fix(distance): draw the New Expensify e-receipt instead of the Classic PDF

Author: yuwenmemon

src/components/Attachments/AttachmentView/index.tsx

@@ -28,7 +28,7 @@ import addEncryptedAuthTokenToURL from '@libs/addEncryptedAuthTokenToURL';
 import {canUseTouchScreen} from '@libs/DeviceCapabilities';
 import {getFileResolution, isHighResolutionImage} from '@libs/fileDownload/FileUtils';
 import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
-import {hasEReceipt, hasReceiptSource, isPerDiemRequest, shouldRenderLocalDistanceEReceipt} from '@libs/TransactionUtils';
+import {hasEReceipt, hasReceiptSource, isMapBasedDistanceRequest, isPerDiemRequest} from '@libs/TransactionUtils';
 
 import type {ColorValue} from '@styles/utils/types';
 import variables from '@styles/variables';
@@ -255,10 +255,13 @@ function AttachmentView({
         );
     }
 
-    // A distance expense normally shows the receipt file that the server generated, which the PDF branch below
-    // renders. Draw the card only for the cases that file cannot cover, and do it before that branch so the
-    // stale URL of a receipt that the server is rebuilding is never requested.
-    if (transaction && shouldRenderLocalDistanceEReceipt(transaction)) {
+    // New Expensify builds the distance e-receipt from the expense, which is why the server stores only the route
+    // map as the thumbnail for it to draw around. The generated PDF beside it is for Expensify Classic, which
+    // cannot build one in the frontend, and it prints the routed trip rather than what the expense bills. Showing
+    // that PDF here made the enlarged receipt contradict every other surface, so draw the card instead. This runs
+    // before the PDF branch below, which would otherwise return first.
+    // See https://github.com/Expensify/Expensify/issues/545298 and https://github.com/Expensify/App/issues/97013.
+    if (transaction && isMapBasedDistanceRequest(transaction)) {
         return <DistanceEReceipt transaction={transaction} />;
     }
 

src/components/DistanceEReceipt.tsx

@@ -7,7 +7,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
 
 import {getThumbnailAndImageURIs} from '@libs/ReceiptUtils';
 import {getTransactionDetails} from '@libs/ReportUtils';
-import {getWaypointIndex, hasReceipt, hasUsableStoredDistanceReceipt} from '@libs/TransactionUtils';
+import {getWaypointIndex, hasDistanceRouteErrors, hasPendingDistanceReceiptRegeneration, hasReceipt} from '@libs/TransactionUtils';
 import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
 
 import type {TranslationPaths} from '@src/languages/types';
@@ -41,9 +41,9 @@ function DistanceEReceipt({transaction, hoverPreview = false}: DistanceEReceiptP
     const {amount: transactionAmount, currency: transactionCurrency, merchant: transactionMerchant, created: transactionDate} = getTransactionDetails(transaction) ?? {};
     const formattedTransactionAmount = convertToDisplayString(transactionAmount, transactionCurrency);
     const thumbnailSource = tryResolveUrlFromApiRoot(thumbnail ?? '');
-    // The thumbnail is a page of the stored receipt file, so this card must not show it once the rest of the app
-    // has stopped trusting that file.
-    const canShowStoredReceiptPage = hasUsableStoredDistanceReceipt(transaction);
+    // The thumbnail this card draws is the stored route map. An edit that makes the server rebuild the receipt
+    // invalidates its URL, and a route error means there is no trip to draw, so show the pending map for both.
+    const canShowStoredRouteMap = !hasPendingDistanceReceiptRegeneration(transaction) && !hasDistanceRouteErrors(transaction);
     const waypoints = useMemo(() => transaction?.comment?.waypoints ?? {}, [transaction?.comment?.waypoints]);
     const sortedWaypoints = useMemo<WaypointCollection>(
         () =>
@@ -68,7 +68,7 @@ function DistanceEReceipt({transaction, hoverPreview = false}: DistanceEReceiptP
                     />
 
                     <View style={[styles.moneyRequestViewImage, styles.mh0, styles.mt0, styles.mb5, styles.borderNone]}>
-                        {!canShowStoredReceiptPage || !thumbnailSource ? (
+                        {!canShowStoredRouteMap || !thumbnailSource ? (
                             <PendingMapView />
                         ) : (
                             <ReceiptImage

src/components/ReportActionItem/MoneyRequestReceiptView.tsx

@@ -57,7 +57,6 @@ import {
     isDistanceRequest as isDistanceRequestTransactionUtils,
     isMapBasedDistanceRequest,
     isScanning,
-    shouldRenderLocalDistanceEReceipt,
 } from '@libs/TransactionUtils';
 import ViolationsUtils, {filterReceiptViolations} from '@libs/Violations/ViolationsUtils';
 
@@ -186,12 +185,13 @@ function MoneyRequestReceiptView({
     const displayedTransaction = updatedTransaction ?? transaction;
     const isDistanceRequest = isDistanceRequestTransactionUtils(displayedTransaction);
 
+    // The hover overlay shows the full distance e-receipt (map + amount + waypoints), so only surface it for
+    // map/route-based distance expenses. Odometer and pure-manual distance expenses have no map and must be excluded.
     const isMapDistanceRequest = isMapBasedDistanceRequest(displayedTransaction);
     const isPendingReceiptRegeneration = hasPendingDistanceReceiptRegeneration(displayedTransaction);
-    // The overlay draws the distance e-receipt card from the expense, therefore it must appear only where the
-    // receipt box has no generated file to show. Over a generated file it would state a total and a mileage that
-    // the file itself can contradict, which is the whole of https://github.com/Expensify/App/issues/97013.
-    const canShowDistanceEReceipt = shouldRenderLocalDistanceEReceipt(displayedTransaction);
+    // While the receipt is regenerating (e.g. after an offline waypoint edit) the stored map is stale and can't be
+    // redrawn locally, so don't surface the e-receipt overlay — the receipt box already shows the pending map.
+    const canShowDistanceEReceipt = isMapDistanceRequest && !isPendingReceiptRegeneration;
     // The Expand button opens the full-screen receipt on the stored map. While regeneration is pending that map is
     // stale and can't be redrawn locally, so disable Expand for map distance requests until the refreshed receipt arrives.
     const shouldDisableExpandReceipt = isMapDistanceRequest && isPendingReceiptRegeneration;

src/components/ReportActionItem/ReportActionItemImage.tsx

@@ -14,12 +14,12 @@ import {getReportIDForExpense} from '@libs/MergeTransactionUtils';
 import Navigation from '@libs/Navigation/Navigation';
 import {getThumbnailAndImageURIs} from '@libs/ReceiptUtils';
 import {
-    hasDistanceRouteErrors,
     hasEReceipt,
     hasPendingDistanceReceiptRegeneration,
     hasReceiptSource,
     isDistanceRequest,
     isManualDistanceRequest,
+    isMapBasedDistanceRequest,
     isPerDiemRequest,
 } from '@libs/TransactionUtils';
 import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
@@ -138,9 +138,7 @@ function ReportActionItemImage({
     const icons = useMemoizedLazyExpensifyIcons(['Receipt']);
     const {report: contextReport, transactionThreadReport} = useShowContextMenuState();
     const isMapDistanceRequest = !!transaction && isDistanceRequest(transaction) && !isManualDistanceRequest(transaction);
-    // Any error on the expense keeps the tile on the live route, not the route errors alone, because a tile that
-    // shows a red dot next to a stale receipt reads as though the receipt itself failed.
-    const hasErrors = !isEmptyObject(transaction?.errors) || hasDistanceRouteErrors(transaction);
+    const hasErrors = !isEmptyObject(transaction?.errors) || !isEmptyObject(transaction?.errorFields?.route) || !isEmptyObject(transaction?.errorFields?.waypoints);
     // While the receipt is regenerating its stored URL is stale, so draw the live route from `routes.coordinates`
     // (via `ConfirmedRoute`) instead of loading the now-404'd image.
     const showMapAsImage = isMapDistanceRequest && (hasErrors || hasPendingDistanceReceiptRegeneration(transaction));
@@ -223,9 +221,10 @@ function ReportActionItemImage({
     // A remote PDF is shown as the server's low-resolution JPG thumbnail, which blurs when hover-zoomed.
     // Where zooming is available (web only), render the actual PDF on top of the thumbnail so the magnified
     // view stays sharp. The thumbnail stays underneath as an instant preview and as a fallback if the PDF fails.
-    // A distance receipt is a PDF too, therefore it zooms the same way as any other receipt.
+    // Map/route distance requests are excluded: their hover overlay is a DistanceEReceipt card, not the PDF.
+    // isMapBasedDistanceRequest covers map, GPS, and manual-typed transactions that still carry waypoints.
     const pdfSourceURL = typeof originalImageSource === 'string' && !!originalImageSource ? originalImageSource : undefined;
-    const isRemotePDF = !!isPDF && !effectiveIsLocalFile && !isEReceipt && !!pdfSourceURL;
+    const isRemotePDF = !!isPDF && !effectiveIsLocalFile && !isEReceipt && !isMapBasedDistanceRequest(transaction) && !!pdfSourceURL;
     const shouldOverlayHighResPDF = canZoomReceipt && isRemotePDF && hasHoverSupport();
 
     const renderReceiptContent = (receiptImage: React.ReactNode) =>

src/components/TransactionItemRow/ReceiptPreview/index.tsx

@@ -9,7 +9,7 @@ import useResponsiveLayoutOnWideRHP from '@hooks/useResponsiveLayoutOnWideRHP';
 import useThemeStyles from '@hooks/useThemeStyles';
 import useWindowDimensions from '@hooks/useWindowDimensions';
 
-import {hasReceiptSource, isPerDiemRequest, shouldRenderLocalDistanceEReceipt} from '@libs/TransactionUtils';
+import {hasReceiptSource, isDistanceRequest, isManualDistanceRequest, isPerDiemRequest} from '@libs/TransactionUtils';
 
 import variables from '@styles/variables';
 
@@ -45,9 +45,7 @@ type ReceiptPreviewProps = {
 };
 
 function ReceiptPreview({source, hovered, isEReceipt = false, transactionItem, anchorPosition}: ReceiptPreviewProps) {
-    // A distance expense usually has a generated receipt file, and `source` already points at a page of it, so the
-    // preview shows the file. The card is for the expenses that have no file to show.
-    const isDistanceEReceipt = shouldRenderLocalDistanceEReceipt(transactionItem);
+    const isDistanceEReceipt = isDistanceRequest(transactionItem) && !isManualDistanceRequest(transactionItem);
     const isPerDiemEReceipt = isPerDiemRequest(transactionItem) && !hasReceiptSource(transactionItem) && !!transactionItem.transactionID;
     const styles = useThemeStyles();
     const [eReceiptScaleFactor, setEReceiptScaleFactor] = useState(0);

src/libs/TransactionUtils/index.ts

@@ -1291,40 +1291,6 @@ function hasDistanceRouteErrors(transaction: OnyxInputOrEntry<Transaction>): boo
     return !isEmptyObject(transaction?.errorFields?.route) || !isEmptyObject(transaction?.errorFields?.waypoints);
 }
 
-/**
- * Whether the receipt file that the server generated for a distance expense can be shown. It cannot when there
- * is no file, when the server is building a new one after an edit, or when the route failed.
- */
-function hasUsableStoredDistanceReceipt(transaction: OnyxEntry<Transaction>): boolean {
-    return hasReceiptSource(transaction) && !hasPendingDistanceReceiptRegeneration(transaction) && !hasDistanceRouteErrors(transaction);
-}
-
-/**
- * Whether a distance expense must render the local `DistanceEReceipt` card instead of the receipt file the
- * server generated.
- *
- * The generated file records the routed trip, therefore its total and its "84.57 mi @ $0.725 / mi" line can
- * differ from what the expense bills. Expensify Classic shows that file on every surface, and New Expensify
- * must do the same. If one surface draws the card and another draws the file, the two contradict each other.
- * The card is only the fallback for a receipt file we cannot show.
- */
-function shouldRenderLocalDistanceEReceipt(transaction: OnyxEntry<Transaction>): boolean {
-    // Only map and GPS distance expenses get a generated map receipt. An odometer expense shows the photos of
-    // the odometer that the user took, and a manual distance expense has no map. Both keep their own handling.
-    if (!isDistanceRequest(transaction) || isOdometerDistanceRequest(transaction) || isManualDistanceRequest(transaction)) {
-        return false;
-    }
-
-    if (!hasUsableStoredDistanceReceipt(transaction)) {
-        return true;
-    }
-
-    // The server generates a distance receipt as a PDF that carries its own total and mileage. An older expense
-    // stored the bare map image instead, therefore the card must supply those two lines around it.
-    const receiptSource = typeof transaction?.receipt?.source === 'string' ? transaction.receipt.source : '';
-    return !Str.isPDF(transaction?.receipt?.filename ?? '') && !Str.isPDF(receiptSource);
-}
-
 /**
  * Return the merchant field from the transaction, return the modifiedMerchant if present.
  */
@@ -3472,8 +3438,6 @@ export {
     hasLocallyKnownDistance,
     hasPendingDistanceReceiptRegeneration,
     hasDistanceRouteErrors,
-    hasUsableStoredDistanceReceipt,
-    shouldRenderLocalDistanceEReceipt,
     isExpensifyCardTransaction,
     isManagedCardTransaction,
     isDuplicate,

src/pages/media/AttachmentModalScreen/routes/TransactionReceiptModalContent.tsx

@@ -32,10 +32,8 @@ import {
     hasOdometerImageSource,
     hasReceipt,
     hasReceiptSource,
-    hasUsableStoredDistanceReceipt,
     isOdometerDistanceRequest,
     isReceiptBeingScanned,
-    shouldRenderLocalDistanceEReceipt,
 } from '@libs/TransactionUtils';
 import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
 
@@ -294,9 +292,7 @@ function TransactionReceiptModalContent({navigation, route}: AttachmentModalScre
         draftTransactionID,
     });
 
-    // Withhold Download only when the card on screen is not backed by a stored file. An older distance expense
-    // also shows the card, but around the very image it would download, so that one stays available.
-    const allowDownload = !isEReceipt && (!shouldRenderLocalDistanceEReceipt(transaction) || hasUsableStoredDistanceReceipt(transaction));
+    const allowDownload = !isEReceipt;
 
     const applyDurableReceipt = useCallback(
         (imageUri: string, filename: string, file: File, isSameReceipt?: boolean) => {

tests/unit/TransactionUtilsTest.ts

@@ -1266,57 +1266,28 @@ describe('TransactionUtils', () => {
         });
     });
 
-    describe('shouldRenderLocalDistanceEReceipt', () => {
+    describe('isMapBasedDistanceRequest', () => {
         const UPDATE = CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE;
         const PDF_RECEIPT = {source: 'https://www.expensify.com/receipts/w_abc123.pdf', filename: 'w_abc123.pdf'};
 
         function generateMapDistanceTransaction(values: Partial<Transaction> = {}): Transaction {
             return generateTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MAP, receipt: PDF_RECEIPT, ...values});
         }
 
-        it('returns false for a map distance expense whose generated PDF receipt is current', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction())).toBe(false);
-        });
-
-        it('returns true when there is no receipt file yet', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({receipt: undefined}))).toBe(true);
-        });
-
-        it('returns true while the server rebuilds the receipt after an edit', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({pendingFields: {waypoints: UPDATE}}))).toBe(true);
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({pendingFields: {merchant: UPDATE}}))).toBe(true);
-        });
-
-        it('returns true when the route failed', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({errorFields: {route: {someError: 'No route found'}}}))).toBe(true);
+        // New Expensify draws its own distance e-receipt for these, so the generated PDF beside them is never shown.
+        it('is true for a map distance expense whichever receipt it stores', () => {
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateMapDistanceTransaction())).toBe(true);
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateMapDistanceTransaction({receipt: undefined}))).toBe(true);
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateMapDistanceTransaction({pendingFields: {merchant: UPDATE}}))).toBe(true);
         });
 
