feat: Replace vacation delegate policy diff warning with RHP invite step by jakubstec · Pull Request #99460 · Expensify/App

jakubstec · 2026-08-25T18:47:03Z

Explanation of Change

Replaces the vacation delegate's blocking "policy diff" warning with an RHP step (VacationDelegateMissingWorkspacesPage). When the delegate isn't a member of every affected workspace, it lets the user invite them straight into the workspaces they admin - any workspace they don't admin still gets its real admins emailed, or skip entirely and just save the delegate, with no invites or emails sent to admin policies.

Fixed Issues

$ #99078
PROPOSAL:

Tests

Go to Settings > Status > Vacation delegate.
Select a user missing only from workspaces you don't administer > verify the RHP shows just the
"member of" section and a single "Confirm" button > tap Confirm > verify the delegate is saved.
Select a user missing only from workspaces you do administer > verify the RHP shows just the "admin
of" section with Invite/Skip buttons > tap Invite > verify the delegate is saved and the user is
added as a member to each listed workspace.
Select a user missing from a mix of both > verify the RHP shows both sections with Invite/Skip
buttons > tap Skip > verify the delegate is saved without adding the user to any workspace.
Repeat any of the above, but dismiss the RHP (back button/swipe) before tapping a button > verify the
delegate reverts to what it was before.

Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

Same as Tests

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

MacOS: Chrome / Safari

Screen.Recording.2026-09-04.at.08.36.53.mov

melvin-bot · 2026-08-25T18:47:08Z

Hey, I noticed you changed src/languages/en.ts in a PR from a fork. For security reasons, translations are not generated automatically for PRs from forks.

If you want to automatically generate translations for other locales, an Expensify employee will have to:

Look at the code and make sure there are no malicious changes.
Run the Generate static translations GitHub workflow. If you have write access and the K2 extension, you can simply click: [this button]

Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running:

npx bun ./scripts/generateTranslations.ts --help

Typically, you'd want to translate only what you changed by running npx bun ./scripts/generateTranslations.ts --compare-ref main

codecov · 2026-08-25T18:50:50Z

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.

Files with missing lines	Coverage Δ
src/ROUTES.ts	`29.39% <ø> (+1.56%)`	⬆️
src/SCREENS.ts	`100.00% <ø> (ø)`
src/components/VacationDelegateMenuItem.tsx	`100.00% <ø> (ø)`
src/libs/API/types.ts	`100.00% <ø> (ø)`
...igation/linkingConfig/RELATIONS/SETTINGS_TO_RHP.ts	`100.00% <ø> (ø)`
src/libs/Navigation/linkingConfig/config.ts	`62.50% <ø> (-8.93%)`	⬇️
src/libs/UserUtils.ts	`84.04% <100.00%> (ø)`
src/libs/actions/Domain.ts	`65.68% <ø> (+1.40%)`	⬆️
src/libs/actions/VacationDelegate.ts	`97.87% <100.00%> (+97.87%)`	⬆️
src/libs/getVacationDelegateErrors.ts	`100.00% <100.00%> (ø)`
... and 10 more
... and 880 files with indirect coverage changes

jakubstec · 2026-08-25T18:51:23Z

@codex review

jakubstec · 2026-08-25T18:51:28Z

@MelvinBot review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7316a47f7e

ℹ️ 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".

MelvinBot · 2026-08-25T18:59:12Z

Review — replace vacation delegate policy diff warning with an RHP invite step

Overall this is a clean, well-tested change. The refactor of setVacationDelegate to an options object, the persisted-write-vs-side-effect split (with the offline reasoning in the comments), and the two new test files (action + UI) are all high quality. CI is green apart from the two ignorable checklist checks. I have one behavior question worth confirming before merge, plus a couple of minor robustness nits.

1. Confirm the "Skip" behavior in the mixed case (member of some + admin of some)

In the mixed diff, the copy notAMemberMixed promises two independent things:

Admins of the workspaces you don't manage will be asked to add them. Do you want to invite them to the ones you are an admin of now?

So the admins of the workspaces you don't manage are supposed to be emailed regardless of the Invite/Skip choice — that choice is only about the workspaces you do administer.

But the "Skip" button calls submit(true), i.e. shouldSkipPolicyInviteEmails: true, which is the single Skip shown for both "admin of all" and "mixed" diffs. If skipPolicyInviteEmails suppresses all invite emails on the backend, then Skip-in-mixed would silently drop the emails to the non-admin workspaces' admins — contradicting the copy. It's only harmless if the flag is scoped on the backend to only the caller's admin workspaces.

