[Payment Due 2024-08-20] [$250] App crashes when the user initiates the send invoice flow twice

[rezkiy37]

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: v9.0.16-4
Reproducible in staging?: Yes
Reproducible in production?: No
Expensify/Expensify Issue URL:
Issue reported by: @rezkiy37
Slack conversation: https://callstack-hq.slack.com/archives/C049HHMV9SM/p1722876158696939, https://callstack-hq.slack.com/archives/C06SPA2JTPC/p1722865278681679

Action Performed:

Break down in numbered steps

1. Open an invoice room.
2. Initiate the Send Invoice flow from the room.
3. Close it.
4. Initiate the Send Invoice flow one more time.

Expected Result:

The app initiates the Send invoice and does not crash.

Actual Result:

The app crashes.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

Bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011b5e32288a6dbf2c
• Upwork Job ID: 1820648846301758830
• Last Price Increase: 2024-08-06

Issue Owner

Current Issue Owner: @brunovjk

[melvin-bot]

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

[daledah]

Proposal

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

The app crashes.

What is the root cause of that problem?

Optional chaining wasn't used for transaction?.comment here:

App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

Line 611 in 8cfeb33

iouComment={transaction?.comment.comment ?? ''}

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

Add optional chaining for transaction?.comment to prevent undefined error:

iouComment={transaction?.comment?.comment ?? ''}

What alternative solutions did you explore? (Optional)

[VictoriaExpensify]

Agree this is a bug that should be fixed

[melvin-bot]

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

[melvin-bot]

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

[rezkiy37]

@daledah I am developing the invoicing feature and I've found this bug. So I will take care of the fix. Thank you for the proposal.

[rezkiy37]

Hi, I’m Michael (Mykhailo) from Callstack and I would like to work on this issue.

[rezkiy37]

Preparing the PR - #46869.

[rezkiy37]

The PR has been opened for review 🙂

[brunovjk]

Reviewing after lunch.

[daledah]

@rezkiy37 According to the Expensify Contributing Guideline, existing proposals should be prioritized, so I think your PR should be put on HOLD, or closed while proposals are being reviewed.

If there are existing proposals, BZ will put the issue on hold. Contributor+ will review the existing proposals. If a contributor’s proposal is accepted then the contributor will be assigned to the issue. If not the issue will be assigned to the agency-employee.

@brunovjk Could you help review my proposal here according to the guideline? Thanks!

[daledah]

I think @rezkiy37 is saying that while developing something, he found a bug and already found a solution.

If this is the case a comment should be posted on the GH before proposals come in. Otherwise I think we should stick to the Contributing Guideline, if not it'd be unfair for contributors who follow the guideline and who post proposals to help with the issue.

[brunovjk]

Hi @VictoriaExpensify, Can you assist with the assignment for this issue? What should I review? Thank you.

[davidcardoza]

Hey everyone, after chatting with the internal Expensify team we are going to move forward with @rezkiy37's PR.

[VictoriaExpensify]

Sounds good, thanks @davidcardoza and @rezkiy37 !

[rezkiy37]

The PR (#46869) is ready for review.

[melvin-bot]

Triggered auto assignment to @Beamanator, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[daledah]

Hey everyone, after chatting with the internal Expensify team we are going to move forward with @rezkiy37's PR.

Hi @davidcardoza would appreciate if you can provide the reasoning for this decision? (As it's an exception to the contributor guideline)

[brunovjk]

I the PR has now been on prod for a week. Deployed on August 13th: #47219 (comment)

cc: @VictoriaExpensify, @Beamanator

[Beamanator]

Agreed 👍

[brunovjk]

@Beamanator, please remove the Reviewing label and bump to Daily as payment is due. Thank you :D

[Beamanator]

Done 😅

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[brunovjk]

• [@brunovjk] The PR that introduced the bug has been identified. Link to the PR: N/A
• [@brunovjk] 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
• [@brunovjk] 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
• [@brunovjk] Determine if we should create a regression test for this bug. Yes
• [@brunovjk] 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.

Regression Test Proposal

Send invoice

• Open an invoice room.
• Initiate the Send Invoice flow from the room.
• Close it.
• Initiate the Send Invoice flow one more time.
• Verify that the app does not crash and that the flow works properly.

Pay

• Go to DM.
• Click +
• Click Pay.
• Enter amount.
• Click Next.
• Verify that the app does not crash and that the flow works properly.
• Step 2
• Step 3

Do we agree 👍 or 👎

[brunovjk]

Hey @VictoriaExpensify, just a heads up that the payment for this issue is still pending. When you have a moment, could you help process it so we can close this out? Appreciate it!

[mallenexpensify]

@brunovjk can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~011b5e32288a6dbf2c

[brunovjk]

@mallenexpensify Offer accepted, thanks.

[mallenexpensify]

Contributor+: @brunovjk paid $250 via Upwork

@brunovjk] 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. N/A

@brunovjk can you provide reasoning why we don't want a regression test here? Thx

[brunovjk]

@brunovjk can you provide reasoning why we don't want a regression test here? Thx

@mallenexpensify In light of the recent type-safe changes across multiple files and flows, I initially considered that a regression test might not be necessary. However, after reviewing, I don’t see why we shouldn’t create a test. I updated this comment with the test.

[mallenexpensify]

Thanks @brunovjk , when in doubt, let's always create the test cases cuz QA reviews them and they can decide what's best.

• https://github.com/Expensify/Expensify/issues/423342

[brunovjk]

Good to know, thank you @mallenexpensify 😄