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

[HOLD for payment 2024-08-20] Bank account - Unable to proceed after website step when website is auto populated #47227

Closed
6 tasks done
IuliiaHerets opened this issue Aug 12, 2024 · 30 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Aug 12, 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.19-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  • User is logged in with Expensifail account.
  1. Go to staging.new.expensify.com
  2. Go to workspace settings > Workflows
  3. Click Connect bank account > Connect manually
  4. Proceed to Step 4 - Enter Company's Tax ID page
  5. Enter Tax ID > Next
  6. Note that the website field is auto populated with https://www.applause.expensifail.com./
  7. Without editing the field, click Next.

Expected Result:

The setup will proceed to the next step.

Actual Result:

The setup is stuck on the same website step until user adds some edit to it.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6569734_1723462954257.bank.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @RachCHopkins
@IuliiaHerets IuliiaHerets added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. DeployBlocker Indicates it should block deploying the API labels Aug 12, 2024
Copy link

melvin-bot bot commented Aug 12, 2024

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

Copy link

melvin-bot bot commented Aug 12, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Aug 12, 2024
Copy link
Contributor

👋 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.

@IuliiaHerets
Copy link
Author

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

@shubham1206agra
Copy link
Contributor

Looks related to #46796 (Not 100% sure).

@hannojg Can you look into this?

@hannojg
Copy link
Contributor

hannojg commented Aug 12, 2024

Looks related to #46796 (Not 100% sure).

@hannojg Can you look into this?

Hmm, on the first look it doesn't appear to me as related. The PR only changed the composer of the report page.

(continuing to look to be sure …)

@hannojg
Copy link
Contributor

hannojg commented Aug 12, 2024

Yeah, none of the components used on the respective page with the bug was touched by the mentioned PR. I don't think the PR is related.

@NikkiWines
Copy link
Contributor

NikkiWines commented Aug 12, 2024

Repro'd locally - reverted this locally and it resolved the issue while I was still on the add website page. But, of course, re-introduced the bug of skipping the website form if I restarted adding a bank account

cc: @bernhardoj @neil-marcellini @suneox

@suneox
Copy link
Contributor

suneox commented Aug 12, 2024

I'll start checking now

@suneox
Copy link
Contributor

suneox commented Aug 12, 2024

@NikkiWines I’ve started testing with a private email, and this issue only happened once. It cannot be reproduced on the latest main branch, so I think it might be related to the backend response during the reimbursementAccount step. I’d like to confirm if this issue can still be reproduced.

@NikkiWines
Copy link
Contributor

@suneox I'm still able to reproduce the issue

Screen.Recording.2024-08-13.at.11.43.59.mov

@bernhardoj
Copy link
Contributor

I found the issue. This happens specifically after removing this code.

What happen in this issue is that, when we press Next, the website draft Onyx data is still empty. It's only filled once we type into the text input. The linked code purpose is to save the generated website to the website draft Onyx.

But if we re-add that, then the website step gonna be skipped again because when we are at the tax ID step (step before the website step) and press next, the website step is mounted, the generated website is saved to the draft, then, a loading view is shown and then the step is remounted which will skip the website step because it's now contains a valid website.

We need to wait for #45278 before we can re-add the above logic back. Every time a step (not sub-step) view is mounted, the initial sub-step will be evaluated and #45278 will prevent the step to be re-mounted every time we save a sub-step.

@NikkiWines
Copy link
Contributor

Thanks for the summary @bernhardoj. Maybe, since we need to wait on the issue you linked, we can remove the default website for now and require the user to input a value to move forward?

Once that issue is complete, we can re-add the default website to bring things back to our normal flow

@bernhardoj
Copy link
Contributor

That works. I'll prepare a PR for that.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Aug 13, 2024
@Beamanator
Copy link
Contributor

Fix is being CP'd 👍

@Beamanator
Copy link
Contributor

@IuliiaHerets can you please retest this on staging? 🙏

@Beamanator Beamanator removed the DeployBlockerCash This issue or pull request should block deployment label Aug 13, 2024
@Beamanator
Copy link
Contributor

Confirmed this is fixed in staging!

@IuliiaHerets
Copy link
Author

@Beamanator sorry for delay. Website field is not auto populated now, but when enter it manually, Next button works

bandicam.2024-08-13.22-37-50-589.mp4

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Aug 13, 2024
@melvin-bot melvin-bot bot changed the title Bank account - Unable to proceed after website step when website is auto populated [HOLD for payment 2024-08-20] Bank account - Unable to proceed after website step when website is auto populated Aug 13, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Aug 13, 2024
Copy link

melvin-bot bot commented Aug 13, 2024

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

Copy link

melvin-bot bot commented Aug 13, 2024

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

If no regressions arise, payment will be issued on 2024-08-20. 🎊

Copy link

melvin-bot bot commented Aug 13, 2024

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@NikkiWines] The PR that introduced the bug has been identified. Link to the PR: Fix company's website step is skipped #46940
  • [@NikkiWines] 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: N/A PR author was involved in resolving the bug
  • [@NikkiWines] 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
  • [@NikkiWines] Determine if we should create a regression test for this bug. N/A - this was caught by existing regression testing
  • [@NikkiWines] 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.
  • [@RachCHopkins] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@bernhardoj
Copy link
Contributor

I was offline. Thanks for handling the PR!

@NikkiWines
Copy link
Contributor

This should be on prod now cc: @RachCHopkins

@RachCHopkins
Copy link
Contributor

@NikkiWines sorry I'm a little new to this, is this the standard $250? I believe @bernhardoj is paid via newdot manual requests, so we don't need an upwork job. And does anyone else need to be paid here?

@shubham1206agra
Copy link
Contributor

@RachCHopkins Nobody needs to be paid here as this was a regression and it was fixed by original author/C+.

@RachCHopkins
Copy link
Contributor

I am confused. It says hold for payment and I am being tagged. Do I simply... close the issue?

@shubham1206agra
Copy link
Contributor

Yes

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Aug 20, 2024
Copy link

melvin-bot bot commented Aug 20, 2024

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

Copy link

melvin-bot bot commented Sep 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants