From 254485568b97d7e3813bc16a876c555ba2985170 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Fri, 25 Aug 2023 14:27:27 -0700 Subject: [PATCH 1/8] Remove the route clearing on new waypoint --- src/libs/actions/Transaction.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index 02927bf7d11..ab9facf9a19 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -78,14 +78,6 @@ function saveWaypoint(transactionID, index, waypoint) { route: null, }, - // Clear the existing route so that we don't show an old route - routes: { - route0: { - geometry: { - coordinates: null, - }, - }, - }, }); } From 3ba94ad2cdaf918a50f431ad31b25d23d8c6c3d8 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Mon, 28 Aug 2023 11:53:00 -0700 Subject: [PATCH 2/8] Set waypoints to null if we are adding one offline --- src/pages/iou/WaypointEditor.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pages/iou/WaypointEditor.js b/src/pages/iou/WaypointEditor.js index c0c621e353c..bfc513f999e 100644 --- a/src/pages/iou/WaypointEditor.js +++ b/src/pages/iou/WaypointEditor.js @@ -87,6 +87,8 @@ function WaypointEditor({transactionID, route: {params: {iouType = '', waypointI // Therefore, we're going to save the waypoint as just the address, and the lat/long will be filled in on the backend if (network.isOffline && waypointValue) { const waypoint = { + lat: null, + lng: null, address: waypointValue, }; From 8528ce5b8485df5d2fb5efb783f11118ed079926 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Mon, 28 Aug 2023 12:18:29 -0700 Subject: [PATCH 3/8] Prevent waypoints offline --- src/libs/actions/Transaction.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index 94b34eb4d6e..ebf6894cb40 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -84,7 +84,23 @@ function saveWaypoint(transactionID, index, waypoint) { route: null, }, + // Clear the existing route so that we don't show an old route + routes: { + route0: { + geometry: { + coordinates: null, + }, + }, + }, + }); + + // You can save offline waypoints without verifying the address (we will geocode it on the backend) + // We're going to prevent saving those addresses in the recent waypoints though since they could be invalid addresses + if (!waypoint.lat || !waypoint.lng) { + return; + } + const recentWaypointAlreadyExists = _.find(recentWaypoints, (recentWaypoint) => recentWaypoint.address === waypoint.address); if (!recentWaypointAlreadyExists) { const clonedWaypoints = _.clone(recentWaypoints); From 9dd6abdcf6b876d9cb9ccb2232a6c954e663adee Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Mon, 28 Aug 2023 15:46:42 -0700 Subject: [PATCH 4/8] Add check for if route exists --- src/libs/actions/Transaction.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index ebf6894cb40..2b9e0a3b5b1 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -92,7 +92,6 @@ function saveWaypoint(transactionID, index, waypoint) { }, }, }, - }); // You can save offline waypoints without verifying the address (we will geocode it on the backend) From f0c7355f3179267241b892f6d323ca2bb51ab30f Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Mon, 28 Aug 2023 15:46:46 -0700 Subject: [PATCH 5/8] Update DistanceRequest.js --- src/components/DistanceRequest.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index ecc36a82452..a41b8b3d476 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -71,11 +71,12 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) { const hasRouteError = Boolean(lodashGet(transaction, 'errorFields.route')); const previousWaypoints = usePrevious(waypoints); const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints); - const shouldFetchRoute = haveWaypointsChanged && !isOffline && !isLoadingRoute && TransactionUtils.validateWaypoints(waypoints); + const doesRouteExist = Boolean(lodashGet(transaction, 'routes.route0.geometry.coordinates')); + const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && TransactionUtils.validateWaypoints(waypoints); const waypointMarkers = _.filter( _.map(waypoints, (waypoint, key) => { - if (!waypoint || waypoint.lng === undefined || waypoint.lat === undefined) { + if (!waypoint || waypoint.lng === undefined || waypoint.lat === undefined || waypoint.lat === null || waypoint.lng === null) { return; } @@ -127,13 +128,14 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) { }; // Handle fetching the route when there are at least 2 waypoints + // TODO: Handle first mount useEffect(() => { - if (!shouldFetchRoute) { + if (isOffline || !shouldFetchRoute) { return; } Transaction.getRoute(transactionID, waypoints); - }, [shouldFetchRoute, transactionID, waypoints]); + }, [shouldFetchRoute, transactionID, waypoints, isOffline]); useEffect(updateGradientVisibility, [scrollContainerHeight, scrollContentHeight]); From 9992a80fd9b259924759cb2af60c12b49b0c9ef4 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Mon, 28 Aug 2023 15:53:16 -0700 Subject: [PATCH 6/8] Comments --- src/components/DistanceRequest.js | 2 -- src/libs/actions/Transaction.js | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index a41b8b3d476..ccd4a533429 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -127,8 +127,6 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) { setShouldShowGradient(visibleAreaEnd < scrollContentHeight); }; - // Handle fetching the route when there are at least 2 waypoints - // TODO: Handle first mount useEffect(() => { if (isOffline || !shouldFetchRoute) { return; diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index 2b9e0a3b5b1..9dbe681ad7f 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -96,6 +96,7 @@ function saveWaypoint(transactionID, index, waypoint) { // You can save offline waypoints without verifying the address (we will geocode it on the backend) // We're going to prevent saving those addresses in the recent waypoints though since they could be invalid addresses + // However, in the backend once we verify the address, we will save the waypoint in the recent waypoints NVP if (!waypoint.lat || !waypoint.lng) { return; } From 1cabd2b24a9cdc1ab6c2bb7fbe9fc70e252d00dc Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Thu, 31 Aug 2023 17:05:33 -0700 Subject: [PATCH 7/8] Review comments / adding lodash has --- src/components/DistanceRequest.js | 7 ++++--- src/libs/actions/Transaction.js | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index ccd4a533429..747c0a58bc5 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -1,6 +1,7 @@ import React, {useEffect, useState} from 'react'; import {ScrollView, View} from 'react-native'; import lodashGet from 'lodash/get'; +import lodashHas from 'lodash/has'; import _ from 'underscore'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; @@ -68,15 +69,15 @@ function DistanceRequest({transactionID, transaction, mapboxAccessToken}) { const lastWaypointIndex = numberOfWaypoints - 1; const isLoadingRoute = lodashGet(transaction, 'comment.isLoading', false); - const hasRouteError = Boolean(lodashGet(transaction, 'errorFields.route')); + const hasRouteError = lodashHas(transaction, 'errorFields.route'); const previousWaypoints = usePrevious(waypoints); const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints); - const doesRouteExist = Boolean(lodashGet(transaction, 'routes.route0.geometry.coordinates')); + const doesRouteExist = lodashHas(transaction, 'routes.route0.geometry.coordinates'); const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && TransactionUtils.validateWaypoints(waypoints); const waypointMarkers = _.filter( _.map(waypoints, (waypoint, key) => { - if (!waypoint || waypoint.lng === undefined || waypoint.lat === undefined || waypoint.lat === null || waypoint.lng === null) { + if (!waypoint || !lodashHas(waypoint, 'lat') || !lodashHas(waypoint, 'lng')) { return; } diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index 9dbe681ad7f..4771e0debb3 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -1,6 +1,7 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; import lodashGet from 'lodash/get'; +import lodashHas from 'lodash/has'; import ONYXKEYS from '../../ONYXKEYS'; import * as CollectionUtils from '../CollectionUtils'; import * as API from '../API'; @@ -97,7 +98,7 @@ function saveWaypoint(transactionID, index, waypoint) { // You can save offline waypoints without verifying the address (we will geocode it on the backend) // We're going to prevent saving those addresses in the recent waypoints though since they could be invalid addresses // However, in the backend once we verify the address, we will save the waypoint in the recent waypoints NVP - if (!waypoint.lat || !waypoint.lng) { + if (!waypoint || (lodashHas(waypoint, 'lat') && waypoint.lat === 0) || (lodashHas(waypoint, 'lng') && waypoint.lng === 0)) { return; } From cab7be121c8fd11482d06976a7bf394d8042cbc5 Mon Sep 17 00:00:00 2001 From: Jack Nam Date: Thu, 31 Aug 2023 17:30:46 -0700 Subject: [PATCH 8/8] update exists --- src/libs/actions/Transaction.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Transaction.js b/src/libs/actions/Transaction.js index 4771e0debb3..b16e8c8753a 100644 --- a/src/libs/actions/Transaction.js +++ b/src/libs/actions/Transaction.js @@ -98,7 +98,7 @@ function saveWaypoint(transactionID, index, waypoint) { // You can save offline waypoints without verifying the address (we will geocode it on the backend) // We're going to prevent saving those addresses in the recent waypoints though since they could be invalid addresses // However, in the backend once we verify the address, we will save the waypoint in the recent waypoints NVP - if (!waypoint || (lodashHas(waypoint, 'lat') && waypoint.lat === 0) || (lodashHas(waypoint, 'lng') && waypoint.lng === 0)) { + if (!lodashHas(waypoint, 'lat') || !lodashHas(waypoint, 'lng')) { return; }