Skip to content
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 #49445][$250] Profile - The incorrect magic code sign is not displayed when wrong code is inserted #50653

Closed
1 of 6 tasks
lanitochka17 opened this issue Oct 11, 2024 · 12 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Oct 11, 2024

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.48
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

  1. Go to staging.new.expensify.com
  2. Click "Account setting" > "Profile"
  3. Click "Contact method"
  4. Click "New Contact Method" and enter a secondary email address that has not been used to sign in to an Expensify account.
  5. observe that Magic code has been sent to your email
  6. Insert wrong magic code

Expected Result:

An incorrect magic code sign is shown, allowing the user to easily identify the error and edit the magic code

Actual Result:

The error message that says "Failed to add the contact method" will be shown, but the exact error will not be visible to the user

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6623778_1727998570348.Bug_video_05.mp4

fail

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021846210499801072215
  • Upwork Job ID: 1846210499801072215
  • Last Price Increase: 2024-10-15
Issue OwnerCurrent Issue Owner: @akinwale
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 11, 2024
Copy link

melvin-bot bot commented Oct 11, 2024

Triggered auto assignment to @OfstadC (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@Shahidullah-Muffakir
Copy link
Contributor

Shahidullah-Muffakir commented Oct 12, 2024

Edited by proposal-police: This proposal was edited at 2023-10-06T15:45:00Z.

Proposal

Please re-state the problem that we are trying to solve in this issue.

When adding a new contact method (email), if the user enters an incorrect Magic code during the email verification step, no clear error message is displayed. Instead, the user is redirected to the next page without being informed that the Magic code was wrong. The expected behavior is to show a clear error message and prevent the user from moving forward.

What is the root cause of that problem?

As we are using addNewContactMethod action in the NewContactMethodPage:

User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);

  1. In the addNewContactMethod function, when there’s a failure, the error is saved under the addedLogin key:
    Here
  2. In the NewContactMethodPage component, the code is incorrectly accessing the validateLogin key, which always returns an empty object, rather than the addedLogin key:
    Here
    and Here
  3. we are not memoizing this clearError function using useCallback:
    clearError={() => User.clearContactMethodErrors(contactMethod, 'validateLogin')}

    which case to the run the following useEffect in each state update:
    useEffect(() => {
    clearError();
    }, [clearError]);

    so even if there are errors, this useEffect is clearing them.

As a result, the error object remains empty, and the user is redirected without seeing the error.

What changes do you think we should make in order to solve the problem?

  1. Update the NewContactMethodPage component to use the addedLogin key instead of validateLogin in both places.

  2. Modify the error message in the addNewContactMethod function:
    Update this line: here
    to
    ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('contacts.genericFailureMessages.validateSecondaryLogin')

  3. if there is an error don't allow user to go to the next page,
    add this check:
    if(validateLoginError) return;
    here:

    User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);

    as:

  const addNewContactMethod = useCallback(
       (magicCode: string) => {
           User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);
           if(validateLoginError) return;
           Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.route);
       },
       [pendingContactAction?.contactMethod],
   );
  1. wrap this clearError function in the useCallback:
    clearError={() => User.clearContactMethodErrors(contactMethod, 'validateLogin')}
new.mov

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Oct 14, 2024

Testing now

@melvin-bot melvin-bot bot removed the Overdue label Oct 14, 2024
@OfstadC OfstadC added the External Added to denote the issue can be worked on by a contributor label Oct 15, 2024
@melvin-bot melvin-bot bot changed the title Profile - The incorrect magic code sign is not displayed when wrong code is inserted [$250] Profile - The incorrect magic code sign is not displayed when wrong code is inserted Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021846210499801072215

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 15, 2024
Copy link

melvin-bot bot commented Oct 15, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

@mkzie2
Copy link
Contributor

mkzie2 commented Oct 15, 2024

Proposal

Please re-state the problem that we are trying to solve in this issue.

The error message that says "Failed to add the contact method" will be shown, but the exact error will not be visible to the user

What is the root cause of that problem?

We always navigate back to the list contact method page after we submit the magic code

const addNewContactMethod = useCallback(
(magicCode: string) => {
User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);
Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.route);
},
[pendingContactAction?.contactMethod],
);

Another problem here is we added the addedLogin error and pending field here and here

But we get the error with validateLogin here

const validateLoginError = ErrorUtils.getEarliestErrorField(loginData, 'validateLogin');

What changes do you think we should make in order to solve the problem?

We should only navigate back to the list contact method page if we enter the correct magic code.

  1. Add a new callback in addNewContactMethod function and call the callback after the API is complete
function addNewContactMethod(contactMethod: string, validateCode = '', callback = () => {}) {
...
API.write(WRITE_COMMANDS.ADD_NEW_CONTACT_METHOD, parameters, {optimisticData, successData, failureData}).then(callback);
  1. Move the call Navigate here as the callback param to addNewContactMethod function and only navigate back if we don't have any error
const addNewContactMethod = useCallback(
    (magicCode: string) => {
        User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode, () => {
            if (Object.values(validateLoginError).length) {
                return;
            }
            Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.route);
        });
    },
    [pendingContactAction?.contactMethod, validateLoginError],
);

const addNewContactMethod = useCallback(
(magicCode: string) => {
User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);
Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.route);
},
[pendingContactAction?.contactMethod],
);

  1. We should get the error from addedLogin field here or we can change the field in addNewContactMethod to validateLogin

const validateLoginError = ErrorUtils.getEarliestErrorField(loginData, 'validateLogin');

What alternative solutions did you explore? (Optional)

For points 1 and 2 we can return the promise in addNewContactMethod and do the navigate logic in this promise

const addNewContactMethod = useCallback(
(magicCode: string) => {
User.addNewContactMethod(pendingContactAction?.contactMethod ?? '', magicCode);
Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHODS.route);
},
[pendingContactAction?.contactMethod],
);

@OfstadC
Copy link
Contributor

OfstadC commented Oct 17, 2024

@akinwale Could you review the proposals by Monday? Thank you! 😃

@melvin-bot melvin-bot bot added the Overdue label Oct 17, 2024
@Shahidullah-Muffakir
Copy link
Contributor

Since refactoring is currently taking place on the same files and they are implementing a different modal, I suggest we hold off on this issue until those changes are completed.
PR

@hungvu193
Copy link
Contributor

Yes, @OfstadC please hold this one for #49445, we fixed this issue there

@OfstadC OfstadC changed the title [$250] Profile - The incorrect magic code sign is not displayed when wrong code is inserted [Hold for #49445][$250] Profile - The incorrect magic code sign is not displayed when wrong code is inserted Oct 18, 2024
@OfstadC OfstadC added Weekly KSv2 and removed Daily KSv2 labels Oct 18, 2024
@melvin-bot melvin-bot bot removed the Overdue label Oct 18, 2024
@OfstadC
Copy link
Contributor

OfstadC commented Oct 25, 2024

@hungvu193 - looks like #49445 has been deployed. Do we need to do anything else for this issue? Or does that cover this fix?

@hungvu193
Copy link
Contributor

Verified it's fixed on Staging. We can close this issue 😄

Screen.Recording.2024-10-25.at.21.44.29.mov

@OfstadC
Copy link
Contributor

OfstadC commented Oct 25, 2024

Thank youuu!

@OfstadC OfstadC closed this as completed Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Weekly KSv2
Projects
Development

No branches or pull requests

6 participants