fix(twenty-server): preserve input order in createMany response#17412
fix(twenty-server): preserve input order in createMany response#17412FelixMalfait merged 2 commits intomainfrom
Conversation
04b407a to
3c065e5
Compare
Greptile OverviewGreptile SummaryThis PR fixes a critical ordering bug in the Key Changes:
Impact:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant CreateManyRunner
participant Repository
participant Database
Client->>CreateManyRunner: createMany(data: [record1, record2, record3])
CreateManyRunner->>Repository: insert/upsert records
Repository->>Database: INSERT INTO table VALUES (...)
Database-->>Repository: generatedMaps: [{id: "uuid-1"}, {id: "uuid-2"}, {id: "uuid-3"}]
Repository-->>CreateManyRunner: InsertResult with ordered generatedMaps
Note over CreateManyRunner: Extract orderedIds from generatedMaps<br/>to preserve input order
CreateManyRunner->>Repository: WHERE id IN (uuid-1, uuid-2, uuid-3)
Repository->>Database: SELECT * FROM table WHERE id IN (...)
Database-->>Repository: Records in arbitrary order
Repository-->>CreateManyRunner: upsertedRecords (unordered)
Note over CreateManyRunner: Map records by ID<br/>recordsById.set(id, record)
Note over CreateManyRunner: Reorder using orderedIds<br/>orderedIds.map(id => recordsById.get(id))
CreateManyRunner-->>Client: [record1, record2, record3] (preserves input order)
|
3c065e5 to
929b390
Compare
Bulk APIs (createMany, mergeMany, findDuplicates) were returning records in arbitrary order because they use WHERE id IN (...) without ORDER BY. Fixed by reordering fetched records to match the original input order. Affected endpoints: - createMany: fetchUpsertedRecords - mergeMany: getRecordsToMerge (for dry-run and actual merge) - findDuplicates: when fetching by IDs
929b390 to
5e59949
Compare
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:12634 This environment will automatically shut down when the PR is closed or after 5 hours. |
|
|
||
| return upsertedRecords as ObjectRecord[]; | ||
| // Preserve original input order by sorting results to match orderedIds | ||
| const recordsById = new Map<string, ObjectRecord>( |
| flatFieldMetadataMaps, | ||
| }); | ||
|
|
||
| const orderedIds = objectRecords.generatedMaps.map((record) => record.id); |
There was a problem hiding this comment.
i'm confused on how this will preserver the order in the input: objectRecords does not seem to be the input. I wonder if the sort method should be done in a parent method that stil has access to the input
There was a problem hiding this comment.
For simple inserts, generatedMaps does preserve input order (TypeORM guarantee), so the current fix works. For upserts, you're right - the order is scrambled because updates are processed before inserts. Fixing that properly would require a bigger change, I don't think it's worth it. I will look into it but will likely merge like that if I find out it's too much code changes just for this upsert edge-case
| ); | ||
|
|
||
| objectRecords = args.ids | ||
| .map((id) => recordsById.get(id)) |
| }) | ||
| .getMany()) as ObjectRecord[]; | ||
|
|
||
| // Preserve original input order |
47a4f15 to
f0b995d
Compare
f0b995d to
f574c6f
Compare
…tyhq#17412) ## Summary - The `createMany` API was returning records in arbitrary order because `fetchUpsertedRecords` used `WHERE id IN (...)` without `ORDER BY` - This caused flaky tests that assumed input order was preserved (e.g., `people-merge-many.integration-spec.ts`) - Fixed by reordering the fetched records to match the original input order from `generatedMaps` ## Root Cause ```typescript // Before: No ORDER BY, so SQL returns in arbitrary order const upsertedRecords = await queryBuilder .where({ id: In(objectRecords.generatedMaps.map((record) => record.id)) }) .getMany(); // Returns in arbitrary order! ``` ## Fix ```typescript // After: Reorder results to match original input order const orderedIds = objectRecords.generatedMaps.map((record) => record.id); const upsertedRecords = await queryBuilder .where({ id: In(orderedIds) }) .getMany(); // Preserve original input order const recordsById = new Map(upsertedRecords.map((record) => [record.id, record])); return orderedIds .map((id) => recordsById.get(id)) .filter((record) => record !== undefined); ``` ## Test plan - [x] Run `people-merge-many.integration-spec.ts` multiple times - passes consistently - [x] Lint passes
Summary
createManyAPI was returning records in arbitrary order becausefetchUpsertedRecordsusedWHERE id IN (...)withoutORDER BYpeople-merge-many.integration-spec.ts)generatedMapsRoot Cause
Fix
Test plan
people-merge-many.integration-spec.tsmultiple times - passes consistently