Disable copilot row while its removal is pending offline - #100622
Disable copilot row while its removal is pending offline#100622MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: Roji Philip <rojiphil@users.noreply.github.com>
|
🤖 Android test steps verification results: ❌ fail I couldn't run the flow on Android. The PR body's
To get this verified on Android, the run needs a session whose account is already validated and already has a saved copilot seeded before the run — both account validation and delegate creation end in magic-code screens I can't clear. Until then, the |
|
🤖 Web test steps verification results: ❌ fail I couldn't test the fix on dev NewDot web. The PR body's
To make this PR testable in CI, the run needs an account that is already validated and already has a copilot seeded before the session starts. Both account validation and delegate creation end in magic-code screens I can't clear, so every copilot-row test stops at the same wall — the Android run hit it identically. Until then, the |




Explanation of Change
When you remove a copilot while offline,
removeDelegatekeeps the delegate in the list and marks it withpendingAction: 'delete', setting bothpendingFields.emailandpendingFields.role. The row renders with a strikethrough, but it stays pressable.The row's
onPressinCopilotPagebranched only on the shape ofpendingFields, never on the value ofpendingAction.addDelegatewrites the identical{email, role}shape, so a pending-delete row was indistinguishable from an interrupted add and fell through to the final branch, navigating toSETTINGS_DELEGATE_CONFIRM— an RHP with a tappable access-level selector for a copilot the user just removed.This makes a delegate row non-interactive while its removal is pending: it sets
disabledwhenpendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETEand suppresses the three-dot affordance for that row, so there is no dead-looking control.This mirrors the delegator rows a few lines below in the same file, which already use
disabled: isPending || isCurrentUser, and matches existing precedent for pending-delete rows inDomainsListPage,WorkspacesListPage, andPaymentMethodList.A unit test was added to
tests/unit/pages/settings/CopilotPageTest.tsxcovering the pending-delete row: it asserts the three-dot icon is gone and that pressing the row does not navigate. The test fails without the source change.AI tests run
Run locally against the changed files:
npm run lint-changed— passnpm run typecheck— passnpm run spell-changed -- <changed files>— passnpm run react-compiler-compliance-check check src/pages/settings/Copilot/CopilotPage.tsx— passnpm run test -- --silent tests/unit/pages/settings tests/unit/withAgentAccessDenied.test.tsx— 14 suites / 84 tests passCould not run:
npm run react-compiler-compliance-check check-changed— fails in this environment before it does any checking, withCould not get commit hash for origin/(empty base ref,git merge-baseerror). The single-filecheckform above was run instead. Same environment limitation hitnpm run spell-changedwith no arguments, so the changed files were passed explicitly.A live browser verification of the offline flow was attempted but the test session rendered a blank page and never reached Settings, so there is no screenshot evidence attached. The flow still needs a manual pass.
Fixed Issues
$ #97948
PROPOSAL: #97948 (comment)
Tests
// TODO: The human co-author must fill out the tests they ran before marking this PR as "ready for review".
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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