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

[$250] [CVP] Account is validated but still being asked to validate to add a VBBA #49813

Closed
1 of 6 tasks
IuliiaHerets opened this issue Sep 26, 2024 · 46 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Sep 26, 2024

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.39-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4992364&group_by=cases:section_id&group_order=asc&group_id=283225
Issue reported by: Applause Internal Team

Action Performed:

  1. Open the app
  2. Log in with a new Expensifail account
  3. Create a workspace
  4. Navigate to Workspace settings - More features
  5. Enable "Workflows"
  6. Navigate to Workflows - Connect bank account
  7. Click on the note to open the process in a browser

Expected Result:

The account is already validated so the message shouldn't appear.

Actual Result:

Web page opened from the app asks to validate my expensifail account.

Workaround:

Unknown

Platforms:

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6610078_1726848225304.Screen_Recording_2024-09-20_At_8.50.37.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021841225609645325117
  • Upwork Job ID: 1841225609645325117
  • Last Price Increase: 2024-10-01
  • Automatic offers:
    • suneox | Reviewer | 104246212
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 26, 2024
Copy link

melvin-bot bot commented Sep 26, 2024

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

@IuliiaHerets
Copy link
Author

We think that this bug might be related to #wave-collect - Release 1

@IuliiaHerets
Copy link
Author

@muttmuure 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

@melvin-bot melvin-bot bot added the Overdue label Sep 30, 2024
Copy link

melvin-bot bot commented Sep 30, 2024

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

@melvin-bot melvin-bot bot removed Overdue labels Oct 1, 2024
@trjExpensify trjExpensify changed the title Desktop - Bank account - Web page opened from the app asks to validate my expensifail account [CVP] Desktop - Bank account - Web page opened from the app asks to validate my expensifail account Oct 1, 2024
@trjExpensify
Copy link
Contributor

I can repro this:

image

@deetergp & I are also running into it in this flow:

  • Send an IOU to a user without an account
  • Click the Pay button in the email
  • The user is signed in and the account is validated:
        },
        "domainControlled": false,
        "email": "<redacted>",
        "isApprovedAccountant": false,
        "isApprovedAccountantClient": false,
        "isUnapprovedAccountant": false,
        "personalPolicyID": "0BBDE5F6A57A234E",
        "samlRequired": false,
        "samlSupported": false,
        "subscribed": 1,
        "twoFactorAuthEnabled": false,
        "validated": true

but in onyx, account.validated is false

image

This is a critical viral flow, so we'd appreciate a quick turnaround on getting to the bottom of it!

CC: @francoisl as I tagged you for eyes here.

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label Oct 1, 2024
@melvin-bot melvin-bot bot changed the title [CVP] Desktop - Bank account - Web page opened from the app asks to validate my expensifail account [$250] [CVP] Desktop - Bank account - Web page opened from the app asks to validate my expensifail account Oct 1, 2024
Copy link

melvin-bot bot commented Oct 1, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 1, 2024
Copy link

melvin-bot bot commented Oct 1, 2024

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

@trjExpensify trjExpensify changed the title [$250] [CVP] Desktop - Bank account - Web page opened from the app asks to validate my expensifail account [$250] [CVP] Account is validated but still being asked to validate in the VBBA flow Oct 2, 2024
@trjExpensify trjExpensify changed the title [$250] [CVP] Account is validated but still being asked to validate in the VBBA flow [$250] [CVP] Account is validated but still being asked to validate to add a VBBA Oct 2, 2024
@koko57
Copy link
Contributor

koko57 commented Oct 3, 2024

Hey! I'm Agata from Callstack and I would like to help with this issue 🙂

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 3, 2024
Copy link

melvin-bot bot commented Oct 3, 2024

📣 @suneox 🎉 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

@trjExpensify
Copy link
Contributor

Wahoo, thanks!

@koko57
Copy link
Contributor

koko57 commented Oct 3, 2024

I've checked the reproduction steps and I could repro when I wasn't logged in the account - because we're not sending validated in the OpenApp response - shouldn't it be always true after the user logs in to their account with the magic code for the first time (or verify the account the other way)?
Screenshot 2024-10-03 at 21 25 07
Screenshot 2024-10-03 at 21 24 47
So if we're not getting validated - it's undefined and falsy - so we have the error displayed.

I couldn't reproduce the scenario that I was logged in and then validate turned to false, but I wasn't following the steps @trjExpensify mentioned, I will check it tomorrow.

