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
2.99k stars 2.5k forks source link

[$250] Split expense - Scan receipt does not show red brick road error details #41256

Open m-natarajan opened 2 weeks ago

m-natarajan commented 2 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: 1.4.67-2 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @shawnborton Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1714397696317219

Action Performed:

  1. Open NewDot app
  2. Click on Fab > Split expense
  3. Scan a receipt with details missing and split
  4. Open the receipt from the preview

    Expected Result:

    The error message on the preview says "Receipt missing details" but when you go to view the Split details, you don't know which details are actually missing. We should highlight under the specific inputs which details were missing, for example, if the Merchant name couldn't be found, let's highlight the Merchant row

    Actual Result:

    In the split details view, there is no indication of which fields need to be fixed.

    Workaround:

    unknown

    Platforms:

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

    • [ ] Android: Native
    • [ ] Android: mWeb Chrome
    • [ ] iOS: Native
    • [ ] iOS: mWeb Safari
    • [x] MacOS: Chrome / Safari
    • [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/38435837/4af32aeb-8837-4e8f-afaa-05b5dd9aceb6

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01395d5820a0885f64
  • Upwork Job ID: 1786772949119090688
  • Last Price Increase: 2024-05-18
Issue OwnerCurrent Issue Owner: @shubham1206agra
melvin-bot[bot] commented 2 weeks ago

Triggered auto assignment to @bfitzexpensify (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 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

bfitzexpensify commented 2 weeks ago

Added to vip-split, and sending external to see if this can be worked on by contributors.

nkdengineer commented 1 week ago

Proposal

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

In the split details view, there is no indication of which fields need to be fixed.

What is the root cause of that problem?

We pass the split draft transaction to MoneyRequestComfirmationList if it exists. If we edit any field before the scan is complete, we will create a split draft transaction in Onyx and the state of the scan will be ready. After the scan is complete, the receipt state is updated in the current transaction and it's not updated in split draft transaction.

https://github.com/Expensify/App/blob/8db67c4da2e5547abd08441503ca92fe06363d7f/src/pages/iou/SplitBillDetailsPage.tsx#L129

So the error field will not appear in confirm page because we use the draft transaction which has the scan state is ready.

https://github.com/Expensify/App/blob/8db67c4da2e5547abd08441503ca92fe06363d7f/src/components/MoneyRequestConfirmationList.tsx#L335

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

In SplitBillDetailPage, we already pass hasSmartScanFailed to confirm page which check the current transaction. So we can do the same by creating hasMissingSmartscanFields variable on SplitBillDetailPage and passing hasMissingSmartscanFields it to confirm page

const hasMissingSmartscanFields = TransactionUtils.hasMissingSmartscanFields(transaction);

https://github.com/Expensify/App/blob/8db67c4da2e5547abd08441503ca92fe06363d7f/src/pages/iou/SplitBillDetailsPage.tsx#L77-L78

And then replace TransactionUtils.hasMissingSmartscanFields(transaction) check here with this prop

!!hasSmartScanFailed && hasMissingSmartscanFields

https://github.com/Expensify/App/blob/8db67c4da2e5547abd08441503ca92fe06363d7f/src/components/MoneyRequestConfirmationList.tsx#L335

What alternative solutions did you explore? (Optional)

NA

melvin-bot[bot] commented 1 week ago

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

ishpaul777 commented 1 week ago

i'll review proposal soon 🙇 Sorry for delay

melvin-bot[bot] commented 1 week ago

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

ishpaul777 commented 1 week ago

i tried reproducing this, but i was not able to, for a long time the reciept keeps scanning, and eventually when scan fails there was a RBR on preview as well as on Split details page. @nkdengineer Are you able to reproduce?

nkdengineer commented 6 days ago

@ishpaul777 I can reproduce this bug by editing any field before the scan is complete. As the video below, I edited the category field before the scan is complete.

https://github.com/Expensify/App/assets/161821005/4d02f5e8-d202-47ab-99fe-107aa4278867

melvin-bot[bot] commented 5 days ago

@bfitzexpensify @ishpaul777 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!

ishpaul777 commented 3 days ago

I have my plate full right now, i won't able to priortize, Please reassign 🙇

melvin-bot[bot] commented 3 days ago

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

shubham1206agra commented 20 hours ago

@nkdengineer I am not able to understand your solution completely. Why do we need to take out TransactionUtils.hasMissingSmartscanFields(transaction) out of the memo? Can you please explain it in a bit more detail.

melvin-bot[bot] commented 16 hours ago

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