Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.92k forks source link

[HOLD for payment 2024-11-11] [$250] VBA - Continue modal is not shown after several attempts #50422

Closed lanitochka17 closed 1 week ago

lanitochka17 commented 1 month ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: 9.0.46-2 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/49687

Action Performed:

Precondition: user has several workspaces, e.g. A, B, C

  1. Go to workspace A > More features > enable Expensify Cards
  2. Go to Expensify Cards tab > issue new card
  3. Start add VBA flow
  4. Close VBA flow RHN on Personal info step
  5. Go to workspace B > More features > enable Expensify Cards
  6. Go to Expensify Cards tab (ws B) > issue new card
  7. Repeat step 2-6 several times

Expected Result:

On step 5, starting page of VBA with setup types displayed On step 6, Continue page with options 'Continue' and 'Start over' displayed

Actual Result:

On step 5, starting page of VBA with setup types displayed On step 6, after several attempts, user is navigated to Personal info step he left on step 4

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/66aa74b0-b7f9-42ff-b2ca-1a81c01aa669

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843708220772796908
  • Upwork Job ID: 1843708220772796908
  • Last Price Increase: 2024-10-08
  • Automatic offers:
    • DylanDylann | Contributor | 104330185
Issue OwnerCurrent Issue Owner: @fedirjh
melvin-bot[bot] commented 1 month ago

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

github-actions[bot] commented 1 month ago

:wave: 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.
marcochavezf commented 1 month ago

Seems this issue stems from this PR. Given that PR fixed a crash and this workflow is not common, I wouldn't consider it a deploy blocker, but it would still require a fix cc @koko57 @mountiny @grgia @DylanDylann Feel free to take it over since you have more context than I do :)

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

mountiny commented 1 month ago

@koko57 @DylanDylann can you please have a look? thanks!

koko57 commented 1 month ago

@mountiny ok I will look into this today, you can assign me 🙂

koko57 commented 1 month ago

I haven't much time to look into this one yet, I'll try to check it today but even so I probably won't be able to fix it till EOD. I'm ooo next Mon and Tue I will come back to it on Wednesday.

Probably there is some problem with setting state, maybe also a problem with updating the data in Onyx. While working on the original issue (with the crash) I was wondering if we should change this continue modal and the start page (with manual/plaid connection choice and the image) into a separate step - we need to display one of them always anyway. This means a bit more work, but maybe it will be more stable than the current solution.

mountiny commented 1 month ago

Thanks for the insights, this is a niche bug so we can take time to get back to this

melvin-bot[bot] commented 1 month ago

@koko57, @mountiny, @DylanDylann Whoops! This issue is 2 days overdue. Let's get this updated quick!

mountiny commented 1 month ago

This is a lower priority

DylanDylann commented 1 month ago

I am working on PR

DylanDylann commented 1 month ago

This is my summary of the PR: Bug 1: The CTA text on the Expensify Card Page is incorrect when changing the workspace Bug 2: The Continue modal is not shown after changing the workspace

Step: Precondition: user has several workspaces, e.g. A, B, C

  1. Go to workspace A > More features > enable Expensify Cards
  2. Go to Expensify Cards tab > issue new card
  3. Start add VBA flow
  4. Close VBA flow RHN on Personal info step
  5. Go to workspace B > More features > enable Expensify Cards
  6. Go to Expensify Cards tab (ws B) > issue new card
  7. Back to workspace A > Click Expensify Cards
  8. See that the CTA text is "Issue new card" (The expect should be "Finish Setup")
  9. Click on "Issue new card"
  10. The Bank Account setup display (The expect should be Continue modal Page)

Before my PR

https://github.com/user-attachments/assets/6a26b12d-7dc4-497b-bb19-c50140891e85

After my PR

https://github.com/user-attachments/assets/8f5f0946-1fc0-4f77-b4bb-39c70177cb61

melvin-bot[bot] commented 3 weeks ago

⚠️ 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.

melvin-bot[bot] commented 3 weeks ago

⚠️ 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.

mountiny commented 3 weeks ago

@DylanDylann is investigating the dbs

melvin-bot[bot] commented 3 weeks ago

⚠️ 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.

koko57 commented 3 weeks ago

@DylanDylann @mountiny I saw that the PR was reverted - do you need some help here?

DylanDylann commented 3 weeks ago

@koko57 Yess, please. We need to create a new PR to address this issue

koko57 commented 3 weeks ago

@DylanDylann @mountiny I think we can also fix this issue here https://github.com/Expensify/App/issues/51799. Looks like we have some problems with these useEffects resulting in all the issues from the original one to the ones that were found after your PR was merged. IMHO #51799 is also caused by that. I'm inclining towards a complete refactor of this step as I mentioned here: https://github.com/Expensify/App/issues/50422#issuecomment-2406707019. Moving the modal to a separate step will make this convoluted logic unnecessary.

I have some other issues on my plate that I need to address first, but I can work on this one when I finish them.

melvin-bot[bot] commented 3 weeks ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 3 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.56-9 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-11-11. :confetti_ball:

For reference, here are some details about the assignees on this issue:

melvin-bot[bot] commented 3 weeks ago

⚠️ 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.

mountiny commented 3 weeks ago

@koko57 @DylanDylann sounds good to me, thanks!

koko57 commented 2 weeks ago

@mountiny maybe when the payment is done here we can close this one and I will work on the refactor in this issue: https://github.com/Expensify/App/issues/51799

mountiny commented 2 weeks ago

@koko57 Sorry, maybe I m not understanding. I think we need a new PR, the original was reverted so there is no payment here

DylanDylann commented 2 weeks ago

If we handle this issue in https://github.com/Expensify/App/issues/51799, please make sure test this bug again in the PR phase and now we can close this one

melvin-bot[bot] commented 2 weeks ago

Issue is ready for payment but no BZ is assigned. @adelekennedy you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

mountiny commented 2 weeks ago

I believe no payments here as the PR was reverted, still discussing if we going to handle the fix here or elsewhere