-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HOLD for payment 2024-10-14] [$250] Sign in–Magic code expired appears briefly if use Request one here link & login via magic link #47883
Comments
Triggered auto assignment to @greg-schroeder ( |
@greg-schroeder FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
ProposalPlease re-state the problem that we are trying to solve in this issue.The user sees the What is the root cause of that problem?Here in ValidateLoginPage:
This condition returns true even when shouldStartSignInWithValidateCode is true . This caused the ValidateCodeModal to be shown to the user.
What changes do you think we should make in order to solve the problem?By changing the condition to // If session.autoAuthState isn't available yet, we use shouldStartSignInWithValidateCode to conditionally render the component instead of local autoAuthState which contains a default value of NOT_STARTED
const shouldValidateModal = !shouldStartSignInWithValidateCode && autoAuthState === CONST.AUTO_AUTH_STATE.NOT_STARTED && !exitTo;
const isFailed = autoAuthState === CONST.AUTO_AUTH_STATE.FAILED;
const isJustSignedIn2FA = autoAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && is2FARequired && !isSignedIn && login;
const isJustSignedIn = autoAuthState === CONST.AUTO_AUTH_STATE.JUST_SIGNED_IN && isSignedIn && !exitTo && login;
return (
<>
{isFailed && <ExpiredValidateCodeModal />}
{isJustSignedIn2FA && <JustSignedInModal is2FARequired />}
{isJustSignedIn && <JustSignedInModal is2FARequired={false} />}
{shouldValidateModal && (
<ValidateCodeModal
accountID={Number(accountID)}
code={validateCode}
/>
)}
{!shouldValidateModal && !isJustSignedIn && !isJustSignedIn2FA && <FullScreenLoadingIndicator />}
</>
); |
@greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Job added to Upwork: https://www.upwork.com/jobs/~015ba582e21228c322 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat ( |
Looks like we have one proposal to review when you get a chance, Rajat! |
ProposalPlease re-state the problem that we are trying to solve in this issue.The magic code expired is shown briefly when open the magic link for the 2nd time. What is the root cause of that problem?When we open the expired magic link for the first time, the
What changes do you think we should make in order to solve the problem?I suggest that we clear the App/src/components/ValidateCode/ExpiredValidateCodeModal.tsx Lines 38 to 45 in d5cfecf
|
Checking... |
@parasharrajat, @greg-schroeder Huh... This is 4 days overdue. Who can take care of this? |
Couldn't spend much time testing this bug on Friday. Going to look more in the bug before picking up a proposal. |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@parasharrajat @greg-schroeder this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
Let me know how you go @parasharrajat! |
@parasharrajat, @greg-schroeder Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
I will check this tomorrow. |
Thanks for the update @fabioh8010! Looks like @MonilBhavsar you're back up for a new review <3 |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.44-12 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-10-11. 🎊 For reference, here are some details about the assignees on this 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:
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.45-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-10-14. 🎊 For reference, here are some details about the assignees on this 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:
|
Payment summary: Contributor: @bernhardoj - $250 - Can make a manual request via ND to be paid on 10/14 |
Requested in ND. |
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:
Regression Test Steps
Do you agree 👍 or 👎 ? |
Oops - regression period is actually 10/14 because the 10/11 date is for the Onyx bump PR, not the PR that fixes this. Updated |
Payment Summary
BugZero Checklist (@greg-schroeder)
|
All right, these manual requests can be approved per #47883 (comment) |
Adding regression test, closing! |
$250 approved for @bernhardoj |
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: 9.0.24-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4886771
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Action Performed:
Expected Result:
Magic code expired not appears briefly
Actual Result:
Magic code expired appears briefly
Workaround:
Unknown
Platforms:
Screenshots/Videos
Bug6579619_1724356216934.MC.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @greg-schroederThe text was updated successfully, but these errors were encountered: