Handle cards optimistically when workspace is deleted#92486
Conversation
|
@nyomanjyotisa Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ea294c07d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d04b9ec25
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| if (!hasDeleteWorkspaceExpensifyCardsError && workspaceAccountID) { |
There was a problem hiding this comment.
Allow frozen-card cleanup while offline
When the user deletes a workspace while offline and the workspace only has frozen Expensify Cards, the callers still set hasDeleteWorkspaceExpensifyCardsError because filterInactiveCards keeps frozen suspended cards. This guard then skips the new optimistic card-removal updates, so the frozen cards remain in WORKSPACE_CARDS_LIST/CARD_LIST, which means the advertised offline fix does not work for the exact frozen-card scenario unless the user is online.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
When user is offline we block removing workspace anyway so it doesn't apply (recording from main):
main-.mov
|
Looks like jules is on this one, so unassigning |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Test cases look good to me.
Explanation of Change
The problem is that frozen cards are not removed from Onyx. Looking at the backend code it looks like the update is correctly scheduled. I can't debug it because of WARP related issues. In the PR above I'm removing cards optimistically from Onyx.
Fixed Issues
$ #88380
PROPOSAL: N/A
Tests
Precondition: Workspace has Expensify card feature enabled and a freeze-issued card
Offline tests
Precondition: Workspace has Expensify card feature enabled and a freeze-issued card
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Precondition: Workspace has Expensify card feature enabled and a freeze-issued card
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
frozen-card-removed.mov