Search #7237

ijreilly · 2024-09-24T14:30:07Z

Steps to test

Run metadata migrations
Run sync-metadata on your workspace
Enable the following feature flags:
IS_SEARCH_ENABLED
IS_QUERY_RUNNER_TWENTY_ORM_ENABLED
IS_WORKSPACE_MIGRATED_FOR_SEARCH
Type Cmd + K and search anything

…nsion)

packages/twenty-server/src/engine/twenty-orm/decorators/workspace-index.decorator.ts

greptile-apps

PR Summary

This PR introduces comprehensive search functionality across the Twenty application, focusing on both frontend and backend implementations. Here are the key changes:

Added new search GraphQL query and resolver for efficient full-text search
Implemented TsVector field type and GIN indexing for PostgreSQL full-text search
Created frontend hooks and utilities for search operations (useSearchRecords, useSearchRecordsQuery)
Integrated feature flags for controlled rollout of search functionality
Modified CommandMenu component to utilize new search capabilities
Updated GraphQL schema and type definitions to support search operations
Added migrations for new database structures supporting search features

_{30 file(s) reviewed, 13 comment(s)}
_{Edit PR Review Bot Settings}

packages/twenty-front/src/modules/object-record/hooks/useSearchRecords.ts

packages/twenty-front/src/modules/object-record/hooks/useSearchRecordsQuery.ts

greptile-apps · 2024-09-30T17:51:04Z

packages/twenty-front/src/modules/object-record/utils/generateSearchRecordsQuery.ts

+import { getSearchRecordsQueryResponseField } from '@/object-record/utils/getSearchRecordsQueryResponseField';
+import { capitalize } from '~/utils/string/capitalize';
+
+export type QueryCursorDirection = 'before' | 'after';


style: QueryCursorDirection is defined but not used in this file. Consider removing if unnecessary.

...s/twenty-server/src/database/typeorm/metadata/migrations/1726848397026-addTypeOrmMetadata.ts

...ase/typeorm/metadata/migrations/1727699709905-addIsCustomAddIsCustomColumnToIndexMetadata.ts

...enty-server/src/engine/api/graphql/workspace-query-builder/factories/search-query.factory.ts

...erver/src/engine/api/graphql/workspace-resolver-builder/factories/search-resolver-factory.ts

greptile-apps · 2024-09-30T18:00:05Z

.../src/engine/api/graphql/workspace-schema-builder/factories/object-type-definition.factory.ts

 import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
 import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
+import { isRelationFieldMetadataType } from 'src/engine/utils/is-relation-field-metadata-type.util';
+import { pascalCase } from 'src/utils/pascal-case';


style: Import order changed. Consider using a consistent import ordering strategy across the codebase.

Weiko

Great Job @ijreilly! I left a few comments and still need to test a bit locally. Can't wait to merge it 💪

packages/twenty-server/src/database/typeorm-seeds/core/feature-flags.ts

...ase/typeorm/metadata/migrations/1727699709905-addIsCustomAddIsCustomColumnToIndexMetadata.ts

...es/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-runner.service.ts

Weiko · 2024-10-01T12:33:29Z

packages/twenty-server/src/modules/company/standard-objects/company.workspace-entity.ts

