Repository navigation
Reapply opening a deep link captured before onboarding, without the Concierge regressions - #103681
Conversation
…ing it" This reverts commit 4e385e2, bringing back Expensify#101558 unchanged. The regressions that caused the revert are fixed in the next commit.
A parked /concierge link was replayed by navigating to the concierge route, a URL entry page that shows a skeleton and redirects itself. On wide layouts it slid in as a panel for close to a second after onboarding. The replay now opens the chat with navigateToConciergeChat, as the rest of the app does. When the app starts on /concierge for an account that has not finished onboarding, ConciergePage mounts under the onboarding modal, and its redirect dismisses the top modal, closing onboarding early. It now waits until guided setup is complete.
|
@eh2077 @QichenZhu One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
This PR adds a new |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Tests openReportFromDeepLink's onboarding path: the link is parked instead of opened under onboarding, replayed only after report data loads, opened in the Concierge chat for /concierge, held until a linked report arrives, and dropped for links the app never replays.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbb8e80e48
ℹ️ 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".
The Concierge page waited for the onboarding flag, but workspace members, invited users and other accounts the app never sends to onboarding keep that flag false, so the page stayed on its skeleton. Keep waiting only while onboarding can actually open. Match the parked Concierge link by its parsed route so concierge/ and concierge?x=1 also open the chat directly, and add the missing When/Then comments to the parked link tests.
chuckdries
left a comment
chuckdries
left a comment
There was a problem hiding this comment.
I approve these connectWithoutView uses. Though, since they're one-shot reads that disconnect themselves, they're prime candidates for using Onyx.get on the off chance that #102135 lands first
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-10-15.at.8.56.29.PM.movAndroid: mWeb ChromeScreen.Recording.2026-10-15.at.8.49.04.PM.moviOS: HybridAppScreen.Recording.2026-10-15.at.8.54.22.PM.moviOS: mWeb SafariScreen.Recording.2026-10-15.at.8.50.05.PM.movMacOS: Chrome / SafariScreen.Recording.2026-10-15.at.8.46.18.PM.mov |
one thing though, admins room is still auto read which is half bug mentioned in #102954. And it is not related to Steps
Expected: #admins is unread, it has the Concierge "Welcome to Expensify! What do you want to do first?" message Actual: #admins shows as read. The report has RCA: Desktop hides it because the app opens #admins after onboarding, which refetches the report. So I propose we will raise a separate PR for #102954 later regarding this bug. @luacmartins does this sound good?! cc @eh2077 |
Co-authored-by: Rocio Perez-Cano <[email protected]>
Co-authored-by: Rocio Perez-Cano <[email protected]>
Co-authored-by: Rocio Perez-Cano <[email protected]>
|
@pecanoro could you pls take a look at #102956 (comment) before merging this? Do you think we should include fix for #102956 here? It's related, but not regression from original PR. |
|
@mukhrr I think this PR already included changes to fix #102956 (comment), am I right? |
|
Yes you are |
|
Chatted with @mukhrr on slack, let me try to summarize above comments, there're two relevant issues found in the first version PR.
This PR already included changes to fix it. @mukhrr commented on the issue and mentioned it isn't a regression because it still exists after reverting the first PR. They also asked #103681 (comment) if need to include fix in this PR. I think we should, or it'll block QA testing on Android App (physical device). I'll help comment the expanded scope work about payment if needed later. @pecanoro do you agree? This PR fixed it. During testing this issue, @mukhrr spotted a new issue about unread admin chat as mentioned in #103681 (comment), they investigated with RCA and asked to reopen the closed issue to apply a follow-up fix. While I think it's better file a new issue for it. @luacmartins Wdyt? Other than the above, this PR should be good to merge. |
|
Replied here |
Yeah, I agree, it sounds good to me |
|
🚧 pecanoro has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/pecanoro in version: 9.5.14-0 🚀
|
|
No help site update needed. This PR is a bug fix: a link opened before signing up now opens after onboarding finishes, which is what users already expect, and no article in |
|
Deploy Blocker #104391 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.5.14-3 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
A deep link opened before signing up was dropped once the new account finished onboarding, and the earlier fix was reverted because replaying a
/conciergelink briefly showed the Concierge page's loading skeleton. This reapplies that fix, opens a replayed/conciergelink directly in the chat, and stops the Concierge page from redirecting while onboarding is still open, which on Android closed onboarding early.Fixed Issues
$ #99940
$ #102954
$ #102956
PROPOSAL: #99940 (comment)
Tests
You need email addresses that have never been used to sign up.
/conciergeto the app URL and load it./conciergeapp link so it opens in the app.Note
The deep link is held in memory only, so refreshing the page during onboarding drops it and onboarding ends on the default destination instead of the Concierge chat. This is expected and not a regression.
Offline tests
Completing onboarding writes optimistically, so the parked route is replayed from client state.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged theExpensify/designteam so 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
android.mp4
iOS: Native
iOS: mWeb Safari
ios.mWeb.mp4
MacOS: Chrome / Safari
web1.mp4