Fix infinite loading and missing error on the workspace add work email pages - #100044
Fix infinite loading and missing error on the workspace add work email pages#100044MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: Eric Han <eh2077@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
🤖 Android test steps verification results: I signed in on Android with the gmail.com test account, but I could not exercise the actual fix: both workspace "Add your work email" pages were unreachable on this account. The account has exactly one auto-created Collect workspace with no company card feed, and per the source the work-email step only fires when you select a feed that already exists on another workspace in the same domain. Company cards > "Add cards" hit an account-validation magic-code gate I can't pass, and Expensify Card > "Issue new card" fell through to bank-account setup. The onboarding "What's your work email?" regression check passed.
To make steps 2-5 testable, the account needs a validated login plus a second workspace on the same domain that already has a linked company card / Expensify Card feed — that "other feeds" selection is the only entry point to the "Add your work email" page. |
|
🤖 Web test steps verification results: I verified the account state (0 workspaces, no company card feed anywhere in its domain) but could not reach either workspace "Add your work email" page, so the fix itself was not exercised on web. Two independent blockers: the feed-selector precondition (a card feed already linked to a different workspace in the same domain) cannot be satisfied on a fresh account, and the "Add cards" entry point is gated behind an email magic-code "Validate your account" wall. The onboarding regression step is not reachable either, because this session's account is already onboarded.
Same outcome as the Android run: to make steps 2-5 testable, the account needs a validated login plus a second workspace on the same domain that already has a linked company card / Expensify Card feed — selecting that "other feed" is the only entry point to the "Add your work email" page. view run · no recording available |












Explanation of Change
The workspace "Add your work email" pages hang forever with no error when the backend rejects the request.
AddWorkEmailwas written for onboarding and hardcodes onboarding Onyx keys: it writesisLoadingtoONYXKEYS.FORMS.ONBOARDING_WORK_EMAIL_FORMand routes failures toONBOARDING_ERROR_MESSAGE_TRANSLATION_KEY/NVP_ONBOARDING.isMergingAccountBlocked. The two workspace card pages that also call it bind theirFormProvidertoONYXKEYS.FORMS.ADD_WORK_EMAIL_FORM, which nothing ever writes. So the press spinner never hands off to a real loading flag and never stops, and the failure lands on keys those pages don't render. On top of that, a failure in a workspace card flow flipped persistent onboarding state, which later renders the onboarding "merge blocked" screen.This PR routes the request lifecycle to the form that submitted it:
AddWorkEmailtakes an optionalformIDthat defaults to the onboarding form, so onboarding is unchanged. The optimistic/success/failure data writeisLoadingto that form.ADD_WORK_EMAIL_FORM.errorFields, mapping the specific backend reasons (2FA, SSO, closed work account) to their existing copy and falling back tocommon.genericErrorMessagefor anything else. The backend rejects this command generically in this flow (jsonCode: 666,message: "403 Forbidden"), which is exactly the case that previously produced no feedback at all. Onboarding-only writes are skipped for workspace callers.WorkspaceCompanyCardAddWorkEmailPageandWorkspaceExpensifyCardAddWorkEmailPagepass their form.FormProviderthen stops the spinner and renders the error natively.addErrorMessage({}, …)on a throwaway object, so that error was silently discarded. It now usessetErrorFields, matching the Expensify Card page.Note on approach: making the action return a promise and handling loading/errors per page was considered but conflicts with
DATA-BINDING.md("Action methods SHOULD not return a promise"), and it would leave both pages coupled to onboarding state.Added
tests/unit/AddWorkEmailTest.ts, which covers both callers. 4 of its 6 cases fail onmainand all 6 pass with this change.Fixed Issues
$ #98664
PROPOSAL: #98664 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Automated coverage added by this PR (
npm test -- tests/unit/AddWorkEmailTest.ts):403 Forbidden->addWorkEmailForm.isLoadingisfalseand a generic error is set on the email field.Offline tests
Submitting while offline is already blocked by the page's own validation, which shows "We couldn't add your work email as you appear to be offline". That behavior is unchanged.
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.
Suggested steps, based on the issue's reproduction:
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