-        it('returns true for an older expense that stored the bare map image', () => {
-            const imageReceipt = {source: 'https://www.expensify.com/receipts/w_abc123.jpg', filename: 'w_abc123.jpg'};
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({receipt: imageReceipt}))).toBe(true);
-            // That file is still the one the card draws, so it stays downloadable.
-            expect(TransactionUtils.hasUsableStoredDistanceReceipt(generateMapDistanceTransaction({receipt: imageReceipt}))).toBe(true);
+        it('is true for a GPS distance expense, which also has a route to draw', () => {
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_GPS}))).toBe(true);
         });
 
-        it('keeps showing the generated receipt when an unrelated error lands on the expense', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({errors: {someError: 'Payment failed'}}))).toBe(false);
-        });
-
-        it('returns false for odometer, manual distance and non-distance expenses', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER, receipt: undefined}))).toBe(
-                false,
-            );
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL, receipt: undefined}))).toBe(
-                false,
-            );
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateTransaction({receipt: undefined}))).toBe(false);
-        });
-
-        it('treats a GPS distance expense the same way, because it also gets a generated receipt', () => {
-            expect(TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_GPS}))).toBe(false);
-            expect(
-                TransactionUtils.shouldRenderLocalDistanceEReceipt(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_GPS, pendingFields: {merchant: UPDATE}})),
-            ).toBe(true);
+        it('is false for odometer and non-distance expenses, which keep their own receipt', () => {
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateMapDistanceTransaction({iouRequestType: CONST.IOU.REQUEST_TYPE.DISTANCE_ODOMETER}))).toBe(false);
+            expect(TransactionUtils.isMapBasedDistanceRequest(generateTransaction({receipt: undefined}))).toBe(false);
         });
     });