Could you confirm the backend semantics of skipPolicyInviteEmails? If it's a global "skip all emails" flag, mixed-case Skip should probably be submit(false) (still email the non-admin workspaces' admins) rather than submit(true).

2. Minor robustness nits

getMenuItemsForPolicies uses policies?.[…]?.name directly for title. If a policy in the diff isn't in Onyx yet, the row renders with an empty title. A fallback (e.g. the policy name or a generic label) would render more gracefully.
In onSelectRow, any non-SUCCESS, non-305 response — including an undefined response — falls into showErrorModal. Worth double-checking the selection row can't be triggered while offline (the side-effect request won't resolve), so a user doesn't get a spurious generic error.

What I checked

Verified component/util signatures used by the new page against main: addMembersToWorkspace (8-arg call matches), getMemberAccountIDsForWorkspace(employeeList, false, false), getAllPolicyExpenseChatReportActions, Button.Text composed pattern, styles.renderHTML, CONST.JSON_CODE.POLICY_DIFF_WARNING = 305.
Confirmed all referenced translation keys exist (common.invite/skip/confirm/vacationDelegate, statusPage.vacationDelegate.*, workspace.common.welcomeNote/workspace).
Walked the state machine: optimistic pendingAction: UPDATE on select → 305 merges policyDiff + clears pendingAction (no error written, avoiding a red brick road) → RHP step → override branch uses a persisted API.write so the delegate can't be dropped offline behind the invites. The unmount rollback + submittedInput snapshot to survive the flow-state null are both covered by tests.
Direct interpolation of the delegate email into <strong>${email}</strong> for RenderHTML matches the established app-wide pattern in en.ts, so no new concern there.
The Invite path (client addMembersToWorkspace per admin policy, then submit(false)) is sequenced correctly behind the persisted queue so the backend only emails the remaining non-admin workspaces — nicely done.

I did not exercise the flow in a live session: reaching the new RHP step requires a 305 policy-diff response, which needs a delegate who is missing from a specific mix of admin/non-admin workspaces — not reproducible against the test account. The mixed-case Invite/Skip path (item 1) is the key scenario to cover in manual QA.

view run

- Debounce rapid delegate-row taps so overlapping SET_VACATION_DELEGATE requests can't race and navigate on a stale response - Keep the missing-workspaces screen rendering from the submitted input instead of Onyx once submit clears the flow state, so it no longer flashes NotFoundPage/the previous delegate while the RHP is closing - Only roll back the delegate on unmount if this screen ever saw an active policy-diff flow, so a stray visit after the flow ended can't wipe out an unrelated, already-saved delegate - Fall back to a placeholder name for a workspace missing from Onyx

jakubstec · 2026-09-01T09:30:32Z

@MelvinBot could you check backend/OldDot parity with the current implementation in this PR? For the "Admin of some" (mixed) case, does Classic send an email to the owners of the workspaces you don't administer when skipping, or does it match this PR's current behavior of sending no email at all?

MelvinBot · 2026-09-01T09:37:08Z

Classic sends the email. In the "admin of some" (mixed) case, Classic emails the owner (with the other admins in cc) of every missing workspace you don't administer, whether you invite now or do it later. It has no way to suppress that — "do it later" issues the same plain SetVacationDelegate(overridePolicyDiffWarning: true) call, and that path always queues those emails. So this PR's mixed Skip → submit(true) → skipPolicyInviteEmails: true → no email at all is a divergence from Classic, and it contradicts your own copy in en.ts:3819-3820 ("Admins of the workspaces you don't manage will be asked to add them").

One more signal that Skip is over-reaching: the backend only ever emails workspaces where you are not an admin. In the "admin of all" case nonAdminPolicies is empty, so there is nothing to suppress and skipPolicyInviteEmails: true is a no-op. The only case where the flag changes observable behavior is the mixed case — exactly the case where Classic sends the emails.

Suggested change: use submit(false) for Skip in the mixed case (keeps Classic parity and matches the copy), leaving submit(true) for the admin-of-all case. That's a one-line change gated on nonAdminPolicies.length === 0.

