Handle agent deleted on approval workflow in offline mode#92286
Handle agent deleted on approval workflow in offline mode#92286NicolasBonet wants to merge 3 commits into
Conversation
|
🚧 @NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here. |
35ae306 to
e7e3bd8
Compare
|
@mkhutornyi 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] |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7e3bd8ab8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| approvalLimit: employee.approvalLimit, | ||
| overLimitForwardsTo: employee.overLimitForwardsTo, | ||
| overLimitForwardsToDisplayName: getOverLimitForwardsToDisplayName(employee.overLimitForwardsTo, personalDetailsByEmail), | ||
| pendingAction: employee.pendingAction, |
There was a problem hiding this comment.
Propagate delete errors to approver rows
When DeleteAgent fails for a policy, the new failure update stores errors on policy.employeeList[agentEmail], but this conversion only copies pendingAction into the Approver. In the online failure state, ApprovalWorkflowSection passes approver.errors to OfflineWithFeedback; without those errors, a pendingAction: delete row is hidden instead of showing the failed-delete error/RBR, so admins lose the failed approver row on the workflows page. Please copy employee.errors onto the approver alongside pendingAction.
Useful? React with 👍 / 👎.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
@mkhutornyi any news? |
Looks like QA Steps describe the bug, not the fix |
|
Can you please add my account to |
Added
Adjusted |
|
@mkhutornyi waiting for you review 🙏 |
|
Please merge main to fix failing check |
aca2a6a to
9a70999
Compare
@mkhutornyi done, but let's not hold reviews for that reason in the future please |
mkhutornyi
left a comment
There was a problem hiding this comment.
- Tap on approver to open "Edit Approval" RHP.
- Click on "Approver" field where agent is an approver two times to reach "Set Approver".
- Click on agent to remove it as approver.
- Reopen approver field.
- Click on agent again.
- Note that agent is the approver, but with its name crossed-out.
Should we remove these QA steps as failing here?
Otherwise looks good.
|
@neil-marcellini 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] |
|
I checked workspace member (as approver) offline delete case. And noticed a different behavior.
member.movcc: @trjExpensify |
…g delete in associated policies
9a70999 to
0da19f9
Compare
|
@mkhutornyi please provide a step by step of how to reproduce the error and what would be the expected outcome, because based on your video that is a different production bug, not quite what we are resolving in here. |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nothing new. I just followed your Tests steps:
Looks good until Step 11. You can just follow Tests steps you created yourself.
|
|
So this is the flow in this branch: Screen.Recording.2026-06-05.at.3.28.09.PM.movThe problem you have is that the agent is no longer shown in the set approver list around second 43? @mkhutornyi I don't think you should see it anymore, since it cannot become an approver optimistically speaking. |
Right. And I am afraid QA will mark this as regression since agent is shown in set approver list in production. |
ok then remove invalid steps (10 - 15) in Tests. |
On that was the error man, I just changed that. |





Explanation of Change
Shows the agents with their names crossed-out if they are removed in offline mode.
Fixed Issues
$ #92262
PROPOSAL: #92262
Tests
Offline tests
Same as tests
QA Steps
Same as tests
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
Screen.Recording.2026-06-01.at.5.02.30.PM.mov