[HOLD for payment 2024-12-17] [$250] Account setting- Secondary account can't be added unless start over after an error

[lanitochka17]

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.62-2 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

Prerequisite User A start a chat with User B All steps are done as User A

1. Navigate to http://www.staging.new.expensify.com/
2. Navigate to Setting > Profile> add user B as secondary contact
3. After magic code entered notice error for the fail
4. Click X of the error and navigate back once of RHP to edit the email
5. Edit the email to a new account (non existing user)
6. Magic code sent go to email and notice there is no email sent
7. Back to the app and navigate back twice from RHP to "New contact method" button
8. enter the same email address as step 5
9. Go to email and notice email received

Expected Result:Prerequisite

Email of magic code received in step 6 as the magic code is already sent

Actual Result:

Email for the magic code received only if the process steps from start "New contact method"

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/d57adf0e-a6ec-41a0-a009-fe5d739d1434

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021858738599400775577
• Upwork Job ID: 1858738599400775577
• Last Price Increase: 2024-11-19

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot[bot]]

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

[daledah]

Proposal

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

Email for the magic code received only if the process steps from start "New contact method"

What is the root cause of that problem?

• We only call the sendValidateCode() one time:

https://github.com/Expensify/App/blob/753de15139c000ed387be380362733ed3373bb44/src/components/ValidateCodeActionModal/index.tsx#L44-L51

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

• We can expose a function resetFirstRenderRef in:

https://github.com/Expensify/App/blob/753de15139c000ed387be380362733ed3373bb44/src/components/ValidateCodeActionModal/index.tsx#L43

    const resetFirstRenderRef = () => {
        firstRenderRef.current = true;
    };
    useImperativeHandle(ref, () => ({resetFirstRenderRef}), []);

• In:

https://github.com/Expensify/App/blob/753de15139c000ed387be380362733ed3373bb44/src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L153

we can pass the ref prop like:

                <ValidateCodeActionModal
                    ref={validateModalRef}

then can use the ref in:

https://github.com/Expensify/App/blob/753de15139c000ed387be380362733ed3373bb44/src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L52

    const handleValidateMagicCode = useCallback(
        (values: FormOnyxValues<typeof ONYXKEYS.FORMS.NEW_CONTACT_METHOD_FORM>) => {
            if (values.phoneOrEmail !== pendingContactAction?.contactMethod) {
                validateModalRef.current?.resetFirstRenderRef?.();
            }

or

https://github.com/Expensify/App/blob/753de15139c000ed387be380362733ed3373bb44/src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L133

                        <InputWrapper
                            onValueChange={()=>{
                                validateModalRef.current?.resetFirstRenderRef?.();
                            }}

What alternative solutions did you explore? (Optional)

[Shahidullah-Muffakir]

Proposal

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

When a user goes back and again click on the Add, the magic code is not being sent.

What is the root cause of that problem?

1. The root cause stems from the original implementation using firstRenderRef.current which was added to solve the double-sending problem because the ValidateCodeActionModal component renders multiple times when it first mounts

2. Each render was triggering the useEffect, causing multiple sendValidateCode() calls

3. new problem created by the fix

   • The firstRenderRef.current solution only allows the code to be sent on the very first render of the component
   • Once set to false, it never resets, so returning to the modal never triggers a new code send https://github.com/Expensify/App/blob/a6f1348d08a441f4f9b48a8ef05e5e95c96b53d7/src/components/ValidateCodeActionModal/index.tsx#L48

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

We should replace firstRenderRef with hasCodeBeenSentRef that resets when the modal closes:

const hasCodeBeenSentRef = useRef(false);

const hide = useCallback(() => {
    clearError();
    onClose();
    hasCodeBeenSentRef.current = false;  // Reset when modal closes
}, [onClose, clearError]);

useEffect(() => {
    if (!isVisible || hasMagicCodeBeenSent || hasCodeBeenSentRef.current) {
        return;
    }
    hasCodeBeenSentRef.current = true;
    sendValidateCode();
}, [isVisible, sendValidateCode, hasMagicCodeBeenSent]);

What alternative solutions did you explore? (Optional)

we can just add this on the hide function(which is also called when user go back), so when the this page closes, we are setting it back to true: firstRenderRef.current = true;

it is same as my main approach, the difference is just the naming of variable, in the first approach i think the naming of variable is more readable

[huult]

Edited by proposal-police: This proposal was edited at 2024-11-16 06:39:01 UTC.

Proposal

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

Secondary account can't be added unless start over after an error

What is the root cause of that problem?

As per the current logic, we check if it’s not the first render, and if it’s not the first render, we don’t send the validate code. This is to prevent sending the validate code multiple times. https://github.com/Expensify/App/blob/29d054cea0d6e84ec44f522c0d31590a973bfb87/src/components/ValidateCodeActionModal/index.tsx#L52-L59

But when we input the wrong validate code, go back, and change to a new email, firstRenderRef still exists as false because we didn't update it when the email changed. As a result, the function to send the validate code will not execute because it returns early due to firstRenderRef, and this issue occurs.

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

To resolve this issue, we should check if the email has changed, and if so, we must reset firstRenderRef. The code should be modified like this:

1 We need to store the previous phone number or email to compare when the 'Add' button is clicked. We must check this in the add function because the user is able to re-enter the email after it has been changed.

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L51
+    const previousPhoneOrEmail = useRef('');
+    const [isChangedPhoneOrEmail, setIsChangedPhoneOrEmail] = useState(false);

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L52
      const handleValidateMagicCode = useCallback((values: FormOnyxValues<typeof ONYXKEYS.FORMS.NEW_CONTACT_METHOD_FORM>) => {
        const phoneLogin = LoginUtils.getPhoneLogin(values.phoneOrEmail);
        const validateIfnumber = LoginUtils.validateNumber(phoneLogin);
        const submitDetail = (validateIfnumber || values.phoneOrEmail).trim().toLowerCase();

+        setIsChangedPhoneOrEmail(previousPhoneOrEmail?.current !== submitDetail);
+        previousPhoneOrEmail.current = submitDetail;
        User.addPendingContactMethod(submitDetail);
        setIsValidateCodeActionModalVisible(true);
    }, []);
// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L159
      <ValidateCodeActionModal
          ... 
+          isChangedPhoneOrEmail={isChangedPhoneOrEmail}
      />

2 In ValidateCodeActionModal, we need check if isChangedPhoneOrEmail is then we need reset firstRenderRef

// src/components/ValidateCodeActionModal/index.tsx#L45
+    useEffect(() => {
+        if (!isChangedPhoneOrEmail) {
+            return;
+        }
+        firstRenderRef.current = true;
+    }, [isChangedPhoneOrEmail]);

3. We need to reset isChangedPhoneOrEmail after sending the validation code to ensure the email validation works correctly.

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L172
    <ValidateCodeActionModal
            ...
            sendValidateCode={() => {
+                 setIsChangedPhoneOrEmail(false);
                  User.requestValidateCodeAction();
            }}
            descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod})}
            isChangedPhoneOrEmail={isChangedPhoneOrEmail}
      />

