From 6fef12596536d1a0857f153433ad133d9fc44b92 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 18 Oct 2024 14:50:04 +0200 Subject: [PATCH] Use search instead of findMany in relation pickers (#7798) First step of #https://github.com/twentyhq/twenty/issues/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 --- .../effect-components/GotoHotkeysEffect.tsx | 1 + ...ionDrawerSectionForObjectMetadataItems.tsx | 1 + ...bjectMetadataItemsRelationPickerEffect.tsx | 29 -------- .../object-record/hooks/useSearchRecords.ts | 10 ++- .../components/RelationFromManyFieldInput.tsx | 2 - .../RecordDetailRelationSection.tsx | 2 - .../SingleEntitySelectMenuItemsWithSearch.tsx | 4 -- .../hooks/useRelationPickerEntitiesOptions.ts | 17 ++--- .../utils/generateSearchRecordsQuery.ts | 8 ++- .../useFilteredSearchEntityQuery.test.tsx | 4 +- .../hooks/useFilteredSearchEntityQuery.ts | 69 +++---------------- .../twenty-front/src/testing/graphqlMocks.ts | 46 +++++++++++++ .../graphql-query-search-resolver.service.ts | 46 ++++++++++--- .../workspace-resolvers-builder.interface.ts | 5 +- .../utils/get-resolver-args.util.ts | 4 ++ 15 files changed, 123 insertions(+), 125 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsRelationPickerEffect.tsx diff --git a/packages/twenty-front/src/modules/app/effect-components/GotoHotkeysEffect.tsx b/packages/twenty-front/src/modules/app/effect-components/GotoHotkeysEffect.tsx index 15d371f9f..202b58b96 100644 --- a/packages/twenty-front/src/modules/app/effect-components/GotoHotkeysEffect.tsx +++ b/packages/twenty-front/src/modules/app/effect-components/GotoHotkeysEffect.tsx @@ -11,6 +11,7 @@ export const GotoHotkeys = () => { return nonSystemActiveObjectMetadataItems.map((objectMetadataItem) => ( diff --git a/packages/twenty-front/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx b/packages/twenty-front/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx index 9960670d1..f90a160b5 100644 --- a/packages/twenty-front/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx +++ b/packages/twenty-front/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx @@ -85,6 +85,7 @@ export const NavigationDrawerSectionForObjectMetadataItems = ({ objectMetadataItemsForNavigationItems.map( (objectMetadataItem) => ( ), diff --git a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsRelationPickerEffect.tsx b/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsRelationPickerEffect.tsx deleted file mode 100644 index 1d2d1ecd7..000000000 --- a/packages/twenty-front/src/modules/object-metadata/components/ObjectMetadataItemsRelationPickerEffect.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { useEffect } from 'react'; - -import { useRelationPicker } from '@/object-record/relation-picker/hooks/useRelationPicker'; - -export const ObjectMetadataItemsRelationPickerEffect = ({ - relationPickerScopeId, -}: { - relationPickerScopeId?: string; -} = {}) => { - const { setSearchQuery } = useRelationPicker({ relationPickerScopeId }); - - const computeFilterFields = (relationPickerType: string) => { - if (relationPickerType === 'company') { - return ['name']; - } - - if (['workspaceMember', 'person'].includes(relationPickerType)) { - return ['name.firstName', 'name.lastName']; - } - - return ['name']; - }; - - useEffect(() => { - setSearchQuery({ computeFilterFields }); - }, [setSearchQuery]); - - return <>; -}; diff --git a/packages/twenty-front/src/modules/object-record/hooks/useSearchRecords.ts b/packages/twenty-front/src/modules/object-record/hooks/useSearchRecords.ts index 175f84554..7c1f90162 100644 --- a/packages/twenty-front/src/modules/object-record/hooks/useSearchRecords.ts +++ b/packages/twenty-front/src/modules/object-record/hooks/useSearchRecords.ts @@ -13,10 +13,11 @@ import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { useQuery, WatchQueryFetchPolicy } from '@apollo/client'; import { useMemo } from 'react'; import { useRecoilValue } from 'recoil'; +import { isDefined } from '~/utils/isDefined'; import { logError } from '~/utils/logError'; export type UseSearchRecordsParams = ObjectMetadataItemIdentifier & - RecordGqlOperationVariables & { + Pick & { onError?: (error?: Error) => void; skip?: boolean; recordGqlFields?: RecordGqlOperationGqlRecordFields; @@ -29,6 +30,7 @@ export const useSearchRecords = ({ searchInput, limit, skip, + filter, recordGqlFields, fetchPolicy, }: UseSearchRecordsParams) => { @@ -45,10 +47,14 @@ export const useSearchRecords = ({ const { data, loading, error, previousData } = useQuery(searchRecordsQuery, { skip: - skip || !objectMetadataItem || !currentWorkspaceMember || !searchInput, + skip || + !objectMetadataItem || + !currentWorkspaceMember || + !isDefined(searchInput), variables: { search: searchInput, limit: limit, + filter: filter, }, fetchPolicy: fetchPolicy, onError: (error) => { diff --git a/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/RelationFromManyFieldInput.tsx b/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/RelationFromManyFieldInput.tsx index be1cba381..f0cbf686d 100644 --- a/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/RelationFromManyFieldInput.tsx +++ b/packages/twenty-front/src/modules/object-record/record-field/meta-types/input/components/RelationFromManyFieldInput.tsx @@ -1,6 +1,5 @@ import { useContext } from 'react'; -import { ObjectMetadataItemsRelationPickerEffect } from '@/object-metadata/components/ObjectMetadataItemsRelationPickerEffect'; import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { FieldContext } from '@/object-record/record-field/contexts/FieldContext'; import { RelationFromManyFieldInputMultiRecordsEffect } from '@/object-record/record-field/meta-types/input/components/RelationFromManyFieldInputMultiRecordsEffect'; @@ -54,7 +53,6 @@ export const RelationFromManyFieldInput = ({ return ( <> - ) : ( <> - - gql` - query Search${capitalize(objectMetadataItem.namePlural)}($search: String, $limit: Int) { - ${getSearchRecordsQueryResponseField(objectMetadataItem.namePlural)}(searchInput: $search, limit: $limit){ + query Search${capitalize(objectMetadataItem.namePlural)}($search: String, $limit: Int, $filter: ${capitalize( + objectMetadataItem.nameSingular, + )}FilterInput) { + ${getSearchRecordsQueryResponseField(objectMetadataItem.namePlural)}(searchInput: $search, limit: $limit, filter: $filter){ edges { node ${mapObjectMetadataToGraphQLQuery({ objectMetadataItems, diff --git a/packages/twenty-front/src/modules/search/hooks/__tests__/useFilteredSearchEntityQuery.test.tsx b/packages/twenty-front/src/modules/search/hooks/__tests__/useFilteredSearchEntityQuery.test.tsx index 263b70dec..6b2409af4 100644 --- a/packages/twenty-front/src/modules/search/hooks/__tests__/useFilteredSearchEntityQuery.test.tsx +++ b/packages/twenty-front/src/modules/search/hooks/__tests__/useFilteredSearchEntityQuery.test.tsx @@ -80,13 +80,11 @@ describe('useFilteredSearchEntityQuery', () => { setMetadataItems(generatedMockObjectMetadataItems); return useFilteredSearchEntityQuery({ - orderByField: 'name', - filters: [{ fieldNames: ['name'], filter: 'Entity' }], - sortOrder: 'AscNullsLast', selectedIds: ['1'], limit: 10, excludeRecordIds: ['2'], objectNameSingular: 'person', + searchFilter: 'Entity', }); }, { wrapper: Wrapper }, diff --git a/packages/twenty-front/src/modules/search/hooks/useFilteredSearchEntityQuery.ts b/packages/twenty-front/src/modules/search/hooks/useFilteredSearchEntityQuery.ts index 06ba43f92..9d6973385 100644 --- a/packages/twenty-front/src/modules/search/hooks/useFilteredSearchEntityQuery.ts +++ b/packages/twenty-front/src/modules/search/hooks/useFilteredSearchEntityQuery.ts @@ -1,39 +1,26 @@ -import { isNonEmptyString } from '@sniptt/guards'; - import { useMapToObjectRecordIdentifier } from '@/object-metadata/hooks/useMapToObjectRecordIdentifier'; import { DEFAULT_SEARCH_REQUEST_LIMIT } from '@/object-record/constants/DefaultSearchRequestLimit'; -import { RecordGqlOperationFilter } from '@/object-record/graphql/types/RecordGqlOperationFilter'; -import { useFindManyRecords } from '@/object-record/hooks/useFindManyRecords'; +import { useSearchRecords } from '@/object-record/hooks/useSearchRecords'; import { EntitiesForMultipleEntitySelect } from '@/object-record/relation-picker/types/EntitiesForMultipleEntitySelect'; import { EntityForSelect } from '@/object-record/relation-picker/types/EntityForSelect'; import { ObjectRecord } from '@/object-record/types/ObjectRecord'; -import { makeAndFilterVariables } from '@/object-record/utils/makeAndFilterVariables'; -import { makeOrFilterVariables } from '@/object-record/utils/makeOrFilterVariables'; -import { OrderBy } from '@/types/OrderBy'; -import { generateILikeFiltersForCompositeFields } from '~/utils/array/generateILikeFiltersForCompositeFields'; import { isDefined } from '~/utils/isDefined'; -type SearchFilter = { fieldNames: string[]; filter: string | number }; - // TODO: use this for all search queries, because we need selectedEntities and entitiesToSelect each time we want to search // Filtered entities to select are export const useFilteredSearchEntityQuery = ({ - orderByField, - filters, - sortOrder = 'AscNullsLast', selectedIds, limit, excludeRecordIds = [], objectNameSingular, + searchFilter, }: { - orderByField: string; - filters: SearchFilter[]; - sortOrder?: OrderBy; selectedIds: string[]; limit?: number; excludeRecordIds?: string[]; objectNameSingular: string; + searchFilter?: string; }): EntitiesForMultipleEntitySelect => { const { mapToObjectRecordIdentifier } = useMapToObjectRecordIdentifier({ objectNameSingular, @@ -46,55 +33,21 @@ export const useFilteredSearchEntityQuery = ({ const selectedIdsFilter = { id: { in: selectedIds } }; const { loading: selectedRecordsLoading, records: selectedRecords } = - useFindManyRecords({ + useSearchRecords({ objectNameSingular, filter: selectedIdsFilter, - orderBy: [{ [orderByField]: sortOrder }], skip: !selectedIds.length, + searchInput: searchFilter, }); - const searchFilters = filters.map(({ fieldNames, filter }) => { - if (!isNonEmptyString(filter)) { - return undefined; - } - - const formattedFilters = fieldNames.reduce( - (previousValue: RecordGqlOperationFilter[], fieldName) => { - const [parentFieldName, subFieldName] = fieldName.split('.'); - - if (isNonEmptyString(subFieldName)) { - // Composite field - return [ - ...previousValue, - ...generateILikeFiltersForCompositeFields(filter, parentFieldName, [ - subFieldName, - ]), - ]; - } - - return [ - ...previousValue, - { - [fieldName]: { - ilike: `%${filter}%`, - }, - }, - ]; - }, - [], - ); - - return makeOrFilterVariables(formattedFilters); - }); - const { loading: filteredSelectedRecordsLoading, records: filteredSelectedRecords, - } = useFindManyRecords({ + } = useSearchRecords({ objectNameSingular, - filter: makeAndFilterVariables([...searchFilters, selectedIdsFilter]), - orderBy: [{ [orderByField]: sortOrder }], + filter: selectedIdsFilter, skip: !selectedIds.length, + searchInput: searchFilter, }); const notFilterIds = [...selectedIds, ...excludeRecordIds]; @@ -102,11 +55,11 @@ export const useFilteredSearchEntityQuery = ({ ? { not: { id: { in: notFilterIds } } } : undefined; const { loading: recordsToSelectLoading, records: recordsToSelect } = - useFindManyRecords({ + useSearchRecords({ objectNameSingular, - filter: makeAndFilterVariables([...searchFilters, notFilter]), + filter: notFilter, limit: limit ?? DEFAULT_SEARCH_REQUEST_LIMIT, - orderBy: [{ [orderByField]: sortOrder }], + searchInput: searchFilter, }); return { diff --git a/packages/twenty-front/src/testing/graphqlMocks.ts b/packages/twenty-front/src/testing/graphqlMocks.ts index 1cb7c4b3a..d634890a5 100644 --- a/packages/twenty-front/src/testing/graphqlMocks.ts +++ b/packages/twenty-front/src/testing/graphqlMocks.ts @@ -113,6 +113,52 @@ export const graphqlMocks = { }, }); }), + graphql.query('SearchWorkspaceMembers', () => { + return HttpResponse.json({ + data: { + searchWorkspaceMembers: { + edges: mockWorkspaceMembers.map((member) => ({ + node: { + ...member, + messageParticipants: { + edges: [], + __typename: 'MessageParticipantConnection', + }, + authoredAttachments: { + edges: [], + __typename: 'AttachmentConnection', + }, + authoredComments: { + edges: [], + __typename: 'CommentConnection', + }, + accountOwnerForCompanies: { + edges: [], + __typename: 'CompanyConnection', + }, + authoredActivities: { + edges: [], + __typename: 'ActivityConnection', + }, + favorites: { + edges: [], + __typename: 'FavoriteConnection', + }, + connectedAccounts: { + edges: [], + __typename: 'ConnectedAccountConnection', + }, + assignedActivities: { + edges: [], + __typename: 'ActivityConnection', + }, + }, + cursor: null, + })), + }, + }, + }); + }), graphql.query('FindManyViewFields', ({ variables }) => { const viewId = variables.filter.view.eq; diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts index cfd570281..378dfff97 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts @@ -4,16 +4,19 @@ import { ResolverService } from 'src/engine/api/graphql/graphql-query-runner/int import { Record as IRecord, OrderByDirection, + RecordFilter, } from 'src/engine/api/graphql/workspace-query-builder/interfaces/record.interface'; import { IConnection } from 'src/engine/api/graphql/workspace-query-runner/interfaces/connection.interface'; import { WorkspaceQueryRunnerOptions } from 'src/engine/api/graphql/workspace-query-runner/interfaces/query-runner-option.interface'; import { SearchResolverArgs } from 'src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface'; import { QUERY_MAX_RECORDS } from 'src/engine/api/graphql/graphql-query-runner/constants/query-max-records.constant'; +import { GraphqlQueryParser } from 'src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser'; import { ObjectRecordsToGraphqlConnectionHelper } from 'src/engine/api/graphql/graphql-query-runner/helpers/object-records-to-graphql-connection.helper'; import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service'; import { SEARCH_VECTOR_FIELD } from 'src/engine/metadata-modules/constants/search-vector-field.constants'; import { TwentyORMGlobalManager } from 'src/engine/twenty-orm/twenty-orm-global.manager'; +import { isDefined } from 'src/utils/is-defined'; @Injectable() export class GraphqlQuerySearchResolverService @@ -24,11 +27,19 @@ export class GraphqlQuerySearchResolverService private readonly featureFlagService: FeatureFlagService, ) {} - async resolve( + async resolve< + ObjectRecord extends IRecord = IRecord, + Filter extends RecordFilter = RecordFilter, + >( args: SearchResolverArgs, options: WorkspaceQueryRunnerOptions, ): Promise> { - const { authContext, objectMetadataItem, objectMetadataMap } = options; + const { + authContext, + objectMetadataItem, + objectMetadataMapItem, + objectMetadataMap, + } = options; const repository = await this.twentyORMGlobalManager.getRepositoryForWorkspace( @@ -39,7 +50,7 @@ export class GraphqlQuerySearchResolverService const typeORMObjectRecordsParser = new ObjectRecordsToGraphqlConnectionHelper(objectMetadataMap); - if (!args.searchInput) { + if (!isDefined(args.searchInput)) { return typeORMObjectRecordsParser.createConnection({ objectRecords: [], objectName: objectMetadataItem.nameSingular, @@ -54,11 +65,27 @@ export class GraphqlQuerySearchResolverService const limit = args?.limit ?? QUERY_MAX_RECORDS; - const resultsWithTsVector = (await repository - .createQueryBuilder() - .where(`"${SEARCH_VECTOR_FIELD.name}" @@ to_tsquery(:searchTerms)`, { - searchTerms, - }) + const queryBuilder = repository.createQueryBuilder( + objectMetadataItem.nameSingular, + ); + const graphqlQueryParser = new GraphqlQueryParser( + objectMetadataMapItem.fields, + objectMetadataMap, + ); + + const queryBuilderWithFilter = graphqlQueryParser.applyFilterToBuilder( + queryBuilder, + objectMetadataMapItem.nameSingular, + args.filter ?? ({} as Filter), + ); + + const resultsWithTsVector = (await queryBuilderWithFilter + .andWhere( + searchTerms === '' + ? `"${SEARCH_VECTOR_FIELD.name}" IS NOT NULL` + : `"${SEARCH_VECTOR_FIELD.name}" @@ to_tsquery(:searchTerms)`, + searchTerms === '' ? {} : { searchTerms }, + ) .orderBy( `ts_rank("${SEARCH_VECTOR_FIELD.name}", to_tsquery(:searchTerms))`, 'DESC', @@ -84,6 +111,9 @@ export class GraphqlQuerySearchResolverService } private formatSearchTerms(searchTerm: string) { + if (searchTerm === '') { + return ''; + } const words = searchTerm.trim().split(/\s+/); const formattedWords = words.map((word) => { const escapedWord = word.replace(/[\\:'&|!()]/g, '\\$&'); diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface.ts b/packages/twenty-server/src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface.ts index 219b185c4..69bc97777 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-resolver-builder/interfaces/workspace-resolvers-builder.interface.ts @@ -48,8 +48,11 @@ export interface FindDuplicatesResolverArgs< data?: Data[]; } -export interface SearchResolverArgs { +export interface SearchResolverArgs< + Filter extends RecordFilter = RecordFilter, +> { searchInput?: string; + filter?: Filter; limit?: number; } diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/utils/get-resolver-args.util.ts b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/utils/get-resolver-args.util.ts index 7e1755218..b50047e62 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/utils/get-resolver-args.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/utils/get-resolver-args.util.ts @@ -147,6 +147,10 @@ export const getResolverArgs = ( type: GraphQLInt, isNullable: true, }, + filter: { + kind: InputTypeDefinitionKind.Filter, + isNullable: true, + }, }; default: throw new Error(`Unknown resolver type: ${type}`);