From 962e999f40441f934e16ff7677e2cd04fc452826 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sun, 25 Aug 2024 16:52:57 +0800 Subject: [PATCH 1/5] use useFetchRoute and use it in confirm page too --- src/hooks/useFetchRoute.ts | 34 +++++++++++++++++++ .../step/IOURequestStepConfirmation.tsx | 3 ++ .../request/step/IOURequestStepDistance.tsx | 15 ++------ 3 files changed, 39 insertions(+), 13 deletions(-) create mode 100644 src/hooks/useFetchRoute.ts diff --git a/src/hooks/useFetchRoute.ts b/src/hooks/useFetchRoute.ts new file mode 100644 index 000000000000..736e99a31d66 --- /dev/null +++ b/src/hooks/useFetchRoute.ts @@ -0,0 +1,34 @@ +import {isEqual} from 'lodash'; +import {useEffect} from 'react'; +import type {OnyxEntry} from 'react-native-onyx'; +import * as IOUUtils from '@libs/IOUUtils'; +import * as TransactionUtils from '@libs/TransactionUtils'; +import * as TransactionAction from '@userActions/Transaction'; +import type {IOUAction} from '@src/CONST'; +import type {Transaction} from '@src/types/onyx'; +import type {WaypointCollection} from '@src/types/onyx/Transaction'; +import useNetwork from './useNetwork'; +import usePrevious from './usePrevious'; + +export default function useFetchRoute(transaction: OnyxEntry, waypoints: WaypointCollection | undefined, action: IOUAction) { + const {isOffline} = useNetwork(); + const hasRouteError = !!transaction?.errorFields?.route; + const hasRoute = TransactionUtils.hasRoute(transaction); + const isRouteAbsentWithoutErrors = !hasRoute && !hasRouteError; + const isLoadingRoute = transaction?.comment?.isLoading ?? false; + const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints); + const previousValidatedWaypoints = usePrevious(validatedWaypoints); + const haveValidatedWaypointsChanged = !isEqual(previousValidatedWaypoints, validatedWaypoints); + const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); + const shouldFetchRoute = isDistanceRequest && (isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && Object.keys(validatedWaypoints).length > 1; + + useEffect(() => { + if (isOffline || !shouldFetchRoute || !transaction?.transactionID) { + return; + } + + TransactionAction.getRoute(transaction.transactionID, validatedWaypoints, IOUUtils.shouldUseTransactionDraft(action)); + }, [shouldFetchRoute, transaction?.transactionID, validatedWaypoints, isOffline, action]); + + return {shouldFetchRoute, validatedWaypoints}; +} diff --git a/src/pages/iou/request/step/IOURequestStepConfirmation.tsx b/src/pages/iou/request/step/IOURequestStepConfirmation.tsx index b33ce6f56600..0e3141ab363c 100644 --- a/src/pages/iou/request/step/IOURequestStepConfirmation.tsx +++ b/src/pages/iou/request/step/IOURequestStepConfirmation.tsx @@ -10,6 +10,7 @@ import MoneyRequestConfirmationList from '@components/MoneyRequestConfirmationLi import {usePersonalDetails} from '@components/OnyxProvider'; import ScreenWrapper from '@components/ScreenWrapper'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import useFetchRoute from '@hooks/useFetchRoute'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; import useThemeStyles from '@hooks/useThemeStyles'; @@ -159,6 +160,8 @@ function IOURequestStepConfirmation({ const isPolicyExpenseChat = useMemo(() => participants?.some((participant) => participant.isPolicyExpenseChat), [participants]); const formHasBeenSubmitted = useRef(false); + useFetchRoute(transaction, transaction?.comment?.waypoints, action); + useEffect(() => { const policyExpenseChat = participants?.find((participant) => participant.isPolicyExpenseChat); if (policyExpenseChat?.policyID && policy?.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { diff --git a/src/pages/iou/request/step/IOURequestStepDistance.tsx b/src/pages/iou/request/step/IOURequestStepDistance.tsx index 1798a95490a7..868f1752dbbf 100644 --- a/src/pages/iou/request/step/IOURequestStepDistance.tsx +++ b/src/pages/iou/request/step/IOURequestStepDistance.tsx @@ -13,6 +13,7 @@ import DotIndicatorMessage from '@components/DotIndicatorMessage'; import DraggableList from '@components/DraggableList'; import withCurrentUserPersonalDetails from '@components/withCurrentUserPersonalDetails'; import type {WithCurrentUserPersonalDetailsProps} from '@components/withCurrentUserPersonalDetails'; +import useFetchRoute from '@hooks/useFetchRoute'; import useLocalize from '@hooks/useLocalize'; import useNetwork from '@hooks/useNetwork'; import usePrevious from '@hooks/usePrevious'; @@ -87,6 +88,7 @@ function IOURequestStepDistance({ }, [optimisticWaypoints, transaction], ); + const {shouldFetchRoute, validatedWaypoints} = useFetchRoute(transaction, waypoints, action); const waypointsList = Object.keys(waypoints); const previousWaypoints = usePrevious(waypoints); const numberOfWaypoints = Object.keys(waypoints).length; @@ -95,12 +97,6 @@ function IOURequestStepDistance({ const isLoadingRoute = transaction?.comment?.isLoading ?? false; const isLoading = transaction?.isLoading ?? false; const hasRouteError = !!transaction?.errorFields?.route; - const hasRoute = TransactionUtils.hasRoute(transaction); - const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints); - const previousValidatedWaypoints = usePrevious(validatedWaypoints); - const haveValidatedWaypointsChanged = !isEqual(previousValidatedWaypoints, validatedWaypoints); - const isRouteAbsentWithoutErrors = !hasRoute && !hasRouteError; - const shouldFetchRoute = (isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && Object.keys(validatedWaypoints).length > 1; const [shouldShowAtLeastTwoDifferentWaypointsError, setShouldShowAtLeastTwoDifferentWaypointsError] = useState(false); const isWaypointEmpty = (waypoint?: Waypoint) => { if (!waypoint) { @@ -159,13 +155,6 @@ function IOURequestStepDistance({ return MapboxToken.stop; }, []); - useEffect(() => { - if (isOffline || !shouldFetchRoute) { - return; - } - TransactionAction.getRoute(transactionID, validatedWaypoints, action === CONST.IOU.ACTION.CREATE); - }, [shouldFetchRoute, transactionID, validatedWaypoints, isOffline, action]); - useEffect(() => { if (numberOfWaypoints <= numberOfPreviousWaypoints) { return; From e740011fb3f39d5bf487683b6356942f3adb154b Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sun, 25 Aug 2024 16:57:52 +0800 Subject: [PATCH 2/5] use transaction draft when categorizing --- src/components/TaxPicker.tsx | 2 +- src/libs/IOUUtils.ts | 5 +++++ src/pages/iou/request/step/IOURequestStepDate.tsx | 2 +- src/pages/iou/request/step/IOURequestStepDescription.tsx | 2 +- src/pages/iou/request/step/IOURequestStepDistance.tsx | 7 ++++--- src/pages/iou/request/step/IOURequestStepWaypoint.tsx | 9 +++++---- .../iou/request/step/withFullTransactionOrNotFound.tsx | 2 +- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/components/TaxPicker.tsx b/src/components/TaxPicker.tsx index c62b84911835..fb7ab9ce53c1 100644 --- a/src/components/TaxPicker.tsx +++ b/src/components/TaxPicker.tsx @@ -131,7 +131,7 @@ export default withOnyx({ }, transaction: { key: ({transactionID, action}) => { - if (action === CONST.IOU.ACTION.CREATE || IOUUtils.isMovingTransactionFromTrackExpense(action)) { + if (IOUUtils.shouldUseTransactionDraft(action)) { return `${ONYXKEYS.COLLECTION.TRANSACTION_DRAFT}${transactionID}` as `${typeof ONYXKEYS.COLLECTION.TRANSACTION}${string}`; } return `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`; diff --git a/src/libs/IOUUtils.ts b/src/libs/IOUUtils.ts index d1bf8fcd8c8c..55ac4ba2ac36 100644 --- a/src/libs/IOUUtils.ts +++ b/src/libs/IOUUtils.ts @@ -156,11 +156,16 @@ function isMovingTransactionFromTrackExpense(action?: IOUAction) { return false; } +function shouldUseTransactionDraft(action: IOUAction | undefined) { + return action === CONST.IOU.ACTION.CREATE || isMovingTransactionFromTrackExpense(action); +} + export { calculateAmount, insertTagIntoTransactionTagsString, isIOUReportPendingCurrencyConversion, isMovingTransactionFromTrackExpense, + shouldUseTransactionDraft, isValidMoneyRequestType, navigateToStartMoneyRequestStep, updateIOUOwnerAndTotal, diff --git a/src/pages/iou/request/step/IOURequestStepDate.tsx b/src/pages/iou/request/step/IOURequestStepDate.tsx index fbeec4e97f1f..b5b7b1d02cea 100644 --- a/src/pages/iou/request/step/IOURequestStepDate.tsx +++ b/src/pages/iou/request/step/IOURequestStepDate.tsx @@ -99,7 +99,7 @@ function IOURequestStepDate({ return; } - const isTransactionDraft = action === CONST.IOU.ACTION.CREATE || IOUUtils.isMovingTransactionFromTrackExpense(action); + const isTransactionDraft = IOUUtils.shouldUseTransactionDraft(action); IOU.setMoneyRequestCreated(transaction?.transactionID ?? '-1', newCreated, isTransactionDraft); diff --git a/src/pages/iou/request/step/IOURequestStepDescription.tsx b/src/pages/iou/request/step/IOURequestStepDescription.tsx index 993bf580f038..9b4cb97beba6 100644 --- a/src/pages/iou/request/step/IOURequestStepDescription.tsx +++ b/src/pages/iou/request/step/IOURequestStepDescription.tsx @@ -131,7 +131,7 @@ function IOURequestStepDescription({ navigateBack(); return; } - const isTransactionDraft = action === CONST.IOU.ACTION.CREATE || IOUUtils.isMovingTransactionFromTrackExpense(action); + const isTransactionDraft = IOUUtils.shouldUseTransactionDraft(action); IOU.setMoneyRequestDescription(transaction?.transactionID ?? '-1', newComment, isTransactionDraft); diff --git a/src/pages/iou/request/step/IOURequestStepDistance.tsx b/src/pages/iou/request/step/IOURequestStepDistance.tsx index 868f1752dbbf..9131bc4b3d39 100644 --- a/src/pages/iou/request/step/IOURequestStepDistance.tsx +++ b/src/pages/iou/request/step/IOURequestStepDistance.tsx @@ -20,6 +20,7 @@ import usePrevious from '@hooks/usePrevious'; import useThemeStyles from '@hooks/useThemeStyles'; import DistanceRequestUtils from '@libs/DistanceRequestUtils'; import * as ErrorUtils from '@libs/ErrorUtils'; +import * as IOUUtils from '@libs/IOUUtils'; import Navigation from '@libs/Navigation/Navigation'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import * as ReportUtils from '@libs/ReportUtils'; @@ -188,7 +189,7 @@ function IOURequestStepDistance({ TransactionEdit.removeBackupTransaction(transaction?.transactionID ?? '-1'); return; } - TransactionEdit.restoreOriginalTransactionFromBackup(transaction?.transactionID ?? '-1', action === CONST.IOU.ACTION.CREATE); + TransactionEdit.restoreOriginalTransactionFromBackup(transaction?.transactionID ?? '-1', IOUUtils.shouldUseTransactionDraft(action)); }; // eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps }, []); @@ -383,8 +384,8 @@ function IOURequestStepDistance({ setOptimisticWaypoints(newWaypoints); Promise.all([ - TransactionAction.removeWaypoint(transaction, emptyWaypointIndex.toString(), action === CONST.IOU.ACTION.CREATE), - TransactionAction.updateWaypoints(transactionID, newWaypoints, action === CONST.IOU.ACTION.CREATE), + TransactionAction.removeWaypoint(transaction, emptyWaypointIndex.toString(), IOUUtils.shouldUseTransactionDraft(action)), + TransactionAction.updateWaypoints(transactionID, newWaypoints, IOUUtils.shouldUseTransactionDraft(action)), ]).then(() => { setOptimisticWaypoints(null); }); diff --git a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx index 71c42acefdaa..28dcbfd3094f 100644 --- a/src/pages/iou/request/step/IOURequestStepWaypoint.tsx +++ b/src/pages/iou/request/step/IOURequestStepWaypoint.tsx @@ -21,6 +21,7 @@ import useNetwork from '@hooks/useNetwork'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; import * as ErrorUtils from '@libs/ErrorUtils'; +import * as IOUUtils from '@libs/IOUUtils'; import Navigation from '@libs/Navigation/Navigation'; import * as ValidationUtils from '@libs/ValidationUtils'; import * as Modal from '@userActions/Modal'; @@ -113,13 +114,13 @@ function IOURequestStepWaypoint({ return errors; }; - const saveWaypoint = (waypoint: FormOnyxValues<'waypointForm'>) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE); + const saveWaypoint = (waypoint: FormOnyxValues<'waypointForm'>) => Transaction.saveWaypoint(transactionID, pageIndex, waypoint, IOUUtils.shouldUseTransactionDraft(action)); const submit = (values: FormOnyxValues<'waypointForm'>) => { const waypointValue = values[`waypoint${pageIndex}`] ?? ''; // Allows letting you set a waypoint to an empty value if (waypointValue === '') { - Transaction.removeWaypoint(transaction, pageIndex, action === CONST.IOU.ACTION.CREATE); + Transaction.removeWaypoint(transaction, pageIndex, IOUUtils.shouldUseTransactionDraft(action)); } // While the user is offline, the auto-complete address search will not work @@ -140,7 +141,7 @@ function IOURequestStepWaypoint({ }; const deleteStopAndHideModal = () => { - Transaction.removeWaypoint(transaction, pageIndex, action === CONST.IOU.ACTION.CREATE); + Transaction.removeWaypoint(transaction, pageIndex, IOUUtils.shouldUseTransactionDraft(action)); setRestoreFocusType(CONST.MODAL.RESTORE_FOCUS_TYPE.DELETE); setIsDeleteStopModalOpen(false); goBack(); @@ -155,7 +156,7 @@ function IOURequestStepWaypoint({ keyForList: `${values.name ?? 'waypoint'}_${Date.now()}`, }; - Transaction.saveWaypoint(transactionID, pageIndex, waypoint, action === CONST.IOU.ACTION.CREATE); + Transaction.saveWaypoint(transactionID, pageIndex, waypoint, IOUUtils.shouldUseTransactionDraft(action)); goBack(); }; diff --git a/src/pages/iou/request/step/withFullTransactionOrNotFound.tsx b/src/pages/iou/request/step/withFullTransactionOrNotFound.tsx index 9bf454b2c27d..491c37c9a402 100644 --- a/src/pages/iou/request/step/withFullTransactionOrNotFound.tsx +++ b/src/pages/iou/request/step/withFullTransactionOrNotFound.tsx @@ -73,7 +73,7 @@ export default function Date: Sun, 25 Aug 2024 16:58:25 +0800 Subject: [PATCH 3/5] fix backup is cleared because of strict mode --- src/libs/actions/TransactionEdit.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/TransactionEdit.ts b/src/libs/actions/TransactionEdit.ts index 2ce2f775586e..a21055bd16bd 100644 --- a/src/libs/actions/TransactionEdit.ts +++ b/src/libs/actions/TransactionEdit.ts @@ -3,6 +3,8 @@ import type {OnyxEntry} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; +let connectionID: number; + /** * Makes a backup copy of a transaction object that can be restored when the user cancels editing a transaction. */ @@ -11,6 +13,7 @@ function createBackupTransaction(transaction: OnyxEntry) { return; } + Onyx.disconnect(connectionID); const newTransaction = { ...transaction, }; @@ -27,10 +30,10 @@ function removeBackupTransaction(transactionID: string) { } function restoreOriginalTransactionFromBackup(transactionID: string, isDraft: boolean) { - const connection = Onyx.connect({ + connectionID = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`, callback: (backupTransaction) => { - Onyx.disconnect(connection); + Onyx.disconnect(connectionID); // Use set to completely overwrite the original transaction Onyx.set(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, backupTransaction ?? null); From 91f362dd5c235b6f2e249883930997989774741d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sun, 25 Aug 2024 17:10:56 +0800 Subject: [PATCH 4/5] add comment --- src/libs/actions/TransactionEdit.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libs/actions/TransactionEdit.ts b/src/libs/actions/TransactionEdit.ts index a21055bd16bd..ffae1ce7db30 100644 --- a/src/libs/actions/TransactionEdit.ts +++ b/src/libs/actions/TransactionEdit.ts @@ -13,6 +13,9 @@ function createBackupTransaction(transaction: OnyxEntry) { return; } + // In Strict Mode, the backup logic useEffect is triggered twice on mount. The restore logic is delayed because we need to connect to the onyx first, + // so it's possible that the restore logic is executed after creating the backup for the 2nd time which will completely clear the backup. + // To avoid that, we need to cancel the pending connection. Onyx.disconnect(connectionID); const newTransaction = { ...transaction, From 8c69a4d4ba4387b0170736735765ffc5b3f6aa55 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sun, 25 Aug 2024 21:56:47 +0800 Subject: [PATCH 5/5] lint --- src/libs/actions/TransactionEdit.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/actions/TransactionEdit.ts b/src/libs/actions/TransactionEdit.ts index ffae1ce7db30..a76cb8f25b75 100644 --- a/src/libs/actions/TransactionEdit.ts +++ b/src/libs/actions/TransactionEdit.ts @@ -1,9 +1,9 @@ import Onyx from 'react-native-onyx'; -import type {OnyxEntry} from 'react-native-onyx'; +import type {Connection, OnyxEntry} from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; -let connectionID: number; +let connection: Connection; /** * Makes a backup copy of a transaction object that can be restored when the user cancels editing a transaction. @@ -16,7 +16,7 @@ function createBackupTransaction(transaction: OnyxEntry) { // In Strict Mode, the backup logic useEffect is triggered twice on mount. The restore logic is delayed because we need to connect to the onyx first, // so it's possible that the restore logic is executed after creating the backup for the 2nd time which will completely clear the backup. // To avoid that, we need to cancel the pending connection. - Onyx.disconnect(connectionID); + Onyx.disconnect(connection); const newTransaction = { ...transaction, }; @@ -33,10 +33,10 @@ function removeBackupTransaction(transactionID: string) { } function restoreOriginalTransactionFromBackup(transactionID: string, isDraft: boolean) { - connectionID = Onyx.connect({ + connection = Onyx.connect({ key: `${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${transactionID}`, callback: (backupTransaction) => { - Onyx.disconnect(connectionID); + Onyx.disconnect(connection); // Use set to completely overwrite the original transaction Onyx.set(`${isDraft ? ONYXKEYS.COLLECTION.TRANSACTION_DRAFT : ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, backupTransaction ?? null);