Test branch

POC

https://github.com/user-attachments/assets/781ebc91-0f34-41c3-833a-0a548e9d908e

What alternative solutions did you explore? (Optional)

Alternatively, if we want to send the validate code only once, we can add the email that the code was sent to into a list and compare it when the 'Add' button is clicked. This list will reset when the modal is closed.

1. Create a list to add a phone number or email that sent a validation code

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L51
+    const previousPhoneOrEmailList = useRef<string[]>([]);
+    const [isChangedPhoneOrEmail, setIsChangedPhoneOrEmail] = useState(false);

2. Compare the phone number or email in the list. If it's a new phone number or email, set setIsChangedPhoneOrEmail to true to send the validation code; otherwise, set it to false to not send the code.

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L52
    const handleValidateMagicCode = useCallback((values: FormOnyxValues<typeof ONYXKEYS.FORMS.NEW_CONTACT_METHOD_FORM>) => {
        const phoneLogin = LoginUtils.getPhoneLogin(values.phoneOrEmail);
        const validateIfnumber = LoginUtils.validateNumber(phoneLogin);
        const submitDetail = (validateIfnumber || values.phoneOrEmail).trim().toLowerCase();

+        if (!previousPhoneOrEmailList.current.includes(submitDetail)) {
+            setIsChangedPhoneOrEmail(true);
+            previousPhoneOrEmailList.current.push(submitDetail);
+        } else {
+            setIsChangedPhoneOrEmail(false);
+        }
        User.addPendingContactMethod(submitDetail);
        setIsValidateCodeActionModalVisible(true);
    }, []);

3. Send isChangedPhoneOrEmail to ValidateCodeActionModal to determine whether to reset firstRenderRef

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L159
      <ValidateCodeActionModal
          ... 
+          isChangedPhoneOrEmail={isChangedPhoneOrEmail}
      />

4. In ValidateCodeActionModal, we need check if isChangedPhoneOrEmail is then we need reset firstRenderRef

// src/components/ValidateCodeActionModal/index.tsx#L45
+    useEffect(() => {
+        if (!isChangedPhoneOrEmail) {
+            return;
+        }
+        firstRenderRef.current = true;
+    }, [isChangedPhoneOrEmail]);

