Expensify
[$500] Sign In - Open in app pop-up not displayed when user successfully Signed In on public room
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.34-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal Team Slack conversation:
Action Performed:
Precondition: user should be signed out
- Open public room deeplink https://staging.new.expensify.com/r/5408450846930023
- Click in Sign In button
- Sign In with valid email
Expected Result:
Pop-up "Open in app" should be displayed
Actual Result:
"Open in app" pop-up not displayed
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [ ] iOS: Native
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/Expensify/App/assets/115492554/e46f7224-660b-4adb-9bab-b15947ca4e0e
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~01b45ef5fa722b5aec
- Upwork Job ID: 1753369448122564608
- Last Price Increase: 2024-02-02
- Automatic offers:
- Ollyws | Reviewer | 28144604
- dukenv0307 | Contributor | 28144605
Job added to Upwork: https://www.upwork.com/jobs/~01b45ef5fa722b5aec
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Ollyws (External)
Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
@izarutskaya What is "Open in app" pop-up ? Please Attach a screenshot of the pop-up to make it clear
Yeah, that's a fair point. It's unclear what that relates to unless you're familiar with the app! @izarutskaya can we make sure to keep an eye on making the action steps accessible for everyone to understand? I think what you're referring to here is the desktop app?
@shahinyan11 if you install our macOS desktop app, when you open a URL on Web, there's usually a centered modal asking if you want to open the (desktop) app instead.
You can download the macOS desktop app from this page: https://new.expensify.com/settings/about/app-download-links
Proposal
Please re-state the problem that we are trying to solve in this issue.
"Open in app" pop-up not displayed
What is the root cause of that problem?
In here, we're setting the hasShownPrompt state to true prematurely without checking all conditions, particularly the Session.isAnonymousUser(), which is only checked once we've initiated the open in desktop prompt here.
So the problem is we didn't really prompt user due to the user is anonymous user at the time, but we set to hasShownPrompt to true already, leading to the prompt not showing again once the user is properly logged in.
What changes do you think we should make in order to solve the problem?
Add || Session.isAnonymousUser() to the check here so it will early return there and doesn't set hasShownPrompt to true if all the conditions for showing the prompt are not met.
If there're other similar such conditions that are checked after setHasShownPrompt(true);, it needs to be moved there also.
What alternative solutions did you explore? (Optional)
Not strictly neceessary but we can also add session (or specifically session?.authTokenType) to the dependency list here if we want the useEffect to always be evaluated again when anonymous/not-anonymous status changes.
Another approach is setHasShownPrompt(true) should only be called when openRouteInDesktopApp is actually triggered, like in here, we can pass setHasShownPrompt down as param to relevant function and call it in those places. But this will be more complex.
Triggered auto assignment to @marcochavezf, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
Thanks for the review @Ollyws, I agree with the solution. Assigning @dukenv0307 🚀
📣 @Ollyws 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!
📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!
Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑💻 Keep in mind: Code of Conduct | Contributing 📖
@trjExpensify The PR was deployed to production last week. I think we're ready for payment now.
Nice! Confirming the payment summary as follows:
- @dukenv0307 $500 for the fix - paid!
- @Ollyws $500 for the C+ review - you need to accept the offer.