From 6cb1c755bd850b3e2108475a2b07dd2dab3ec5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Thu, 16 Oct 2025 12:18:24 +0200 Subject: [PATCH 1/5] refactor: isolate saveWaypoint from Onyx.connect data --- Mobile-Expensify | 2 +- src/libs/actions/Transaction.ts | 14 +++-- tests/unit/TransactionTest.ts | 90 ++++++++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/Mobile-Expensify b/Mobile-Expensify index 3926c6df4837..a42ddea7e10d 160000 --- a/Mobile-Expensify +++ b/Mobile-Expensify @@ -1 +1 @@ -Subproject commit 3926c6df48372506768405daf2b35e13153fb034 +Subproject commit a42ddea7e10dc448e5b68b460c14ce3a941b05bb diff --git a/src/libs/actions/Transaction.ts b/src/libs/actions/Transaction.ts index 5350d4c73d67..ad587b754c73 100644 --- a/src/libs/actions/Transaction.ts +++ b/src/libs/actions/Transaction.ts @@ -111,7 +111,15 @@ function isViolationWithName(violation: unknown): violation is {name: string} { return !!(violation && typeof violation === 'object' && typeof (violation as {name?: unknown}).name === 'string'); } -function saveWaypoint(transactionID: string, index: string, waypoint: RecentWaypoint | null, isDraft = false) { +type SaveWaypointProps = { + transactionID: string; + index: string; + waypoint: RecentWaypoint | null; + isDraft?: boolean; + recentWaypointsList: RecentWaypoint[]; +}; + +function saveWaypoint({transactionID, index, waypoint, isDraft = false, recentWaypointsList}: SaveWaypointProps) { Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, { comment: { waypoints: { @@ -153,9 +161,9 @@ function saveWaypoint(transactionID: string, index: string, waypoint: RecentWayp if (deepEqual(waypoint?.address, CONST.YOUR_LOCATION_TEXT)) { return; } - const recentWaypointAlreadyExists = recentWaypoints.find((recentWaypoint) => recentWaypoint?.address === waypoint?.address); + const recentWaypointAlreadyExists = recentWaypointsList.find((recentWaypoint) => recentWaypoint?.address === waypoint?.address); if (!recentWaypointAlreadyExists && waypoint !== null) { - const clonedWaypoints = lodashClone(recentWaypoints); + const clonedWaypoints = lodashClone(recentWaypointsList); const updatedWaypoint = {...waypoint, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}; clonedWaypoints.unshift(updatedWaypoint); Onyx.merge(ONYXKEYS.NVP_RECENT_WAYPOINTS, clonedWaypoints.slice(0, CONST.RECENT_WAYPOINTS_NUMBER)); diff --git a/tests/unit/TransactionTest.ts b/tests/unit/TransactionTest.ts index e1e39a30660e..1bec9c0fe4fe 100644 --- a/tests/unit/TransactionTest.ts +++ b/tests/unit/TransactionTest.ts @@ -1,6 +1,7 @@ import type {OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; -import {changeTransactionsReport} from '@libs/actions/Transaction'; +import OnyxUtils from 'react-native-onyx/dist/OnyxUtils'; +import {changeTransactionsReport, saveWaypoint} from '@libs/actions/Transaction'; import DateUtils from '@libs/DateUtils'; import {getAllNonDeletedTransactions} from '@libs/MoneyRequestReportUtils'; import {rand64} from '@libs/NumberUtils'; @@ -10,7 +11,7 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {Attendee} from '@src/types/onyx/IOU'; import type {ReportCollectionDataSet} from '@src/types/onyx/Report'; import * as TransactionUtils from '../../src/libs/TransactionUtils'; -import type {ReportAction, ReportActions, Transaction} from '../../src/types/onyx'; +import type {RecentWaypoint, ReportAction, ReportActions, Transaction} from '../../src/types/onyx'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; function generateTransaction(values: Partial = {}): Transaction { @@ -384,4 +385,89 @@ describe('Transaction', () => { expect(result.at(0)).toEqual(transaction); }); }); + + describe('saveWaypoint', () => { + it('should save a waypoint with lat/lng and not YOUR_LOCATION_TEXT', async () => { + const transactionID = 'txn1'; + const index = '0'; + const waypoint: RecentWaypoint = { + address: '123 Main St', + lat: 10, + lng: 20, + }; + const recentWaypointsList: RecentWaypoint[] = []; + saveWaypoint({transactionID, index, waypoint, isDraft: false, recentWaypointsList}); + await waitForBatchedUpdates(); + + const transaction = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`); + const updatedRecentWaypoints = await OnyxUtils.get(ONYXKEYS.NVP_RECENT_WAYPOINTS); + + expect(transaction?.comment?.waypoints?.[`waypoint${index}`]).toEqual(waypoint); + expect(updatedRecentWaypoints?.[0]?.address).toBe('123 Main St'); + }); + + it('should not save waypoint if missing lat/lng', async () => { + const transactionID = 'txn2'; + const index = '1'; + const waypoint: RecentWaypoint = { + address: 'No LatLng', + }; + const recentWaypointsList: RecentWaypoint[] = []; + saveWaypoint({transactionID, index, waypoint, isDraft: false, recentWaypointsList}); + await waitForBatchedUpdates(); + + const updatedRecentWaypoints = await OnyxUtils.get(ONYXKEYS.NVP_RECENT_WAYPOINTS); + expect(updatedRecentWaypoints?.length ?? 0).toBe(0); + }); + + it('should not save waypoint if address is YOUR_LOCATION_TEXT', async () => { + const transactionID = 'txn3'; + const index = '2'; + const waypoint: RecentWaypoint = { + address: CONST.YOUR_LOCATION_TEXT, + lat: 1, + lng: 2, + }; + const recentWaypointsList: RecentWaypoint[] = []; + saveWaypoint({transactionID, index, waypoint, isDraft: false, recentWaypointsList}); + await waitForBatchedUpdates(); + + const updatedRecentWaypoints = await OnyxUtils.get(ONYXKEYS.NVP_RECENT_WAYPOINTS); + expect(updatedRecentWaypoints?.length ?? 0).toBe(0); + }); + + it('should reset amount for draft transactions', async () => { + const transactionID = 'txn4'; + const index = '0'; + const waypoint: RecentWaypoint = { + address: 'Draft Waypoint', + lat: 5, + lng: 6, + }; + const recentWaypointsList: RecentWaypoint[] = []; + saveWaypoint({transactionID, index, waypoint, isDraft: true, recentWaypointsList}); + await waitForBatchedUpdates(); + + const transaction = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}`); + expect(transaction?.amount).toBe(CONST.IOU.DEFAULT_AMOUNT); + }); + + it('should clear errorFields and routes', async () => { + const transactionID = 'txn5'; + const index = '0'; + const waypoint: RecentWaypoint = { + address: 'Clear Error', + lat: 7, + lng: 8, + }; + const recentWaypointsList: RecentWaypoint[] = []; + saveWaypoint({transactionID, index, waypoint, isDraft: false, recentWaypointsList}); + await waitForBatchedUpdates(); + + const transaction = await OnyxUtils.get(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`); + expect(transaction?.errorFields?.route ?? null).toBeNull(); + expect(transaction?.routes?.route0?.distance ?? null).toBeNull(); + expect(transaction?.routes?.route0?.geometry?.coordinates ?? null).toBeNull(); + }); + }); }); From 8e08fb32e682834e46fb76ee0643aa7756dc742c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Thu, 16 Oct 2025 12:30:43 +0200 Subject: [PATCH 2/5] fix: update saveWaypoint props in IOURequestStepWaypoint --- src/pages/iou/request/step/IOURequestStepWaypoint.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx index c14c32c80a3f..38785339aecb 100644 --- a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx @@ -4,6 +4,7 @@ import type {TextInput} from 'react-native'; import {View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import AddressSearch from '@components/AddressSearch'; +import type {PredefinedPlace} from '@components/AddressSearch/types'; import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; import ConfirmModal from '@components/ConfirmModal'; import FormProvider from '@components/Form/FormProvider'; @@ -39,7 +40,7 @@ import withWritableReportOrNotFound from './withWritableReportOrNotFound'; // Only grab the most recent 20 waypoints because that's all that is shown in the UI. This also puts them into the format of data // that the google autocomplete component expects for it's "predefined places" feature. -function recentWaypointsSelector(waypoints: RecentWaypoint[] = []) { +function recentWaypointsSelector(waypoints: RecentWaypoint[] = []): PredefinedPlace[] { return waypoints .slice(0, CONST.RECENT_WAYPOINTS_NUMBER) .filter((waypoint) => waypoint.keyForList?.includes(CONST.YOUR_LOCATION_TEXT) !== true) @@ -124,7 +125,8 @@ function IOURequestStepWaypoint({ return errors; }; - const save = (waypoint: FormOnyxValues<'waypointForm'>) => saveWaypoint(transactionID, pageIndex, waypoint, shouldUseTransactionDraft(action)); + const save = (waypoint: FormOnyxValues<'waypointForm'>) => + saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: recentWaypoints as RecentWaypoint[]}); const submit = (values: FormOnyxValues<'waypointForm'>) => { const waypointValue = values[`waypoint${pageIndex}`] ?? ''; @@ -166,7 +168,7 @@ function IOURequestStepWaypoint({ keyForList: `${values.name ?? 'waypoint'}_${Date.now()}`, }; - saveWaypoint(transactionID, pageIndex, waypoint, shouldUseTransactionDraft(action)); + saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: recentWaypoints as RecentWaypoint[]}); goBack(); }; From 98e6e20325368a09670ec04be007d4135412796e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Thu, 16 Oct 2025 14:34:45 +0200 Subject: [PATCH 3/5] fix: fix passing not whole subset of recent waypoints --- .../request/step/IOURequestStepWaypoint.tsx | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx index 38785339aecb..8b52b4be01c3 100644 --- a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx @@ -4,7 +4,6 @@ import type {TextInput} from 'react-native'; import {View} from 'react-native'; import type {OnyxEntry} from 'react-native-onyx'; import AddressSearch from '@components/AddressSearch'; -import type {PredefinedPlace} from '@components/AddressSearch/types'; import FullPageNotFoundView from '@components/BlockingViews/FullPageNotFoundView'; import ConfirmModal from '@components/ConfirmModal'; import FormProvider from '@components/Form/FormProvider'; @@ -40,7 +39,7 @@ import withWritableReportOrNotFound from './withWritableReportOrNotFound'; // Only grab the most recent 20 waypoints because that's all that is shown in the UI. This also puts them into the format of data // that the google autocomplete component expects for it's "predefined places" feature. -function recentWaypointsSelector(waypoints: RecentWaypoint[] = []): PredefinedPlace[] { +function recentWaypointsSelector(waypoints: RecentWaypoint[] = []) { return waypoints .slice(0, CONST.RECENT_WAYPOINTS_NUMBER) .filter((waypoint) => waypoint.keyForList?.includes(CONST.YOUR_LOCATION_TEXT) !== true) @@ -83,6 +82,7 @@ function IOURequestStepWaypoint({ const [userLocation] = useOnyx(ONYXKEYS.USER_LOCATION, {canBeMissing: true}); const [recentWaypoints] = useOnyx(ONYXKEYS.NVP_RECENT_WAYPOINTS, {selector: recentWaypointsSelector, canBeMissing: true}); + const [allRecentWaypoints] = useOnyx(ONYXKEYS.NVP_RECENT_WAYPOINTS, {canBeMissing: true}); const waypointDescriptionKey = useMemo(() => { switch (parsedWaypointIndex) { @@ -125,8 +125,12 @@ function IOURequestStepWaypoint({ return errors; }; - const save = (waypoint: FormOnyxValues<'waypointForm'>) => - saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: recentWaypoints as RecentWaypoint[]}); + const save = (waypoint: FormOnyxValues<'waypointForm'>) => { + if (!allRecentWaypoints) { + return; + } + saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: allRecentWaypoints}); + }; const submit = (values: FormOnyxValues<'waypointForm'>) => { const waypointValue = values[`waypoint${pageIndex}`] ?? ''; @@ -160,6 +164,10 @@ function IOURequestStepWaypoint({ }; const selectWaypoint = (values: Waypoint) => { + if (!allRecentWaypoints) { + return; + } + const waypoint = { lat: values.lat ?? 0, lng: values.lng ?? 0, @@ -168,7 +176,7 @@ function IOURequestStepWaypoint({ keyForList: `${values.name ?? 'waypoint'}_${Date.now()}`, }; - saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: recentWaypoints as RecentWaypoint[]}); + saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: allRecentWaypoints}); goBack(); }; From 1cc7ee39f36ef6d131e98a8e36d3d0352ca570f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Fri, 24 Oct 2025 10:18:14 +0200 Subject: [PATCH 4/5] review: fix test --- tests/unit/TransactionTest.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/unit/TransactionTest.ts b/tests/unit/TransactionTest.ts index 1bec9c0fe4fe..18d672f245d5 100644 --- a/tests/unit/TransactionTest.ts +++ b/tests/unit/TransactionTest.ts @@ -461,6 +461,23 @@ describe('Transaction', () => { lng: 8, }; const recentWaypointsList: RecentWaypoint[] = []; + // Ensure there is an existing transaction with errorFields and routes + const existingTransaction = generateTransaction({transactionID, reportID: '1'}); + // Add errorFields and routes so saveWaypoint can clear them + // Populate with realistic non-null values + existingTransaction.errorFields = {route: {some: 'value'}}; + existingTransaction.routes = { + route0: { + distance: 123, + geometry: { + coordinates: [ + [0, 0], + [1, 1], + ], + }, + }, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, existingTransaction); saveWaypoint({transactionID, index, waypoint, isDraft: false, recentWaypointsList}); await waitForBatchedUpdates(); From 72bafe95ac057a43fe8afbd007a4f7beaa824b10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Ska=C5=82ka?= Date: Wed, 29 Oct 2025 10:34:16 +0100 Subject: [PATCH 5/5] review: remove early returns --- src/libs/actions/Transaction.ts | 4 ++-- src/pages/iou/request/step/IOURequestStepWaypoint.tsx | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/libs/actions/Transaction.ts b/src/libs/actions/Transaction.ts index 006fc70ea796..0969cecbba44 100644 --- a/src/libs/actions/Transaction.ts +++ b/src/libs/actions/Transaction.ts @@ -117,10 +117,10 @@ type SaveWaypointProps = { index: string; waypoint: RecentWaypoint | null; isDraft?: boolean; - recentWaypointsList: RecentWaypoint[]; + recentWaypointsList?: RecentWaypoint[]; }; -function saveWaypoint({transactionID, index, waypoint, isDraft = false, recentWaypointsList}: SaveWaypointProps) { +function saveWaypoint({transactionID, index, waypoint, isDraft = false, recentWaypointsList = []}: SaveWaypointProps) { Onyx.merge(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, { comment: { waypoints: { diff --git a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx index 8b52b4be01c3..da0912559dfe 100644 --- a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx @@ -126,9 +126,6 @@ function IOURequestStepWaypoint({ }; const save = (waypoint: FormOnyxValues<'waypointForm'>) => { - if (!allRecentWaypoints) { - return; - } saveWaypoint({transactionID, index: pageIndex, waypoint, isDraft: shouldUseTransactionDraft(action), recentWaypointsList: allRecentWaypoints}); }; @@ -164,10 +161,6 @@ function IOURequestStepWaypoint({ }; const selectWaypoint = (values: Waypoint) => { - if (!allRecentWaypoints) { - return; - } - const waypoint = { lat: values.lat ?? 0, lng: values.lng ?? 0,