Scope the distance commuter-exclusion check to the expense destination - #101022
Scope the distance commuter-exclusion check to the expense destination#101022MelvinBot wants to merge 1 commit into
Conversation
Co-authored-by: cretadn22 <cretadn22@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. 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. |
|
The failing check is not caused by this PR, so I haven't pushed anything.
EvidenceThe failure All three failures read a hard-coded route index and get Each one resolves Why it isn't this PR This PR changes exactly three files, none of them navigation:
And the same failure reproduces on unrelated open PRs:
Likely source (not confirmed) These three tests were added by #100387 ("Fix back navigation from Workspace Search results", merged 2026-09-11 18:17 UTC), which changed
Per the "don't fix unrelated failures" rule I've left this alone — it needs a fix on |
|
🤖 Android test steps verification results: ✅ pass The PR's No JS console errors. Two notes on how the run was set upBeta override. "Exclude commutes" is gated on Composer entry point. In both the Self-DM and the workspace expense chat, composer Create expense offers only view run · no recording available |
|
🤖 Web test steps verification results: ✅ pass The PR's
Setup notes and one caveat on console errorsBeta override. The Exclude commutes row is gated behind the Setup prerequisite worth documenting. "Exclude usual commute" cannot be selected until the workspace has a company address ( Entry point. In both the Self-DM and the workspace expense chat, composer Create expense offers only Manual and Scan tabs — no Distance tab. Track distance is the map-distance entry point, and that is the path used for steps 4-8. Console errors: not collected. Browser console capture is unavailable on this web driver, so I cannot assert the "no JS console errors" checklist item. No in-app error banners, red-dot errors, or failed-action states were observed during the run. The observed behavior matches the scoping in view run · no recording available |
| // Odometer tabs do. `policy` here can be the member's active workspace even for a Self-DM track expense, and | ||
| // personal expenses are exempt from a workspace's commuter rule. | ||
| const blockDistanceRequestIfNeeded = useBlockDistanceRequest({ | ||
| policyID: report?.policyID ?? (shouldAutoReportToDefaultWorkspace ? defaultExpensePolicy?.id : undefined), |
There was a problem hiding this comment.
@MelvinBot This will fail if report?.policyID is FAKE, because FAKE is truthy and won't trigger the fallback
There was a problem hiding this comment.
You're right about the ?? semantics — _FAKE_ is truthy, so it short-circuits the fallback. But I traced it and it's a no-op here, for two independent reasons:
1. _FAKE_ and undefined behave identically inside the hook. getBlockReason only uses the ID as a key lookup into policiesRequiringMapOrGPS, which is built from the real POLICY collection:
src/hooks/useBlockDistanceRequest.ts:91
if (!policyIDToCheck || !policiesRequiringMapOrGPS?.[policyIDToCheck] || isEditingExistingDistanceRequest) {No policy is ever stored under _FAKE_, so it bails at the second condition instead of the first. Same result, and no risk of a wrong workspace name leaking into the modal copy.
2. The fallback branch is unreachable when policyID is _FAKE_. The fallback only matters if shouldAutoReportToDefaultWorkspace is true, and that hard-gates on iouType === create:
src/libs/shouldUseDefaultExpensePolicy.ts:18
Every create distance entry point passes a freshly generated report ID that has no Onyx report — e.g. FloatingActionButtonAndPopover.tsx:36 feeding TrackDistanceMenuItem.tsx:53, plus the generateReportID() call sites in QuickCreationActionsBar, SearchActionsBarCreateButton, and useCreateNavigationSuggestions. So report is undefined, report?.policyID is undefined, and the fallback fires as intended. Conversely, when a report actually carries _FAKE_ (Self-DM / DM, from buildOptimisticChatReport's default), iouType is track/submit, so shouldAutoReportToDefaultWorkspace is false and the fallback would resolve to undefined anyway.
I couldn't construct a case where a _FAKE_ report reaches this step with iouType === create. If you know of one, say so and I'll fix it.
One note on scope: this expression is copied verbatim from main — DynamicIOURequestStepDistanceManual.tsx:161 and IOURequestStepDistanceOdometer.tsx:176. If you'd still rather normalize _FAKE_ defensively, I'd do it in all three at once so they don't drift — happy to push that if you want it.
view run · no recording available
The failing test isn't caused by this PR
Root cause: It's a semantic merge conflict: #100387 branched before Fix (verified locally — 14/14 pass): in those 3 tests, add This belongs in its own PR against The other two red checks ( EvidenceEach run below is
This PR's own test file passes: The three broken tests and their
Compare with a passing test in the same file, which includes |
Update: the fix is already up and approved — nothing to do on this branchRe-checked. Situation on this PR is unchanged: What's new: someone already put the fix up — #101024 ("Fix Workspace navigation tests after Insights tab addition", by So the path to green here is: merge #101024, then re-run Heads up: #101016 ("[No QA] Look up the Workspace navigator by name in NavigateTests") is a duplicate of the same fix and is still open — worth closing one of the two. The other two red checks ( |

















Explanation of Change
A member whose default workspace excludes commutes by "Calculate by home and office" was blocked by the "Home address is required" modal when creating a personal map distance expense in their own Self-DM. Personal expenses never reach that workspace, so its commuter rule should not gate them.
Two call sites were checking the rule against the member's default workspace instead of the expense's destination:
IOURequestStepDistanceMap.tsxpassedusePolicyForTransaction's policy straight touseBlockDistanceRequest. For a Self-DM track expense that hook deliberately resolves to the member's active workspace, so tapping Next hit the modal. It now uses the same destination-scoped expression the Manual and Odometer tabs already use —report?.policyID ?? (shouldAutoReportToDefaultWorkspace ? defaultExpensePolicy?.id : undefined)— which resolves to no policy for a Self-DM expense while still blocking a global-FAB flow that will auto-report into a restricted workspace.useExpenseSubmission.tshad the same unscopedpolicyID: policy?.id. Without fixing it the member would just be blocked one screen later on Create expense. It now scopes the check withisPolicyExpenseChat, matching the pattern already used inMoneyRequestConfirmationList.tsx.Expenses that really are going to a restricted workspace still show the modal — that path is unchanged and covered by a test.
Fixed Issues
$ #100969
PROPOSAL: #100969 (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 validate your change worked.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
// The human co-author must fill out the QA tests before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas they need to test for regressions.
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