fix(front): sanitize optimistic input when creating a record (#21076)
## Summary Closes #15800. Clicking **+ Add New** from a relation cell to create a **Task** or **Note** (e.g. from a custom object's Tasks/Notes section in the list view) throws: ``` Uncaught (in promise) Error: Should never occur, encountered unknown fields name in objectMetadataItem task ``` ### Root cause `useCreateOneRecord` computes a **sanitized** input (with `sanitizeRecordInput`, which strips fields that don't belong to the object) and sends it to the GraphQL mutation. But it still feeds the **raw** input to the optimistic cache computation: ```ts const sanitizedInput = { ...sanitizeRecordInput({ objectMetadataItem, recordInput }), id: idForCreation }; const optimisticRecordInput = computeOptimisticRecordFromInput({ ... recordInput: { ...computeOptimisticCreateRecordBaseRecordInput(objectMetadataItem), ...recordInput, // ← raw input, may contain fields unknown to the object id: idForCreation, }, ... }); // mutation uses the sanitized input: mutate({ variables: { input: sanitizedInput } }); ``` `computeOptimisticRecordFromInput` asserts that every input key maps to a field on the object and `throw`s otherwise. So when the create input carries a field the target object doesn't have (the relation-create path passes a `name`, but Task/Note use `title`), the optimistic step throws before the mutation ever runs. `useCreateManyRecords` does **not** have this problem — it already feeds the sanitized input to `computeOptimisticRecordFromInput`. ### Fix Feed the sanitized input to the optimistic computation in `useCreateOneRecord`, exactly as `useCreateManyRecords` does: ```ts recordInput: { ...computeOptimisticCreateRecordBaseRecordInput(objectMetadataItem), ...sanitizedInput, }, ``` This is safe and behavior-preserving for valid creates: `computeOptimisticRecordFromInput` only ever reads *known* fields (it iterates the object's field metadata); unknown input keys never contribute to the optimistic record — they only trip the invariant. Relations are resolved through their join columns, which sanitization keeps. ## Test plan - [x] `npx oxlint --type-aware` — passes on the changed files - [x] `npx oxfmt --check` — passes - [x] `tsc --noEmit` — no type errors in the changed files - [x] `npx jest computeOptimisticRecordFromInput` — passes, including a new case asserting that input which has been through `sanitizeRecordInput` no longer trips the "Should never occur, encountered unknown fields" invariant (the existing test already covers the raw input throwing) - [ ] Manual: from a custom object's Notes/Tasks relation, use **+ Add New** to create a Note/Task — no error, the record is created ### Note on test scope The crash only reproduces through the full relation-create flow with live metadata; at the hook level in jsdom the create resolves regardless, so a hook-level test would not guard the regression. The added test instead locks the underlying mechanism the fix relies on — that sanitized input is safe for `computeOptimisticRecordFromInput` — alongside the existing test that proves raw unknown fields throw. --------- Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
@@ -133,7 +133,7 @@ export const useCreateManyRecords = <
|
||||
currentWorkspaceMember: currentWorkspaceMember,
|
||||
recordInput: {
|
||||
...baseOptimisticRecordInputCreatedBy,
|
||||
...recordToCreate,
|
||||
...sanitizedRecord,
|
||||
},
|
||||
objectPermissionsByObjectMetadataId,
|
||||
}),
|
||||
|
||||
@@ -101,8 +101,7 @@ export const useCreateOneRecord = <
|
||||
objectMetadataItems,
|
||||
recordInput: {
|
||||
...computeOptimisticCreateRecordBaseRecordInput(objectMetadataItem),
|
||||
...recordInput,
|
||||
id: idForCreation,
|
||||
...sanitizedInput,
|
||||
},
|
||||
objectPermissionsByObjectMetadataId,
|
||||
});
|
||||
|
||||
+28
@@ -5,6 +5,7 @@ import { updateRecordFromCache } from '@/object-record/cache/utils/updateRecordF
|
||||
import { generateDepthRecordGqlFieldsFromRecord } from '@/object-record/graphql/record-gql-fields/utils/generateDepthRecordGqlFieldsFromRecord';
|
||||
import { type FieldActorForInputValue } from '@/object-record/record-field/ui/types/FieldMetadata';
|
||||
import { computeOptimisticRecordFromInput } from '@/object-record/utils/computeOptimisticRecordFromInput';
|
||||
import { sanitizeRecordInput } from '@/object-record/utils/sanitizeRecordInput';
|
||||
import { type WorkspaceMember } from '@/workspace-member/types/WorkspaceMember';
|
||||
import { InMemoryCache } from '@apollo/client';
|
||||
import { mockedWorkspaceMemberRecords } from '~/testing/mock-data/generated/data/workspaceMembers/mock-workspaceMembers-data';
|
||||
@@ -274,4 +275,31 @@ describe('computeOptimisticRecordFromInput', () => {
|
||||
`"Should never occur, encountered unknown fields unknwon, foo, bar in objectMetadataItem person"`,
|
||||
);
|
||||
});
|
||||
|
||||
// Regression test for #15800
|
||||
it('should not throw when the input has been sanitized first', () => {
|
||||
const cache = new InMemoryCache();
|
||||
const personObjectMetadataItem = getMockObjectMetadataItemOrThrow('person');
|
||||
|
||||
const sanitizedInput = sanitizeRecordInput({
|
||||
objectMetadataItem: personObjectMetadataItem,
|
||||
recordInput: {
|
||||
city: 'Paris',
|
||||
nonExistentField: 'should be stripped',
|
||||
},
|
||||
});
|
||||
|
||||
const result = computeOptimisticRecordFromInput({
|
||||
currentWorkspaceMember,
|
||||
objectMetadataItems: getTestEnrichedObjectMetadataItemsMock(),
|
||||
objectMetadataItem: personObjectMetadataItem,
|
||||
recordInput: sanitizedInput,
|
||||
cache,
|
||||
objectPermissionsByObjectMetadataId: {},
|
||||
});
|
||||
|
||||
expect(result).toEqual({
|
||||
city: 'Paris',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user