Skip to content

Start Using BeginSignIn command and remove obsolete API command calls (Take 2)#10269

Merged
yuwenmemon merged 3 commits into
mainfrom
revert-10266-revert-10222-yuwen-beginSignIn
Aug 8, 2022
Merged

Start Using BeginSignIn command and remove obsolete API command calls (Take 2)#10269
yuwenmemon merged 3 commits into
mainfrom
revert-10266-revert-10222-yuwen-beginSignIn

Conversation

@yuwenmemon

@yuwenmemon yuwenmemon commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

@robertjchen please review

Second try of #10222

Details

Start using the BeginSignIn command added in https://github.com/Expensify/Web-Expensify/pull/34408

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/218743

Tests

  1. Do a normal sign-in flow for a user that already has a validated account, make sure it works properly:
    https://user-images.githubusercontent.com/4741899/182488142-8044224b-396a-46b2-97a2-69599c0ee76d.mp4

  1. With the same account do the same sign in. Click "Forgot?" on the password form.

  2. Make sure you see the following:
    Screen Shot 2022-08-02 at 4 03 52 PM

  3. And that you get send a password reset link email.

  4. Click the "Resend link" button.

  5. Make sure you get another email


  1. Sign in with an email that DOES NOT already have an account

  2. Make sure that right away you see:
    Screen Shot 2022-08-02 at 4 14 01 PM

  3. ...And that you get an email with a sign-in link:

Screen Shot 2022-08-02 at 4 14 25 PM

  1. Before validating that account by clicking the link in the email, sign-in with that account again, make sure that you get an email with a sign-in link again.

  1. Sign-in with a phone number that does not have an account yet, make sure you see the following:
    Screen Shot 2022-08-02 at 4 16 39 PM

  2. And that you get a text message with your magic sign-in link:
    IMG_8397


Error handling:

  1. Sign in with a number that is not actually a phone number, make sure you get the following error message:

Screen Shot 2022-08-08 at 2 51 13 PM

  1. Sign in with a login that is not actually an email, make sure you get the following error message:

Screen Shot 2022-08-08 at 2 51 49 PM

@yuwenmemon yuwenmemon requested a review from a team as a code owner August 5, 2022 18:12
@yuwenmemon yuwenmemon self-assigned this Aug 5, 2022
@melvin-bot

melvin-bot Bot commented Aug 5, 2022

Copy link
Copy Markdown

Looks like you modified deprecatedAPI.js! To be clear, you should not be adding any code to this file.

Instead, all new API commands should use API.js, and follow our guidelines for writing new API commands.

Unsure if your change is okay? Drop a note in #expensify-open-source!

@yuwenmemon yuwenmemon changed the title [WIP] Start Using BeginSignIn command and remove obsolete API command calls (Round 2) [WIP] Start Using BeginSignIn command and remove obsolete API command calls (Take 2) Aug 5, 2022
@melvin-bot melvin-bot Bot requested review from alex-mechler and removed request for a team August 5, 2022 18:12
@yuwenmemon yuwenmemon changed the title [WIP] Start Using BeginSignIn command and remove obsolete API command calls (Take 2) Start Using BeginSignIn command and remove obsolete API command calls (Take 2) Aug 8, 2022
@yuwenmemon yuwenmemon requested a review from robertjchen August 8, 2022 21:42

@robertjchen robertjchen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@yuwenmemon yuwenmemon merged commit 9a76034 into main Aug 8, 2022
@yuwenmemon yuwenmemon deleted the revert-10266-revert-10222-yuwen-beginSignIn branch August 8, 2022 22:30
@melvin-bot melvin-bot Bot added the Emergency label Aug 8, 2022
@melvin-bot

melvin-bot Bot commented Aug 8, 2022

Copy link
Copy Markdown

@yuwenmemon looks like this was merged without passing tests. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

@OSBotify

OSBotify commented Aug 8, 2022

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

value: {
isLoading: false,
errors: {
[DateUtils.getMicroseconds()]: 'Cannot get account details, please try again',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw this randomly. This is wrong, this is a string that should be localized. Can you send a PR to fix that please?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a link for the context for this? These updates to errors have been breakneck - I want to make sure I have everything straight.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait I see it. Yeah, my bad, updating!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants