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

[$250] Receipt audit shows "Receipt ° Verified √" when there's a violations related to SmartScan #46791

Closed
1 of 6 tasks
m-natarajan opened this issue Aug 5, 2024 · 26 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@m-natarajan
Copy link

m-natarajan commented Aug 5, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Version Number: 9.0.16-5
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: @cead22
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1722635347734459

Action Performed:

  • Scan a real receipt that will SmartScan successfully
  • Wait for it to SmartScan
  • Once SmartScan finishes, update the date of the receipt
  • You should see a violation under the date field "date differs from scanned receipt"
  • You should also see "Receipt ° Verified √" above the receipt, this is the bug
  • Update the amount of the receipt to something higher
  • You should see "Amount greater than scanned receipt"
  • You should also see "Receipt ° Verified √" above the receipt. Again, this is the bug

Expected Result:

When "Date differs from scanned receipt" and/or "amount greater than scanned receipt" are visible, the receipt audit component shouldn't show "Receipt ° Verified √"
Instead, it should say "Receipt ° Issue found" or "Receipt ° Issues found"
However, we should not show "Date differs from scanned receipt" or "Amount greater than scanned receipts" under the receipt (ie, in the ReceiptAuditMessages component)
We should show the "Date differs from scanned receipt" under the date field, and "Amount greater than scanned receipt" under the amount field

Actual Result:

You see "Receipt ° Verified √" when one of the two mentioned violations are present

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

Recording.3152.mp4

image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01925fef567df5c3b7
  • Upwork Job ID: 1820487169849591150
  • Last Price Increase: 2024-08-05
Issue OwnerCurrent Issue Owner: @muttmuure
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

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.

@bernhardoj
Copy link
Contributor

Proposal

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

Receipt audit shows verified even though there is a receipt violation.

What is the root cause of that problem?

The "Issue found" will be shown if there is a violation that is included in receiptViolationNames array.

const receiptViolationNames: OnyxTypes.ViolationName[] = [
CONST.VIOLATIONS.RECEIPT_REQUIRED,
CONST.VIOLATIONS.RECEIPT_NOT_SMART_SCANNED,
CONST.VIOLATIONS.CASH_EXPENSE_WITH_NO_RECEIPT,
CONST.VIOLATIONS.SMARTSCAN_FAILED,
];
const receiptViolations =
transactionViolations?.filter((violation) => receiptViolationNames.includes(violation.name)).map((violation) => ViolationsUtils.getViolationTranslation(violation, translate)) ?? [];

Both "Date differs from scanned receipt" and "Amount greater than scanned receipt" violations are not in the array, so the "Verified" is shown.

<ReceiptAudit
notes={receiptViolations}

let auditText = '';
if (notes.length > 0 && shouldShowAuditResult) {
auditText = translate('iou.receiptIssuesFound', notes.length);
} else if (!notes.length && shouldShowAuditResult) {
auditText = translate('common.verified');
}

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

We can just add both CONST.VIOLATIONS.MODIFIED_AMOUNT and CONST.VIOLATIONS.MODIFIED_DATE to receiptViolationNames, but it will also show the violation below the receipt which we don't want.
image

The violation below receipt is shown by ReceiptAuditMessages.

{shouldShowAuditMessage && <ReceiptAuditMessages notes={receiptViolations} />}

So, I propose to rename receiptViolationNames to receiptImageViolationNames and add a new array called receiptFieldViolationNames which contains the new violation name. Then, we will also have a new array called receiptImageViolations which contains the image-only violation (the current one).

const receiptImageViolationNames: OnyxTypes.ViolationName[] = [
    CONST.VIOLATIONS.RECEIPT_REQUIRED,
    CONST.VIOLATIONS.RECEIPT_NOT_SMART_SCANNED,
    CONST.VIOLATIONS.CASH_EXPENSE_WITH_NO_RECEIPT,
    CONST.VIOLATIONS.SMARTSCAN_FAILED,
];
const receiptFieldViolationNames: OnyxTypes.ViolationName[] = [
    CONST.VIOLATIONS.MODIFIED_AMOUNT,
    CONST.VIOLATIONS.MODIFIED_DATE,
    // other violation names if needed
]
const [receiptImageViolations, receiptViolations] = useMemo(() => {
    const imageViolations = [];
    const allViolations = [];

    for (const violation of (transactionViolations ?? [])) {
        const isReceiptFieldViolation = receiptFieldViolationNames.includes(violation.name);
        const isReceiptImageViolation = receiptImageViolationNames.includes(violation.name);
        if (isReceiptFieldViolation || isReceiptImageViolation) {
            const violationMessage = ViolationsUtils.getViolationTranslation(violation, translate);
            allViolations.push(violationMessage);
            if (isReceiptImageViolation) {
                imageViolations.push(violationMessage);
            }
        }
    }
    return [imageViolations, allViolations];
}, []);

