Import - fix import with multiple unique constraints #12784
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
Updates import functionality to handle multiple unique constraints, focusing on the buildWhereConditions method in the GraphQL query resolver.
- Modified
buildWhereConditionsingraphql-query-create-many-resolver.service.tsto prioritize 'id' field when searching for existing records - Added documentation regarding unique constraint handling, noting that uniqueness can be based on field combinations
- TODOs added to improve handling of conflicting fields based on unique indices rather than field metadata
1 file reviewed, 2 comments
Edit PR Review Bot Settings | Greptile
| //TODO : Use one constraint only - id by default - https://github.com/twentyhq/core-team-issues/issues/1115 | ||
| if (field.column === 'id') { | ||
| return { id: In(fieldValues) }; | ||
| } |
There was a problem hiding this comment.
logic: Early return with id-based condition bypasses other unique constraints. Consider the impact on records with multiple unique fields.
There was a problem hiding this comment.
Shouldn't this be a continue ?
There was a problem hiding this comment.
Discussed this with @etiennejouan, we might wanna give a try to an OR condition instead of the early returning on id so we could be sending two unique fields that are not the id one
...ine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts
Outdated
Show resolved
Hide resolved
etiennejouan
changed the title
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:37242 This environment will automatically shut down when the PR is closed or after 5 hours. |
| //TODO : Use one constraint only - id by default - https://github.com/twentyhq/core-team-issues/issues/1115 | ||
| if (field.column === 'id') { | ||
| return { id: In(fieldValues) }; | ||
| } |
| //TODO : Use one constraint only - id by default - https://github.com/twentyhq/core-team-issues/issues/1115 | ||
| if (field.column === 'id') { | ||
| return { id: In(fieldValues) }; | ||
| } |
There was a problem hiding this comment.
Shouldn't this be a continue ?
|
As discussed with @prastoin, the best solution is to check for existing records, parallelising conditions on conflicting fields |
...ine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts
Outdated
Show resolved
Hide resolved
...ine/api/graphql/graphql-query-runner/resolvers/graphql-query-create-many-resolver.service.ts
Outdated
Show resolved
Hide resolved
Test :