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-07-17][$250] Expense - App crashes when opening an expense report after submitting another expense #44960

Closed
1 of 6 tasks
lanitochka17 opened this issue Jul 8, 2024 · 38 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 8, 2024

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

Action Performed:

  1. Sign in to an account
  2. Go to FAB > submit expense > Manual expense
  3. Enter an amount
  4. Select a user you have never submitted an expense to
  5. Submit the expense
  6. Navigate to the chat with the user you just submitted the expense to
  7. Click on the "+" icon
  8. Click on submit expense > Manual expense > Enter amount >Next > Submit
  9. Go to the combined expense report and open the first expense you submitted

Expected Result:

The expense report opens and the app does not crash

Actual Result:

App crashes after opening the report

Workaround:

Unknown

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

Bug6535811_1720442411664.2024-07-08_15_15_43.mp4

logs (2).txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01ab200eebc7b06724
  • Upwork Job ID: 1810395750787214444
  • Last Price Increase: 2024-07-08
  • Automatic offers:
    • brunovjk | Reviewer | 103047644
    • nkdengineer | Contributor | 103047648
Issue OwnerCurrent Issue Owner: @allroundexperts
@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Jul 8, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

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

Copy link
Contributor

github-actions bot commented Jul 8, 2024

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

@lanitochka17
Copy link
Author

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

@Julesssss
Copy link
Contributor

Crash seems to be related to Policy key changing? Are we temporarily setting it to FAKE

Screenshot 2024-07-08 at 14 46 08

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

Taking a look at this, weird one

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

It looks like we are creating the first money request in a different policyID other than _FAKE_, and when we open that report again we set it back to fake, which then crashes. Looking like a backend bug.

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

not happening in prod though, and apparently we didn't deploy anything related to staging in Web-E

@Julesssss
Copy link
Contributor

Hmm, nice. At least it points back to app.

I wonder if any of the PRs seem associated with policies. Does this issue seem related?

@Julesssss
Copy link
Contributor

Julesssss commented Jul 8, 2024

Oops lol, ignore that 🤦

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

Yeah, tested with the production Web-E branch and still happens in dev, so I am gonna go ahead and remove the DeployBlocker for Web-E, will continue looking after lunch.

@rlinoz rlinoz added External Added to denote the issue can be worked on by a contributor and removed DeployBlocker Indicates it should block deploying the API labels Jul 8, 2024
@melvin-bot melvin-bot bot changed the title Expense - App crashes when opening an expense report after submitting another expense [$250] Expense - App crashes when opening an expense report after submitting another expense Jul 8, 2024
@rlinoz rlinoz added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 8, 2024
Copy link

melvin-bot bot commented Jul 8, 2024

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

Copy link

melvin-bot bot commented Jul 8, 2024

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

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

I am having a hard time finding what started causing the issue, so let's ask for some external help.

Also, I think we probably shouldn't be sending the report with a policyID other than FAKE, so that is weird, but it is also happening in prod.

Edit: Oh, the policyID of the threadReportID is the personal policyID in DB but it is _FAKE_ in the FE, trying to undertand why..

@rlinoz
Copy link
Contributor

rlinoz commented Jul 8, 2024

@nkdengineer
Copy link
Contributor

Proposal

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

App crashes after opening the report

What is the root cause of that problem?

When we submit an expense, we create an optimistic transaction thread here

const transactionThread = buildTransactionThread(iouAction, iouReport, existingTransactionThreadReportID);

When we submit an expense in a new DM, we create an optimistic iou report with policyID as undefined here

