[FIX] Optimistically compute position only for objectMetadataItem that has the field
#10510
Conversation
prastoin
changed the title
objectMetadataItem that has the fieldposition only for objectMetadataItem that has the field
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR fixes a regression in optimistic cache handling for record creation by correctly identifying objects that have position fields.
- Added new utility
hasObjectMetadataItemPositionFieldin/packages/twenty-front/src/modules/object-metadata/utils/hasObjectMetadataItemPositionField.tsto check if an object actually has a position field - Removed incorrect
hasPositionField.tswhich assumed all non-remote objects had position fields - Created new utility
computeOptimisticCreateRecordBaseRecordInputto conditionally add position and createdBy fields only when they exist - Updated multiple components to use the new utilities, ensuring position fields are only computed for objects that actually have them
- Renamed
checkObjectMetadataItemHasFieldCreatedBytohasObjectMetadataItemFieldCreatedByfor naming consistency
8 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
...twenty-front/src/modules/object-record/utils/computeOptimisticCreateRecordBaseRecordInput.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
As discussed with @lucasbordeau refactored failing test using jest each pattern and created a standard regarding it and applied to an other test also using it.each
packages/twenty-front/src/modules/object-metadata/utils/hasObjectMetadataItemPositionField.ts
Show resolved
Hide resolved
| } | ||
|
|
||
| if (hasObjectMetadataItemPositionField(objectMetadataItem)) { | ||
| baseRecordInput.position = Number.NEGATIVE_INFINITY; |
There was a problem hiding this comment.
Should we really use this NEGATIVE_INFINITY ?
There was a problem hiding this comment.
Indeed this might seems a bit radical but calculating the next index would quite complex for a very small plus value regarding this solution here
There was a problem hiding this comment.
Note that this isn't sent to the api and will override on api response
…10519) Following this discussion #10510 (comment) with @ijreilly
Introduction
In this PR #10493 introduced a regression on optimistic cache for record creation, by expecting a
positionfieldMetadataItemon everyObjectMetadataItemwhich is a wrong assertionSome
TasksandApiKeysdo not have oneFix
Dynamically compute optimistic record input position depending on current
ObjectMetadataItemRefactor
Refactored a failing test following jest each pattern to avoid error prone duplicated env tests. Created a "standard' and applied it to an other test also using
it.each.