5. We need to reset isChangedPhoneOrEmail after sending the validation code to ensure the email validation works correctly.

// src/pages/settings/Profile/Contacts/NewContactMethodPage.tsx#L172
    <ValidateCodeActionModal
            ...
            sendValidateCode={() => {
+                 setIsChangedPhoneOrEmail(false);
                  User.requestValidateCodeAction();
            }}
            descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod})}
            isChangedPhoneOrEmail={isChangedPhoneOrEmail}
      />

POC

https://github.com/user-attachments/assets/2622d62a-e453-4a53-aa93-42d0b9364aa9

[melvin-bot[bot]]

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

[strepanier03]

Working on testing this one now.

[strepanier03]

Repro'd with steps as described.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[daledah]

Proposal updated

[thesahindia]

@Shahidullah-Muffakir's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @Shahidullah-Muffakir You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[daledah]

@yuwenmemon @thesahindia The selected proposal does not match the expected behavior in the OP, it always call ResendValidateCode when opening the ValidateCodeActionModal, but the expected behavior is that, we only call "ResendValidateCode" if user enter new email.

[Shahidullah-Muffakir]

@yuwenmemon @thesahindia The selected proposal does not match the expected behavior in the OP, it always call ResendValidateCode when opening the ValidateCodeActionModal, but the expected behavior is that, we only call "ResendValidateCode" if user enter new email.

I don't think it is the expected behavior, as a user if i click on the Add button, i want to receive a majic code, even if the email did not change.

[daledah]

I think we need the confirmation from internal team here

[huult]

I think it should send a magic link only once per email added, similar to the alternative solution in the video. This is useful to avoid spamming by sending too many verification codes

[thesahindia]

@yuwenmemon, could we please confirm the expected result here?

[melvin-bot[bot]]

@yuwenmemon, @strepanier03, @thesahindia, @Shahidullah-Muffakir Huh... This is 4 days overdue. Who can take care of this?

[thesahindia]

@yuwenmemon, could we please confirm the expected result here?

bump @yuwenmemon

[melvin-bot[bot]]

@yuwenmemon @strepanier03 @thesahindia @Shahidullah-Muffakir 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!

[Shahidullah-Muffakir]

Hi @yuwenmemon, when you have a moment, could you confirm the expected behavior for this flow?

Context:

1. A user adds a secondary email and clicks the "Add" button.
2. They are taken to a page to enter a magic code sent to their primary email.
3. If they navigate back to the previous page without changing the email, and click "Add" again, I believe a new magic code should still be sent.

This behavior seems intuitive because the user expects a magic code each time they click "Add." Not sending the code may confuse users, as they could think the feature isn't working.

My proposal here was selected because it ensures the magic code is sent upon each re-entry to the modal, resetting the state appropriately when the modal closes.

[yuwenmemon]

We have a limit on the number of codes we'll send you - what if we allow you to request a new code after 30 seconds, similar to what we have in the login screen?

But to answer your question, yes - I think that should be the expected behavior that each time you navigate into the modal a new validate code is sent to you - we just need to be careful to throttle the user from requesting too many codes at once.

[Shahidullah-Muffakir]

Thanks for confirming, @yuwenmemon.

We have a limit on the number of codes we'll send you - what if we allow you to request a new code after 30 seconds, similar to what we have in the login screen?

We already support the 30-second resend feature. Our current approach will not affect it, as in that case, the user can click the button displayed under the magic code input after 30 seconds. This directly calls ResendValidateCode api endpoint, without relying on the conditions we are adding in the useEffect.

In short, the 30-second resend functionality is already working and will remain unaffected by the changes we're making.

But to answer your question, yes - I think that should be the expected behavior that each time you navigate into the modal a new validate code is sent to you - we just need to be careful to throttle the user from requesting too many codes at once.

The throttling concern is already handled; if a user requests too many codes, they see this error message: Failed to send a new magic code. Please wait a bit and try again. This ensures we limit excessive requests while aligning with the expected behavior you described.

[Shahidullah-Muffakir]

📣 @Shahidullah-Muffakir You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

The PR will be ready for review within 24 hrs.

[melvin-bot[bot]]

@yuwenmemon, @strepanier03, @thesahindia, @Shahidullah-Muffakir Eep! 4 days overdue now. Issues have feelings too...

[Shahidullah-Muffakir]

@thesahindia PR https://github.com/Expensify/App/pull/53462 is ready for review, Thank you.

[melvin-bot[bot]]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[Shahidullah-Muffakir]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

This doesn't appear to be a regression. The issue persists even after reverting the changes from the PR. Thanks.

[thesahindia]

Thanks for verifying!