Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contact method - "Please enter the magic code sent to..." is missing current email #54714

Closed
5 of 8 tasks
IuliiaHerets opened this issue Jan 1, 2025 · 30 comments
Closed
5 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering

Comments

@IuliiaHerets
Copy link

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.80-1
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from TestRail: exp
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team
Device used: Mac 15.0 / Chrome
App Component: User Settings

Action Performed:

  1. Go to staging.new.expensify.com
  2. Go to Account settings > Profile.
  3. Click Contact method.
  4. Click New contact method.
  5. Enter a new contact method.
  6. Click Next.

Expected Result:

The message "Please enter the magic code sent to..." will include the current email.

Actual Result:

The message "Please enter the magic code sent to . It should arrive within a minute or two." is missing email.

This issue also happens on magic code page when assigning a new card.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6704764_1735725510740.20250101_175459.mp4

View all open jobs on GitHub

@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Bug Something is broken. Auto assigns a BugZero manager. labels Jan 1, 2025
Copy link

melvin-bot bot commented Jan 1, 2025

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

Copy link

melvin-bot bot commented Jan 1, 2025

Triggered auto assignment to @madmax330 (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Jan 1, 2025
Copy link

melvin-bot bot commented Jan 1, 2025

💬 A slack conversation has been started in #expensify-open-source

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Jan 1, 2025
Copy link
Contributor

github-actions bot commented Jan 1, 2025

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@parasharrajat
Copy link
Member

I can't reproduce this. I can think of only reason where it can happen. We use account.primaryLogin but that this key is empty '' until user add displayname on onbarding so it might be the case that you didn't finish the onboarding for this user and trying to add contact. In that case, we will end up with '' string given that we are using ?? operator here.

App/src/libs/UserUtils.ts

Lines 260 to 262 in 701c68d

function getContactMethod(): string {
return account?.primaryLogin ?? session?.email ?? '';
}

To solve this and as a fail safe, we should change the operator to ||.

@madmax330 If you agree, I can create the PR asap.

@jliexpensify
Copy link
Contributor

I don't think this is a deploy blocker, just looks strange in the UI.

@jliexpensify jliexpensify added Daily KSv2 and removed Hourly KSv2 labels Jan 1, 2025
@Beamanator Beamanator removed the DeployBlockerCash This issue or pull request should block deployment label Jan 2, 2025
@Beamanator
Copy link
Contributor

I also couldn't reproduce, so marking this as not a blocker for now

@Beamanator
Copy link
Contributor

Actually what I'm seeing is that we're sending the magic code to the user's primary contact method even when trying to validate a new one 🤔 that seems wrong..

@Beamanator
Copy link
Contributor

Ooh but that's exactly what happens on prod - so i won't close this out, but it's not a blocker

@Kalydosos
Copy link
Contributor

the missing email error is located in the tag of this ticket 9.0.80-1
Capture d’écran de 2025-01-02 17-22-15

it's not present in main because its source was that ONYXKEYS.ACCOUNT was excluded from merges in the tag

function flushQueue(): Promise<void> {
if (!currentAccountID && !CONFIG.IS_TEST_ENV) {
const preservedKeys: OnyxKey[] = [
ONYXKEYS.NVP_TRY_FOCUS_MODE,
ONYXKEYS.PREFERRED_THEME,
ONYXKEYS.NVP_PREFERRED_LOCALE,
ONYXKEYS.SESSION,
ONYXKEYS.IS_LOADING_APP,
ONYXKEYS.CREDENTIALS,
ONYXKEYS.IS_SIDEBAR_LOADED,
];

but not in main

function flushQueue(): Promise<void> {
if (!currentAccountID && !CONFIG.IS_TEST_ENV) {
const preservedKeys: OnyxKey[] = [
ONYXKEYS.NVP_TRY_FOCUS_MODE,
ONYXKEYS.PREFERRED_THEME,
ONYXKEYS.NVP_PREFERRED_LOCALE,
ONYXKEYS.SESSION,
ONYXKEYS.IS_LOADING_APP,
ONYXKEYS.CREDENTIALS,
ONYXKEYS.IS_SIDEBAR_LOADED,
ONYXKEYS.ACCOUNT,
ONYXKEYS.IS_CHECKING_PUBLIC_ROOM,
ONYXKEYS.MODAL,
ONYXKEYS.NETWORK,
ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT,
ONYXKEYS.PRESERVED_USER_SESSION,
];

the wrong email error can be solved by changing the following line from

descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod})}

to

descriptionPrimary={translate('contacts.enterMagicCode', {contactMethod: pendingContactAction?.contactMethod ?? ''})}

but sending the validation code to the new contact method will require BE changes

@Kalydosos
Copy link
Contributor

but sending a validation code to the new contact method and a confirmation code to the current contact method will require changes from FE also

@parasharrajat
Copy link
Member

Actually what I'm seeing is that we're sending the magic code to the user's primary contact method even when trying to validate a new one 🤔 that seems wrong..

@Beamanator This screen the verification screen for the account owner. The contact verification happens later.

@melvin-bot melvin-bot bot added the Overdue label Jan 4, 2025
Copy link

melvin-bot bot commented Jan 7, 2025

@madmax330, @jliexpensify Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Jan 9, 2025

@madmax330, @jliexpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@jliexpensify
Copy link
Contributor

Bumping @Beamanator and @madmax330 - how should we tackle this? Is this something we're looking at fixing? I think from a UI perspective, we should - can we make it External?

@melvin-bot melvin-bot bot removed the Overdue label Jan 9, 2025
@Kalydosos
Copy link
Contributor

Kalydosos commented Jan 9, 2025

@Beamanator This screen the verification screen for the account owner. The contact verification happens later.

@parasharrajat @Beamanator yes indeed, but there is a demand to verify the former primary login as new secondary login but the validation failed (twice in my test case). Have you tried validating the secondary login ? Also when i requested another code as the previous was rejected i receive 2 validation emails in my test case.

Capture d’écran de 2025-01-09 17-09-23

@parasharrajat
Copy link
Member

Here is the flow.

  1. When you add a new login.
  2. System asks you to verify yourself(logined account or primary account). Use code from primary email.
  3. THen you can click the secondary login which was added in step 1 to verify it.
  4. Now, the system will ask you to verify it. Use code from secondary email.

@Kalydosos
Copy link
Contributor

@parasharrajat @Beamanator

here is a demo of the issue #54714 (comment)

2025-01-14.00-29-50.mp4

Copy link

melvin-bot bot commented Jan 14, 2025

@madmax330, @jliexpensify Huh... This is 4 days overdue. Who can take care of this?

@madmax330
Copy link
Contributor

So the issue is that when you verify the secondary login it does not actually verify it right?

@melvin-bot melvin-bot bot removed the Overdue label Jan 14, 2025
@Kalydosos
Copy link
Contributor

Kalydosos commented Jan 14, 2025

So the issue is that when you verify the secondary login it does not actually verify it right?

@madmax330 yes in my test cases the verifiction code is rejected even if you request a new one. If it was intended that the "new" secondary login is validated then indeed it's not working in my test cases. It's a different bug though.

Copy link

melvin-bot bot commented Jan 15, 2025

@madmax330 @jliexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Jan 17, 2025
@madmax330
Copy link
Contributor

In my opinion after you enter the code to validate the secondary login it should not ask you to validate anything anymore. Since the primary login is already validated

@melvin-bot melvin-bot bot removed the Overdue label Jan 17, 2025
@Kalydosos
Copy link
Contributor

@madmax330 some investigation and maybe some choice should be made indeed as maybe the code is rejected because that contact method is considered already validated (and indeed a new validation should not be requested) or some other reason, but i guess this will require a ticket of its own

@melvin-bot melvin-bot bot added the Overdue label Jan 20, 2025
Copy link

melvin-bot bot commented Jan 21, 2025

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

@madmax330
Copy link
Contributor

Hmm then I'm not sure what the issue we're addressing here is...

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 21, 2025
Copy link

melvin-bot bot commented Jan 27, 2025

@madmax330, @jliexpensify Huh... This is 4 days overdue. Who can take care of this?

@jliexpensify
Copy link
Contributor

Maybe I'm looking at this too simply, but isn't the issue that there's no email here @madmax330 ?

Image

That should reflect whatever email you added in the previous step, in the New Contact Method

Image

But it sounds like there are 2 bugs here: the one Applause has reported (with a missing email) and a new ticket that needs to be opened (bug 2)

@melvin-bot melvin-bot bot removed the Overdue label Jan 27, 2025
@Kalydosos
Copy link
Contributor

@madmax330 @jliexpensify the issue here seems not present in main version just the tag version the last time i checked

@parasharrajat
Copy link
Member

We can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering
Projects
Status: Done
Development

No branches or pull requests

6 participants