[$125] mWeb - 2FA - In 2FA magic code page, after refresh briefly error message is displayed

[IuliiaHerets]

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: V9. 0.75-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
Issue reported by: Applause Internal Team
Device used: Redmi note 10s android 13
App Component: User Settings

Action Performed:

Pre-condition: Login with unverified gmail account

1. Go to https://staging.new.expensify.com/home
2. Go to settings- security - two factor authentication
3. Enter incorrect magic code
4. Refresh the page

Expected Result:

In 2FA magic code page, after refresh briefly error message must not be displayed.

Actual Result:

In 2FA magic code page, after refresh briefly error message is displayed.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6691899_1733991788664.Screenrecorder-2024-12-12-13-48-04-520.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021867646284163514204
• Upwork Job ID: 1867646284163514204
• Last Price Increase: 2025-01-03

Issue Owner

Current Issue Owner: @dominictb

[melvin-bot]

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.

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-12-12 13:36:14 UTC.

Proposal

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

2FA - In 2FA magic code page, after refresh briefly error message is displayed

What is the root cause of that problem?

When the page is refreshed while there were error previously the error will only be cleared after the server response of the resend validate code requested here

App/src/components/ValidateCodeActionModal/index.tsx

Lines 44 to 50 in 9603c2f

	useEffect(() => {
	if (!firstRenderRef.current || !isVisible || hasMagicCodeBeenSent) {
	return;
	}
	firstRenderRef.current = false;
	sendValidateCode();

so it will take a while to get cleared

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

We should clear errors on mount (or unmount/useBeforeRemove) just like we clear when user start entering code here

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Lines 177 to 185 in 9603c2f

	const onTextInput = useCallback(
	(text: string) => {
	setValidateCode(text);
	setFormError({});
	if (!isEmptyObject(validateError)) {
	clearError();
	User.clearValidateCodeActionError('actionVerified');
	}

We can do that in BaseValidateCodeForm for more general fix like:

useEffect(() => {
        if (!isEmptyObject(validateError)) {
            clearError();
            User.clearValidateCodeActionError('actionVerified');
        }
    }, []);

Optionally, we might also similarly clear the error here too so that the error will be cleared immediately instead of waiting for BE response on pressing Didn't receive a magic code button

Alternatively, we can clear the errors on mount or before sendValidateCode here

App/src/components/ValidateCodeActionModal/index.tsx

Line 50 in 9603c2f

sendValidateCode();

Despite the claim by the below proposal here is a demo that clearing error on mount approach works well on mobile:

2024-12-12.17-44-00.mp4

### What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

We can also consider clearing errors on User.requestValidateCodeAction but this function is used in several other places and the errors are different in different instances so we can pass clearError callback to the function and run the respective callbacks on the start of the function to clear errors before requesting code resend is requested so that we don't wait to clear errors for the BE response.

[nyomanjyotisa]

Proposal

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

In 2FA magic code page, after refresh briefly error message is displayed

What is the root cause of that problem?

On reload page we need to wait for API call response to clear the errors

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

Clear the errors on mount and on unmount using useEffect hook

    useEffect(() => {
        clearError();

        return () => {
            clearError();
        };
    }, []);

And only allow to show error if validateAndSubmitForm executed, since clearing the errors on mount and on unmount alone still briefly show the error in mWeb Android & iOS(#52303 (comment)). We did the same approach here #52303

const [canShowError, setCanShowError] = useState<boolean>(false);

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Line 193 in c2285e6

const validateAndSubmitForm = useCallback(() => {

const validateAndSubmitForm = useCallback(() => {
        setCanShowError(true);

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Lines 217 to 218 in c2285e6

	errorText={formError?.validateCode ? translate(formError?.validateCode) : ErrorUtils.getLatestErrorMessage(account ?? {})}
	hasError={!isEmptyObject(validateError)}

errorText={canShowError ? formError?.validateCode ? translate(formError?.validateCode) : ErrorUtils.getLatestErrorMessage(account ?? {}) : ''}
hasError={canShowError ? !isEmptyObject(validateError) : false}

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Line 262 in c2285e6

errors={validateError}

errors={canShowError ? validateError : undefined}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

N/A

What alternative solutions did you explore? (Optional)

N/A

[huult]

Edited by proposal-police: This proposal was edited at 2024-12-12 16:40:23 UTC.

Proposal

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

In 2FA magic code page, after refresh briefly error message is displayed

What is the root cause of that problem?

We sent an incorrect magic code to the API, which responds with errorFields, and it is then stored in Onyx.

• 
• 

App/src/pages/settings/Security/TwoFactorAuth/Steps/CodesStep.tsx

Line 57 in 3031b6d

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

When refreshing the 2FA page, the errorFields data still exists (the error is still displayed). After reloading, ResendValidateCode is called. If ResendValidateCode responds without errorFields, the ONYXKEYS.LOGIN_LIST is updated, and the error is removed. However, if ResendValidateCode returns a 402 (sent too many times) error, the errorFields cannot be removed, and the error remains displayed.

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

The validateError appears when we input a valid magic code (6 digits) and submit it to the API for verification. When the page is reloaded, the code is removed. I think we should check if the magic code exists and is valid. If so, we should display the validateError. If it does not exist or is invalid, we should also display the validateError. This will resolve the issue.

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Line 74 in 6a5e1fb

function BaseValidateCodeForm({

Add this code to check validate code valid in BaseValidateCodeForm

    const isValidateCodeValid = !!validateCode.trim() || !!ValidationUtils.isValidValidateCode(validateCode);

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Line 262 in 6a5e1fb

errors={validateError}

To

 errors={isValidateCodeValid ? validateError : undefined}

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Line 218 in 6a5e1fb

hasError={!isEmptyObject(validateError)}

To

  hasError={!isEmptyObject(isValidateCodeValid ? validateError : undefined)}

POC

Screen.Recording.2024-12-12.at.23.38.05.mov

Test branch

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This is UI bug no need the test

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[OfstadC]

I'm currently OoO - but will be back on Tuesday - Reassigning to get the ball rolling, but happy to take back on Tuesday

[melvin-bot]

Triggered auto assignment to @michaelkwardrop (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.

[melvin-bot]

⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.

[melvin-bot]

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

[melvin-bot]

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

[michaelkwardrop]

@dominictb please attempt reproduction and review the above proposals - thanks!

[dominictb]

Will review tomorrow morning.

[melvin-bot]

@OfstadC, @dominictb, @michaelkwardrop Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[michaelkwardrop]

@dominictb lmk if this needs to wait until the new year! 🎄

[dominictb]

on it now, I'm just stuck on other PR

[dominictb]

@nyomanjyotisa's proposal looks promising. Onyx update is the async action and calling func in did-mount happens after rendering the component.

So using canShowError and clear the error on mount are the good ideas to solve the issue and be consistent with this PR

But I just wonder:

1. Should we call clearValidateCodeActionError along with clearError like what we did here?
2. If we use canShowError, I don't think we need to clear error in clean-up function. Please correct me if I miss something.

[huult]

@dominictb Could you review my proposal? thanks

[FitseTLT]

@nyomanjyotisa's proposal looks promising. Onyx update is the async action and calling func in did-mount happens after rendering the component.

So using canShowError and clear the error on mount are the good ideas to solve the issue and be consistent with this PR

But I just wonder:

1. Should we call clearValidateCodeActionError along with clearError like what we did here?
2. If we use canShowError, I don't think we need to clear error in clean-up function. Please correct me if I miss something.

@dominictb Why are we even considering a problem that is hypothetical raised linked to other PR. I have uploaded a video on my proposal as POC that the problem @nyomanjyotisa is claiming doesn't even exist. The approach I am suggesting is used in other places in our code base such as here here @nyomanjyotisa proposal is my proposal plus a solution for a problem that doesn't even exist from my testing. BTW useOnyx is late in loading data I remember issues arising when withFullTransactionOrNotFound HOC was migrated to use useOnyx from withOnyx and transaction was being undefined on useEffects run on render that used to work well with withOnyx and regarding the pr comment linked by @nyomanjyotisa you can see that the migration commit was made later. It might be because we are getting the error data from useOnyx that I am not reproducing the issue he is claiming so we need to make sure that the problem is real in our situation before applying unnecessary workaround solution.
Can you provide an evidence of the brief display of the error occuring in this issue's case?

[dominictb]

@huult Dismissing the error is the common technique we're using in the App so we should do the same.

[dominictb]

@FitseTLT Here's the flow:

1. ValidateError has value
2. Render the component within validateError
3. Logic in useEffect is triggered to clear validateError

The question here is: how do you ensure the validateError is not shown in step 2?

BTW we faced this issue before here, so we need to avoid it.

If you can prove the issue doesn't happen anymore (by plain text not just record the video), I'll consider the decision again. Thanks 🙏

[FitseTLT]

If you can prove the issue doesn't happen anymore (by plain text not just record the video), I'll consider the decision again. Thanks 🙏

I am not saying my video should be taken as evidence but if you guys are sure the problem can also happen here give me a proof otherwise what is the point of applying a workaround solution for an edge case problem that's not even there.
The explanation that I can give you here is

BTW useOnyx is late in loading data I remember an issue arising on mWeb of not auto-populating tag on iou confirmation page when withFullTransactionOrNotFound HOC was migrated to use useOnyx from withOnyx due to transaction being undefined when useEffect executed on rendering but that used to work well with withOnyx and regarding the pr #52303 (comment) linked by @nyomanjyotisa you can see that the useOnys migration commit was made later. It might be because we are getting the error data from useOnyx that I am not reproducing the issue he is claiming and the problem wouldn't occur here because we would be clearing the error even before it is read from onyx in mWeb.

[dominictb]

It doesn't really convince me. Can you show me the code?

[FitseTLT]

It doesn't really convince me. Can you show me the code?

@dominictb Just proceed with your review and let the assigned engineer decide whether we should apply a fix for an invisible bug or not 👍

[melvin-bot]

@OfstadC @dominictb @michaelkwardrop 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!

[melvin-bot]

@OfstadC, @dominictb, @michaelkwardrop Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

@OfstadC, @dominictb, @michaelkwardrop 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[melvin-bot]

@OfstadC, @dominictb, @michaelkwardrop Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

[melvin-bot]

@OfstadC, @dominictb, @michaelkwardrop 10 days overdue. I'm getting more depressed than Marvin.

[melvin-bot]

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