+    icon: 'IconUser',
+    generatedType: 'STORED',
+    asExpression: getTsVectorColumnExpressionFromFields([
+      { name: NAME_FIELD_NAME, type: FieldMetadataType.TEXT },


Should we use standardFieldId instead here?

In the util at this stage we cannot fetch fieldMetadata based on the standardFieldId, can we?

Probably not yet. Should be part of the refactoring of sync-metadata but we already started to use hashmap in some places with standard-id, nameSingular, namePlural, etc... as keys.
You can keep it as it is for now indeed

packages/twenty-server/src/engine/metadata-modules/index-metadata/index-metadata.entity.ts

packages/twenty-front/src/modules/object-record/graphql/types/RecordGqlOperationFilter.ts

.../src/database/typeorm/metadata/migrations/1725893697807-addIndexTypeColumnToIndexMetadata.ts

packages/twenty-front/src/modules/object-record/cache/utils/getRecordsFromRecordConnection.ts

...c/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts

Weiko

Almost there 💪
Also I noticed we were not querying search for custom objects on the FE, is it expected? Looks like the BE should be ready? LGTM if we want to do it in another PR 👍

.../src/database/typeorm/metadata/migrations/1725893697807-addIndexTypeColumnToIndexMetadata.ts

...c/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts

...twenty-server/src/engine/api/graphql/workspace-schema-builder/utils/generate-fields.utils.ts

packages/twenty-server/src/engine/metadata-modules/index-metadata/index-metadata.entity.ts

Weiko · 2024-10-02T16:36:50Z

packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.service.ts

+      isActive: false,
+      isSystem: true,
+      type: FieldMetadataType.TS_VECTOR,
+      name: SEARCH_VECTOR_FIELD_NAME,


I see, maybe we can create an object then?

const SEARCH_VECTOR_FIELD = { name: 'searchVector', label: 'Search Vector', ... }

not a strong requirement, up to you if you feel like those const are enough

...nty-server/src/engine/metadata-modules/utils/constants/search-vector-field-name.constants.ts

packages/twenty-server/src/engine/twenty-orm/decorators/workspace-index.decorator.ts

...rc/engine/workspace-manager/workspace-migration-runner/workspace-migration-runner.service.ts

...twenty-server/src/engine/workspace-manager/workspace-sync-metadata/standard-objects/index.ts

Weiko · 2024-10-02T17:01:42Z

packages/twenty-server/src/modules/company/standard-objects/company.workspace-entity.ts

+    icon: 'IconUser',
+    generatedType: 'STORED',
+    asExpression: getTsVectorColumnExpressionFromFields([
+      { name: NAME_FIELD_NAME, type: FieldMetadataType.TEXT },


Probably not yet. Should be part of the refactoring of sync-metadata but we already started to use hashmap in some places with standard-id, nameSingular, namePlural, etc... as keys.
You can keep it as it is for now indeed

Weiko

Great job!! Just left a few comments but LGTM 👏

.../src/database/typeorm/metadata/migrations/1725893697807-addIndexTypeColumnToIndexMetadata.ts

Weiko · 2024-10-03T14:26:55Z

.../src/database/typeorm/metadata/migrations/1725893697807-addIndexTypeColumnToIndexMetadata.ts

+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(
+      `CREATE TYPE metadata.INDEX_TYPES AS ENUM ('BTREE', 'GIN')`,


Just realised but I think that's not the naming convention we follow currently.
We put the name of the table as well. Can you double check please? 🙏

Weiko · 2024-10-03T14:29:24Z

...c/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts

+    }
+
+    if (!args.searchInput) {
+      return {} as IConnection<ObjectRecord>;


You should be able to return this

return typeORMObjectRecordsParser.createConnection( [], objectMetadataItem.nameSingular, 0, 0, [{ id: OrderByDirection.AscNullsFirst }], false, false, );

Steps to test 1. Run metadata migrations 2. Run sync-metadata on your workspace 3. Enable the following feature flags: IS_SEARCH_ENABLED IS_QUERY_RUNNER_TWENTY_ORM_ENABLED IS_WORKSPACE_MIGRATED_FOR_SEARCH 4. Type Cmd + K and search anything

ijreilly added 30 commits August 23, 2024 12:36

fix test

dd5055d

Merge branch 'main' of github.com:twentyhq/twenty into search

0904917

Comments with twenty orm hints

81d6005

Merge branch 'main' of github.com:twentyhq/twenty into search

c343155

WIP - search with query builder and trgm works (prereq: add trgm exte…

8d76f96

…nsion)

WIP

ed90ee9

wip - Add search parameter

66f8747

wip - search on trigram works in front

77f4063

ts_vector works

0989b70

wip

b758fc8

wip - spilo dockerfile works

570b9d3

Merge branch 'main' of github.com:twentyhq/twenty into search

c612fc0

Merge branch 'main' of github.com:twentyhq/twenty into search

324ac9b

implementation of search endpoint

3cbccd3

Add limit arg to search endpoint

6d1092f

Add feature flag

552b4e0

wip

b258648

Merge branch 'main' of github.com:twentyhq/twenty into search

cc4764a

wip - searchvector created and works but typeorm issue + no indexes yet

3401747

split emails in two words for tsvector

a1213c5

Add searchVector to custom object entity

51dcb01

create searchVector field at custom object creation

0716119

Handle gin index creation with decorator

fc053e3

Merge branch 'main' of github.com:twentyhq/twenty into search

7e50d5a

remove merge conflict trace

3929800

wip

1d67d22

Merge branch 'main' of github.com:twentyhq/twenty into search

2a1103c

generate graphql for front

24c7d6e

wip

6b1d822

Add typeOrmMetadata column

4059b61

ijreilly commented Sep 27, 2024

View reviewed changes

packages/twenty-server/src/engine/twenty-orm/decorators/workspace-index.decorator.ts Outdated Show resolved Hide resolved

ijreilly added 6 commits September 27, 2024 10:54

Remove code from paradedb tests

9a06c25

clean code

581045b

Merge branch 'main' of github.com:twentyhq/twenty into search

eb39b2d

minor improvements

88a4d19

Extend sync-index to standard indexes on custom objects

1c52263

Add feature flag in FE

3d7bbac

ijreilly marked this pull request as ready for review September 30, 2024 17:45

ijreilly requested a review from Weiko September 30, 2024 17:47

greptile-apps bot reviewed Sep 30, 2024

View reviewed changes

Weiko reviewed Oct 1, 2024

View reviewed changes

ijreilly added 8 commits October 1, 2024 15:11

Fix standard index factory issue

5605645

Merge branch 'main' of github.com:twentyhq/twenty into search

ef9ab33

Code improvements

b05c027

Fix index creation

54c17d1

Fix SQL injection breach

4652fc7

Refactor generateFields

cae5738

Nullify searchVector when deletedAt is not null

5771352

improve code quality

d404c22

github-advanced-security bot found potential problems Oct 2, 2024

View reviewed changes

...c/engine/api/graphql/graphql-query-runner/resolvers/graphql-query-search-resolver.service.ts Fixed Show fixed Hide fixed

Fix escapedWord

2e95942

Weiko reviewed Oct 2, 2024

View reviewed changes

Weiko added the -PR: awaiting author label Oct 2, 2024

ijreilly added 2 commits October 3, 2024 15:54

improve code quality

c6fe5f9

Fix BTREE indexes inclusion of deletedAt + add tests

adfc8ee

Weiko approved these changes Oct 3, 2024

View reviewed changes

Follow conventions for addIndexType

1bed77f

ijreilly merged commit 5f9435c into main Oct 3, 2024
7 of 13 checks passed

ijreilly deleted the search branch October 3, 2024 15:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search #7237

Search #7237

ijreilly commented Sep 24, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Sep 30, 2024

greptile-apps bot Sep 30, 2024

Weiko left a comment

Weiko Oct 1, 2024

ijreilly Oct 1, 2024

Weiko Oct 2, 2024

Weiko left a comment •

edited

Loading

Weiko Oct 2, 2024

Weiko Oct 2, 2024

Weiko left a comment

Weiko Oct 3, 2024

Weiko Oct 3, 2024

Search #7237

Search #7237

Conversation

ijreilly commented Sep 24, 2024 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Sep 30, 2024

Choose a reason for hiding this comment

greptile-apps bot Sep 30, 2024

Choose a reason for hiding this comment

Weiko left a comment

Choose a reason for hiding this comment

Weiko Oct 1, 2024

Choose a reason for hiding this comment

ijreilly Oct 1, 2024

Choose a reason for hiding this comment

Weiko Oct 2, 2024

Choose a reason for hiding this comment

Weiko left a comment • edited Loading

Choose a reason for hiding this comment

Weiko Oct 2, 2024

Choose a reason for hiding this comment

Weiko Oct 2, 2024

Choose a reason for hiding this comment

Weiko left a comment

Choose a reason for hiding this comment

Weiko Oct 3, 2024

Choose a reason for hiding this comment

Weiko Oct 3, 2024

Choose a reason for hiding this comment

ijreilly commented Sep 24, 2024 •

edited

Loading

Weiko left a comment •

edited

Loading