-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUGFIX] Create optimistic cache generate recordGqlFields
from prefilled record
#10493
Conversation
@@ -102,6 +102,7 @@ export const useCreateOneRecord = < | |||
recordInput: { | |||
...baseOptimisticRecordInputCreatedBy, | |||
...recordInput, | |||
position: Number.NEGATIVE_INFINITY, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review: Might be quite radical but we always want new record to be displayed on top
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR fixes a bug where newly created empty records weren't appearing in the table view due to optimistic cache issues.
- In
packages/twenty-front/src/modules/object-record/cache/hooks/useCreateOneRecordInCache.ts
, the record is now prefilled before generating GraphQL fields, ensuring all required fields are included in the optimistic cache - In
packages/twenty-front/src/modules/object-record/hooks/useCreateOneRecord.ts
, addedposition: Number.NEGATIVE_INFINITY
to optimistic record input, ensuring new records appear in the table before server response - The changes ensure that optimistic records fulfill all mutation response field requirements, preventing them from being considered invalid
- The fix properly handles the position field which is critical for record ordering in the UI
- These changes maintain backward compatibility while addressing the specific issue where empty records weren't refreshing the view
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
packages/twenty-front/src/modules/object-record/cache/hooks/useCreateOneRecordInCache.ts
Show resolved
Hide resolved
## Introduction This refactor results from this #10493 review Introduced a new abstraction to the extinsting `generateDepthOneRecordGqlFields` that was accepting an optional record in arg in order to map generated `recordGqlFields` to the keys in the record 1/ Created a dedicated util method `generateDepthOneRecordGqlFieldsFromRecord` to do so 2/ Updated each previous `generateDepthOneRecordGqlFields` passing a record to call new `generateDepthOneRecordGqlFieldsFromRecord`
… that has the field (#10510) # Introduction In this PR #10493 introduced a regression on optimistic cache for record creation, by expecting a `position` `fieldMetadataItem` on every `ObjectMetadataItem` which is a wrong assertion Some `Tasks` and `ApiKeys` do not have one ## Fix Dynamically compute optimistic record input position depending on current `ObjectMetadataItem` ## Refactor Refactored a failing test following [jest each](https://jestjs.io/docs/api#describeeachtablename-fn-timeout) pattern to avoid error prone duplicated env tests. Created a "standard' and applied it to an other test also using `it.each`.
Introduction
The record does not appear in the table because the optimistic record input cached does not fulfill the mutation response fields, which result in it being considered as invalid response
close #10199
close #10153