From 7ea05ae06eb9841ba2381a57a8828faa143ae427 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Fri, 1 Dec 2023 15:43:42 +0700 Subject: [PATCH 1/8] revert transaction when not saving distance in edit page --- src/components/DistanceRequest/index.js | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index 6fa5dfede620..2a2027a35bdc 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -23,6 +23,7 @@ import useThemeStyles from '@styles/useThemeStyles'; import variables from '@styles/variables'; import * as MapboxToken from '@userActions/MapboxToken'; import * as Transaction from '@userActions/Transaction'; +import * as TransactionEdit from '@userActions/TransactionEdit'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; import DistanceRequestFooter from './DistanceRequestFooter'; @@ -90,12 +91,27 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe const haveValidatedWaypointsChanged = !_.isEqual(previousValidatedWaypoints, validatedWaypoints); const isRouteAbsentWithoutErrors = !hasRoute && !hasRouteError; const shouldFetchRoute = (isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && _.size(validatedWaypoints) > 1; + const transactionWasSaved = useRef(false); useEffect(() => { MapboxToken.init(); return MapboxToken.stop; }, []); + useEffect(() => { + if (isEditing) { + TransactionEdit.createBackupTransaction(transaction); + } + + return () => { + if (transactionWasSaved.current && !isEditing) { + return; + } + TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps -- we only want this effect when component is mounted + }, []); + useEffect(() => { const transactionWaypoints = lodashGet(transaction, 'comment.waypoints', {}); if (!lodashGet(transaction, 'transactionID') || !_.isEmpty(transactionWaypoints)) { @@ -182,8 +198,15 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe setHasError(true); return; } + + if (isEditing) { + Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); + transactionWasSaved.current = true; + return; + } + onSubmit(waypoints); - }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints]); + }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditing, iouType, reportID]); const content = ( <> From 02debe0c9c1f6522a40c8362c9795bbeb75cc71b Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 13:34:55 +0700 Subject: [PATCH 2/8] dry code --- src/components/DistanceRequest/index.js | 14 ++++++++------ src/pages/EditRequestDistancePage.js | 23 ----------------------- 2 files changed, 8 insertions(+), 29 deletions(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index 2a2027a35bdc..fbfabec4b69f 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -99,12 +99,12 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe }, []); useEffect(() => { - if (isEditing) { + if (isEditing || isEditingRequest) { TransactionEdit.createBackupTransaction(transaction); } return () => { - if (transactionWasSaved.current && !isEditing) { + if (transactionWasSaved.current || (!isEditing && !isEditingRequest)) { return; } TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); @@ -199,14 +199,16 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe return; } - if (isEditing) { - Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); + if (isEditing || isEditingRequest) { transactionWasSaved.current = true; - return; + if (isEditing) { + Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); + return; + } } onSubmit(waypoints); - }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditing, iouType, reportID]); + }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditing, iouType, reportID, isEditingRequest]); const content = ( <> diff --git a/src/pages/EditRequestDistancePage.js b/src/pages/EditRequestDistancePage.js index 48b80890dc49..0ea295c0780b 100644 --- a/src/pages/EditRequestDistancePage.js +++ b/src/pages/EditRequestDistancePage.js @@ -12,7 +12,6 @@ import useNetwork from '@hooks/useNetwork'; import usePrevious from '@hooks/usePrevious'; import Navigation from '@libs/Navigation/Navigation'; import * as IOU from '@userActions/IOU'; -import * as TransactionEdit from '@userActions/TransactionEdit'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import reportPropTypes from './reportPropTypes'; @@ -52,7 +51,6 @@ const defaultProps = { function EditRequestDistancePage({report, route, transaction, transactionBackup}) { const {isOffline} = useNetwork(); const {translate} = useLocalize(); - const transactionWasSaved = useRef(false); const hasWaypointError = useRef(false); const prevIsLoading = usePrevious(transaction.isLoading); @@ -66,26 +64,6 @@ function EditRequestDistancePage({report, route, transaction, transactionBackup} } }, [transaction, prevIsLoading, report]); - useEffect(() => { - // This effect runs when the component is mounted and unmounted. It's purpose is to be able to properly - // discard changes if the user cancels out of making any changes. This is accomplished by backing up the - // original transaction, letting the user modify the current transaction, and then if the user ever - // cancels out of the modal without saving changes, the original transaction is restored from the backup. - - // On mount, create the backup transaction. - TransactionEdit.createBackupTransaction(transaction); - - return () => { - // If the user cancels out of the modal without without saving changes, then the original transaction - // needs to be restored from the backup so that all changes are removed. - if (transactionWasSaved.current) { - return; - } - TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - /** * Save the changes to the original transaction object * @param {Object} waypoints @@ -101,7 +79,6 @@ function EditRequestDistancePage({report, route, transaction, transactionBackup} return; } - transactionWasSaved.current = true; IOU.editMoneyRequest(transaction, report.reportID, {waypoints}); // If the client is offline, then the modal can be closed as well (because there are no errors or other feedback to show them From e799f4f88cf44f1040a50f4333c90838d90b71b9 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 17:24:27 +0700 Subject: [PATCH 3/8] centralize logic --- src/components/DistanceRequest/index.js | 23 +++++++++++------------ src/pages/iou/NewDistanceRequestPage.js | 17 +++++++++++++++-- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index fbfabec4b69f..7c62c37c010a 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -39,6 +39,9 @@ const propTypes = { /** Are we editing an existing distance request, or creating a new one? */ isEditingRequest: PropTypes.bool, + /** Are we editing a new distance request */ + isEditingNewRequest: PropTypes.bool, + /** Called on submit of this page */ onSubmit: PropTypes.func.isRequired, @@ -62,17 +65,17 @@ const defaultProps = { transactionID: '', report: {}, isEditingRequest: false, + isEditingNewRequest: false, transaction: {}, }; -function DistanceRequest({transactionID, report, transaction, route, isEditingRequest, onSubmit}) { +function DistanceRequest({transactionID, report, transaction, route, isEditingRequest, isEditingNewRequest, onSubmit}) { const styles = useThemeStyles(); const {isOffline} = useNetwork(); const {translate} = useLocalize(); const [optimisticWaypoints, setOptimisticWaypoints] = useState(null); const [hasError, setHasError] = useState(false); - const isEditing = Navigation.getActiveRoute().includes('address'); const reportID = lodashGet(report, 'reportID', ''); const waypoints = useMemo(() => optimisticWaypoints || lodashGet(transaction, 'comment.waypoints', {waypoint0: {}, waypoint1: {}}), [optimisticWaypoints, transaction]); const waypointsList = _.keys(waypoints); @@ -99,12 +102,12 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe }, []); useEffect(() => { - if (isEditing || isEditingRequest) { + if (isEditingNewRequest || isEditingRequest) { TransactionEdit.createBackupTransaction(transaction); } return () => { - if (transactionWasSaved.current || (!isEditing && !isEditingRequest)) { + if (transactionWasSaved.current || (!isEditingNewRequest && !isEditingRequest)) { return; } TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); @@ -150,7 +153,7 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe }, [waypoints, previousWaypoints]); const navigateBack = () => { - Navigation.goBack(isEditing ? ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID) : ROUTES.HOME); + Navigation.goBack(isEditingNewRequest ? ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID) : ROUTES.HOME); }; /** @@ -199,16 +202,12 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe return; } - if (isEditing || isEditingRequest) { + if (isEditingNewRequest || isEditingRequest) { transactionWasSaved.current = true; - if (isEditing) { - Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); - return; - } } onSubmit(waypoints); - }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditing, iouType, reportID, isEditingRequest]); + }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditingNewRequest, isEditingRequest]); const content = ( <> @@ -263,7 +262,7 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe ); - if (!isEditing) { + if (!isEditingNewRequest) { return content; } diff --git a/src/pages/iou/NewDistanceRequestPage.js b/src/pages/iou/NewDistanceRequestPage.js index fc4549be3b33..dd8031c6cda6 100644 --- a/src/pages/iou/NewDistanceRequestPage.js +++ b/src/pages/iou/NewDistanceRequestPage.js @@ -1,13 +1,15 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useEffect} from 'react'; +import React, {useCallback, useEffect} from 'react'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import DistanceRequest from '@components/DistanceRequest'; +import Navigation from '@libs/Navigation/Navigation'; import reportPropTypes from '@pages/reportPropTypes'; import * as IOU from '@userActions/IOU'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; +import ROUTES from '@src/ROUTES'; import {iouPropTypes} from './propTypes'; const propTypes = { @@ -44,6 +46,8 @@ const defaultProps = { // You can't use Onyx props in the withOnyx mapping, so we need to set up and access the transactionID here, and then pass it down so that DistanceRequest can subscribe to the transaction. function NewDistanceRequestPage({iou, report, route}) { const iouType = lodashGet(route, 'params.iouType', 'request'); + const reportID = lodashGet(route, 'params.reportID', ''); + const isEditing = Navigation.getActiveRoute().includes('address'); useEffect(() => { if (iou.transactionID) { @@ -52,12 +56,21 @@ function NewDistanceRequestPage({iou, report, route}) { IOU.setUpDistanceTransaction(); }, [iou.transactionID]); + const onSubmit = useCallback(() => { + if (isEditing) { + Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); + return; + } + IOU.navigateToNextPage(iou, iouType, report); + }, [iou, iouType, isEditing, report, reportID]); + return ( IOU.navigateToNextPage(iou, iouType, report)} + onSubmit={onSubmit} /> ); } From 1a23e1cbcb547f1005819e80967273e5a44144cc Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 17:29:17 +0700 Subject: [PATCH 4/8] add comment --- src/components/DistanceRequest/index.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index 7c62c37c010a..72b12dce3197 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -102,17 +102,28 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe }, []); useEffect(() => { + if (!isEditingNewRequest && !isEditingRequest) { + return () => {}; + } + // This effect runs when the component is mounted and unmounted. It's purpose is to be able to properly + // discard changes if the user cancels out of making any changes. This is accomplished by backing up the + // original transaction, letting the user modify the current transaction, and then if the user ever + // cancels out of the modal without saving changes, the original transaction is restored from the backup. + + // On mount, create the backup transaction. if (isEditingNewRequest || isEditingRequest) { TransactionEdit.createBackupTransaction(transaction); } return () => { + // If the user cancels out of the modal without without saving changes, then the original transaction + // needs to be restored from the backup so that all changes are removed. if (transactionWasSaved.current || (!isEditingNewRequest && !isEditingRequest)) { return; } TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); }; - // eslint-disable-next-line react-hooks/exhaustive-deps -- we only want this effect when component is mounted + // eslint-disable-next-line react-hooks/exhaustive-deps }, []); useEffect(() => { From 7835a8b257588e1dc15855f3e047b3dcc19220c5 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 18:04:12 +0700 Subject: [PATCH 5/8] remove un-use component --- src/components/DistanceRequest/index.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index 72b12dce3197..771dc2a997ae 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -111,14 +111,12 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe // cancels out of the modal without saving changes, the original transaction is restored from the backup. // On mount, create the backup transaction. - if (isEditingNewRequest || isEditingRequest) { - TransactionEdit.createBackupTransaction(transaction); - } + TransactionEdit.createBackupTransaction(transaction); return () => { // If the user cancels out of the modal without without saving changes, then the original transaction // needs to be restored from the backup so that all changes are removed. - if (transactionWasSaved.current || (!isEditingNewRequest && !isEditingRequest)) { + if (transactionWasSaved.current) { return; } TransactionEdit.restoreOriginalTransactionFromBackup(transaction.transactionID); From e6182a3ae491db48ee4513be85856663090eba43 Mon Sep 17 00:00:00 2001 From: dukenv0307 Date: Mon, 4 Dec 2023 21:20:36 +0700 Subject: [PATCH 6/8] remove reportID variable --- src/pages/iou/NewDistanceRequestPage.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pages/iou/NewDistanceRequestPage.js b/src/pages/iou/NewDistanceRequestPage.js index dd8031c6cda6..0cf85caefac4 100644 --- a/src/pages/iou/NewDistanceRequestPage.js +++ b/src/pages/iou/NewDistanceRequestPage.js @@ -46,7 +46,6 @@ const defaultProps = { // You can't use Onyx props in the withOnyx mapping, so we need to set up and access the transactionID here, and then pass it down so that DistanceRequest can subscribe to the transaction. function NewDistanceRequestPage({iou, report, route}) { const iouType = lodashGet(route, 'params.iouType', 'request'); - const reportID = lodashGet(route, 'params.reportID', ''); const isEditing = Navigation.getActiveRoute().includes('address'); useEffect(() => { @@ -58,11 +57,11 @@ function NewDistanceRequestPage({iou, report, route}) { const onSubmit = useCallback(() => { if (isEditing) { - Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID)); + Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, report.reportID)); return; } IOU.navigateToNextPage(iou, iouType, report); - }, [iou, iouType, isEditing, report, reportID]); + }, [iou, iouType, isEditing, report]); return ( Date: Tue, 5 Dec 2023 14:31:08 +0700 Subject: [PATCH 7/8] rename variable --- src/components/DistanceRequest/index.js | 2 +- src/pages/iou/NewDistanceRequestPage.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/DistanceRequest/index.js b/src/components/DistanceRequest/index.js index 771dc2a997ae..20deac09c721 100644 --- a/src/components/DistanceRequest/index.js +++ b/src/components/DistanceRequest/index.js @@ -39,7 +39,7 @@ const propTypes = { /** Are we editing an existing distance request, or creating a new one? */ isEditingRequest: PropTypes.bool, - /** Are we editing a new distance request */ + /** Are we editing the distance while creating a new distance request */ isEditingNewRequest: PropTypes.bool, /** Called on submit of this page */ diff --git a/src/pages/iou/NewDistanceRequestPage.js b/src/pages/iou/NewDistanceRequestPage.js index 0cf85caefac4..e8479d4700d3 100644 --- a/src/pages/iou/NewDistanceRequestPage.js +++ b/src/pages/iou/NewDistanceRequestPage.js @@ -46,7 +46,7 @@ const defaultProps = { // You can't use Onyx props in the withOnyx mapping, so we need to set up and access the transactionID here, and then pass it down so that DistanceRequest can subscribe to the transaction. function NewDistanceRequestPage({iou, report, route}) { const iouType = lodashGet(route, 'params.iouType', 'request'); - const isEditing = Navigation.getActiveRoute().includes('address'); + const isEditingNewRequest = Navigation.getActiveRoute().includes('address'); useEffect(() => { if (iou.transactionID) { @@ -56,12 +56,12 @@ function NewDistanceRequestPage({iou, report, route}) { }, [iou.transactionID]); const onSubmit = useCallback(() => { - if (isEditing) { + if (isEditingNewRequest) { Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, report.reportID)); return; } IOU.navigateToNextPage(iou, iouType, report); - }, [iou, iouType, isEditing, report]); + }, [iou, iouType, isEditingNewRequest, report]); return ( Date: Tue, 5 Dec 2023 14:44:32 +0700 Subject: [PATCH 8/8] fix lint --- src/pages/iou/NewDistanceRequestPage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/iou/NewDistanceRequestPage.js b/src/pages/iou/NewDistanceRequestPage.js index e8479d4700d3..750ac5d0141e 100644 --- a/src/pages/iou/NewDistanceRequestPage.js +++ b/src/pages/iou/NewDistanceRequestPage.js @@ -67,7 +67,7 @@ function NewDistanceRequestPage({iou, report, route}) {