[$250] Split & Invoice -Preview shows empty receipt placeholder when there is no way to add a receipt

[IuliiaHerets]

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.71-0 Reproducible in staging?: Yes Reproducible in production?: No If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A If this was caught during regression testing, add the test name, ID and link from TestRail: Exp Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Click + > Split expense > Manual.
4. Submit a split manual expense.
5. Note that the split preview has an empty receipt placeholder when there is no option to manually add a receipt to the split details.
6. Go to FAB > Send invoice.
7. Send an invoice to anyone.
8. Note that the invoice preview also has an empty receipt placeholder when it is not possible to add a receipt to invoice.

Expected Result:

Empty receipt placeholder should not be displayed for split and invoice preview because there is no option to add a receipt in the first place.

Actual Result:

Empty receipt placeholder is displayed for split and invoice preview when there is no option to add a receipt in the first place.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/6f213267-53e7-492d-8743-1876a69d8e29

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021864292349376469824
• Upwork Job ID: 1864292349376469824
• Last Price Increase: 2024-12-04

Issue Owner

Current Issue Owner: @rojiphil

[melvin-bot[bot]]

Triggered auto assignment to @garrettmknight (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.

[github-actions[bot]]

:wave: 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.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[garrettmknight]

I don't think we'll need to block the deploy for this fix.

[mkzie2]

Edited by proposal-police: This proposal was edited at 2024-12-04 13:06:23 UTC.

Proposal

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

Empty receipt placeholder is displayed for split and invoice preview when there is no option to add a receipt in the first place.

What is the root cause of that problem?

We always show the ReportActionItemImages. When the transaction doesn't have the receipt, isEmptyReceipt is true and this component will display the ReceiptEmptyState

https://github.com/Expensify/App/blob/146204878d1b9d4358d91e9a0a81169a7819d96f/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L351

https://github.com/Expensify/App/blob/20b4c7db47969f7d2209f1c0b8fd765ce883f92c/src/components/ReportActionItem/ReportPreview.tsx#L475C1-L480C27

https://github.com/Expensify/App/blob/361a024c382249524a381a05edbe1b4a5f1d8775/src/components/ReportActionItem/ReportActionItemImages.tsx#L87

https://github.com/Expensify/App/blob/361a024c382249524a381a05edbe1b4a5f1d8775/src/components/ReceiptImage.tsx#L111-L115

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

We should hide the receipt component if the transaction doesn't have the receipt and it's split in MoneyRequestPreviewContent

const shouldHideReceipt = !hasReceipt && isBillSplit;
{!shouldHideReceipt && (
    <ReportActionItemImages
        images={receiptImages}
        isHovered={isHovered || isScanning}
        size={1}
        onPress={shouldDisableOnPress ? undefined : onPreviewPressed}
    />
)}

https://github.com/Expensify/App/blob/146204878d1b9d4358d91e9a0a81169a7819d96f/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L351

And hide it in ReportPreview if it's an invoice

{!isInvoiceRoom && (
    <ReportActionItemImages
        images={lastThreeReceipts}
        total={allTransactions.length}
        size={CONST.RECEIPT.MAX_REPORT_PREVIEW_RECEIPTS}
        onPress={openReportFromPreview}
    />
)}

https://github.com/Expensify/App/blob/20b4c7db47969f7d2209f1c0b8fd765ce883f92c/src/components/ReportActionItem/ReportPreview.tsx#L475C1-L480C27

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

It's a UI bug and I don't think we need a test case here.

Or we can create a util shouldHideReceipt, use it here and add some tests to verify that it works as expected for some cases:

• The transaction has receipt

• The transaction has no receipt and the the iou report is money request report or it's a track expense action

• The transaction has no receipt and it's a split bill or send invoice

What alternative solutions did you explore? (Optional)

[rojiphil]

@mkzie2 Thanks for your proposal. The RCA is correct as we are showing the receipt preview without checking if we can show receipt preview or not. However, instead of focusing on the condition to show, can you please update the proposal to focus on when not to show (i.e. when split and invoice preview).

[mkzie2]

@rojiphil Updated my proposal with the condition to hide the receipt.

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-12-04 14:28:36 UTC.

Proposal

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

Split & Invoice -Preview shows empty receipt placeholder when there is no way to add a receipt

What is the root cause of that problem?

• We try to get the receipts of the last three transactions and if the transaction does not have any receipt, we return {isEmptyReceipt: true} from getThumbnailAndImageURIs and when the array is passed to ReportActionItemImages the ReportActionItemImages will render an empty placeholder if isEmptyReceipt is true https://github.com/Expensify/App/blob/0296f801a4c8c2a755f4dd9de1168a3fbe8240c1/src/components/ReportActionItem/ReportPreview.tsx#L176-L177 https://github.com/Expensify/App/blob/0296f801a4c8c2a755f4dd9de1168a3fbe8240c1/src/components/ReportActionItem/ReportPreview.tsx#L475-L480

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

• We should a the same condition used in MoneyRequestView to show the empty state and using the condition we will filter out the transactions which doesn't have an receipt and receipt can't be updated/added to it. https://github.com/Expensify/App/blob/0296f801a4c8c2a755f4dd9de1168a3fbe8240c1/src/components/ReportActionItem/MoneyRequestView.tsx#L362-L364

    // This is just pseudocode, we can refactor this in the PR.
    const isInvoice = ReportUtils.isInvoiceReport(iouReport);
    const canUserPerformWriteAction = !!ReportUtils.canUserPerformWriteAction(iouReport);
    const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
    const isApprover = ReportUtils.isMoneyRequestReport(iouReport) && iouReport?.managerID !== null && session?.accountID === iouReport?.managerID;
    const currentUserPersonalDetails = useCurrentUserPersonalDetails();
    const isRequestor = currentUserPersonalDetails.accountID === action?.actorAccountID;
    const canEditReceipt = canUserPerformWriteAction && ReportUtils.canEditFieldOfMoneyRequest(action, CONST.EDIT_REQUEST_FIELD.RECEIPT);

    const isSettled = ReportUtils.isSettled(iouReport?.reportID);
    const shouldShowReceiptEmptyState =
        !isInvoice && !isApproved && !isSettled && (canEditReceipt || isAdmin || isApprover || isRequestor) && (canEditReceipt || ReportUtils.isPaidGroupPolicy(iouReport));
    const lastThreeTransactions = allTransactions.slice(-3).filter((t) => TransactionUtils.hasReceipt(t) || shouldShowReceiptEmptyState);
     const lastThreeReceipts = lastThreeTransactions.map((transaction) => ({...ReceiptUtils.getThumbnailAndImageURIs(transaction), transaction}));

• We will hide ReportActionItemImages if lastThreeReceipts length is 0 and in MoneyRequestPreviewContent we will return empty array when we shouldShowReceiptEmptyState condition is not matched and similarly we will not render ReportActionItemImages when receiptImages length is 0. https://github.com/Expensify/App/blob/361a024c382249524a381a05edbe1b4a5f1d8775/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L178 https://github.com/Expensify/App/blob/361a024c382249524a381a05edbe1b4a5f1d8775/src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx#L351-L356
• We can create a util function for getting the value of shouldShowReceiptEmptyState because it will be used in multiple places.
• We also need to do the same changes in MoneyRequestPreviewContent.tsx

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?


• If we add a new util function for checking if we can show the receipt empty view or not then we can add some tests for the util function with some different type of IOU reports.

What alternative solutions did you explore? (Optional)

[Krishna2323]

@rojiphil could you please check my proposal? Thanks!

[rojiphil]

@mkzie2 @Krishna2323 Thanks both for your proposals. However, your proposals lack clarity on both the root cause and the solution and is unacceptable to me in its current form. Can you please be more precise with respect to the root cause and solution?

[Krishna2323]

@rojiphil, Proposal Updated.

[mkzie2]

Updated proposal.

[Krishna2323]

PROPOSAL UPDATED

• The siolution remains the same, just added more explanation.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[rojiphil]

Proposal

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

We should not display empty receipt placeholder for split and invoice previews as there is no provision to add a receipt at all for these cases.

What is the root cause of that problem?

Previous to the PR #52848, we displayed ReportActionItemImages conditionally based on hasReceipt. But the implementation was intentionally changed so that the PR can introduce placeholder thumbnail to expense previews that does not have any receipt. However, in the process, we completely removed the condition of hasReceipt here and here and always displayed the receipt placeholder thumbnail even when this was not required for split and invoice previews. This is the root cause of the problem.

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

For invoice rooms, we need to prevent showing receipt thumbnail for the report preview of all invoices. This can be done here by conditionally displaying as demonstrated below:

                        {!isInvoiceRoom && (
                            <ReportActionItemImages
                                images={lastThreeReceipts}
                                total={allTransactions.length}
                                size={CONST.RECEIPT.MAX_REPORT_PREVIEW_RECEIPTS}
                                onPress={openReportFromPreview}
                            />
                        )}

Further, the receipt thumbnail image for split previews is displayed here. We can leverage the existing isBillSplit variable to prevent display of the thumbnail image as follows:

                    {!isBillSplit && (
                        <ReportActionItemImages
                            images={receiptImages}
                            isHovered={isHovered || isScanning}
                            size={1}
                            onPress={shouldDisableOnPress ? undefined : onPreviewPressed}
                        />
                    )}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

[rojiphil]

@mkzie2 @Krishna2323 Since your proposals are still unacceptable to me in it’s current form, I have put forward a proposal to avoid further delays here. Let me know if you think your proposal is still better.

[mkzie2]

@rojiphil What is the difference between my proposal and your proposal here. Beside, the split bill can have the receipt at the time we create then we should only hide the ReportActionItemImages if we don't have a receipt and it's split bill or invoice room.

[rojiphil]

What is the difference between my proposal and your proposal here

@mkzie2 Maybe I am missing something here. Can you please share a test video based on your proposal? I think that can help us understand the difference.

Beside, the split bill can have the receipt at the time we create then we should only hide the ReportActionItemImages if we don't have a receipt and it's split bill or invoice room.

And, I am not sure of this use case. But it should be clear once we have a test video of your proposal.

[Krishna2323]

@rojiphil, I'm not sure what's missing in my proposal, It uses the same logic as applied in MoneyRequestView, do you need a test branch or something?

[rojiphil]

I'm not sure what's missing in my proposal, It uses the same logic as applied in MoneyRequestView, do you need a test branch or something?

@Krishna2323 Why is there a need to introduce many changes when we can keep the fix simple with minimal changes? But let me know if there is any distinct advantage in your proposal that I missed. And I would be happy to revisit your proposal. Thanks.

[melvin-bot[bot]]

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

[Krishna2323]

Will respond to this within a hour.

[mkzie2]

@mkzie2 Maybe I am missing something here. Can you please share a test video based on your proposal? I think that can help us understand the difference.

I understood what is different here. The invoice case should be updated in ReportPreview. I updated the proposal for this case.

Beside, the split bill can have the receipt at the time we create then we should only hide the ReportActionItemImages if we don't have a receipt and it's split bill or invoice room.

And, I am not sure of this use case. But it should be clear once we have a test video of your proposal.

Here is the case what I thought, the split bill still can have the receipt.

https://github.com/user-attachments/assets/6fb847e7-09f0-49f0-bdd0-df0826a99ac6

[Krishna2323]

@rojiphil, the reason for adding the same logic used in MoneyRequestView is:

Preview shows empty receipt placeholder when there is no way to add a receipt

• When transaction has no receipt and it has been paid, the empty receipt placeholder is not shown on MoneyRequestView page, so to be consistent with that, we should also hide the empty thumbnail on ReportPreview & MoneyRequestPreview.

• The same case happens when the report is approved. So instead of fixing just one case we should only show the thumbnail when receipt can be added on MoneyRequestView.

• On staging, you can see that empty receipt placeholder is shown even if the expense is paid without receipt and has no option to add receipt.

https://github.com/user-attachments/assets/825ecd33-8df2-40c9-8f01-9cf7192b2797

• We will create a util function that will be used in all three components for keeping the code cleaner instead of adding multiple lines in every component.

[rojiphil]

Thanks @mkzie2 @Krishna2323 for the follow-up with your proposals. I agree @mkzie2 that even split expenses can have receipts and we should show the receipt if present. But @Krishna2323 proposal is better as it also covers additional cases (e.g. paid expenses).

Let’s go with @Krishna2323 proposal 🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

Triggered auto assignment to @lakchote, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[lakchote]

@Krishna2323's proposal LGTM.

[melvin-bot[bot]]

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