Repository navigation
Fix agent approver row flicker on reconnect after adding agent offline - #92738
Conversation
…ending Prevents the approver row from briefly flipping to the workspace owner and back to the agent when reconnecting after adding an agent as approver offline. The onReconnect refetch (openPolicyWorkflowsPage) returns the pre-approval-update employeeList and clobbers the optimistic agent-as-approver overlay; skip it while a deferred save for the policy is still pending. Co-authored-by: mkhutornyi <mkhutornyi@users.noreply.github.com>
|
I created this PR from an upstream branch since To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
|
@MelvinBot complete checklist, QA steps (same as linked issue) and mark screenshots as done since they’ll be added in the reviewer checklist. |
|
🤖 Done. I updated the PR description:
|
|
@MelvinBot add and complete 4 missing items in checklist |
|
🤖 Done. The PR Author Checklist was missing 4 items from the current template. I added and checked them off (placed in their canonical position, right after the deeplink item):
|
|
@MelvinBot mark as ready for review |
|
🤖 Done — marked this PR as ready for review. |
|
@thesahindia 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
Code ReviewVerdict: The change is correct, minimal, and well-targeted. No blocking issues found. The guard does exactly what the explanation claims: on reconnect it skips the stale I verified the two things most likely to make this kind of guard silently not work:
One low-severity edge case (non-blocking)If This is acceptable: the admin is already shown an error and is expected to act on it, Same note applies to the initial mount fetch ( The timing-sensitive flicker itself still needs the manual offline→online QA run (requires the |
|
🚧 @NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.4.1-0 🚀
Bundle Size Analysis (Sentry): |
Help site review — no changes requiredI reviewed the changes in this PR against the help site articles under Conclusion: No help site updates are required. Why:
Since no customer-facing behavior changed, I did not open a docs PR. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.4.1-6 🚀
|
Explanation of Change
When an admin adds an agent as the approver while offline and then restores the connection, the Approver row on the Workspace Workflows card briefly flips to the workspace owner and back to the agent.
On reconnect,
WorkspaceWorkflowsPagefiresopenPolicyWorkflowsPageviauseNetwork({onReconnect: fetchData}). That READ returns the pre-approval-updateemployeeList(the agent'ssubmitsTois still the workspace owner, becauseUPDATE_WORKSPACE_APPROVALhasn't been persisted yet). If that response lands afteruseDeferredAgentWorkflowReconciliationhas cleared the optimistic agent-as-approver overlay but before the approval update settles, the card renders the stale server approver (the owner) for a few frames — the blink.This implements Melvin's approved alternative solution: gate the
onReconnectrefetch.fetchDatanow skipsopenPolicyWorkflowsPagewhile aDEFERRED_AGENT_WORKFLOW_SAVESentry for the current policy is still pending, so the stale READ never clobbers the optimistic overlay. The reconciliation path's ownupdateApprovalWorkflowwrite keeps the data fresh, andgetPaymentMethods()still runs unconditionally.AI Tests
Automated checks run locally on the changed file (
src/pages/workspace/workflows/WorkspaceWorkflowsPage.tsx):prettier— clean (no changes)eslint— no new errors (only a pre-existing, grandfatheredrunAfterInteractionsdeprecation warning unrelated to this change)typecheck(tsgo) — passedreact-compiler-compliance-check— file COMPILEDWorkflowUtilsTest,ApprovalWorkflowSectionTest,WorkspaceWorkflowsApprovalsEditPageTest,WorkflowTest)The timing-sensitive reconnect flicker is a manual/QA step (it requires the
CUSTOM_AGENTbeta and the offline→online race) and is left for the human co-author to verify in the steps below.Fixed Issues
$ #92296
PROPOSAL: #92296 (comment)
Tests
CUSTOM_AGENTbeta and sign in with an expensifail account.Offline tests
CUSTOM_AGENTbeta and sign in with an expensifail account.QA Steps
Preconditions: user does not have an agent (enable the
CUSTOM_AGENTbeta).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