[HOLD for payment 2023-09-06] [$1000] Web - Inconsistent error messages text on wrong magic code #22017

kbecciv · 2023-07-01T14:38:49Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Signout if you are logged in
Enter an email > continue
Enter wrong magic code
Login
Go to contact methods
Add an email > verify
Enter wrong magic code

Expected Result:

same error message

Actual Result:

Inconsistent error messages text on wrong magic code

Workaround:

Unknown

Platforms:

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

Version Number: 1.3.35.4
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
Notes/Photos/Videos: Any additional supporting documentation

bandicam_2023-06-28_15-18-13-488.mp4

bandicam_2023-06-28_15-19-37-915-1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @chiragxarora
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688221719614829

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0150dd5082869ecc1c
Upwork Job ID: 1675276504517607424
Last Price Increase: 2023-07-22
Automatic offers:

abdulrahuman5196 | Reviewer | 26274048
chiragxarora | Reporter | 26274049

melvin-bot · 2023-07-01T14:38:56Z

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-07-01T14:38:57Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

kbecciv · 2023-07-01T14:39:32Z

Proposal

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

inconsistent error message on incorrect magic code

What is the root cause of that problem?

Root cause of the issue is the different error messages being passed from backend. When user enters an incorrect code while signing in, different error message comes from backend, when wrong code on secondary login, different error message.

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

We should make the error message consistent on backend, we should pick one error message out of two and use it on both flows.
We should use the one which says "incorrect magic code, please try again or request a new one" because other message mentioning code expiry doesn't make much sense

Results

### What alternative solutions did you explore? (Optional) None

MitchExpensify · 2023-07-01T22:51:49Z

Agree this is expected: "Incorrect magic code, please try again or request a new one"

melvin-bot · 2023-07-01T22:53:40Z

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

melvin-bot · 2023-07-01T22:53:43Z

Current assignee @MitchExpensify is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-07-01T22:53:49Z

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

abdulrahuman5196 · 2023-07-02T07:30:22Z

@chiragxarora Could you provide references for your proposal? #22017 (comment)

From code I could see places where we are setting the magic code error locally. Gentle suggestion would be to provide references and more information in proposal, so that we don't have to assume certain parts of proposal.

chiragxarora · 2023-07-02T08:44:48Z

Updated Proposal to #22017 (comment)

PROPOSAL

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

inconsistent error message on incorrect magic code

What is the root cause of that problem?

Root cause of the issue is backend sending different error messages for the similar process as shown in the videos above.
And while displaying the error,

for signin flow, we pick the error from onyx which has the error received from the backend

App/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.js

Line 251 in fa473e9

    
           {hasError && <FormHelpMessage message={ErrorUtils.getLatestErrorMessage(props.account)} />}

for secondary login flow, we also display the error from onyx which first has the error message received from backend

App/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.js

Line 171 in e9fa9a9

errors={ErrorUtils.getEarliestErrorField(loginData, 'validateLogin')}

but we see a third different error because we have populated the failure data with an extra error field here while making the api call which we haven't done in signin flow

App/src/libs/actions/User.js

Lines 441 to 454 in fa473e9

    
           { 
        
               onyxMethod: Onyx.METHOD.MERGE, 
        
               key: ONYXKEYS.LOGIN_LIST, 
        
               value: { 
        
                   [contactMethod]: { 
        
                       errorFields: { 
        
                           validateLogin: ErrorUtils.getMicroSecondOnyxError('contacts.genericFailureMessages.validateSecondaryLogin'), 
        
                       }, 
        
                       pendingFields: { 
        
                           validateLogin: null, 
        
                       }, 
        
                   }, 
        
               }, 
        
           },

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

To make it completely consistent across frontend and backend, we need to make 2 changes here,

firstly, we should remove/replace the errorField from the failure data and let the onyx use the error message received from backend while validating code
second change would be at the backend which is real rca. As shown in the networks tab attachments of both flow above, we can see that backend is sending different error messages,
- Incorrect magic code. Please try again or request a new code.
- This magic code is invalid or has expired, please request a new code and try again.
  We need to display one error message which is Incorrect magic code. Please try again or request a new code. at both the places
  This second change is important as you can see that both the error messages are currently being stored in onyx db and on slow devices/network, second error shows up

