Closed thienlnam closed 1 year ago
Triggered auto assignment to @johncschuster (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Platforms
in OP are ✅):wave: Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash
deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
Triggered auto assignment to @jasperhuangg (Engineering
), see https://stackoverflow.com/c/expensify/questions/4319 for more details.
To clarify, deep links are still broken on iOS (they redirect you to the web browser, which shows you a screen telling you what the code is), so this issue should only be reproducible on Android at the moment.
Just reproduced on Android 👍 investigating
Hmm it seems deep links are broken on Android now too, I can't even use the link to deep link to the ValidateLogin page on a physical device
They seem to work with the staging build though.
lol wtf we're calling a function Session.signInWithValidateCodeAndNavigate
https://github.com/Expensify/App/blob/d300d6eb6263ea581f809523c3a9b93121566eab/src/pages/ValidateLoginPage/index.js#L48
but that function just straight up doesn't even exist in our Session.js
lib... no wonder the App just crashes
I'm pretty confident my changes should fix the crash, but I'm unable to test them since I can't get deeplinking to work for me on Android
Deeplinking fix involves a few more changes that I haven't been able to finish testing, will keep at this tomorrow
lol wtf we're calling a function Session.signInWithValidateCodeAndNavigate but that function just straight up doesn't even exist in our Session.js lib... no wonder the App just crashes
:wat: that definitely used to exist, was introduced in https://github.com/Expensify/App/pull/14443 - looking into when it got removed
Aha, looks like it was removed in https://github.com/Expensify/App/pull/15505 but somehow missed the usage in ValidateLoginPage/index.js
? cc: @cristipaval
Hey friends! I'm OOO today. Do you need another non-engineer assigned to this?
Oooh... true @NikkiWines! I can create a quick PR to fix this one. @jasperhuangg do you mind if I open a quick PR for this one?
@cristipaval go for it!
I see that you already have a draft PR, I'll continue on top of yours.
^ Open a separate PR instead so that I can review, since I already made commits to my PR
@cristipaval For the deeplinking not working on native, I created an issue for it here.
Reviewing
label has been removed, please complete the "BugZero Checklist".
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.2.96-4 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-04-14. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
@kbecciv , As I also asked here in Slack discussion, do we have separate regression tests for magick link handling on web vs on mobile native?
cc @johncschuster (this is meant to address the forth checkmark from above)
The solution for this issue has been :rocket: deployed to production :rocket: in version 1.2.97-2 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-04-17. :confetti_ball:
After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
I don't think we need to add regression tests here, this was a bug that should have definitely been caught in review cc @cristipaval what do you think?
We do not have regression tests at all for Automatic Authentication. I will propose some regression tests for it today.
Resend code
option and then click on the new magic linkcc @kavimuru
Aha! Why thank you very much good sir!! Checking now!
Great RT @cristipaval! I updated it a bit to have each step be a bit more clear and succinct. I also left comments in parentheses for you to just make sure the steps are super clear!
Thank you @dylanexpensify ! Could you please check this one?
@johncschuster @cristipaval @jasperhuangg this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
Created an issue to update the regression test. Closing this one.
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Platform: iOS Staging / Android staging Version : v1.2.96-0 (edited)
Action Performed:
Break down in numbered steps
Expected Result:
Describe what you think should've happened It should take you to the 2FA code prompt
Actual Result:
Describe what actually happened Got the Uh-oh something went wrong! page.
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: v1.2.96-0 Reproducible in staging?: Y Reproducible in production?: N 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 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL: Issue reported by: Internal @johnmlee101 Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680796507538369
View all open jobs on GitHub