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

[$500] Smartscan: Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully #34178

Closed
6 tasks done
kbecciv opened this issue Jan 9, 2024 · 52 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Jan 9, 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: 1.4.23-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

  1. Go to workspace chat.
  2. Create a Scan request with PDF file as receipt.
  3. When the scanning is complete, open expense report.

Expected Result:

Expense report preview in LHN shows the correct amount.

Actual Result:

Expense report preview in LHN shows 0.00.
This issue is only reproducible when PDF receipt is used.

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

image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e635178117bbd335
  • Upwork Job ID: 1744794404061753344
  • Last Price Increase: 2024-01-30
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jan 9, 2024
@melvin-bot melvin-bot bot changed the title Scan - Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully [$500] Scan - Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully Jan 9, 2024
Copy link

melvin-bot bot commented Jan 9, 2024

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

Copy link

melvin-bot bot commented Jan 9, 2024

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

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

melvin-bot bot commented Jan 9, 2024

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

@Tony-MK
Copy link
Contributor

Tony-MK commented Jan 9, 2024

I think this issue is related to #34064

@s-alves10
Copy link
Contributor

This issue is related to the sidebar and I don't think this is related to #34064

@Tony-MK
Copy link
Contributor

Tony-MK commented Jan 9, 2024

@s-alves10 In this #34064 (comment), Tim Golden instructed.

The LHN and the preview should be using the same logic

If it is not related, then it should be fixed by #34064.

What do you think? 🤔

@s-alves10
Copy link
Contributor

Proposal

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

LHN shows 0.00 for IOU report when receipt is scanned successfully

What is the root cause of that problem?

Let's take a look at getReportPreviewMessage function which is used for displaying second line of LHN

For money request, we get the preview message here

App/src/libs/ReportUtils.ts

Lines 2140 to 2148 in 3acb14a

if (originalMessage?.amount !== undefined && lastActorID && !isPreviewMessageForParentChatReport) {
const amount = originalMessage?.amount;
const currency = originalMessage?.currency ?? report.currency ?? '';
const amountToDisplay = CurrencyUtils.convertToDisplayString(Math.abs(amount), currency);
// We only want to show the actor name in the preview if it's not the current user who took the action
const requestorName = lastActorID && lastActorID !== currentUserAccountID ? getDisplayNameForParticipant(lastActorID, !isPreviewMessageForParentChatReport) : '';
return `${requestorName ? `${requestorName}: ` : ''}${Localize.translateLocal('iou.requestedAmount', {formattedAmount: amountToDisplay})}`;
}

We get the preview message from the original message amount and report currency.

This is the root cause

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

We should get the preview message from the transaction details

We are already doing this for split bill actions here

App/src/libs/ReportUtils.ts

Lines 2067 to 2087 in 3acb14a

if (isNotEmptyObject(reportAction) && !isIOUReport(report) && reportAction && ReportActionsUtils.isSplitBillAction(reportAction)) {
// This covers group chats where the last action is a split bill action
const linkedTransaction = TransactionUtils.getLinkedTransaction(reportAction);
if (isEmptyObject(linkedTransaction)) {
return reportActionMessage;
}
if (isNotEmptyObject(linkedTransaction)) {
if (TransactionUtils.isReceiptBeingScanned(linkedTransaction)) {
return Localize.translateLocal('iou.receiptScanning');
}
if (TransactionUtils.hasMissingSmartscanFields(linkedTransaction)) {
return Localize.translateLocal('iou.receiptMissingDetails');
}
const transactionDetails = getTransactionDetails(linkedTransaction);
const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency ?? '');
return Localize.translateLocal('iou.didSplitAmount', {formattedAmount, comment: transactionDetails?.comment ?? ''});
}
}

Update the above code segments to

    if (isNotEmptyObject(reportAction) && reportAction && ReportActionsUtils.isMoneyRequestAction(reportAction)) {
        // This covers group chats where the last action is a split bill action
        const linkedTransaction = TransactionUtils.getLinkedTransaction(reportAction);
        if (isEmptyObject(linkedTransaction)) {
            return reportActionMessage;
        }

        if (isNotEmptyObject(linkedTransaction)) {
            if (TransactionUtils.isReceiptBeingScanned(linkedTransaction)) {
                return Localize.translateLocal('iou.receiptScanning');
            }

            if (TransactionUtils.hasMissingSmartscanFields(linkedTransaction)) {
                return Localize.translateLocal('iou.receiptMissingDetails');
            }

            const transactionDetails = getTransactionDetails(linkedTransaction);
            const formattedAmount = CurrencyUtils.convertToDisplayString(transactionDetails?.amount ?? 0, transactionDetails?.currency ?? '');
            if (!isIOUReport(report) && ReportActionsUtils.isSplitBillAction(reportAction)) {
                return Localize.translateLocal('iou.didSplitAmount', {formattedAmount, comment: transactionDetails?.comment ?? ''});
            } else {
                const lastActorID = reportAction?.actorAccountID;
                const requestorName = lastActorID && lastActorID !== currentUserAccountID ? getDisplayNameForParticipant(lastActorID, !isPreviewMessageForParentChatReport) : '';
                return `${requestorName ? `${requestorName}: ` : ''}${Localize.translateLocal('iou.requestedAmount', {formattedAmount})}`;
            }
        }
    }

