IuliiaHerets commented 3 weeks 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.57-0 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): applausetester+kh22100010ad@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Go to staging.new.expensify.com
Go to Workspace settings > Workflows.
Click Connect bank account.
Click Update to USD.

Expected Result:

App will not crash when connecting to bank account.

Actual Result:

App crashes when connecting to bank account.

Workaround:

Unknown

Platforms:

[ ] Android: Standalone
[ ] Android: HybridApp
[x] Android: mWeb Chrome
[ ] iOS: Standalone
[ ] iOS: HybridApp
[ ] iOS: mWeb Safari
[x] MacOS: Chrome / Safari
[x] MacOS: Desktop

Screenshots/Videos

0411_2.txt

https://github.com/user-attachments/assets/8a9f0e30-a382-412f-acf0-7b1adacdc431

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021853501012094691283
Upwork Job ID: 1853501012094691283
Last Price Increase: 2024-11-04
Automatic offers:

DylanDylann | Reviewer | 104737674
CyberAndrii | Contributor | 104737676

Issue Owner

Current Issue Owner: @muttmuure

melvin-bot[bot] commented 3 weeks ago

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.

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

github-actions[bot] commented 3 weeks 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:

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

nkdengineer commented 3 weeks ago

Proposal

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

App crashes when connecting to bank account.

What is the root cause of that problem?

we have this error Cannot access 'hasInProgressVBBA' before initialization

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

We should move function hasInProgressVBBA to above place where we define the state shouldShowContinueSetupButton here

https://github.com/Expensify/App/blob/b87ec1ffed2887cf53cfb3bf25a34f691313a9bd/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx#L192-L194

What alternative solutions did you explore? (Optional)

CyberAndrii commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-11-04 16:51:33 UTC.

Proposal

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

Bank account - App crashes when connecting to bank account

What is the root cause of that problem?

Long time ago https://github.com/Expensify/App/commit/98b1256be20fd2e43b4afa1426e885b0628b185f moved the call to getShouldShowContinueSetupButtonInitialValue() above the place where the function is defined.

It was moved back in https://github.com/Expensify/App/pull/51315 but it recently got reverted by https://github.com/Expensify/App/pull/51857.

The PR after which this crash started to occur is https://github.com/Expensify/App/pull/51718. It changed

!requestorStepRef.current

to

 !requestorStepRef?.current

I think this caused the compiler to generate a different lowered code. The following code seems to prevent the crash

!requestorStepRef || !requestorStepRef.current

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

Move this functions a few lines up before the call to getShouldShowContinueSetupButtonInitialValue()

https://github.com/Expensify/App/blob/657531f56427e51d377b642af6ef65faf30605d9/src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx#L183-L206

Also avoid calling it again on every render

-    const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue());
+    const [shouldShowContinueSetupButton, setShouldShowContinueSetupButton] = useState(getShouldShowContinueSetupButtonInitialValue);

DylanDylann commented 3 weeks ago

I also see this bug today. But I don't understand why this bug happens after reverting https://github.com/Expensify/App/pull/51315. TBH, https://github.com/Expensify/App/pull/51315 fix another bug and we already reverted by https://github.com/Expensify/App/pull/51857. Anyone help to investigate why this bug doesn't happen before merging https://github.com/Expensify/App/pull/51315

DylanDylann commented 3 weeks ago

cc @nkdengineer @CyberAndrii

blimpich commented 3 weeks ago

Bringing conversation to Slack: https://expensify.slack.com/archives/C01GTK53T8Q/p1730736248203919

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

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

nkdengineer commented 3 weeks ago

Move this functions a few lines up before the call to getShouldShowContinueSetupButtonInitialValue()

@CyberAndrii I see your solution is no different from the solution I proposed before.

melvin-bot[bot] commented 3 weeks 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 📖

melvin-bot[bot] commented 3 weeks ago

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

melvin-bot[bot] commented 3 weeks ago

📣 @CyberAndrii 🎉 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 📖

blimpich commented 3 weeks ago

@ahmedGaber93 please ignore the ping, assigning @DylanDylann here since they have context

