Navigate to onboarding workspace #admins room on small screens#95752
Navigate to onboarding workspace #admins room on small screens#95752MelvinBot wants to merge 4 commits into
Conversation
Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
I looked into the failing checks — neither is caused by this PR, so I didn't push a fix. This PR only touches 1. 2. That error is in Per the guidelines I don't fix unrelated/pre-existing failures on a PR. This one needs a fix on Checks that passed / are informational
|
|
Update: I re-checked — the state is unchanged, and I still can't push a fix from this PR because neither failing check is caused by (or fixable from) this branch. I did pin down the exact root cause of the
Fix (must land on Why I can't push it from this PR:
So there's nothing for me to push on this PR. The typecheck break needs a quick fix on |
| // When a workspace was created during onboarding, take the user to its #admins room. This applies to both | ||
| // screen sizes: on small screens the #admins room belongs to the onboarding policy, so the findLastAccessedReport | ||
| // fallback below would filter it out and leave the user on HOME instead. |
There was a problem hiding this comment.
@MelvinBot we can remove this comment as it is already covered in a function comment
There was a problem hiding this comment.
Done — removed the block comment in 1b52a3b since the function JSDoc already covers that behavior.
Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
…nsRoomSmallScreen Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
|
🤖 Pushed a fix for the failing checks. Root cause: The failing This PR only touches Fix: Merged the latest Details
|
|
🤖 The fix is already in — no further push needed. I resolved this in my previous comment: the failing Current status: |
|
🤖 ✅ Confirmed fixed — The only remaining failing check is No further action needed from me. |
|
@MelvinBot we can refactor and simplify with below changes Remove
|
…ortID at call site Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
|
Done in 4735299. Removed
(session?.email ?? '').includes('+') ? conciergeReportID : adminsChatReportID,The other three callers ( One behavior nuance to confirm: for test accounts ( |
|
🤖 Status update — the Since my earlier comment, you pushed a reworked commit (
No further action needed from me. |
|
@MelvinBot check for any regression @codex review and check for any regression |
Regression check — no functional regressions found ✅I reviewed the refactor that removes the Call sites are all consistent. All 4 callers of Behavioral diff vs
|
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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 update the test steps too so QA can test accordingly |
|
🤖 Updated the Tests and QA Steps in the PR description to match the refactor ( Key changes for QA:
The |
|
🤖 The fix worked — As explained in my earlier comment, the typecheck error was a pre-existing failure in The only remaining red check is |
|
@MelvinBot mark this ready for review |
|
Done — marked this PR as ready for review. 🚀 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-07-10.at.11.27.42.movSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-07-10.at.11.27.00.moviOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-07-10.at.11.23.22.movScreen.Recording.2026-07-10.at.11.22.41.mov |
Pujan92
left a comment
There was a problem hiding this comment.
- Native(with normal account)
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2026-07-10.at.11.27.00.mov
- Native(with test account)
Simulator.Screen.Recording.-.iPhone.15.Pro.-.2026-07-10.at.11.27.42.mov
- Web(with normal account)
Screen.Recording.2026-07-10.at.11.22.41.mov
- Web(with test account)
Screen.Recording.2026-07-10.at.11.23.22.mov
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #94994 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
|
@MelvinBot complete the author checklist by just ticking all the checkboxes to pass the failing check. I will do the needed in my reviewer checklist |
|
Done — ticked all 38 boxes in the PR Author Checklist section so the |
trjExpensify
left a comment
There was a problem hiding this comment.
#94994 (comment) not sure if this is intentional or not, confirming here before we proceed.
Explanation of Change
After onboarding through "Manage my team's expenses" creates a workspace, the user should land on the new workspace's
#adminsroom. This worked on large screens but not on small screens (Android/native), where the user was dropped on Home instead.The destination is decided by
getReportIDAfterOnboardinginsrc/libs/navigateAfterOnboarding.ts. The two screen sizes were asymmetric:onboardingAdminsChatReportIDdirectly.onboardingAdminsChatReportIDand instead derived the target fromfindLastAccessedReport, which filters out any report whosepolicyID === onboardingPolicyID. The just-created#adminsroom belongs toonboardingPolicyID, so it was always excluded, the function returnedundefined, andnavigateAfterOnboardingfell back toROUTES.HOME.This change hoists the existing admins-room check above the
isSmallScreenWidthsplit, so when a workspace was created during onboarding, the user is navigated to its#adminsroom on both screen sizes. The caller now owns the target-report decision: theshouldPreventOpenAdminRoomboolean was removed and each call site passes the report ID it wants. For+test accounts,BaseOnboardingInterestedFeaturespassesconciergeReportIDinstead of the admins-room ID, so those accounts land on Concierge (where the onboarding tasks live) on both screen sizes.Unit tests in
tests/unit/navigateAfterOnboardingTest.tswere updated to exercise thefindLastAccessedReportfallback with no onboarding admins room, the small-screen#adminscase, and the+-email → Concierge case.Fixed Issues
$ #94994
PROPOSAL: #94994 (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.
+email) on a small-screen device (Android native or narrow layout).#adminsroom (not Home).#adminsroom.+email (e.g.you+test@domain.com) and go through the same "Manage my team's expenses" flow. Verify you land on Concierge (where the onboarding tasks are) on both small and large screens — not Home.Offline tests
N/A — this changes only post-onboarding navigation target selection.
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.
Use a non-
+account for the primary checks below (a+test account is now expected to route to Concierge — see step 4).+account and go through "Manage my team's expenses" > "1-4 employees" > "QuickBooks Online" > "Continue".#admins) report — not Home.#adminsroom.+email and confirm you land on Concierge (not Home / not the#adminsroom).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.