Caveat before you change it: the FE issue's own spec table says Skip → emails "none", so the divergence may be intentional. dannymcclain read it the other way in #99078 ("the Admins will be notified regardless… Invite and Skip really only apply to workspaces that you're already an admin of"), and trjExpensify never answered your question there. Worth getting an explicit call from him before merging, since the answer also decides whether skipPolicyInviteEmails has any purpose at all.

How I verified this

Classic behavior comes from the internal design issue this FE issue hangs off, which documents all three Classic cases. For the mixed case it states: for the workspaces you're an admin of you can invite now or do it later; for the ones you aren't an admin of, the admins of those workspaces get an email. The "no email is sent to the other workspace admins" line that mountiny quoted in [Vacation Delegate Invite] [FE] Replace the policy diff warning modal with an invite step page in the RHP #99078 belongs to the admin-of-all case, not the mixed one — and in that case there are no non-admin workspaces to email anyway, so it isn't evidence about mixed.
Backend email rule (also written up in the body of [Vacation Delegate Invite] [FE] Replace the policy diff warning modal with an invite step page in the RHP #99078): on the override path the backend emails one owner per missing workspace where you are not an admin, other admins in cc, and only where the vacationer is in that workspace's approval workflow. Workspaces you administer never get an email.
skipPolicyInviteEmails semantics: it is brand new — SetVacationDelegateParams on main has only overridePolicyDiffWarning. The internal backend issue specifies that when it's true the delegate still gets their own email but all invite-request emails to workspace owners and admins are skipped — i.e. it is not scoped to your admin workspaces. The Auth side deployed to production on 2026-08-31.
Current PR state at d6a29c2b: submit(shouldSkipPolicyInviteEmails) is called with true from the single Skip button shared by the admin-of-all and mixed variations, and with false from Confirm and from the end of the Invite loop.

I could not read the Auth/PHP implementations directly in this run (no access to those repos), so the "not scoped to admin workspaces" claim rests on the written backend spec rather than on the merged code. If you want that nailed down, ask rayane-d to confirm against the merged Auth change. I also didn't exercise the flow live — reaching the mixed 305 needs a delegate missing from a specific mix of admin/non-admin workspaces, which isn't reproducible on the test account.

view run

…tic delegate when request rejects, disable invite until every admin policy that is missing is loaded, foramt sms delegate logins as phone numbers, add selectors to onyx subscriptions, switch setDomainVacationDelegate to API.write

…selecting a delegate

…ff-warning-rhp-vacation-delegate

jakubstec · 2026-09-01T11:13:07Z

@MelvinBot could you re-review it?

MelvinBot · 2026-09-01T11:37:09Z

Re-review at `d677b48e`

The three commits since my last pass are solid: HTML-escaping the delegate before RenderHTML, the !!policyDiff gate that closes Codex's rollback finding, offline-blocking the footer, double-press guards, and the API.write ordering fix. Verified green: 20/20 tests pass across the three new files, npm run typecheck clean repo-wide, lint clean on all 32 changed files, React Compiler compliance passes. Only Check independent approval is red (ignorable).

Two things to settle before merge, then some pre-existing bugs this flow now walks straight into.

1. Mixed-case Skip is still unresolved (blocking on a product call)

Unchanged at VacationDelegateMissingWorkspacesPage.tsx:210 — Codex flagged the same thing independently. Per my parity check above, Classic does email the owners of the workspaces you don't administer when you skip, so submit(true) in the mixed case diverges from Classic and from your own copy. Needs trjExpensify's call, not a code change from you yet.

2. Please drop the closure-dependent Onyx selector

VacationDelegateMissingWorkspacesPage.tsx:57-60 subscribes to the whole REPORT_ACTIONS collection through an inline selector that closes over allReports. It is correct — I traced hasSelectorChanged in useOnyx, so no stale data — but it re-scans the entire collection every time allReports changes identity, and policyExpenseChatReportsSelector returns full Report objects, so that happens on every message and read receipt in any policy expense chat — exactly what the ExcludedFields comment 10 lines below it warns about.

The fix is also a simplification: subscribe to COLLECTION.REPORT and COLLECTION.REPORT_ACTIONS raw and call getAllPolicyExpenseChatReportActions(allReports, allReportActions) inside invite(). That's what WorkspaceInviteMessageComponent.tsx:184 — the only other caller of addMembersToWorkspace — does, and all 5 users of that util follow the same shape. It also lets you delete the new selector, which is currently used by this page alone.

3. Two pre-existing bugs the new dismissal path makes easy to hit

Both exist on main today, so not regressions — but this PR adds a new exit path that lands on them, and each is a one-liner. Your call whether to fix here or file follow-ups.

a. Dismissing the RHP can resurrect a deleted delegate. deleteVacationDelegate's successData never clears previousDelegate, and the next setVacationDelegate can't clear it either because it writes previousDelegate: currentDelegate and Onyx's merge skips undefined source properties. So: remove delegate A → select B → get a 305 → back out → clearVacationDelegateError('A') sets delegate: 'A' while the server has none. Fix: previousDelegate: null in the delete successData, or currentDelegate ?? null.

b. A failed final write leaves an unsaved delegate on screen. failureData writes the error but doesn't restore delegate, unlike deleteVacationDelegate's failureData. Status page then shows the new delegate with a dismissible red dot; if the user never taps the X they believe it saved. Adding delegate: currentDelegate makes the optimistic update self-healing.

Nits, and what I checked

Nits

VacationDelegatePage.tsx:55-59 navigates from the .then of an in-flight request with no focus check. Select a delegate, immediately press the header back button, and when the 305 lands the RHP force-pushes over whatever the user is now looking at. isSelectingRef doesn't cover this.
useOnyx at line 54 ignores the loading metadata, so a cold deeplink/refresh onto this route renders one frame of NotFoundPage before Onyx hydrates — and usePreventRemove is disarmed for that frame. Seeding submittedInput once policyDiff first becomes truthy would close it.
invite() doesn't set isSubmittingRef.current = true before its addMembersToWorkspace loop (line 116). Harmless today because the whole handler is synchronous, but if that loop ever throws, partial invites are sent with no delegate write and the ref stays false.
The page omits addMembersToWorkspace's optional 9th approverEmail arg that the invite page passes. Intentional?
The new UI test emits a lot of The action 'REPLACE' ... was not handled by any navigator console noise from goBackToStatus, if you care about clean output.

What I verified

Every symbol and translation key the new page uses resolves with a matching signature — the 8-arg addMembersToWorkspace call is arg-for-arg identical to the invite page's, getPersonalDetailsOnyxDataForOptimisticUsers(newLogins, newAccountIDs, formatPhoneNumber) order is right, and Str.htmlEncode before an HTML translation has direct precedent in ConciergeAutoMatchVendorContent.tsx.
Codex's first P2 (direct-route visit wiping a saved delegate) is fixed: usePreventRemove's preventRemove: false path returns before invoking the callback, so clearVacationDelegateError(undefined) is unreachable without a policyDiff. Re-entrancy is also closed upstream by react-navigation's visited-route-keys stamp.
Double-press orderings (Skip×2, Confirm×2, Invite→Skip, Skip→Invite) are all genuinely guarded; the API.write switch really does keep SetVacationDelegate behind the invites in the sequential queue.
Offline is properly blocked on both pages via FullPageOfflineBlockingView; the goes-offline-mid-request error modal is legitimate, only the copy is generic.
Untested paths, if you want more coverage: no-previous-delegate rollback, deleteVacationDelegate (no test at all), RHP double-press, failureData contents, and offline. The RHP test file stubs usePreventRemove wholesale, so dismissal assertions validate the callback body rather than the navigation contract.

Live test: failed on session infrastructure, not on your code. The browser tab went fully blank (0 DOM nodes) before the tester took control and never recovered; the dev server itself was healthy (HTTP 200, clean build). Console logs aren't obtainable on this web session, so I can't distinguish a headless-tab crash from a client error — but nothing here is evidence against the PR. The mixed 305 path still isn't reachable on a test account anyway.

Step	Status	Observation
Navigate to Settings > Profile > Status > Vacation delegate, no console errors	fail	Render tree empty (0 nodes), screenshot pure white; dev server returned 200 with valid HTML and a clean webpack build, so the blockage was client/session-side
List offers a selectable person, or shows the cannot-set message	fail	Settings/Status never reachable
Selecting a delegate doesn't flash a not-found page and saves	fail	Not reachable
No JS console errors throughout	fail	Not obtainable — log access unsupported for this web session

Evidence (3)

Last known good state: CI prestart reached the sign-up screen at 11:14 UTC

Blank white page when the tester took over at 11:30 UTC

Still blank after waits and a probe press

view run

…isting selector instead of newly created policyExpenseChatReportsSelector, build the policy expense chat report actions at invite time instead of in a per render useOnyx selector, remove no-op guard from delete branch, skip setVacationDelegate response navigation when the screen has lost focus, show loading indicator while vacation delegate is still hydrating

…-warning-rhp-vacation-delegate

… OldDot's

…r clearing up the selected delegate

…ff-warning-rhp-vacation-delegate

…PI and align the styling

mkhutornyi · 2026-09-14T17:27:39Z

@MelvinBot are those regressions caused by this PR, not happening on main?

MelvinBot · 2026-09-14T17:33:09Z

Yes — all four are PR-caused. I re-checked each one by diffing base f777fabb against head 35cac1ff, not just reading the new code. Here's the before/after for each.

One correction first: the screenshot I attached to #1 was not proof. rory@expensifail.com has no display name in that account, and main falls back to the login in that case too — so main would have rendered the same thing. The code and the deleted test are the evidence; the screenshot isn't.

1. Display name → raw login

main VacationDelegatePage.tsx:84 passed option.text into the modal, and line 47 rendered Str.removeSMSDomain(delegateDisplayName ?? delegateLogin). The PR renders formatPhoneNumber(pendingDelegate) (MissingWorkspacesIntro.tsx:26), and pendingDelegate is always a login.

The strongest evidence is the test this PR deleted, which asserted exactly this:

await selectRowAndFlush(EMAIL_DELEGATE, 'Jane Doe');
expect(mockShowConfirmModal).toHaveBeenCalledWith(expect.objectContaining({prompt: 'statusPage.vacationDelegateWarning(Jane Doe)'}));

So on main a delegate with a display name showed "Jane Doe". Delegates without one showed the login on both branches — that part is unchanged.

2. Selection blocked behind the write queue

main's setVacationDelegate was not async and had no barrier — it went straight to makeRequestWithSideEffects at line 60, so prepareRequest applied optimisticData synchronously on tap. The PR inserts await API.waitForWrites(...) at VacationDelegate.ts:94 before that call, so both the optimistic write and the request now wait for the whole sequential queue to drain.

Confirmed by code only — I didn't build an offline backlog and reproduce the visible stall.

3. 305 without `policyDiff`

main VacationDelegatePage.tsx:83-84 gated on jsonCode alone and never read the payload; Confirm then called setVacationDelegate(..., true, ...) at line 54. The PR adds && response.data?.policyDiff (VacationDelegate.ts:102) and routes the else to failureData, and the override is now reachable only through the RHP. That narrowing is real and PR-introduced.

As I said before, Auth does send policyDiff today — my earlier run got the RHP — so this is a missing fallback rather than a live break.

4. Dropped test coverage

The PR deletes the file, so yes. Three of its four cases are covered by the new UI tests; uses the display name for an email account in the warning prompt is the one that isn't, because the behavior it asserted is gone (#1).

5. `shouldHideOnDelete` (the LOW one)

main's VacationDelegateMenuItem passed no shouldHideOnDelete, and OfflineWithFeedback defaults it to true (line 78). The PR sets it to false (line 60), so an online delete keeps the row visible instead of hiding it — on the Domain member details page too, which shares the component.

The two items I labelled speculative both hang off code this PR introduced (the persisted override write, and the new isSelectingRef guard), so they can't happen on main either — but I haven't reproduced either one, so treat them as unproven rather than confirmed.

view run · no recording available

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 35cac1ff22

ℹ️ 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".

mountiny · 2026-09-16T07:07:45Z

@mkhutornyi can you please complete the review?

mkhutornyi · 2026-09-16T07:09:16Z

Waiting for #99460 (comment) and bot comments to be addressed

chatgpt-codex-connector · 2026-09-16T10:43:31Z

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 4f2ef8c145

ℹ️ 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".

MelvinBot · 2026-09-16T10:50:43Z

Reviewed the current head (4f2ef8c) against main: the diff, every resolved review thread, and CI. Nothing blocking, and I found no regressions. All checks are green except Check independent approval, which just needs a second reviewer.

Four things left worth deciding on, all minor. Only the first is one I'd actually fix.

1. The pick shows no feedback while it waits on the write queue

This is the one open Codex comment I'd still act on. await API.waitForWrites(...) runs before the request is prepared, and optimistic data is only applied inside prepareRequest:

So during that await there's no pendingAction on the row, and VacationDelegatePage.tsx:53 has already set isSelectingRef, so further taps are swallowed. The tap looks like it did nothing.

Offline is safe — SequentialQueue.ts:673 resolves isReadyPromise when offline, so it won't hang. But with another persisted write in flight, or the queue paused for a data-gap sync, it's a real dead window on a high-traffic account. Applying the optimistic merge (or a local selecting state that renders the pending row) before the await closes it without giving up the ordering guard.

2. `Invite` can dead-end offline

index.tsx:80 requires a non-empty employeeList, and the only recovery is openWorkspaceMembersPage — an API.read, which is discarded offline. Enter the page offline with an admin workspace whose employeeList never loaded and Invite is disabled with no stated reason, leaving only Skip. That workspace also renders as an "Unavailable" row in the admin section, which reads oddly next to a dead button.

By your own argument on the Codex thread (employeeList only feeds the optimistic #announce room, which the server settles), the guard could be dropped, or kept but paired with a reason in the UI.

3. `policyDiff` + `pendingDelegate` can be stranded in the persisted NVP

Rollback lives only in usePreventRemove, so killing the app on this RHP leaves both fields set indefinitely. Not a correctness bug — VacationDelegatePage.tsx:54 folds policyDiff into hasUnconfirmedChange, and re-picking nulls both. But the Status page never surfaces the parked pick either, so it's dead state that survives restarts. Clearing it on Status page mount would tidy it up if you care.

4. Copy nit

Two contractions for the same phrase in one flow: en.ts:3874 and en.ts:3875 say "is not a member", en.ts:3877 says "isn't a member". All 10 locales were generated off these, so fixing it means regenerating.

What I checked and found clean

Earlier review comments — all genuinely addressed

skipPolicyInviteEmails is gone; Skip and Confirm both send only overridePolicyDiffWarning, so non-admin workspace admins still get emailed. Matches the copy.
The direct-visit rollback bug is fixed: usePreventRemove is gated on !!policyDiff, so an unrelated visit to the URL renders NotFoundPage and never rolls back.
The page is decomposed into MissingWorkspacesIntro / WorkspaceSection / WorkspaceRow / MissingWorkspacesFooter, using the composable MenuItem API and Section children rather than menuItems. The policy lookup happens once per row.
mhn1 is gone from the codebase — alignment is now Section p0/mh0 plus ph5 on the title, matching MenuItem.Root.
The invite note is fully localized via workspace.common.invitedYouToWorkspace + welcomeNote; no hardcoded English left.
API.waitForWrites uses the existing API import.
The offline blocking wrapper is gone, so Invite/Skip/Confirm queue offline as persisted writes.
New-file header comment and comment style both fixed.
All 10 locales carry the 4 new keys; common.confirm/invite/skip all exist.

Correctness of the flow

submit() sets isSubmittingRef before setSubmittedInput, so the usePreventRemove rollback can't fire on its own goBack. submittedInput keeps delegate/policyDiff alive after the write nulls them, so the page doesn't flash NotFoundPage on the way out.
Invite ordering is right: addMembersToWorkspace calls are queued before the persisted SetVacationDelegate, and only the last invite carries successData/failureData for the optimistic personal details.
addMembersToWorkspace and openWorkspaceMembersPage arg order matches their signatures. policyMemberAccountIDs only reaches buildOptimisticAnnounceChat, not reportCreationData — your read on that thread is correct.
getVacationDelegateErrors gating on pendingAction correctly suppresses the 305 errors payload while keeping real failures visible, since every real-failure path clears pendingAction alongside the errors.
Rollback targets are consistent: previousDelegate survives failureData, and hasUnconfirmedChange picks it over delegate when a change is unsettled.
Intro copy branches map correctly to admin-only / non-admin-only / mixed, and the section order matches the mixed sentence.
Delegate name is Str.htmlEncoded before going into RenderHTML.
Route, screen, navigator, linking config and SETTINGS_TO_RHP are all wired; usePreventRemove is on react-navigation 7.1.33 and already used in shared (non-.native) code.
Hooks all run before the early returns, and the useEffect no-ops when policyDiff is absent.
Codex's P1 about the failed-response test is stale — the test was reworked and all 8 Jest shards pass.

Next Steps: Reply with @MelvinBot <your feedback> if you want me to dig into any of these further.

jakubstec · 2026-09-16T11:14:38Z

@mkhutornyi could you take a look? thanks!

mountiny · 2026-09-17T08:48:15Z

+
+WorkspaceSection.displayName = 'WorkspaceSection';


Not needed

Suggested change

WorkspaceSection.displayName = 'WorkspaceSection';

Btw I noticed many new feature PRs keep adding displayName in new components despite the rule already exists in guideline:

https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#function-component-style

Should we add lint rule to automatically prevent this further?

… then

…en then structure

mountiny

Thanks for addressing those

github-actions · 2026-09-17T10:51:04Z

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

OSBotify · 2026-09-17T10:53:30Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-09-17T10:55:03Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #99460.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-c0a5c94-9b6d912-b754c7694e3f483e3e2fa37642871e62bf70542d/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-c0a5c94-9b6d912-d60230030923073f6412fd51a11ab7c47372043f/index.html

Web 🕸️
https://99460.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

OSBotify · 2026-09-17T22:20:04Z

🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.82-0 🚀

platform	result
🕸 web 🕸	success ✅
🤖 android 🤖	cancelled 🔪
🍎 iOS 🍎	cancelled 🔪

MelvinBot · 2026-09-17T22:41:38Z

🤖 Yes, help site changes are required. Draft PR: Docs updates for E/A#99460 vacation delegate missing workspaces invite step

docs/articles/new-expensify/settings/Delegate-when-out-of-office.md only documented the old behavior. Its FAQ said that when a delegate isn't part of your workspace, "Workspace Admins will be notified by email and prompted to add them" — with no mention that you can now invite the delegate yourself from the new RHP step.

What the article now covers:

A new ## How to invite a Vacation delegate who is missing from a workspace section: the two workspace groups (You are an admin of these workspaces: and You are a member of these workspaces:), the three button sets (Invite / Skip, or Confirm alone when you administer none of the listed workspaces), and that leaving the page without choosing one leaves the delegate unchanged.
Corrected assign and remove steps. The old steps told members to click Save and to "clear the delegate's email or phone number" — neither exists. Selecting a person saves immediately, and selecting them again removes them.
The email-only FAQ answer replaced, plus a new Does skipping the invite still save my delegate? entry.

Verification and guideline compliance

Every UI label was verified against a live dev NewDot web session rather than read from the source strings alone: the Account > Profile > Status path, the Vacation delegate row, the Name, email, or phone number field, the You are an admin of these workspaces: heading, and the Invite / Skip buttons. The missing-workspaces step was reproduced by inviting a test account to one workspace and creating a second one it wasn't a member of. The absence of a confirmation prompt on removal was also confirmed live.

Per docs/HELPSITE_NAMING_CONVENTIONS.md, the article now uses the UI-exact Vacation delegate (lowercase "delegate") instead of "Vacation Delegate". Per docs/HELP_AUTHORING_GUIDELINES.md the file was brought to a single # title plus # FAQ, task-based ## headings, and the required internalScope metadata — it previously failed all three.

No other article needed changes. The Expensify Classic delegate article documents Classic, which this PR doesn't touch, and the cross-link from Manage-Copilot-Access.md still resolves.

@war-in, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR Ready for review

view run · view recording

mitarachim · 2026-09-18T00:40:29Z

Deploy Blocker #101498 was identified to be related to this PR.

feat: Replace vacation delegate policy diff warning with RHP invite step

7316a47

melvin-bot Bot assigned jakubstec Aug 25, 2026

jakubstec mentioned this pull request Aug 25, 2026

[Vacation Delegate Invite] [FE] Replace the policy diff warning modal with an invite step page in the RHP #99078

Open

chatgpt-codex-connector Bot reviewed Aug 25, 2026

View reviewed changes

Comment thread src/pages/settings/Profile/CustomStatus/VacationDelegateMissingWorkspacesPage.tsx Outdated

Comment thread src/pages/settings/Profile/CustomStatus/VacationDelegateMissingWorkspacesPage.tsx Outdated

rayane-d assigned war-in Aug 26, 2026

jakubstec added 3 commits September 1, 2026 12:19

fix: race condition causing not found page appearing on the rhp when …

58c4eba

…selecting a delegate

Merge remote-tracking branch 'upstream/main' into jakubstec/policy-di…

d677b48

…ff-warning-rhp-vacation-delegate

jakubstec added 5 commits September 1, 2026 14:25

Merge remote-tracking branch 'origin/main' into jakubstec/policy-diff…

a4a3524

…-warning-rhp-vacation-delegate

chore: align sending emails to non-admin workspace owners behavior to…

457fd18

… OldDot's

fix: pre-existing bug causing vacation delegate row to disappear afte…

32a13a3

…r clearing up the selected delegate

fix: request race condition causing not found page bug

49bfd30

war-in reviewed Sep 4, 2026

View reviewed changes

jakubstec added 2 commits September 7, 2026 08:44

Merge remote-tracking branch 'upstream/main' into jakubstec/policy-di…

1e11823

…ff-warning-rhp-vacation-delegate

fix: handle request failure properly, refactor to the new MenuItems A…

bea0666

…PI and align the styling

jakubstec marked this pull request as ready for review September 7, 2026 07:42

jakubstec requested review from a team as code owners September 7, 2026 07:42

tgolen previously approved these changes Sep 14, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Sep 14, 2026

View reviewed changes

Comment thread src/pages/settings/Profile/CustomStatus/VacationDelegateMissingWorkspacesPage/index.tsx Outdated

jakubstec added 3 commits September 16, 2026 10:40

fix: remove hardcoded welcome message

1d81018

fix: regression causing showing email names instead of full names

0dcdb41

fix: address codex comment

4f2ef8c

jakubstec dismissed tgolen’s stale review via 4f2ef8c September 16, 2026 10:37

mkhutornyi approved these changes Sep 16, 2026

View reviewed changes

mountiny requested changes Sep 17, 2026

View reviewed changes

jakubstec added 4 commits September 17, 2026 11:07

fix: remove unneccesary displayName props naming

dc4256a

fix: update test comments, ensure proper format of test: given, when,…

4201657

… then

fix: ensure all test files related to vacation delegate have given wh…

a12c1bc

…en then structure

fix: add missing comments to DomainTest.ts

e75bec3

mountiny approved these changes Sep 17, 2026

View reviewed changes

mountiny merged commit c0a5c94 into Expensify:main Sep 17, 2026
35 of 36 checks passed

OSBotify mentioned this pull request Sep 17, 2026

Deploy Checklist: New Expensify 2026-09-17 #101475

Open

81 tasks

MelvinBot mentioned this pull request Sep 17, 2026

Docs updates for E/A#99460 vacation delegate missing workspaces invite step #101491

Open

40 tasks

applause-bot Bot mentioned this pull request Sep 18, 2026

Vacation delegate - Workspace are not arranged in alphabetical order #101498

Open

7 tasks

Conversation

jakubstec commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Aug 25, 2026

Uh oh!

codecov Bot commented Aug 25, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jakubstec commented Aug 25, 2026

Uh oh!

jakubstec commented Aug 25, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

MelvinBot commented Aug 25, 2026

Review — replace vacation delegate policy diff warning with an RHP invite step

1. Confirm the "Skip" behavior in the mixed case (member of some + admin of some)

2. Minor robustness nits

Uh oh!

jakubstec commented Sep 1, 2026

Uh oh!

MelvinBot commented Sep 1, 2026

Uh oh!

jakubstec commented Sep 1, 2026

Uh oh!

MelvinBot commented Sep 1, 2026

Re-review at d677b48e

1. Mixed-case Skip is still unresolved (blocking on a product call)

2. Please drop the closure-dependent Onyx selector

3. Two pre-existing bugs the new dismissal path makes easy to hit

Last known good state: CI prestart reached the sign-up screen at 11:14 UTC

Blank white page when the tester took over at 11:30 UTC

Still blank after waits and a probe press

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkhutornyi commented Sep 14, 2026

Uh oh!

MelvinBot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Display name → raw login

2. Selection blocked behind the write queue

3. 305 without policyDiff

4. Dropped test coverage

5. shouldHideOnDelete (the LOW one)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

mountiny commented Sep 16, 2026

Uh oh!

mkhutornyi commented Sep 16, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 16, 2026

Uh oh!

MelvinBot commented Sep 16, 2026

1. The pick shows no feedback while it waits on the write queue

2. Invite can dead-end offline

3. policyDiff + pendingDelegate can be stranded in the persisted NVP

4. Copy nit

Uh oh!

jakubstec commented Aug 25, 2026 •

edited

Loading

codecov Bot commented Aug 25, 2026 •

edited by github-actions Bot

Loading

Re-review at `d677b48e`

MelvinBot commented Sep 14, 2026 •

edited

Loading

3. 305 without `policyDiff`

5. `shouldHideOnDelete` (the LOW one)

2. `Invite` can dead-end offline

3. `policyDiff` + `pendingDelegate` can be stranded in the persisted NVP

MelvinBot commented Sep 17, 2026 •

edited

Loading