ahmedGaber93 commented 3 weeks ago

please ignore the ping, assigning @DylanDylann here since they have context

Ok, no problem, I also faced this problem yesterday and can help. It is a pretty straightforward JS issue and not requires any context.

blimpich commented 3 weeks ago

@ahmedGaber93 thank you! We'll stick with @DylanDylann as the C+ for now but will let you know if we need help 🙂

blimpich commented 3 weeks ago

@nkdengineer hello there is definitely similarity with the proposals. I preemptively assigned @CyberAndrii since they were active in the Slack chat I opened up about this. My priority is to fix this quickly so I weighed their responsiveness in Slack heavily when assigning 👍

nkdengineer commented 3 weeks ago

@blimpich thanks for explaining, i understand

blimpich commented 3 weeks ago

Update: PR is up, waiting for @DylanDylann to come back online to review. I'm about to log off for the day, will check back in in the evening to see if PR is ready to merge and CP to staging.

TODO:

[x] get PR approved by @DylanDylann
[x] get PR approved and merged to main by @blimpich
[x] CP fix to staging
[x] have QA retest this and close it

Julesssss commented 3 weeks ago

Thanks all. @blimpich, the changes looked good, so I have merged them to unblock the deploy. CP requested here.

izarutskaya commented 3 weeks ago

Not reproduced Build v9.0.57-7

https://github.com/user-attachments/assets/c5496e75-e581-40e0-a99f-522036825bb8

Julesssss commented 3 weeks ago

Checked off

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.57-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/51981

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

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

@CyberAndrii requires payment automatic offer (Contributor)
@DylanDylann requires payment automatic offer (Reviewer)

melvin-bot[bot] commented 3 weeks ago

@DylanDylann @muttmuure The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot[bot] commented 2 weeks ago

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

https://github.com/Expensify/App/pull/51981

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

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

@CyberAndrii requires payment automatic offer (Contributor)
@DylanDylann requires payment automatic offer (Reviewer)

melvin-bot[bot] commented 2 weeks ago

@DylanDylann @muttmuure The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

melvin-bot[bot] commented 1 week ago

@blimpich, @CyberAndrii, @muttmuure, @DylanDylann Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

DylanDylann commented 1 week ago

BugZero Checklist:

[x] [Contributor] Classify the bug:

Bug classification

Source of bug: - [ ] 1a. Result of the original design (eg. a case wasn't considered) - [x] 1b. Mistake during implementation - [ ] 1c. Backend bug - [ ] 1z. Other: Where bug was reported: - [ ] 2a. Reported on production - [x] 2b. Reported on staging (deploy blocker) - [ ] 2c. Reported on both staging and production - [ ] 2d. Reported on a PR - [ ] 2z. Other: Who reported the bug: - [ ] 3a. Expensify user - [ ] 3b. Expensify employee - [ ] 3c. Contributor - [x] 3d. QA - [ ] 3z. Other:

[ ] [Contributor] 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:
[x] [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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: https://expensify.slack.com/archives/C01GTK53T8Q/p1730736248203919
[x] [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.
[ ] [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

Link to issue:

Regression Test Proposal

Test:

Go to Workspace settings > Workflows.
Click Connect bank account.
Click Update to USD to change the currency
Verify the app does not crash.

Do we agree 👍 or 👎

melvin-bot[bot] commented 6 days ago

@blimpich, @CyberAndrii, @muttmuure, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

muttmuure commented 6 days ago

@CyberAndrii - $250 for C @DylanDylann - $250 for C+

melvin-bot[bot] commented 21 hours ago

@blimpich, @CyberAndrii, @muttmuure, @DylanDylann Huh... This is 4 days overdue. Who can take care of this?

muttmuure commented 12 hours ago

This is paid up and regression test created

Expensify / App

[HOLD for payment 2024-11-14] [HOLD for payment 2024-11-13] [$250] Bank account - App crashes when connecting to bank account #51961

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

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

What is the root cause of that problem?

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

BugZero Checklist:

Regression Test Proposal

Test: