-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Restore transaction when not saving distance in edit page #32334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7ea05ae
d2a9809
3cec4a3
02debe0
e799f4f
1a23e1c
7835a8b
e6182a3
32dc728
8341e91
0cc3061
455b18a
118e910
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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'; | ||
|
|
@@ -38,6 +39,9 @@ const propTypes = { | |
| /** Are we editing an existing distance request, or creating a new one? */ | ||
| isEditingRequest: PropTypes.bool, | ||
|
|
||
| /** Are we editing the distance while creating a new distance request */ | ||
| isEditingNewRequest: PropTypes.bool, | ||
|
|
||
| /** Called on submit of this page */ | ||
| onSubmit: PropTypes.func.isRequired, | ||
|
|
||
|
|
@@ -61,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); | ||
|
|
@@ -90,12 +94,36 @@ 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 (!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. | ||
| 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 | ||
| }, []); | ||
|
|
||
| useEffect(() => { | ||
| const transactionWaypoints = lodashGet(transaction, 'comment.waypoints', {}); | ||
| if (!lodashGet(transaction, 'transactionID') || !_.isEmpty(transactionWaypoints)) { | ||
|
|
@@ -134,7 +162,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); | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -182,8 +210,13 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe | |
| setHasError(true); | ||
| return; | ||
| } | ||
|
|
||
| if (isEditingNewRequest || isEditingRequest) { | ||
| transactionWasSaved.current = true; | ||
| } | ||
|
neil-marcellini marked this conversation as resolved.
|
||
|
|
||
| onSubmit(waypoints); | ||
| }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints]); | ||
| }, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditingNewRequest, isEditingRequest]); | ||
|
|
||
| const content = ( | ||
| <> | ||
|
|
@@ -238,7 +271,7 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe | |
| </> | ||
| ); | ||
|
|
||
| if (!isEditing) { | ||
| if (!isEditingNewRequest) { | ||
|
Comment on lines
-241
to
+274
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this change, would you please explain it and include a comment in the code?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @neil-marcellini We add
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok yes I understand isEditingNewRequest is true when we click the distance field from the confirmation screen while creating a new request. This condition is deciding whether to return only content, or to wrap it with a screen wrapper and header. We don't need the header for the new distance request page, nor for the editing request page, but we are missing a header when editing a new distance request with the current change 😕
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dukenv0307 please fix this bug
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @neil-marcellini Not see any header is missing from my testing. web21.mov |
||
| return content; | ||
| } | ||
|
|
||
|
|
||

Uh oh!
There was an error while loading. Please reload this page.