What alternative solutions did you explore? (Optional)

None

alitoshmatov · 2023-07-02T13:39:41Z

I think backend should change the error message, because right now it is showing what backend is sending in error

App/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.js

Line 168 in c614ed9

errors={ErrorUtils.getLatestErrorField(loginData, 'validateLogin')}

HezekielT · 2023-07-03T10:26:05Z

Proposal

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

Inconsistent error messages text on wrong magic code.

What is root cause of that problem?

The error message we get by calling ValidateSecondaryLogin api call and SigninUser api call is not the same – that is the error message returned by ValidateSecondaryLogin is
Failed to validate contact method with given magic code. Please request a new code and try again.
while SigninUser api call returns
Incorrect magic code. Please try again or request a new code.
Which caused the inconsistency.

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

We need to change the error message we get when calling ValidateSecondaryLogin to
Incorrect magic code, please try again or request a new code. .

I noticed that we set the error fields for contactMethod in failureData of ValidateSecondaryLogin api method as shown below.

App/src/libs/actions/User.js

Lines 447 to 455 in b0add94

    
           const failureData = [ 
        
               { 
        
                   onyxMethod: Onyx.METHOD.MERGE, 
        
                   key: ONYXKEYS.LOGIN_LIST, 
        
                   value: { 
        
                       [contactMethod]: { 
        
                           errorFields: { 
        
                               validateLogin: ErrorUtils.getMicroSecondOnyxError('contacts.genericFailureMessages.validateSecondaryLogin'), 
        
                           },

Where the value of contacts.genericFailureMessages.validateSecondaryLogin
in en.js is

App/src/languages/en.js

Line 445 in b0add94

    
           validateSecondaryLogin: 'Failed to validate contact method with given magic code. Please request a new code and try again.',

so we have to change this generic failure message to
Incorrect magic code, please try again or request a new code. in en.js and its correct translation in es.js

Result

Login page error message

Contact Page error message

What alternative solutions did you explore? (Optional)

None

melvin-bot · 2023-07-05T20:10:37Z

@MitchExpensify, @abdulrahuman5196 Whoops! This issue is 2 days overdue. Let's get this updated quick!

MitchExpensify · 2023-07-07T01:33:42Z

We have a fresh proposal for you to review @abdulrahuman5196

melvin-bot · 2023-07-08T16:00:03Z

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

abdulrahuman5196 · 2023-07-08T16:34:18Z

@MitchExpensify

In the signin page we show This magic code is invalid or has expired, please request a new code and try again which is got from backend

In contact verification page we show Failed to validate contact method with given magic code. Please request a new code and try again. which is shown with local data.

Which one should be the right one to maintain consistency? Based on that we can make the change.

HezekielT · 2023-07-08T16:45:15Z

@abdulrahuman5196 I think the expected one is Incorrect magic code, please try again or request a new code. according to this comment

MitchExpensify · 2023-07-10T21:31:30Z

This is expected: "Incorrect magic code, please try again or request a new one"

nishancx · 2023-07-11T03:01:10Z

I think this has been fixed, error message while verifying contact method has been replaced by a more appropriate message.

abdulrahuman5196 · 2023-07-12T20:01:11Z

The text is changed but still inconsistent as per the OP. Correct me if wrong. @MitchExpensify

HezekielT · 2023-08-23T12:43:27Z

@abdulrahuman5196 PR #25762 is ready for review. Thanks!

chiragxarora · 2023-08-23T16:48:18Z

bumping @cead22 for response to this #22017 (comment)

cead22 · 2023-08-23T23:07:15Z

The proposal is complete because we only expect front end changes in the proposal, or at least that's my understanding. If you feel differently, I think you should feel encouraged to start a discussion about this in #expensfy-open-source, but I don't think it's worth changing this decision after having selected a proposal and having a PR submitted

chiragxarora · 2023-08-24T06:09:39Z

The PR has a one line change so I don't think that should be the reason to stick to the same decision.

I'm commenting here because in my proposal, I had suggested the complete solution (frontend plus backend (and why it's needed)) which I think is accepted

Anyways I'm okay with whatever decision is made

cc @cead22

melvin-bot · 2023-08-28T08:19:46Z

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

when @HezekielT got assigned: 2023-08-23 01:21:59 Z
when the PR got merged: 2023-08-28 08:19:45 UTC
days elapsed: 3

On to the next one 🚀

flodnv · 2023-08-28T08:22:00Z

@MitchExpensify this is actually eligible for the bonus. The only reason for the delay was different timezones and working hours.

MitchExpensify · 2023-08-29T01:51:00Z

Payment summary:

Reporting: @chiragxarora $250
C: @HezekielT $1500
C+: @abdulrahuman5196 $1500

melvin-bot · 2023-08-30T01:13:23Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-08-30T01:13:28Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.58-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: Inconsistent error messages shown on wrong magic code #25762

If no regressions arise, payment will be issued on 2023-09-06. 🎊

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.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@HezekielT requires payment
@abdulrahuman5196 requires payment offer (Reviewer)
@chiragxarora requires payment offer (Reporter)

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-08-30T01:13:30Z

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:

[@abdulrahuman5196] The PR that introduced the bug has been identified. Link to the PR: NA - "Not a regression. We have made the error message consistent." [HOLD for payment 2023-09-06] [$1000] Web - Inconsistent error messages text on wrong magic code #22017 (comment)
[@abdulrahuman5196] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: NA
[@abdulrahuman5196] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: NA
[@abdulrahuman5196] Determine if we should create a regression test for this bug.
[@abdulrahuman5196] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@MitchExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/316080

abdulrahuman5196 · 2023-09-04T10:32:12Z

The PR that introduced the bug has been identified. Link to the PR:
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

Not a regression. We have made the error message consistent.

Determine if we should create a regression test for this bug.
If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression test needs to be added. But can only be added after backend fix is also done. So we can add regression test part of that.

@MitchExpensify / @cead22 Could you kindly create separate issue for backend string update if required?

MitchExpensify · 2023-09-06T14:13:56Z

@MitchExpensify / @cead22 Could you kindly create separate issue for backend string update if required?

Is this update required do you think? @cead22

MitchExpensify · 2023-09-06T16:02:14Z

Reporting: @chiragxarora $250 - Paid and contract ended
C: @HezekielT $1500 - Please accept the Upwork offer 👍
C+: @abdulrahuman5196 $1500 - Paid and contract ended

HezekielT · 2023-09-06T16:04:16Z

@MitchExpensify Accepted the offer. Thanks!

MitchExpensify · 2023-09-06T17:01:29Z

Thanks @HezekielT , Paid and contract ended 👍

cead22 · 2023-09-07T00:00:02Z

Is this update required do you think? @cead22

Not really since the PR to fix the backend message is already merged and deployed. @abdulrahuman5196 given that can you add the regression test now?

abdulrahuman5196 · 2023-09-11T17:58:28Z

Regression test:

Go to login
Enter your email address and click continue.
Add incorrect magic codes in the magic code inputs.
Verify that an error messages that says Incorrect or invalid magic code. Please try again or request a new code. is displayed.
Now enter the correct magic code and login.
Navigate to Settings => Profile => Contact methods
Click on New contact method.
Enter your email and click on Add option.
From Contact methods options click on Please verify this contact method.
Add incorrect magic codes in the magic code inputs.
Verify that an error messages that says Incorrect or invalid magic code. Please try again or request a new code. is displayed.
Change the language to Spanish.
Repeat step 10 and 11.
Verify that an error messages that says Código mágico incorrecto o no válido. Inténtalo de nuevo o solicita otro código. is displayed in spanish.

@cead22 @MitchExpensify

MitchExpensify · 2023-09-12T14:44:45Z

Added the test request. Thanks all! Closing

kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 1, 2023

melvin-bot bot assigned MitchExpensify Jul 1, 2023

MitchExpensify added the External Added to denote the issue can be worked on by a contributor label Jul 1, 2023

melvin-bot bot changed the title ~~Web - Inconsistent error messages text on wrong magic code~~ [$1000] Web - Inconsistent error messages text on wrong magic code Jul 1, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 1, 2023

melvin-bot bot assigned abdulrahuman5196 Jul 1, 2023

melvin-bot bot added the Overdue label Jul 4, 2023

melvin-bot bot removed the Overdue label Jul 7, 2023

melvin-bot bot added the Overdue label Jul 10, 2023

melvin-bot bot removed the Overdue label Jul 10, 2023

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Aug 23, 2023

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 30, 2023

melvin-bot bot changed the title ~~[$1000] Web - Inconsistent error messages text on wrong magic code~~ [HOLD for payment 2023-09-06] [$1000] Web - Inconsistent error messages text on wrong magic code Aug 30, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Aug 30, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 5, 2023

melvin-bot bot added the Overdue label Sep 11, 2023

melvin-bot bot removed the Overdue label Sep 11, 2023

MitchExpensify closed this as completed Sep 12, 2023

[HOLD for payment 2023-09-06] [$1000] Web - Inconsistent error messages text on wrong magic code #22017

[HOLD for payment 2023-09-06] [$1000] Web - Inconsistent error messages text on wrong magic code #22017

Comments

kbecciv commented Jul 1, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jul 1, 2023

melvin-bot bot commented Jul 1, 2023 • edited by MitchExpensify Loading

Bug0 Triage Checklist (Main S/O)

kbecciv commented Jul 1, 2023

Proposal

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

What is the root cause of that problem?

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

MitchExpensify commented Jul 1, 2023 • edited Loading

melvin-bot bot commented Jul 1, 2023

melvin-bot bot commented Jul 1, 2023

melvin-bot bot commented Jul 1, 2023

abdulrahuman5196 commented Jul 2, 2023

chiragxarora commented Jul 2, 2023 • edited Loading

PROPOSAL

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

alitoshmatov commented Jul 2, 2023

HezekielT commented Jul 3, 2023 • edited Loading

Proposal

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

What is root cause of that problem?

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

Result

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jul 5, 2023

MitchExpensify commented Jul 7, 2023

melvin-bot bot commented Jul 8, 2023

abdulrahuman5196 commented Jul 8, 2023

HezekielT commented Jul 8, 2023 • edited Loading

MitchExpensify commented Jul 10, 2023

nishancx commented Jul 11, 2023

abdulrahuman5196 commented Jul 12, 2023

HezekielT commented Aug 23, 2023

chiragxarora commented Aug 23, 2023

cead22 commented Aug 23, 2023

chiragxarora commented Aug 24, 2023

melvin-bot bot commented Aug 28, 2023

flodnv commented Aug 28, 2023

MitchExpensify commented Aug 29, 2023

melvin-bot bot commented Aug 30, 2023

melvin-bot bot commented Aug 30, 2023

melvin-bot bot commented Aug 30, 2023 • edited by MitchExpensify Loading

abdulrahuman5196 commented Sep 4, 2023

MitchExpensify commented Sep 6, 2023 • edited Loading

MitchExpensify commented Sep 6, 2023

HezekielT commented Sep 6, 2023

MitchExpensify commented Sep 6, 2023

cead22 commented Sep 7, 2023

abdulrahuman5196 commented Sep 11, 2023

MitchExpensify commented Sep 12, 2023

kbecciv commented Jul 1, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jul 1, 2023 •

edited by MitchExpensify

Loading

MitchExpensify commented Jul 1, 2023 •

edited

Loading

chiragxarora commented Jul 2, 2023 •

edited

Loading

HezekielT commented Jul 3, 2023 •

edited

Loading

HezekielT commented Jul 8, 2023 •

edited

Loading

melvin-bot bot commented Aug 30, 2023 •

edited by MitchExpensify

Loading

MitchExpensify commented Sep 6, 2023 •

edited

Loading