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.56k stars 2.9k forks source link

[$250] Android - Scan - In review duplicates, tapping "keep all" app crashes #46711

Closed lanitochka17 closed 2 months ago

lanitochka17 commented 3 months 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.16-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4801447 Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch app
  2. Login as applausetester+pendingreceipt@applause.expensifail.com
  3. Tap on Do not delete - pending receipt chat
  4. Create 2 scanned expense with same valid receipt so that scan will be successful
  5. Tap on the preview
  6. Open one of the expense
  7. Tap review duplicates in header
  8. Tap "keep all" in header

Expected Result:

In review duplicates, tapping "keep all" app must not crash

Actual Result:

In review duplicates, tapping "keep all" app crashes

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/92325e67-52fe-43e4-a553-35956b90dc17

logs (2).txt

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017ea914a2c756b016
  • Upwork Job ID: 1822889161149535059
  • Last Price Increase: 2024-09-16
Issue OwnerCurrent Issue Owner: @alitoshmatov
melvin-bot[bot] commented 3 months ago

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.

lanitochka17 commented 3 months ago

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

lanitochka17 commented 3 months ago

We think that this bug might be related to #vip-split

melvin-bot[bot] commented 3 months ago

@RachCHopkins Whoops! This issue is 2 days overdue. Let's get this updated quick!

RachCHopkins commented 3 months ago

Duplicate detection is part of #wave-control

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

@RachCHopkins, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 3 months ago

@RachCHopkins @alitoshmatov this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

alitoshmatov commented 3 months ago

No proposals yet

Krishna2323 commented 3 months ago

@alitoshmatov, is this issue reproducible? have you tried on android native?

melvin-bot[bot] commented 3 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Huh... This is 4 days overdue. Who can take care of this?

kubabutkiewicz commented 2 months ago

Hi! I am Jakub from Callstack - expert contributor group. I’d like to work on this job

kubabutkiewicz commented 2 months ago

Hey @lanitochka17 are you able to share the receipt which you used when noticed the crash?

lanitochka17 commented 2 months ago

@kubabutkiewicz Receipt Example

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

kubabutkiewicz commented 2 months ago

@lanitochka17 Are you able to reproduce that? I was trying but no luck

kubabutkiewicz commented 2 months ago

Ok, I was able to reproduce that today

kubabutkiewicz commented 2 months ago

I was able to reproduce that only once but I know what is a problem

Proposal

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

In review duplicates, tapping "keep all" causing an app crash

What is the root cause of that problem?

In that function https://github.com/Expensify/App/blob/c2e7f400b13d813cc52ceb5ed9dfdbaf72dff905/src/libs/actions/Transaction.ts#L295-L391 we are creating an currentTransactions here https://github.com/Expensify/App/blob/c2e7f400b13d813cc52ceb5ed9dfdbaf72dff905/src/libs/actions/Transaction.ts#L297 and the problem occurs because this currentTransactions looked like that

[
   {
      "amount":0,
      "bank":"",
      "billable":false,
      "cardID":0,
      "cardName":"Cash Expense",
      "cardNumber":"",
      "category":"",
      "comment":"",
      "created":"2024-08-27",
      "currency":"PLN",
      "filename":"w_d579b03466fbab547f0feb9ff9c95877491b3587.png",
      "hasEReceipt":false,
      "isLoading":false,
      "managedCard":false,
      "mcc":"",
      "merchant":"(none)",
      "modifiedAmount":-650,
      "modifiedCreated":"2023-09-28",
      "modifiedCurrency":"USD",
      "modifiedMCC":"",
      "modifiedMerchant":"Test",
      "originalAmount":0,
      "originalCurrency":"",
      "parentTransactionID":"",
      "pendingFields":{
         "merchant":"update"
      },
      "receipt":{
         "receiptID":987847661721264,
         "source":"https://www.expensify.com/receipts/w_d579b03466fbab547f0feb9ff9c95877491b3587.png",
         "state":"SCANCOMPLETE"
      },
      "reimbursable":true,
      "reportID":"5732908836376120",
      "status":"Posted",
      "tag":"",
      "taxAmount":0,
      "taxCode":"",
      "transactionID":"44320141500630305"
   },
   "undefined",
   "undefined",
   "undefined",
   "undefined"
]

so last 4 items were undefined and then here https://github.com/Expensify/App/blob/c2e7f400b13d813cc52ceb5ed9dfdbaf72dff905/src/libs/actions/Transaction.ts#L298-L299 we are mapping through currentTransactions and because off those undefined the reportID and transactionID don't exists and it make a crash

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

We can use TransactionUtils.getTransaction method here https://github.com/Expensify/App/blob/c2e7f400b13d813cc52ceb5ed9dfdbaf72dff905/src/libs/actions/Transaction.ts#L297 so then the type of currenctTransactions will be OnyxEntry<Transaction>[] and we can use optional chaining in places where transaction can be undefined so it will look like that

    const currentTransactions = transactionIDs.map((id) => TransactionUtils.getTransaction(id));
    const transactionsReportActions = currentTransactions.map((transaction) => ReportActionsUtils.getIOUActionForReportID(transaction?.reportID ?? '', transaction?.transactionID ?? ''));
    const isSubmitter = currentTransactions.every((transaction) => isCurrentUserSubmitter(transaction?.reportID ?? ''));

in that way the app will not crash

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

lanitochka17 commented 2 months ago

Issue is not reproducible on the latest build 9.0.25-10

https://github.com/user-attachments/assets/f6a80722-c21a-425c-a94a-863c54611b70

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov 10 days overdue. Is anyone even seeing these? Hello?

melvin-bot[bot] commented 2 months ago

@RachCHopkins @alitoshmatov this issue is now 4 weeks old, please consider:

Thanks!

kubabutkiewicz commented 2 months ago

@alitoshmatov Can you check my proposal ?

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

alitoshmatov commented 2 months ago

I can't access applause test account, and couldn't reproduce it myself following the OP instructions. So I think I cannot help you here. I will ask for help in C+ channel

kubabutkiewicz commented 2 months ago

I was able to reproduce that on my own account

allgandalf commented 2 months ago

I guess this bug is reproducible for private domains that may explain why only few people are able to reproduce, i will try reproducing this from a private domain and get back

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!

RachCHopkins commented 2 months ago

Any joy @allgandalf?

mvtglobally commented 2 months ago

Issue not reproducible during KI retests. (First week)

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

RachCHopkins commented 2 months ago

@mvtglobally did you retest with a private domain?

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

RachCHopkins commented 2 months ago

Looks like no one here can repro this. @alitoshmatov are you able to?

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

alitoshmatov commented 2 months ago

@RachCHopkins No I am not able to reproduce this issue

mvtglobally commented 2 months ago

Issue not reproducible during KI retests. (Second week)

melvin-bot[bot] commented 2 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 2 months ago

@RachCHopkins, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

RachCHopkins commented 2 months ago

Ok, closing!