-
-
Notifications
You must be signed in to change notification settings - Fork 261
feat: added 'updatedAt' field to claimDraft #7523
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
Conversation
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.
Bug: Updated draft return omits new timestamp
When updating an existing draft, saveOrUpdateClaimDraft writes a merged object with a new updatedAt into state, but returns draft as ClaimDraft (the input), which can omit updatedAt and other persisted fields. This makes the function’s return value inconsistent with state and can break callers expecting the new updatedAt.
packages/claims-controller/src/ClaimsController.ts#L225-L247
core/packages/claims-controller/src/ClaimsController.ts
Lines 225 to 247 in c1748ab
| */ | |
| saveOrUpdateClaimDraft(draft: Partial<ClaimDraft>): ClaimDraft { | |
| const { drafts } = this.state; | |
| const isExistingDraft = drafts.some( | |
| (existingDraft) => | |
| draft.draftId && existingDraft.draftId === draft.draftId, | |
| ); | |
| if (isExistingDraft) { | |
| this.update((state) => { | |
| state.drafts = state.drafts.map((existingDraft) => | |
| existingDraft.draftId === draft.draftId | |
| ? { | |
| ...existingDraft, | |
| ...draft, | |
| updatedAt: new Date().toISOString(), | |
| } | |
| : existingDraft, | |
| ); | |
| }); | |
| return draft as ClaimDraft; | |
| } |
|
@metamaskbot publish-preview |
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.
Bug: Updated draft return omits new timestamp
When updating an existing draft, saveOrUpdateClaimDraft writes updatedAt (and merged fields) into state.drafts, but it returns the input draft cast to ClaimDraft. Callers relying on the return value won’t see the newly generated updatedAt, making the API inconsistent with the “returns the saved draft” behavior used for new drafts.
packages/claims-controller/src/ClaimsController.ts#L233-L247
core/packages/claims-controller/src/ClaimsController.ts
Lines 233 to 247 in 998a0a9
| if (isExistingDraft) { | |
| this.update((state) => { | |
| state.drafts = state.drafts.map((existingDraft) => | |
| existingDraft.draftId === draft.draftId | |
| ? { | |
| ...existingDraft, | |
| ...draft, | |
| updatedAt: new Date().toISOString(), | |
| } | |
| : existingDraft, | |
| ); | |
| }); | |
| return draft as ClaimDraft; | |
| } |
5f79425 to
23cae10
Compare
|
@metamaskbot publish-preview |
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.
Bug: Persisted drafts may lack `updatedAt`
updatedAt is only set when creating or updating via saveOrUpdateClaimDraft, but existing persisted state.drafts from older versions can remain without updatedAt indefinitely (until edited). Consumers that start relying on draft.updatedAt after this change may break or mis-sort drafts for upgraded users.
packages/claims-controller/src/ClaimsController.ts#L225-L273
core/packages/claims-controller/src/ClaimsController.ts
Lines 225 to 273 in 23cae10
| */ | |
| saveOrUpdateClaimDraft(draft: Partial<ClaimDraft>): ClaimDraft { | |
| const { drafts } = this.state; | |
| const isExistingDraft = drafts.some( | |
| (existingDraft) => | |
| draft.draftId && existingDraft.draftId === draft.draftId, | |
| ); | |
| if (isExistingDraft) { | |
| const updatedAt = new Date().toISOString(); | |
| this.update((state) => { | |
| state.drafts = state.drafts.map((existingDraft) => | |
| existingDraft.draftId === draft.draftId | |
| ? { | |
| ...existingDraft, | |
| ...draft, | |
| updatedAt, | |
| } | |
| : existingDraft, | |
| ); | |
| }); | |
| return { ...draft, updatedAt } as ClaimDraft; | |
| } | |
| // generate a new draft id, name and add it to the state | |
| const draftId = `draft-${Date.now()}`; | |
| const newDraft: ClaimDraft = { | |
| ...draft, | |
| draftId, | |
| updatedAt: new Date().toISOString(), | |
| }; | |
| this.update((state) => { | |
| state.drafts.push(newDraft); | |
| }); | |
| return newDraft; | |
| } | |
| /** | |
| * Get the list of claim drafts. | |
| * | |
| * @returns The list of claim drafts. | |
| */ | |
| getClaimDrafts(): ClaimDraft[] { | |
| return this.state.drafts; | |
| } |
packages/claims-controller/src/types.ts#L50-L61
core/packages/claims-controller/src/types.ts
Lines 50 to 61 in 23cae10
| export type ClaimDraft = Partial< | |
| Omit< | |
| Claim, | |
| 'id' | 'shortId' | 'createdAt' | 'intercomId' | 'status' | 'attachments' | |
| > | |
| > & { | |
| /** | |
| * The draft ID. | |
| */ | |
| draftId: string; | |
| }; |
|
Explanation
References
Checklist
Note
Adds an
updatedAttimestamp to claim drafts and updates it on create/update, with tests and changelog updated.updatedAtinClaimDraft(packages/claims-controller/src/types.ts).saveOrUpdateClaimDraftnow setsupdatedAton create and refreshes it on updates; return value includesupdatedAt(packages/claims-controller/src/ClaimsController.ts).updatedAt(packages/claims-controller/src/ClaimsController.test.ts).updatedAtaddition (packages/claims-controller/CHANGELOG.md).Written by Cursor Bugbot for commit 23cae10. This will update automatically on new commits. Configure here.