Then, we pass the receiptImageViolations to ReceiptAuditMessage so it will only show the violation message from receiptImageViolationNames.

{shouldShowAuditMessage && <ReceiptAuditMessages notes={receiptViolations} />}

@cead22 cead22 self-assigned this Aug 5, 2024
@cead22 cead22 added the External Added to denote the issue can be worked on by a contributor label Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

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

@melvin-bot melvin-bot bot changed the title Receipt audit shows "Receipt ° Verified √" when there's a violations related to SmartScan [$250] Receipt audit shows "Receipt ° Verified √" when there's a violations related to SmartScan Aug 5, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Aug 7, 2024
@cead22
Copy link
Contributor

cead22 commented Aug 7, 2024

@thesahindia any thoughts on @bernhardoj 's proposal?

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2024
@thesahindia
Copy link
Member

@bernhardoj's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

Copy link

melvin-bot bot commented Aug 8, 2024

Current assignee @cead22 is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

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

cead22 commented Aug 8, 2024

@bernhardoj do you think you'll have a PR up for this today?

@bernhardoj
Copy link
Contributor

Yes, I'll work on the PR in a few minutes.

@cead22
Copy link
Contributor

cead22 commented Aug 8, 2024

Awesome, thank you!

@bernhardoj
Copy link
Contributor

PR is ready

@bernhardoj
Copy link
Contributor

The PR was deployed to prod 2 weeks ago, so this should be ready for payment.

Requested in ND.

@JmillsExpensify
Copy link

Awaiting payment summary

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Sep 2, 2024
Copy link

melvin-bot bot commented Sep 2, 2024

This issue has not been updated in over 15 days. @cead22, @muttmuure, @bernhardoj, @thesahindia eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@muttmuure muttmuure added Daily KSv2 and removed Monthly KSv2 labels Sep 6, 2024
@muttmuure
Copy link
Contributor

$250 - @bernhardoj
$250 - @thesahindia

@JmillsExpensify
Copy link

$250 approved for @bernhardoj

@cead22
Copy link
Contributor

cead22 commented Sep 11, 2024

Anything else we need to do before we close this one?

@garrettmknight
Copy link
Contributor

$250 approved for @thesahindia

@garrettmknight
Copy link
Contributor

@thesahindia can you complete the checklist?

@thesahindia
Copy link
Member

"Date differs from scanned receipt" and "Amount greater than scanned receipt" violations weren't in the receiptViolationNames array. Because of that we were seeing "Verified".

We can add a test case for them, here are the steps :-

  1. Scan a real receipt that will SmartScan successfully
  2. Wait for it to SmartScan
  3. Once SmartScan finishes, change the date/amount of the receipt
  4. Verify you see a violation under the date field "date differs from scanned receipt"
  5. Verify you see a violation under the amount field "Amount greater than scanned receipt"
  6. Verify you see "Receipt ° Issue found" above the receipt

Copy link

melvin-bot bot commented Sep 19, 2024

@cead22, @muttmuure, @bernhardoj, @thesahindia Whoops! This issue is 2 days overdue. Let's get this updated quick!

Copy link

melvin-bot bot commented Sep 23, 2024

@cead22, @muttmuure, @bernhardoj, @thesahindia 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@cead22
Copy link
Contributor

cead22 commented Sep 25, 2024

@muttmuure bump

@muttmuure
Copy link
Contributor

Created regression test request for applause

@muttmuure
Copy link
Contributor

@thesahindia just needs to submit his request in ND

@garrettmknight
Copy link
Contributor

$250 approved for @thesahindia

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
Status: Done
Development

No branches or pull requests

7 participants