Revert "Revert "[4/n]: migrate the RESTAPI GET /rest/* to use TwentyORM direc…" #11349

martmull · 2025-04-02T12:32:57Z

Reverts #11344

Fixes twentyhq/core-team-issues#735

Depth===2 fails to depth === 1 on workspace with featureFlag IsNewRelationEnabled = false

…RM direc…" This reverts commit c6afb0d.

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-one.integration-spec.ts

greptile-apps

PR Summary

This PR reverts recent changes to restore legacy REST API GET behavior and reverts modifications in GraphQL parsers related to feature flag support.

packages/twenty-server/src/app.module.ts: Reintroduces RequestMethod.GET in MIGRATED_REST_METHODS for legacy GET middleware.
packages/twenty-server/src/engine/api/graphql/.../graphql-query-order.parser.ts, graphql-query-filter-condition.parser.ts, graphql-query-filter-field.parser.ts, and graphql-query.parser.ts: Removed feature flag parameters, reverting to only fieldMetadataMapByName.
packages/twenty-server/src/engine/api/rest/core/controllers/rest-api-core.controller.ts: Reinstates class-level exception filtering and reverts GET endpoint changes.
Reintroduced integration tests for GET endpoints in rest-api-core-find-one.integration-spec.ts and rest-api-core-find-many.integration-spec.ts.
packages/twenty-server/src/engine/api/rest/input-factories/order-by-input.factory.ts & rest-api.module.ts: Restores previous objectMetadata mapping and factory dependencies.

_{12 file(s) reviewed, 1 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-one.integration-spec.ts

charlesBochet · 2025-04-15T15:02:52Z

closing to keep the PR history clean, let's re-open once ready!

greptile-apps

PR Summary

(updates since last review)

This PR reverts a previous reversion, re-implementing the REST API GET endpoints to use TwentyORM directly with improved cursor-based pagination and filtering capabilities.

Implemented cursor-based pagination in rest-api-core-v2.service.ts with forward/backward navigation and metadata handling
Added comprehensive integration tests in rest-api-core-find-many.integration-spec.ts covering pagination, filtering, and error cases
Removed feature flag dependencies from GraphQL query parsers while maintaining core filtering/ordering functionality
Added FilterInputFactory and OrderByInputFactory to support enhanced filtering and ordering capabilities
Potential concern: computeCursorFilter method only uses ID for cursor filtering, which may be insufficient for complex sorting scenarios

_{10 file(s) reviewed, 6 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts

martmull · 2025-05-07T09:51:37Z

@pacyL2K19 jeez that was hard ^^ Feel free to take a look if you want to

pacyL2K19 · 2025-05-07T09:54:03Z

Thanks for handling this from me @martmull
Sorry for being idle recently, got a lot on my plate
But I will review this today and ideally approve
Won't be able to contribute actively until June tho, but I'll be happy to resume asap

Makefile

...graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts

...er/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts

packages/twenty-server/src/engine/api/graphql/workspace-schema.factory.ts

packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts

packages/twenty-server/src/engine/core-modules/cache-storage/services/cache-storage.service.ts

Weiko · 2025-05-07T15:30:56Z

packages/twenty-server/src/engine/api/rest/input-factories/depth-input.factory.ts

+const ALLOWED_DEPTH_VALUES: Depth[] = [0, 1, 2];
+
+@Injectable()
+export class DepthInputFactory {


An Injectable factory seems a bit overkill to me here, why not simply a util function?

to be consistent with other input factories, limit is not more complex

Yes definitely, I didn't know limit was also a factory, I would have made a util for that one too 😛

packages/twenty-server/src/engine/api/rest/core/types/query-variables.type.ts

packages/twenty-server/src/engine/api/rest/core/rest-api-core-v2.service.ts

...er/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query.parser.ts

...ges/twenty-server/src/engine/api/rest/core/resolvers/rest-api-update-one-resolver.service.ts

…RM direc…" (twentyhq#11349)

## Done - add relations in dropdown variables - add relations in worklfow run inputs - use objectMetadataMaps in workflow folder ## To do - does not work with rest api calls, will be fixed after #11349 is merged - waiting for crud action relation fields twentyhq/core-team-issues#509

## Done - add relations in dropdown variables - add relations in worklfow run inputs - use objectMetadataMaps in workflow folder ## To do - does not work with rest api calls, will be fixed after twentyhq#11349 is merged - waiting for crud action relation fields twentyhq/core-team-issues#509

Revert "Revert "[4/n]: migrate the RESTAPI GET /rest/* to use TwentyO…

ddfd6b6

…RM direc…" This reverts commit c6afb0d.

github-advanced-security bot found potential problems Apr 2, 2025

View reviewed changes

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-many.integration-spec.ts Fixed Show fixed Hide fixed

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-one.integration-spec.ts Fixed Show fixed Hide fixed

Fix issues

aff7cce

martmull force-pushed the revert-11344-revert-rest-api-get branch from 78936c2 to aff7cce Compare April 2, 2025 13:59

martmull marked this pull request as ready for review April 2, 2025 14:03

greptile-apps bot reviewed Apr 2, 2025

View reviewed changes

packages/twenty-server/test/integration/rest/suites/rest-api-core-find-one.integration-spec.ts Outdated Show resolved Hide resolved

Merge branch 'main' into revert-11344-revert-rest-api-get

d8adc58

charlesBochet closed this Apr 15, 2025

Merge branch 'main' into revert-11344-revert-rest-api-get

9d9878e

martmull reopened this Apr 17, 2025

greptile-apps bot reviewed Apr 17, 2025

View reviewed changes

martmull added 12 commits April 17, 2025 17:58

WIP: martin's code

c5a7963

Merge branch 'main' into revert-11344-revert-rest-api-get

74f693f

Fix imports

91e7c2d

Fix find one test

d5d8b3d

WIP: Fix find many test

7d09c2e

Add missing tests cases for findMany

9fb6090

Fix tests

2df4830

WIP: improve code

a8e1913

Fix depth parameter

23d315a

Improve style

6287f90

Fix tests

6d04f6c

Fix test for depth 1

b315134

martmull force-pushed the revert-11344-revert-rest-api-get branch from 36f81ad to b315134 Compare April 23, 2025 09:42

Merge branch 'main' into revert-11344-revert-rest-api-get

0375dfe

martmull force-pushed the revert-11344-revert-rest-api-get branch from 3ce148e to 0375dfe Compare April 23, 2025 13:01

martmull added 3 commits April 23, 2025 16:40

Update test so it works with composite fields

8504207

Add test on totalCount

7218abd

Merge branch 'main' into revert-11344-revert-rest-api-get

7e11c06

Code review returns: greptile

6fc27c4

martmull force-pushed the revert-11344-revert-rest-api-get branch from c7f6b3b to 6fc27c4 Compare May 6, 2025 16:04

Fix orderBy

6a6b78d

martmull force-pushed the revert-11344-revert-rest-api-get branch from 0855dec to 6a6b78d Compare May 7, 2025 07:46

Fix cache conflicts betwenn dev environments

d024f13

martmull mentioned this pull request May 7, 2025

Add relations in database event trigger output data #11820

Merged

Weiko reviewed May 7, 2025

View reviewed changes

martmull added 2 commits May 7, 2025 21:49

Merge branch 'main' into revert-11344-revert-rest-api-get

ca9c5ba

Code review returns: Weiko

c1bf418

martmull force-pushed the revert-11344-revert-rest-api-get branch from 3ec6dde to c1bf418 Compare May 7, 2025 22:13

Remove useless stuff

33b512e

martmull force-pushed the revert-11344-revert-rest-api-get branch from 7a50124 to ef2b328 Compare May 8, 2025 21:11

martmull added 2 commits May 8, 2025 23:21

Split v2 service in multiple resolvers

5927847

Create a dedicated module for core rest api

f16ccf4

martmull force-pushed the revert-11344-revert-rest-api-get branch from ef2b328 to f16ccf4 Compare May 8, 2025 21:28

Weiko reviewed May 9, 2025

View reviewed changes

...ges/twenty-server/src/engine/api/rest/core/resolvers/rest-api-update-one-resolver.service.ts Outdated Show resolved Hide resolved

Code review returns: Weiko 2

74e1d64

martmull force-pushed the revert-11344-revert-rest-api-get branch from 00e824e to 74e1d64 Compare May 12, 2025 08:23

Weiko approved these changes May 12, 2025

View reviewed changes

martmull enabled auto-merge (squash) May 12, 2025 08:29

martmull merged commit 650f8f5 into main May 12, 2025
37 checks passed

martmull deleted the revert-11344-revert-rest-api-get branch May 12, 2025 08:32

ehconitin pushed a commit to ehconitin/twenty that referenced this pull request May 12, 2025

Revert "Revert "[4/n]: migrate the RESTAPI GET /rest/* to use TwentyO…

7f338bb

…RM direc…" (twentyhq#11349)

Revert "Revert "[4/n]: migrate the RESTAPI GET /rest/* to use TwentyORM direc…" #11349

Revert "Revert "[4/n]: migrate the RESTAPI GET /rest/* to use TwentyORM direc…" #11349

Uh oh!

Conversation

martmull commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

Uh oh!

charlesBochet commented Apr 15, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martmull commented May 7, 2025

Uh oh!

pacyL2K19 commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Weiko May 7, 2025

Choose a reason for hiding this comment

Uh oh!

martmull May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Weiko May 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

martmull commented Apr 2, 2025 •

edited

Loading