return {

And then the optimistic transaction thread report has policyID as _FAKE_ by default here

moneyRequestReport?.policyID,

policyID: string = CONST.POLICY.OWNER_EMAIL_FAKE,

After BE returns data, the policyID is changed to personal policyID so the app crashes

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

We can change the default policyID here to -1 or undefined and update other places that use this function to pass policyID as undefined if we're passing it as _FAKE_

policyID: string = CONST.POLICY.OWNER_EMAIL_FAKE,

What alternative solutions did you explore? (Optional)

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

melvin-bot bot commented Jul 9, 2024

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

@rlinoz
Copy link
Contributor

rlinoz commented Jul 12, 2024

I think the automation failed, but this is in prod already.

@isabelastisser

@rlinoz rlinoz added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Jul 12, 2024
@isabelastisser
Copy link
Contributor

@rlinoz, to clarify, when is the payment due?

@rlinoz rlinoz changed the title [$250] Expense - App crashes when opening an expense report after submitting another expense [HOLD for payment 2024-07-17][$250] Expense - App crashes when opening an expense report after submitting another expense Jul 12, 2024
@rlinoz
Copy link
Contributor

rlinoz commented Jul 12, 2024

It got to prod 2 days ago, so the 17, updated the title.

@isabelastisser
Copy link
Contributor

thanks!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 18, 2024
@isabelastisser
Copy link
Contributor

The payments were processed in Upwork. All set!

@brunovjk
Copy link
Contributor

@allroundexperts reviewed the PR

@isabelastisser @rlinoz

@mallenexpensify
Copy link
Contributor

Thanks @brunovjk , I just requested a refund, please confirm via a comment here when you've done so.

@brunovjk
Copy link
Contributor

Great @mallenexpensify, I went to make the refund, but the 10% Upwork fee has already been debited, I can refund $225 (Bounty - Upwork fee), okay?
Next week I have other contracts to receive, maybe we'll leave this one alone, and next week we'll cash it. I have no preference, whatever is best for the project.

Note: While we're here, would you know if I can get paid through Expensify instead of Upwork? I don't live in USA. Thank you.

@mallenexpensify
Copy link
Contributor

Thx @brunovjk , that's weird that the fee isn't removed too, hmmm...
How has the 10% fee been debited if it takes them 5 days to pay? Do you have any screenshots you can share.

Currently, all contributors need to be paid via Upwork for 18 months from their first hire, unfortunately you have a while. 2025-08-09 Bruno Rocha

@brunovjk
Copy link
Contributor

brunovjk commented Jul 20, 2024

Thx @brunovjk , that's weird that the fee isn't removed too, hmmm... How has the 10% fee been debited if it takes them 5 days to pay? Do you have any screenshots you can share.

image

It shows that the fee was removed, I tried refund anyway, can you confirm if everything went well?

Currently, all contributors need to be paid via Upwork for 18 months from their first hire, unfortunately you have a while. 2025-08-09 Bruno Rocha

Thank you very much @mallenexpensify for the information!!!! I'll leave a note here, it goes quickly :D

@melvin-bot melvin-bot bot added the Overdue label Jul 22, 2024
@mallenexpensify
Copy link
Contributor

Yup, confirming we got the refund. Thanks @brunovjk , I know it's a bit of a pain. Luckily it happens pretty rarely.

Contributor: @nkdengineer paid $250 via Upwork
Contributor+: @allroundexperts owed $250 via NewDot.

@isabelastisser when you pay contributors can you provide more detail in the description so that we know who's getting paid where, and how much? Thx.

@allroundexperts plz complete the BZ checklist below, seems like we'd want a regression test for this, right?

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:

  • [@] The PR that introduced the bug has been identified. Link to the PR:
  • [@] 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:
  • [@] 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:
  • [@] Determine if we should create a regression test for this bug.
  • [@] 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.
  • [@bz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  • [ ]

Copy link

melvin-bot bot commented Jul 23, 2024

@rlinoz, @allroundexperts, @isabelastisser, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@rlinoz
Copy link
Contributor

rlinoz commented Jul 24, 2024

friendly bump @allroundexperts

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 24, 2024
@allroundexperts
Copy link
Contributor

Checklist

  1. I was not able to pinpoint the exact PR which caused this issue.
  2. N/A
  3. N/A
  4. A regression test would be useful here.

Regression test

  1. Sign in to an account and go to FAB > submit expense > Manual expense
  2. Enter an amount and select a user you have never submitted an expense to
  3. Submit the expense
  4. Navigate to the chat with the user you just submitted the expense to
  5. Click on the "+" icon
  6. Click on submit expense > Manual expense > Enter amount >Next > Submit
  7. Go to the combined expense report and open the first expense you submitted

Verify that the expense report opens and the app does not crash

Do we 👍 or 👎 ?

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
@isabelastisser
Copy link
Contributor

Sorry for the confusion, team! And thanks for handling this, @mallenexpensify!

@rlinoz, can you please review the regression test above? Thanks!

@rlinoz
Copy link
Contributor

rlinoz commented Jul 29, 2024

I think since this was caught during regression testing we can skip creating a new one.

@isabelastisser
Copy link
Contributor

Sounds good, thanks @rlinoz!

This is all set, closing!

@JmillsExpensify
Copy link

$250 approved for @allroundexperts

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. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

9 participants