Use search instead of findMany in relation pickers #7798
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request updates the search functionality in relation pickers to allow for a filter argument, addressing the first step of issue #3298. Key changes include:
- Removed
ObjectMetadataItemsRelationPickerEffectcomponent, simplifying the search logic - Updated
useSearchRecordshook to include a filter parameter for more flexible querying - Modified
GraphqlQuerySearchResolverServiceto apply filters to the query builder - Added a 'filter' argument to the
SearchResolverArgsinterface in the server-side resolver builder - Updated
useFilteredSearchEntityQueryto useuseSearchRecordsinstead ofuseFindManyRecords - Adjusted
generateSearchRecordsQueryto include the filter argument in GraphQL queries
These changes lay the groundwork for improving multi-object requests and pickers across the application.
13 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -11,6 +11,7 @@ export const GotoHotkeys = () => { | |||
|
|
|||
| return nonSystemActiveObjectMetadataItems.map((objectMetadataItem) => ( | |||
| <GoToHotkeyItemEffect | |||
| key={`go-to-hokey-item-${objectMetadataItem.id}`} | |||
There was a problem hiding this comment.
syntax: typo in 'hokey', should be 'hotkey'
| const { loading: selectedRecordsLoading, records: selectedRecords } = | ||
| useFindManyRecords({ | ||
| useSearchRecords({ | ||
| objectNameSingular, | ||
| filter: selectedIdsFilter, | ||
| orderBy: [{ [orderByField]: sortOrder }], | ||
| skip: !selectedIds.length, | ||
| searchInput: searchFilter, | ||
| }); |
There was a problem hiding this comment.
logic: This query might return unnecessary records. Consider adding a limit to avoid performance issues with large datasets.
| const { | ||
| loading: filteredSelectedRecordsLoading, | ||
| records: filteredSelectedRecords, | ||
| } = useFindManyRecords({ | ||
| } = useSearchRecords({ | ||
| objectNameSingular, | ||
| filter: makeAndFilterVariables([...searchFilters, selectedIdsFilter]), | ||
| orderBy: [{ [orderByField]: sortOrder }], | ||
| filter: selectedIdsFilter, | ||
| skip: !selectedIds.length, | ||
| searchInput: searchFilter, | ||
| }); |
There was a problem hiding this comment.
logic: This query seems redundant as it's identical to the previous one. Consider removing or clarifying its purpose.
| const notFilterIds = [...selectedIds, ...excludeRecordIds]; | ||
| const notFilter = notFilterIds.length | ||
| ? { not: { id: { in: notFilterIds } } } | ||
| : undefined; |
There was a problem hiding this comment.
logic: The notFilter logic might not work as expected with the new search implementation. Verify that excluded records are properly filtered out.
| // TODO: use this for all search queries, because we need selectedEntities and entitiesToSelect each time we want to search | ||
| // Filtered entities to select are |
There was a problem hiding this comment.
style: Consider removing or updating this TODO comment as it may no longer be relevant after the changes.
| searchTerms === '' | ||
| ? `"${SEARCH_VECTOR_FIELD.name}" IS NOT NULL` | ||
| : `"${SEARCH_VECTOR_FIELD.name}" @@ to_tsquery(:searchTerms)`, | ||
| searchTerms === '' ? {} : { searchTerms }, |
There was a problem hiding this comment.
style: Consider extracting this condition into a separate method for better readability
|
Great! |
First step of ##3298.
Here we update the search endpoint to allow for a filter argument, which we currently use in the relation pickers to restrict or exclude ids from search.
In a future PR we will try to simplify the search logic in the FE