[C+ Checklist Needs Completion] [$250] Contact method - Secondary contact method can not select Set as default

[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.13-1
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. Login with new gmail account
2. Go to Account settings> Profile> Contact method> New contact method> Add new email> Add
3. Click on newly added secondary contact method> Complete the verification process
4. Click on the verified secondary contact method> Set as default

Expected Result:

There should be no error message. Since the secondary contact method becomes primary contact method - Set as default option should not be present

Actual Result:

"This is already your primary contact" error message appears

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

Bug6554036_1722022412832.Recording__3615.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~012a16b21d55b907d8
• Upwork Job ID: 1818663412557629626
• Last Price Increase: 2024-08-07
• Automatic offers:
• ikevin127 | Reviewer | 103456590

Issue Owner

Current Issue Owner: @ikevin127

[melvin-bot]

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

[lanitochka17]

We think that this bug might be related to #vip-vsp

[lanitochka17]

@greg-schroeder FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[bernhardoj]

Proposal

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

Can't select secondary method as default on a new non-validated account.

What is the root cause of that problem?

When we have a non-validated account, add a secondary login, and then verify the secondary login, the BE actually sets the secondary login as the primary login. It can be verified from an email received after verifying, telling that the primary login has been changed to the secondary login. So, when we try to set the secondary login as the primary login, the BE responds with a message that the secondary login is already the primary login.

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

If the BE behavior is expected, then we can fix this on the FE. What we want to do is, when validating a secondary login, if the primary login isn't validated yet, set the secondary login as the primary login on success verifying.

The loginList will be passed from the function params.

App/src/libs/actions/User.ts

Line 368 in 9901ca3

function validateSecondaryLogin(contactMethod: string, validateCode: string) {

if (!loginList[currentEmail].validatedDate) {
    successData.push(...[
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.ACCOUNT,
            value: {
                primaryLogin: contactMethod,
            },
        },
        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: ONYXKEYS.SESSION,
            value: {
                email: newDefaultContactMethod,
            },
        },
        ...,
    ]);
}

The data that requires update can be seen from the setContactMethodAsDefault optimistic data. (except the login list)

[melvin-bot]

@greg-schroeder Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[ikevin127]

⚠️ @bernhardoj I was not able to successfully verify if your solution fixes the issue, this is what I did (see diff below) to test the solution and the issue is still present. Please let me know if I missed something!

Diff

diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts
index 3019e3dfbb6..ea9fa1e6a60 100644
--- a/src/libs/actions/User.ts
+++ b/src/libs/actions/User.ts
@@ -365,7 +365,7 @@ function validateLogin(accountID: number, validateCode: string) {
 /**
  * Validates a secondary login / contact method
  */
-function validateSecondaryLogin(contactMethod: string, validateCode: string) {
+function validateSecondaryLogin(contactMethod: string, validateCode: string, loginList: Record<string, Login>) {
     const optimisticData: OnyxUpdate[] = [
         {
             onyxMethod: Onyx.METHOD.MERGE,
@@ -444,6 +444,20 @@ function validateSecondaryLogin(contactMethod: string, validateCode: string) {
         },
     ];
 
+    if (!loginList[currentEmail].validatedDate) {
+        successData.push(
+            ...[
+                {
+                    onyxMethod: Onyx.METHOD.MERGE,
+                    key: ONYXKEYS.ACCOUNT,
+                    value: {
+                        primaryLogin: contactMethod,
+                    },
+                },
+            ],
+        );
+    }
+
     const parameters: ValidateSecondaryLoginParams = {partnerUserID: contactMethod, validateCode};
 
     API.write(WRITE_COMMANDS.VALIDATE_SECONDARY_LOGIN, parameters, {optimisticData, successData, failureData});
diff --git a/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.tsx b/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.tsx
index 352260c275f..0ed81cddcfb 100644
--- a/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.tsx
+++ b/src/pages/settings/Profile/Contacts/ValidateCodeForm/BaseValidateCodeForm.tsx
@@ -178,8 +178,8 @@ function BaseValidateCodeForm({account = {}, contactMethod, hasMagicCodeBeenSent
         }
 
         setFormError({});
-        User.validateSecondaryLogin(contactMethod, validateCode);
-    }, [validateCode, contactMethod]);
+        User.validateSecondaryLogin(contactMethod, validateCode, loginList);
+    }, [validateCode, contactMethod, loginList]);
 
     return (
         <>

[bernhardoj]

Oh my bad, the ONYXKEYS.ACCOUNT is duplicated, it should be

{
    onyxMethod: Onyx.METHOD.MERGE,
    key: ONYXKEYS.SESSION,
    value: {
        email: newDefaultContactMethod,
    },
},

This will make sure the secondary login will be the default contact method.

I have updated my proposal to fix that.

[ikevin127]

Thanks for clarifying!

@bernhardoj's updated proposal looks good to me. The RCA is on point and the updated solution fixes the issue in accordance with the Expected result.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

@greg-schroeder, @roryabraham, @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[ikevin127]

We're currently awaiting Rory's input on the reviewed / selected proposal.

[melvin-bot]

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

[ikevin127]

Yes we do!

cc @roryabraham for assignment based on #46350 (comment).

[melvin-bot]

📣 @ikevin127 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[bernhardoj]

PR is ready

cc: @ikevin127

[ikevin127]

ℹ️ Will review on Saturday (PST) when I'll return to office.

[greg-schroeder]

PR merged. Awaiting deploy to staging -> prod

[ikevin127]

⚠️ Production deploy automation failed here -> this should be on paid on 2024-08-20 according to yesterday’s production deploy confirmed in #47132 (comment) and deploy checklist.

cc @greg-schroeder

[greg-schroeder]

Sure, adjusted

[greg-schroeder]

Payment summary:

Contributor: @bernhardoj - $250 - You can make a manual request via ND
C+: @ikevin127 - $250 - Paid via Upwork

[greg-schroeder]

@ikevin127 the automation didn't past the checklist, but can you propose a regression test if one is required here? Thanks!

[bernhardoj]

Requested in ND.

[ikevin127]

@greg-schroeder Thanks for the payment 💯

• [@ikevin127] The PR that introduced the bug has been identified. Link to the PR: Fix stuck navigation & Re-add Contact Method Details pages #15751
• [@ikevin127] 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: https://github.com/Expensify/App/pull/15751/files#r1723731216
• [@ikevin127] 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:N/A.
• [@ikevin127] Determine if we should create a regression test for this bug.
• [@ikevin127] 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 Proposal

1. Login as a new user with a new email address.
2. Add a secondary contact method and validate it.
3. Verify the secondary contact method is assigned as the default contact method.
4. Open the contact method page again and now verify the other contact method.
5. Set the other contact method as default.
6. Verify the other contact method is set as default.
7. Repeat step 5 and 6.
8. Verify that the contact method selected at step (5.) is set as default.

Do we agree 👍 or 👎.

[JmillsExpensify]

$250 approved for @bernhardoj

[melvin-bot]

@greg-schroeder @roryabraham @bernhardoj @ikevin127 this issue is now 4 weeks old, please consider:

• Finding a contributor to fix the bug
• Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[greg-schroeder]

Filed regression, payments done. closing