diff --git a/src/libs/actions/IOU/index.ts b/src/libs/actions/IOU/index.ts index 32854a8020f72..4c0b40a3516f5 100644 --- a/src/libs/actions/IOU/index.ts +++ b/src/libs/actions/IOU/index.ts @@ -240,7 +240,7 @@ import type {BuildPolicyDataKeys} from '@userActions/Policy/Policy'; import {buildOptimisticPolicyRecentlyUsedTags} from '@userActions/Policy/Tag'; import type {GuidedSetupData} from '@userActions/Report'; import {buildInviteToRoomOnyxData, completeOnboarding, notifyNewAction, optimisticReportLastData} from '@userActions/Report'; -import {mergeTransactionIdsHighlightOnSearchRoute, sanitizeRecentWaypoints} from '@userActions/Transaction'; +import {mergeTransactionIdsHighlightOnSearchRoute, sanitizeWaypointsForAPI, stringifyWaypointsForAPI} from '@userActions/Transaction'; import {removeDraftTransaction, removeDraftTransactions, removeDraftTransactionsByIDs} from '@userActions/TransactionEdit'; import {getOnboardingMessages} from '@userActions/Welcome/OnboardingFlow'; import type {OnboardingCompanySize} from '@userActions/Welcome/OnboardingFlow'; @@ -4611,8 +4611,7 @@ function getUpdateMoneyRequestParams(params: GetUpdateMoneyRequestParamsType): U const transactionDetails = getTransactionDetails(updatedTransaction, undefined, undefined, allowNegative); if (transactionDetails?.waypoints) { - // This needs to be a JSON string since we're sending this to the MapBox API - transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints); + transactionDetails.waypoints = stringifyWaypointsForAPI(transactionDetails.waypoints as WaypointCollection); } const dataToIncludeInParams: Partial = Object.fromEntries(Object.entries(transactionDetails ?? {}).filter(([key]) => key in transactionChanges)); @@ -5159,8 +5158,7 @@ function getUpdateTrackExpenseParams( const transactionDetails = getTransactionDetails(updatedTransaction); if (transactionDetails?.waypoints) { - // This needs to be a JSON string since we're sending this to the MapBox API - transactionDetails.waypoints = JSON.stringify(transactionDetails.waypoints); + transactionDetails.waypoints = stringifyWaypointsForAPI(transactionDetails.waypoints as WaypointCollection); } const dataToIncludeInParams: Partial = Object.fromEntries(Object.entries(transactionDetails ?? {}).filter(([key]) => key in transactionChanges)); @@ -5749,7 +5747,9 @@ function updateMoneyRequestDistance({ parentReportNextStep, }: UpdateMoneyRequestDistanceParams) { const transactionChanges: TransactionChanges = { - ...(waypoints && {waypoints: sanitizeRecentWaypoints(waypoints)}), + // Don't sanitize waypoints here - keep all fields for Onyx optimistic data (e.g., keyForList) + // Sanitization happens when building API params + ...(waypoints && {waypoints}), routes, ...(distance && {distance}), ...(odometerStart !== undefined && {odometerStart}), @@ -6413,7 +6413,7 @@ function convertBulkTrackedExpensesToIOU({ const isDistanceRequest = isDistanceRequestTransactionUtils(transaction); const transactionWaypoints = getWaypoints(transaction); - const sanitizedWaypointsForBulk = transactionWaypoints ? JSON.stringify(sanitizeRecentWaypoints(transactionWaypoints)) : undefined; + const sanitizedWaypointsForBulk = transactionWaypoints ? stringifyWaypointsForAPI(transactionWaypoints) : undefined; const convertParams: ConvertTrackedExpenseToRequestParams = { payerParams: { @@ -6697,7 +6697,7 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation): {iouRep const testDriveCommentReportActionID = isTestDrive ? NumberUtils.rand64() : undefined; - const sanitizedWaypoints = waypoints ? JSON.stringify(sanitizeRecentWaypoints(waypoints)) : undefined; + const sanitizedWaypoints = waypoints ? stringifyWaypointsForAPI(waypoints) : undefined; // If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function const isMoneyRequestReport = isMoneyRequestReportReportUtils(report); @@ -7496,7 +7496,7 @@ function trackExpense(params: CreateTrackExpenseParams) { // Pass an open receipt so the distance expense will show a map with the route optimistically const trackedReceipt = validWaypoints ? {source: ReceiptGeneric as ReceiptSource, state: CONST.IOU.RECEIPT_STATE.OPEN, name: 'receipt-generic.png'} : receipt; - const sanitizedWaypoints = validWaypoints ? JSON.stringify(sanitizeRecentWaypoints(validWaypoints)) : undefined; + const sanitizedWaypoints = validWaypoints ? stringifyWaypointsForAPI(validWaypoints) : undefined; const retryParams: CreateTrackExpenseParams = { ...params, @@ -8440,7 +8440,7 @@ function createDistanceRequest(distanceRequestInformation: CreateDistanceRequest let parameters: CreateDistanceRequestParams; let onyxData: OnyxData; - const sanitizedWaypoints = !isManualDistanceRequest ? sanitizeRecentWaypoints(validWaypoints) : null; + const sanitizedWaypoints = !isManualDistanceRequest ? sanitizeWaypointsForAPI(validWaypoints) : null; if (iouType === CONST.IOU.TYPE.SPLIT) { const { splitData, diff --git a/src/libs/actions/Transaction.ts b/src/libs/actions/Transaction.ts index d575d9c8c1160..c954d67886ecd 100644 --- a/src/libs/actions/Transaction.ts +++ b/src/libs/actions/Transaction.ts @@ -347,23 +347,47 @@ function getOnyxDataForRouteRequest( } /** - * Sanitizes the waypoints by removing the pendingAction property. + * Sanitizes the waypoints data to only include allowed fields for API requests. + * Only keeps: name (optional), address, lat, lng * * @param waypoints - The collection of waypoints to sanitize. - * @returns The sanitized collection of waypoints. + * @returns The sanitized collection of waypoints with only allowed fields. */ -function sanitizeRecentWaypoints(waypoints: WaypointCollection): WaypointCollection { +function sanitizeWaypointsForAPI(waypoints: WaypointCollection): WaypointCollection { return Object.entries(waypoints).reduce((acc: WaypointCollection, [key, waypoint]) => { - if ('pendingAction' in waypoint) { - const {pendingAction, ...rest} = waypoint; - acc[key] = rest; - } else { - acc[key] = waypoint; + if (!waypoint) { + return acc; + } + + const sanitizedWaypoint: Record = {}; + + if (waypoint.name !== undefined) { + sanitizedWaypoint.name = waypoint.name; + } + if (waypoint.address !== undefined) { + sanitizedWaypoint.address = waypoint.address; + } + if (waypoint.lat !== undefined) { + sanitizedWaypoint.lat = waypoint.lat; } + if (waypoint.lng !== undefined) { + sanitizedWaypoint.lng = waypoint.lng; + } + + acc[key] = sanitizedWaypoint; return acc; }, {}); } +/** + * Sanitizes waypoints and serializes them to a JSON string for API params. + * Preserves keyForList and other Onyx-only fields by sanitizing at the serialization boundary + * rather than when building transactionChanges. + */ +function stringifyWaypointsForAPI(waypoints: WaypointCollection): string { + return JSON.stringify(sanitizeWaypointsForAPI(waypoints)); +} + /** * Gets the route for a set of waypoints * Used so we can generate a map view of the provided waypoints @@ -372,7 +396,7 @@ function sanitizeRecentWaypoints(waypoints: WaypointCollection): WaypointCollect function getRoute(transactionID: string, waypoints: WaypointCollection, routeType: TransactionState = CONST.TRANSACTION.STATE.CURRENT) { const parameters: GetRouteParams = { transactionID, - waypoints: JSON.stringify(sanitizeRecentWaypoints(waypoints)), + waypoints: stringifyWaypointsForAPI(waypoints), }; let command; @@ -1682,7 +1706,8 @@ export { setReviewDuplicatesKey, abandonReviewDuplicateTransactions, openDraftDistanceExpense, - sanitizeRecentWaypoints, + sanitizeWaypointsForAPI, + stringifyWaypointsForAPI, getLastModifiedExpense, revert, changeTransactionsReport, diff --git a/tests/unit/TransactionTest.ts b/tests/unit/TransactionTest.ts index fa61d399d9c20..8103eca0d10b9 100644 --- a/tests/unit/TransactionTest.ts +++ b/tests/unit/TransactionTest.ts @@ -3,7 +3,7 @@ import type {OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; import useOnyx from '@hooks/useOnyx'; -import {changeTransactionsReport, dismissDuplicateTransactionViolation, markAsCash, saveWaypoint} from '@libs/actions/Transaction'; +import {changeTransactionsReport, dismissDuplicateTransactionViolation, markAsCash, sanitizeWaypointsForAPI, saveWaypoint} from '@libs/actions/Transaction'; import DateUtils from '@libs/DateUtils'; import {getAllNonDeletedTransactions} from '@libs/MoneyRequestReportUtils'; import type {buildOptimisticNextStep} from '@libs/NextStepUtils'; @@ -1101,6 +1101,126 @@ describe('Transaction', () => { }); }); + describe('sanitizeWaypointsForAPI', () => { + it('should only include allowed fields (name, address, lat, lng)', () => { + // Given waypoints with extra fields that should be stripped out + const waypointsWithExtraFields = { + waypoint0: { + name: 'Start Location', + address: '123 Main St', + lat: 40.7128, + lng: -74.006, + city: 'New York', + state: 'NY', + zipCode: '10001', + country: 'US', + street: '123 Main St', + street2: 'Apt 4B', + keyForList: 'unique-key-1', + pendingAction: 'add', + extraField: 'should be removed', + }, + waypoint1: { + address: '456 Oak Ave', + lat: 40.7589, + lng: -73.9851, + city: 'New York', + state: 'NY', + zipCode: '10002', + keyForList: 'unique-key-2', + anotherExtraField: 'should also be removed', + }, + }; + + // When sanitizing the waypoints + // Test intentionally passes extra fields not in WaypointCollection to verify they are stripped + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithExtraFields as any); + + // Then only allowed fields should remain + expect(sanitizedWaypoints.waypoint0).toEqual({ + name: 'Start Location', + address: '123 Main St', + lat: 40.7128, + lng: -74.006, + }); + + // waypoint1 has no name field, so it should not be included + expect(sanitizedWaypoints.waypoint1).toEqual({ + address: '456 Oak Ave', + lat: 40.7589, + lng: -73.9851, + }); + }); + + it('should handle waypoints with only some allowed fields', () => { + // Given waypoints with only address + const waypointsWithPartialFields = { + waypoint0: { + address: 'Partial Address Only', + }, + }; + + // When sanitizing the waypoints + // Test uses a partial waypoint object to verify sanitization handles missing fields + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithPartialFields as any); + + // Then only the address should be present + expect(sanitizedWaypoints.waypoint0).toEqual({ + address: 'Partial Address Only', + }); + }); + + it('should handle empty waypoints', () => { + // Given empty waypoints + const emptyWaypoints = {}; + + // When sanitizing the waypoints + const sanitizedWaypoints = sanitizeWaypointsForAPI(emptyWaypoints); + + // Then the result should also be empty + expect(sanitizedWaypoints).toEqual({}); + }); + + it('should skip null waypoint entries without crashing', () => { + // Given waypoints where some entries are null (can happen during rollback of failed distance edits) + const waypointsWithNulls = { + waypoint0: { + address: '123 Main St', + lat: 40.7128, + lng: -74.006, + }, + waypoint1: null, + waypoint2: { + address: '789 Pine Rd', + lat: 40.73, + lng: -73.99, + }, + }; + + // When sanitizing the waypoints + // Null entries can occur at runtime even though WaypointCollection type doesn't include null + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument + const sanitizedWaypoints = sanitizeWaypointsForAPI(waypointsWithNulls as any); + + // Then null entries should be dropped and valid entries sanitized + expect(sanitizedWaypoints).toEqual({ + waypoint0: { + address: '123 Main St', + lat: 40.7128, + lng: -74.006, + }, + waypoint2: { + address: '789 Pine Rd', + lat: 40.73, + lng: -73.99, + }, + }); + expect(sanitizedWaypoints.waypoint1).toBeUndefined(); + }); + }); + describe('markAsCash', () => { it('should optimistically remove RTER violation and add dismissed violation report action', async () => { // Given a transaction with an RTER violation