[HOLD for payment 2023-07-21] [$1000] Login - Inconsistent copy in error message on 2FA entry screen

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Launch New Expensify app.
2. Enter email with 2FA enabled.
3. Enter magic code.
4. Tap Sign in without entering 2FA code.
5. Enter incorrect 2FA code and tap Sign in.

Expected Result:

The term used for 2FA in the error message in Step 4 and 5 should be the same.

Actual Result:

Error message in
Step 4 - Please enter your two-factor code.
Step 5 - Incorrect Two Factor Authentication code. Please try again.

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.30.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6102839_Screen_Recording_20230622_224816_New_Expensify.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0181c49131acb5e082
• Upwork Job ID: 1673380295588896768
• Last Price Increase: 2023-06-26

[melvin-bot]

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

[melvin-bot]

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

[Pujan92]

Proposal

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

Missing 'Authentication' word in error message in 2FA entry screen

What is the root cause of that problem?

In the pleaseFillTwoFactorAuth translation we don't have 'authentication' word within a sentence.

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

If this is a valid inconsistent bug then we can update the translations for en.js and es.js for below key.

App/src/languages/en.js

Line 558 in a777544

pleaseFillTwoFactorAuth: 'Please enter your two-factor code',

Please enter your two-factor authentication code

App/src/languages/es.js

Line 558 in a777544

pleaseFillTwoFactorAuth: 'Por favor, introduce tu código 2 factores',

[sonialiap]

Agreed, let's make it consistent.

Question, why do we have two error codes that say almost the same thing. Do we not have a library of errors that we can reference in both step 4 and 5? It would be more organized to write the error once and reference it in various places, wouldn't it? If that's not our standard practice - fine, but if it is then let's make sure that both cases reference the same error

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Santhosh-Sellavel]

@sonialiap Straight forward we just need the updated copy here in both languages, and we can go with @Pujan92 here!

C+ Reviewed
🎀 👀 🎀

[melvin-bot]

Triggered auto assignment to @amyevans, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Santhosh-Sellavel]

We just need updated copies to get started @amyevans!

[amyevans]

I don't agree this is a bug, the error messages communicate different things. In the first case, the user hasn't entered any 2FA code at all, so they're instructed to enter one. In the second case, they entered a code, but it was incorrect, so the error message communicates that. It's a tad subtle, but I think it's helpful and in service of helping the customer. @sonialiap thoughts?

[sonialiap]

Oh I misread the scenarios, I agree, the difference does make sense - let's ignore my suggestion of using the same error message for both!

I do think that it makes sense to standardize on a format for the 2FA name. In one error it's two-factor code in the other it's Two Factor Authentication code

Not a bug, per-say, but something that does make sense to me to clean up. I recommend we standardize on Two Factor Authentication code and update the Please enter your two-factor code. to Please enter your Two Factor Authentication code.

[amyevans]

Sure, cleaning it up a bit sounds good 👍. I am not sure if Two Factor Authentication code is grammatically the correct thing to standardize on, though.

For context, it looks like we're exclusively using the hyphenated, lowercase phrase in the App repo. In Web-E it looks like we're largely standardized on "Two Factor Authentication" but not exclusively. It also seems like moving to capitalization was intentional (in https://github.com/Expensify/Web-Expensify/pull/36159, cc @NikkiWines as I'd be curious for more context). The Incorrect Two Factor Authentication code. Please try again error is coming from Web-E.

It's not a proper noun, so I think lowercase is correct. I think hyphenated is also correct since two-factor is a compound adjective.

So based on that my suggestion would be to update the 3 instances of pleaseFillTwoFactorAuth in App from Please enter your two-factor code to Please enter your two-factor authentication code (and twoFactorCode from Two-factor code to Two-factor authentication code), and in Web-E change Incorrect Two Factor Authentication code. Please try again to Incorrect two-factor authentication code. Please try again. But open to feedback.

[NikkiWines]

IIRC we standardized on Two Factor Authentication when first implementing 2FA in expensify.com, but cc: @bondydaa for clarity on that since he headed up that project. I personally don't mind what version we use, so long as we're consistent across the board 😊

[bondydaa]

here's the last time this was brought up, https://github.com/Expensify/Expensify/issues/175847#issuecomment-910290429.

[Santhosh-Sellavel]

@amyevans Bump!

[amyevans]

Slack thread for internal reference: https://expensify.slack.com/archives/C21FRDWCV/p1688764627188419

In requesting the translation, @iwiznia pointed out that we have a rule

only names and the first words of sentences should be capitalized in new dot

that should trump any older rules we were following for OldDot.

So I think the best course of action is to leave the frontend error messages and translations as they are, and update the backend error (https://github.com/Expensify/Web-Expensify/blob/main/lib/UserAPI.php#L2701) to be Incorrect two-factor authentication code. Please try again.

The method is only used by NewDot, which is a detail I was missing before. So while it might be inconsistent within the code in Web-E, it would not be inconsistent within the app (OldDot would handle it one way, NewDot would handle it another).

[Santhosh-Sellavel]

Alright sorry for all the back and forth here, but once I took a peek at the actual scope of the update, I agree with others that the juice just isn't worth the squeeze. It'd mean touching code in 5+ repos, including OldApp, which we don't really deploy anymore, so it'd leave things in an inconsistent state for our current customers. So let's just standardize on what we've got in the backend (Two Factor Authentication), and update the frontend translation to Please enter your Two Factor Authentication code

We don't want to do this too? Right?

[amyevans]

Right - just update any errors that get surfaced in NewDot flows, do not touch OldDot flows.

[Pujan92]

So do we only need to update two-factor code with two-factor authentication code to make consistency?

App/src/languages/en.js

Line 571 in 6a7c1b2

pleaseFillTwoFactorAuth: 'Please enter your two-factor code',

[amyevans]

Yes, so that can be Please enter your two-factor authentication code /
Por favor, introduce tu código de autenticación de dos factores

[Pujan92]

@Santhosh-Sellavel @amyevans PR is ready for review, Thanks!

[melvin-bot]

🎯 ⚡️ Woah @Santhosh-Sellavel / @Pujan92, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @Pujan92 got assigned: 2023-07-07 15:47:13 Z
• when the PR got merged: 2023-07-11 20:34:28 UTC

On to the next one 🚀

[melvin-bot]

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

[melvin-bot]

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

• Fix: Login - Inconsistent copy in error message on 2FA entry screen #22651

If no regressions arise, payment will be issued on 2023-07-21. 🎊

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

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]

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:

• [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[Santhosh-Sellavel]

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:

• [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Everything was intentionally added earlier so we can skip this @amyevans

[Santhosh-Sellavel]

Request Payment on ND

[anmurali]

Approved 1500 to Santhosh based on #21321 (comment)

[sonialiap]

@Pujan92 offer sent for fix (+bonus)

[Pujan92]

@sonialiap thanks, accepted the offer.

[sonialiap]

Thanks, paid 🚀