Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/components/ReportActionItem/MoneyRequestReceiptView.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import AttachmentPicker from '@components/AttachmentPicker';
import Badge from '@components/Badge';
import Icon from '@components/Icon';
import {ModalActions} from '@components/Modal/Global/ModalContext';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
Expand Down Expand Up @@ -85,6 +86,7 @@ import type {ValueOf} from 'type-fest';
import {useRoute} from '@react-navigation/native';
import {hasSeenTourSelector} from '@selectors/Onboarding';
import {conciergePersonalDetailSelector, personalDetailsSelector} from '@selectors/PersonalDetails';
import {Str} from 'expensify-common';
import mapValues from 'lodash/mapValues';
import React, {useEffect, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
Expand Down Expand Up @@ -196,6 +198,9 @@ function MoneyRequestReceiptView({
// stale and can't be redrawn locally, so disable Expand for map distance requests until the refreshed receipt arrives.
const shouldDisableExpandReceipt = isMapDistanceRequest && isPendingReceiptRegeneration;
const hasReceipt = hasReceiptTransactionUtils(displayedTransaction);
// The thumbnail only ever renders page 1 of a PDF, so a total sitting on a later page looks like it
// disagrees with the expense amount. Only multi-page receipts need the badge.
const receiptPageCount = displayedTransaction?.receipt?.pageCount ?? 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear stale page counts when replacing receipts

When a multi-page PDF is replaced, replaceReceipt() in src/libs/actions/IOU/Receipt.ts applies an Onyx MERGE whose nested receiptOptimistic object omits pageCount; therefore the previous count remains in Onyx. Since this line trusts that retained value without checking whether the new receipt is a PDF, replacing the PDF with an image—or with a PDF having fewer pages—shows the old Page 1 of N badge after loading, and an image response that omits this PDF-only field may never clear it. Clear pageCount in receipt replacement paths or gate the badge against the current receipt type.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed in 5e7544a. receiptOptimistic is applied with Onyx.METHOD.MERGE, so omitting pageCount left the replaced receipt's count in place until the server responded. Now sends pageCount: null, matching how localSource is already cleared in the same object, and the type allows null.

Worth noting the server side of this is already handled — Auth's attach/replace update always sends pageCount (null when the new receipt has none), so the correction arrives rather than the field simply being omitted. This closes the optimistic window before it.

const isTransactionScanning = isScanning(displayedTransaction);
const didReceiptScanSucceed = hasReceipt && didReceiptScanSucceedTransactionUtils(transaction);
const isInvoice = isInvoiceReport(moneyRequestReport);
Expand Down Expand Up @@ -545,6 +550,12 @@ function MoneyRequestReceiptView({

// Map distance receipts show both hover actions just like regular receipts, so we don't exclude isMapDistanceRequest here.
const canShowReceiptActions = hasReceipt && !isLoading && isEditable && !mergeTransactionID;

// A page count only means anything for a PDF, and optimistic writers that swap the receipt with a
// merge can leave the replaced PDF's count behind, so the current file type decides this rather
// than the count alone. Held back until the receipt has loaded too: the container stretches while
// loading, which would strand the badge at the bottom of that taller box.
const shouldShowReceiptPageCount = receiptPageCount > 1 && Str.isPDF(receiptURIs?.filename ?? '') && !isLoading;
const receiptPendingAction = isDistanceRequest ? getPendingFieldAction('waypoints') : getPendingFieldAction('receipt');
const isReceiptOfflinePending = isOffline && !!receiptPendingAction;
const receiptAuditMessagesRow = (
Expand Down Expand Up @@ -702,6 +713,12 @@ function MoneyRequestReceiptView({
)}
</ReceiptHoverZoom>
</View>
{shouldShowReceiptPageCount && (
<Badge
text={translate('receipt.pageCount', {pageCount: receiptPageCount})}
badgeStyles={[styles.receiptPageCountBadge, styles.pointerEventsNone]}
/>
)}
{canShowReceiptActions && (
<View style={[styles.receiptActionButtonsContainer, styles.pointerEventsBoxNone, !hovered && !isPickerOpen && deviceHasHoverSupport && styles.opacity0]}>
<AttachmentPicker acceptedFileTypes={[...CONST.API_ATTACHMENT_VALIDATIONS.ALLOWED_RECEIPT_EXTENSIONS]}>
Expand Down
1 change: 1 addition & 0 deletions src/languages/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1213,6 +1213,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Beleg hinzufügen',
phrase2: 'oder ziehe eine hierher und lege sie ab',
},
pageCount: ({pageCount}: {pageCount: number}) => `Seite 1 von ${pageCount}`,
},
quickAction: {
scanReceipt: 'Beleg scannen',
Expand Down
1 change: 1 addition & 0 deletions src/languages/el.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Προσθήκη απόδειξης',
phrase2: 'ή σύρετε και αποθέστε ένα εδώ',
},
pageCount: ({pageCount}: {pageCount: number}) => `Σελίδα 1 από ${pageCount}`,
},
quickAction: {
scanReceipt: 'Σαρώστε απόδειξη',
Expand Down
1 change: 1 addition & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,7 @@ const translations = {
addAdditionalReceipt: 'Add additional receipt',
scanFailed: "The receipt couldn't be scanned, as it's missing a merchant, date, or amount.",
crop: 'Crop',
pageCount: ({pageCount}: {pageCount: number}) => `Page 1 of ${pageCount}`,
addAReceipt: {
phrase1: 'Add a receipt',
phrase2: 'or drag and drop one here',
Expand Down
1 change: 1 addition & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ const translations: TranslationDeepObject<typeof en> = {
addAdditionalReceipt: 'Añadir recibo adicional',
scanFailed: 'El recibo no pudo ser escaneado, ya que falta el comerciante, la fecha o el importe.',
crop: 'Recortar',
pageCount: ({pageCount}: {pageCount: number}) => `Página 1 de ${pageCount}`,
addAReceipt: {
phrase1: 'Añade un recibo',
phrase2: 'o arrastra y suelta uno aquí',
Expand Down
1 change: 1 addition & 0 deletions src/languages/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Ajouter un reçu',
phrase2: 'ou faites-en glisser un ici',
},
pageCount: ({pageCount}: {pageCount: number}) => `Page 1 sur ${pageCount}`,
},
quickAction: {
scanReceipt: 'Scanner le reçu',
Expand Down
1 change: 1 addition & 0 deletions src/languages/it.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Aggiungi una ricevuta',
phrase2: 'o trascinalo qui',
},
pageCount: ({pageCount}: {pageCount: number}) => `Pagina 1 di ${pageCount}`,
},
quickAction: {
scanReceipt: 'Scansiona ricevuta',
Expand Down
1 change: 1 addition & 0 deletions src/languages/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: '領収書を追加',
phrase2: 'または、ここにファイルをドラッグ&ドロップしてください',
},
pageCount: ({pageCount}: {pageCount: number}) => `${pageCount} ページ中 1 ページ`,
},
quickAction: {
scanReceipt: 'レシートをスキャン',
Expand Down
1 change: 1 addition & 0 deletions src/languages/nl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Voeg een bon toe',
phrase2: 'of sleep ze hier naartoe',
},
pageCount: ({pageCount}: {pageCount: number}) => `Pagina 1 van ${pageCount}`,
},
quickAction: {
scanReceipt: 'Bon scannen',
Expand Down
1 change: 1 addition & 0 deletions src/languages/pl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Dodaj paragon',
phrase2: 'lub przeciągnij i upuść tutaj',
},
pageCount: ({pageCount}: {pageCount: number}) => `Strona 1 z ${pageCount}`,
},
quickAction: {
scanReceipt: 'Zeskanuj paragon',
Expand Down
1 change: 1 addition & 0 deletions src/languages/pt-BR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: 'Adicionar um recibo',
phrase2: 'ou arraste e solte um aqui',
},
pageCount: ({pageCount}: {pageCount: number}) => `Página 1 de ${pageCount}`,
},
quickAction: {
scanReceipt: 'Digitalizar recibo',
Expand Down
1 change: 1 addition & 0 deletions src/languages/zh-hans.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,7 @@ const translations: TranslationDeepObject<typeof en> = {
phrase1: '添加收据',
phrase2: '或将文件拖放到此处',
},
pageCount: ({pageCount}: {pageCount: number}) => `第 1 页,共 ${pageCount} 页`,
},
quickAction: {
scanReceipt: '扫描收据',
Expand Down
2 changes: 2 additions & 0 deletions src/libs/DebugUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,7 @@ function validateReportActionDraftProperty(key: keyof ReportAction, value: strin
isTestDriveReceipt: 'boolean',
thumbnail: 'string',
receiptTraceId: 'string',
pageCount: 'number',
});
case 'childRecentReceiptTransactionIDs':
return validateObject<ObjectElement<ReportAction, 'childRecentReceiptTransactionIDs'>>(value, {}, 'string');
Expand Down Expand Up @@ -1206,6 +1207,7 @@ function validateTransactionDraftProperty(key: keyof Transaction, value: string)
isTestDriveReceipt: 'boolean',
thumbnail: 'string',
receiptTraceId: 'string',
pageCount: 'number',
});
case 'taxRate':
return validateObject<ObjectElement<Transaction, 'taxRate'>>(value, {
Expand Down
4 changes: 4 additions & 0 deletions src/libs/actions/IOU/Receipt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,10 @@ function replaceReceipt({
state: state ?? CONST.IOU.RECEIPT_STATE.OPEN,
filename: file.name,
receiptTraceId,
// Cleared explicitly because this is a merge: the replacement is not known to be a
// multi-page PDF, so keeping the replaced receipt's count would label it with the old one's
// page count until the server responds.
pageCount: null,
};
const newTransaction = transaction && {...transaction, receipt: receiptOptimistic};
const retryParams: ReplaceReceiptRetryParams = {
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/MergeTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,10 @@ function getOnyxTargetTransactionData({
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction.transactionID}`,
value: {
receipt: mergeTransaction.receipt ?? null,
// pageCount is cleared explicitly because this is a merge: the chosen receipt is not
// necessarily a multi-page PDF, and omitting the key would leave the target's old count
// describing a receipt it no longer has.
receipt: mergeTransaction.receipt ? {pageCount: null, ...mergeTransaction.receipt} : null,
},
});

Expand Down
10 changes: 10 additions & 0 deletions src/styles/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4038,6 +4038,16 @@ const staticStyles = (theme: ThemeColors) =>
gap: 8,
},

// Sits bottom-left because the receipt action buttons occupy the top-right corner, and floats
// over the receipt rather than sitting inside it, so hover-zoom magnifies the receipt and
// leaves the badge alone. Badge applies its own left margin, which is cleared here.
receiptPageCountBadge: {
position: 'absolute',
bottom: 12,
left: 12,
marginLeft: 0,
},

receiptActionButton: {
width: 40,
height: 40,
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ type Receipt = {
/** Collection of reservations */
reservationList?: Reservation[];

/** Number of pages in a receipt stored as a PDF. Absent for images, for PDFs uploaded before the backend reported a count, and null while a replacement receipt is pending. */
pageCount?: number | null;

/** Whether this is a test receipt */
isTestReceipt?: true;

Expand Down
86 changes: 86 additions & 0 deletions tests/ui/components/MoneyRequestReceiptViewTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,16 @@ const transactionWithReceipt: Transaction = {
},
};

const transactionWithMultiPagePDFReceipt: Transaction = {
...transactionWithoutReceipt,
receipt: {
state: CONST.IOU.RECEIPT_STATE.OPEN,
source: 'https://example.com/receipt.pdf',
filename: 'receipt.pdf',
pageCount: 3,
},
};

const transactionWithScanningReceipt: Transaction = {
...transactionWithoutReceipt,
receipt: {
Expand Down Expand Up @@ -274,6 +284,82 @@ describe('MoneyRequestReceiptView', () => {
});
});

describe('receipt page count badge', () => {
it('shows the page count for a multi-page PDF receipt', async () => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TEST_TRANSACTION_ID}`, transactionWithMultiPagePDFReceipt);
});
await waitForBatchedUpdatesWithAct();

render(
<Wrapper>
<MoneyRequestReceiptView report={testReport} />
</Wrapper>,
);
await waitForBatchedUpdatesWithAct();

expect(screen.getByText(translateLocal('receipt.pageCount', {pageCount: 3}))).toBeTruthy();
});

it('does not show the page count for a single page PDF receipt', async () => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TEST_TRANSACTION_ID}`, {
...transactionWithMultiPagePDFReceipt,
receipt: {...transactionWithMultiPagePDFReceipt.receipt, pageCount: 1},
});
});
await waitForBatchedUpdatesWithAct();

render(
<Wrapper>
<MoneyRequestReceiptView report={testReport} />
</Wrapper>,
);
await waitForBatchedUpdatesWithAct();

expect(screen.queryByText(translateLocal('receipt.pageCount', {pageCount: 1}))).toBeNull();
});

// An optimistic merge that swaps a PDF for an image can leave the PDF's count behind, so the
// badge has to follow the current file type rather than the leftover count
it('does not show the page count when a stale count is left on an image receipt', async () => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TEST_TRANSACTION_ID}`, {
...transactionWithMultiPagePDFReceipt,
receipt: {...transactionWithMultiPagePDFReceipt.receipt, source: 'https://example.com/photo.jpg', filename: 'photo.jpg'},
});
});
await waitForBatchedUpdatesWithAct();

render(
<Wrapper>
<MoneyRequestReceiptView report={testReport} />
</Wrapper>,
);
await waitForBatchedUpdatesWithAct();

expect(screen.queryByText(translateLocal('receipt.pageCount', {pageCount: 3}))).toBeNull();
});

// An image receipt carries no page count at all, which is also what a PDF uploaded before the
// backend started reporting one looks like
it('does not show the page count for a receipt without one', async () => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TEST_TRANSACTION_ID}`, transactionWithReceipt);
});
await waitForBatchedUpdatesWithAct();

render(
<Wrapper>
<MoneyRequestReceiptView report={testReport} />
</Wrapper>,
);
await waitForBatchedUpdatesWithAct();

expect(screen.queryByText(translateLocal('receipt.pageCount', {pageCount: 3}))).toBeNull();
});
});

describe('receipt action buttons visibility', () => {
it('does not show action buttons when transaction has no receipt', async () => {
render(
Expand Down
Loading