@deetergp deetergp added Reviewing Has a PR in review Internal Requires API changes or must be handled by Expensify staff and removed External Added to denote the issue can be worked on by a contributor labels Oct 9, 2024
@trjExpensify trjExpensify moved this from Todo to In Progress in [#whatsnext] #convert Oct 15, 2024
@koko57
Copy link
Contributor

koko57 commented Oct 16, 2024

Is anything on the FE side needed to be done here? 🙂

@trjExpensify
Copy link
Contributor

Nope, I don't think so. Daniel re-tested it here: https://expensify.slack.com/archives/C07HPDRELLD/p1729031899739159?thread_ts=1729024865.063539&cid=C07HPDRELLD

We do have a somewhat related bug, but a proposal is in from Manan here: #49576

@trjExpensify
Copy link
Contributor

Cool, closing!

@isagoico
Copy link

Going to reopen this issue as this is still happening on our side for the Hybrid app CC @AndrewGable

@isagoico isagoico reopened this Oct 30, 2024
@trjExpensify trjExpensify moved this from Done to In Progress in [#whatsnext] #convert Nov 1, 2024
@trjExpensify
Copy link
Contributor

Hm, interesting. @koko57 can you give it a re-test and confirm that?

@deetergp
Copy link
Contributor

deetergp commented Nov 6, 2024

Any word on the retest @koko57?

@koko57
Copy link
Contributor

koko57 commented Nov 7, 2024

@trjExpensify @deetergp Sorry I missed Tom's message as I was ooo that day. Will retest today

@koko57
Copy link
Contributor

koko57 commented Nov 7, 2024

The original problem is fixed, works ok:
https://github.com/user-attachments/assets/ece7e513-ab86-4c17-8ddc-fbf0d71b7068

But I've also checked if the validation error will be shown after creating a new account on the web (and not validating it) and the message is not shown:

Screenshot 2024-11-07 at 09 52 02

I think in this case we want it shown, don't we?

I see that some changes were introduced by this PR #51718. The message should be shown because there is a check for account.validated, but for some reason it doesn't.

I'll be working on refactoring the first step of enabling bank account #50422 #51799 - because there are many issues with this step, so I also can fix it there.

@koko57
Copy link
Contributor

koko57 commented Nov 7, 2024

And I've noticed that we have 2 ways of informing that the workspace currency is not set to USD

for the Expensify Card it looks like this:
Screenshot 2024-11-07 at 09 41 07

and for the workflows:
Screenshot 2024-11-07 at 09 40 45

Is there any reason that for the workflows we have only this info in the RHP and we cannot do this in the modal like for ECard? Or should we make it consistent?

cc @shawnborton

@shawnborton
Copy link
Contributor

I don't think I have strong feelings here, but I always love airing on the side of consistency. cc @Expensify/design @trjExpensify for the quick check here as well.

For the connect bank flow, I assume what's happening is that you click on the connect bank account row and you immediately see the RHP with the message about currency? If it's really the first step in the flow like that, I don't see why it couldn't be a modal either to match the Expensify Card page.

@trjExpensify
Copy link
Contributor

Interestingly, in the global reimbursement doc, we decided not to show the "Connect bank account" row if the workspace currency isn't USD, GBP, EUR, AUD or CAD (supported currencies for global reimbursement):

image

We could pull that out to move forward with it for non-USD for now, and extend it to the other currencies when global reimbursement is implemented? CC: @madmax330 for vis.

@trjExpensify
Copy link
Contributor

But I've also checked if the validation error will be shown after creating a new account on the web (and not validating it) and the message is not shown:

I'm not quite following this. Here's a test on staging web, am I missing something?

2024-11-07_13-28-21.mp4

@dannymcclain
Copy link
Contributor

I'm not super passionate but I feel the same as Shawn. If we can make them consistent I think that'd probably be best, and I'd vote for standardizing on the modal approach from the Expensify Card page. I'm happy to defer to what Tom thinks is best though.

@koko57
Copy link
Contributor

koko57 commented Nov 8, 2024

@trjExpensify aaah ok, because of this new feature we no longer have to display this message:
Screenshot 2024-11-08 at 10 31 21
(I meant that this message is not displayed for the accounts that have not been validated)

@trjExpensify
Copy link
Contributor

Yep, exactly!

@koko57
Copy link
Contributor

koko57 commented Nov 8, 2024

@trjExpensify ok, so the original bug is fixed, this issue can be closed 🙂

@trjExpensify
Copy link
Contributor

Great stuff!

We could pull that out to move forward with it for non-USD for now, and extend it to the other currencies when global reimbursement is implemented? CC: @madmax330 for vis.

@madmax330 let us know about the status of this one, and potentially expediting it for non-USD.

@github-project-automation github-project-automation bot moved this from In Progress to Done in [#whatsnext] #convert Nov 8, 2024
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 Internal Requires API changes or must be handled by Expensify staff Reviewing Has a PR in review
Projects
Development

No branches or pull requests

10 participants