This works great

Result

image

What alternative solutions did you explore? (Optional)

@s-alves10
Copy link
Contributor

@Tony-MK

#34064 is related to IOU report preview in chat report and LHN for the report
This issue is related to money request in IOU report and LHN for the IOU report
They are different.

@melvin-bot melvin-bot bot added the Overdue label Jan 12, 2024
@kmbcook
Copy link
Contributor

kmbcook commented Jan 12, 2024

Proposal

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

Scan - Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully

What is the root cause of that problem?

libs/SidebarUtils.getOptionData sets alternateText incorrectly when last visible report action is an iou report action.

This root cause is the same root cause as #33733. The changes proposed here fix both of these issues.

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

In libs/SidebarUtils.getOptionData check to see if last visible report action is an iou. If so, set alternateText correctly, using the latest amount of the iou transaction. Otherwise, use existing logic to set alternateText.

@JmillsExpensify
Copy link

@fedirjh Thoughts on these proposals?

@melvin-bot melvin-bot bot removed the Overdue label Jan 14, 2024
Copy link

melvin-bot bot commented Jan 16, 2024

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

@melvin-bot melvin-bot bot added the Overdue label Jan 16, 2024
@fedirjh
Copy link
Contributor

fedirjh commented Jan 16, 2024

I think this bug and #34064 are related. I think the fix should be on the backend. Let's put it on hold for #34064.

The backend should return the correct text for the IOU action: for reference, this is the IOU action which doesn’t contain the correct text: this action is used to get the text displayed in the LHN

Screenshot 2024-01-16 at 5 02 39 PM

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 16, 2024
Copy link

melvin-bot bot commented Jan 19, 2024

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

@fedirjh
Copy link
Contributor

fedirjh commented Jan 19, 2024

I think this will be fixed with #34717.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 19, 2024
@fedirjh
Copy link
Contributor

fedirjh commented Jan 22, 2024

#34717 is still being reviewed.

Copy link

melvin-bot bot commented Jan 23, 2024

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

@greg-schroeder
Copy link
Contributor

Added Hot Pick

Copy link

melvin-bot bot commented Feb 28, 2024

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

Copy link

melvin-bot bot commented Mar 1, 2024

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

Copy link

melvin-bot bot commented Mar 5, 2024

@JmillsExpensify, @fedirjh 12 days overdue now... This issue's end is nigh!

@JmillsExpensify
Copy link

@trjExpensify This one has been in hot picks, though never got picked up. Maybe we re-highlight in #wave-collect?

@melvin-bot melvin-bot bot removed the Overdue label Mar 6, 2024
@trjExpensify trjExpensify changed the title [$500] [MEDIUM] Smartscan: Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully [$500] Smartscan: Expense report preview in LHN shows 0.00 when PDF receipt is scanned successfully Mar 6, 2024
@trjExpensify
Copy link
Contributor

Will do!

@Gonals Gonals self-assigned this Mar 7, 2024
@Gonals
Copy link
Contributor

Gonals commented Mar 7, 2024

I can take a look at this 😁

@trjExpensify
Copy link
Contributor

Niceee! Removing the Hot picks label and putting this in Release 1 as it's a bad bug to be scanning $0.00s

@trjExpensify trjExpensify removed the Hot Pick Ready for an engineer to pick up and run with label Mar 7, 2024
@Gonals
Copy link
Contributor

Gonals commented Mar 7, 2024

Actually, it seems I can't reproduce this. @kbecciv, could you give it another try and confirm you still see this issue?
Screenshot 2024-03-07 at 4 11 56 PM

cc @trjExpensify

@kbecciv
Copy link
Author

kbecciv commented Mar 7, 2024

Checking

@Gonals
Copy link
Contributor

Gonals commented Mar 8, 2024

Checking

Any news, @kbecciv?

@kbecciv
Copy link
Author

kbecciv commented Mar 8, 2024

Issue is no longer reproduced
Screenshot 2024-03-09 at 00 32 34
!

@melvin-bot melvin-bot bot added the Overdue label Mar 11, 2024
@Gonals
Copy link
Contributor

Gonals commented Mar 11, 2024

Closing, then!

@Gonals Gonals closed this as completed Mar 11, 2024
@melvin-bot melvin-bot bot removed the Overdue label Mar 11, 2024
@lanitochka17
Copy link

Issue is still reproducible on the latest build 9.0.18-1

20240808_204303.mp4

@lanitochka17 lanitochka17 reopened this Aug 8, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 12, 2024
Copy link

melvin-bot bot commented Aug 13, 2024

@JmillsExpensify, @Gonals, @fedirjh Huh... This is 4 days overdue. Who can take care of this?

@JmillsExpensify
Copy link

Can we create a fresh issue for this?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 14, 2024
@Gonals
Copy link
Contributor

Gonals commented Aug 19, 2024

I can't seem to reproduce this (once again). Closing for now 🤷

@Gonals Gonals closed this as completed Aug 19, 2024
@melvin-bot melvin-bot bot removed the Overdue label Aug 19, 2024
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 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
Archived in project
Development

No branches or pull requests