From 109726ef166f05718063de528ce76385e867439b Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 04:22:51 +0500 Subject: [PATCH 1/8] fix: logic for waypoint validation --- src/components/DistanceRequest.js | 9 ++++----- src/libs/TransactionUtils.js | 16 +++++++--------- src/libs/actions/IOU.js | 3 ++- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index ce6b9880a82c..e24ae96b9e47 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -90,8 +90,8 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) const previousWaypoints = usePrevious(waypoints); const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints); const doesRouteExist = lodashHas(transaction, 'routes.route0.geometry.coordinates'); - const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && TransactionUtils.validateWaypoints(waypoints); - + const validatedWaypoints = TransactionUtils.validateWaypoints(waypoints); + const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && _.keys(validatedWaypoints).length > 1; const waypointMarkers = useMemo( () => _.filter( @@ -149,14 +149,13 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) const visibleAreaEnd = lodashGet(event, 'nativeEvent.contentOffset.y', 0) + scrollContainerHeight; setShouldShowGradient(visibleAreaEnd < scrollContentHeight); }; - useEffect(() => { if (isOffline || !shouldFetchRoute) { return; } - Transaction.getRoute(iou.transactionID, waypoints); - }, [shouldFetchRoute, iou.transactionID, waypoints, isOffline]); + Transaction.getRoute(iou.transactionID, validatedWaypoints); + }, [shouldFetchRoute, iou.transactionID, validatedWaypoints, isOffline]); useEffect(updateGradientVisibility, [scrollContainerHeight, scrollContentHeight]); diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 39ec2082d84a..8c777fc72bee 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -274,28 +274,26 @@ function getAllReportTransactions(reportID) { */ function validateWaypoints(waypoints) { const waypointValues = _.values(waypoints); - // Ensure the number of waypoints is between 2 and 25 if (waypointValues.length < 2 || waypointValues.length > 25) { return false; } - for (let i = 0; i < waypointValues.length; i++) { - const currentWaypoint = waypointValues[i]; - const previousWaypoint = waypointValues[i - 1]; - + const validWaypoints = _.reduce(waypointValues, (acc, currentWaypoint, index) => { + const previousWaypoint = waypointValues[index - 1]; // Check if the waypoint has a valid address if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') { - return false; + return acc; } // Check for adjacent waypoints with the same address if (previousWaypoint && currentWaypoint.address === previousWaypoint.address) { - return false; + return acc; } - } + return {...acc, [`waypoint${index}`]: currentWaypoint}; + }, {}); - return true; + return validWaypoints; } /* diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 9eb3658e3e53..1130b86d242f 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -519,6 +519,7 @@ function createDistanceRequest(report, participant, comment, created, transactio null, transactionID, ); + const validatedWaypoints = TransactionUtils.validateWaypoints(transaction.comment.waypoints); API.write( 'CreateDistanceRequest', { @@ -530,7 +531,7 @@ function createDistanceRequest(report, participant, comment, created, transactio createdChatReportActionID, createdIOUReportActionID, reportPreviewReportActionID: reportPreviewAction.reportActionID, - waypoints: JSON.stringify(transaction.comment.waypoints), + waypoints: JSON.stringify(validatedWaypoints), created, }, onyxData, From f7d7796693de4ec8e302f6e1836076ad2204c033 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 04:44:00 +0500 Subject: [PATCH 2/8] fix: lint issues --- src/libs/TransactionUtils.js | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 8c777fc72bee..1f1068c415ef 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -279,19 +279,23 @@ function validateWaypoints(waypoints) { return false; } - const validWaypoints = _.reduce(waypointValues, (acc, currentWaypoint, index) => { - const previousWaypoint = waypointValues[index - 1]; - // Check if the waypoint has a valid address - if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') { - return acc; - } - - // Check for adjacent waypoints with the same address - if (previousWaypoint && currentWaypoint.address === previousWaypoint.address) { - return acc; - } - return {...acc, [`waypoint${index}`]: currentWaypoint}; - }, {}); + const validWaypoints = _.reduce( + waypointValues, + (acc, currentWaypoint, index) => { + const previousWaypoint = waypointValues[index - 1]; + // Check if the waypoint has a valid address + if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') { + return acc; + } + + // Check for adjacent waypoints with the same address + if (previousWaypoint && currentWaypoint.address === previousWaypoint.address) { + return acc; + } + return {...acc, [`waypoint${index}`]: currentWaypoint}; + }, + {}, + ); return validWaypoints; } From 9b1e69320c583538dd97bb3101cb66ced254e4b7 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 11:32:53 +0500 Subject: [PATCH 3/8] fix: add proper typings for waypoint validation --- src/components/DistanceRequest.js | 2 +- src/libs/TransactionUtils.js | 8 ++++---- src/libs/actions/IOU.js | 3 +-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index e24ae96b9e47..3f2713eb7d0c 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -90,7 +90,7 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) const previousWaypoints = usePrevious(waypoints); const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints); const doesRouteExist = lodashHas(transaction, 'routes.route0.geometry.coordinates'); - const validatedWaypoints = TransactionUtils.validateWaypoints(waypoints); + const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints); const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && _.keys(validatedWaypoints).length > 1; const waypointMarkers = useMemo( () => diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 1f1068c415ef..e37b40e475c6 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -268,11 +268,11 @@ function getAllReportTransactions(reportID) { } /** - * Verifies that the provided waypoints are valid + * Filters the waypoints which are valid and returns those * @param {Object} waypoints - * @returns {Boolean} + * @returns {Object} validated waypoints */ -function validateWaypoints(waypoints) { +function getValidWaypoints(waypoints) { const waypointValues = _.values(waypoints); // Ensure the number of waypoints is between 2 and 25 if (waypointValues.length < 2 || waypointValues.length > 25) { @@ -327,7 +327,7 @@ export { getAllReportTransactions, hasReceipt, isReceiptBeingScanned, - validateWaypoints, + getValidWaypoints, isDistanceRequest, hasMissingSmartscanFields, }; diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 1130b86d242f..5fa80d4e2a72 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -519,7 +519,6 @@ function createDistanceRequest(report, participant, comment, created, transactio null, transactionID, ); - const validatedWaypoints = TransactionUtils.validateWaypoints(transaction.comment.waypoints); API.write( 'CreateDistanceRequest', { @@ -531,7 +530,7 @@ function createDistanceRequest(report, participant, comment, created, transactio createdChatReportActionID, createdIOUReportActionID, reportPreviewReportActionID: reportPreviewAction.reportActionID, - waypoints: JSON.stringify(validatedWaypoints), + waypoints: JSON.stringify(TransactionUtils.getValidWaypoints(transaction.comment.waypoints)), created, }, onyxData, From 4bc3a2396a0891705a1712476437171f405bab3d Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 16:18:15 +0500 Subject: [PATCH 4/8] fix: re-arrange indexes --- src/libs/TransactionUtils.js | 9 ++++++--- src/libs/actions/IOU.js | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index e37b40e475c6..8008ac80c732 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -270,13 +270,14 @@ function getAllReportTransactions(reportID) { /** * Filters the waypoints which are valid and returns those * @param {Object} waypoints + * @param {Boolean} reArrangeIndexes * @returns {Object} validated waypoints */ -function getValidWaypoints(waypoints) { +function getValidWaypoints(waypoints, reArrangeIndexes = false) { const waypointValues = _.values(waypoints); // Ensure the number of waypoints is between 2 and 25 if (waypointValues.length < 2 || waypointValues.length > 25) { - return false; + return {}; } const validWaypoints = _.reduce( @@ -292,7 +293,9 @@ function getValidWaypoints(waypoints) { if (previousWaypoint && currentWaypoint.address === previousWaypoint.address) { return acc; } - return {...acc, [`waypoint${index}`]: currentWaypoint}; + + const lastIndex = Math.max(..._.map(_.keys(acc), (key => parseInt(key.replace('waypoint', ''), 10)))); + return {...acc, [`waypoint${reArrangeIndexes && lastIndex > -1 ? lastIndex + 1 : index}`]: currentWaypoint}; }, {}, ); diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 5fa80d4e2a72..509b79f48099 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -530,7 +530,7 @@ function createDistanceRequest(report, participant, comment, created, transactio createdChatReportActionID, createdIOUReportActionID, reportPreviewReportActionID: reportPreviewAction.reportActionID, - waypoints: JSON.stringify(TransactionUtils.getValidWaypoints(transaction.comment.waypoints)), + waypoints: JSON.stringify(TransactionUtils.getValidWaypoints(transaction.comment.waypoints, true)), created, }, onyxData, From 2d34d9833ccafa923a470ecdc9a9b28469bfdd8b Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 16:18:53 +0500 Subject: [PATCH 5/8] lint errors --- src/libs/TransactionUtils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 8008ac80c732..d9bd9202ba65 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -294,7 +294,7 @@ function getValidWaypoints(waypoints, reArrangeIndexes = false) { return acc; } - const lastIndex = Math.max(..._.map(_.keys(acc), (key => parseInt(key.replace('waypoint', ''), 10)))); + const lastIndex = Math.max(..._.map(_.keys(acc), (key) => parseInt(key.replace('waypoint', ''), 10))); return {...acc, [`waypoint${reArrangeIndexes && lastIndex > -1 ? lastIndex + 1 : index}`]: currentWaypoint}; }, {}, From d6de6026560e9da98ef6ef3ce983d2e1f625934d Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 17:05:26 +0500 Subject: [PATCH 6/8] fix: simplify the logic for waypoint validation --- src/libs/TransactionUtils.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index d9bd9202ba65..c4bc45a5f2aa 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -280,10 +280,12 @@ function getValidWaypoints(waypoints, reArrangeIndexes = false) { return {}; } + let lastWaypointIndex = -1; + const validWaypoints = _.reduce( waypointValues, (acc, currentWaypoint, index) => { - const previousWaypoint = waypointValues[index - 1]; + const previousWaypoint = waypointValues[lastWaypointIndex]; // Check if the waypoint has a valid address if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') { return acc; @@ -294,8 +296,11 @@ function getValidWaypoints(waypoints, reArrangeIndexes = false) { return acc; } - const lastIndex = Math.max(..._.map(_.keys(acc), (key) => parseInt(key.replace('waypoint', ''), 10))); - return {...acc, [`waypoint${reArrangeIndexes && lastIndex > -1 ? lastIndex + 1 : index}`]: currentWaypoint}; + const validatedWaypoints = {...acc, [`waypoint${reArrangeIndexes ? lastWaypointIndex + 1 : index}`]: currentWaypoint}; + + lastWaypointIndex += 1; + + return validatedWaypoints; }, {}, ); From ac08f333b78781c5cc031c04620eb44a66b313e2 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Sun, 3 Sep 2023 19:39:41 +0500 Subject: [PATCH 7/8] fix: disable button when valid waypoints are less than 2 --- src/components/DistanceRequest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index 3f2713eb7d0c..644532002951 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -255,7 +255,7 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) style={[styles.w100, styles.mb4, styles.ph4, styles.flexShrink0]} onPress={() => IOU.navigateToNextPage(iou, iouType, reportID, report)} pressOnEnter - isDisabled={waypointMarkers.length < 2} + isDisabled={_.keys(validatedWaypoints).length < 2} text={translate('common.next')} /> From a8dda333f52f278f76acd93ec2df05ad31031040 Mon Sep 17 00:00:00 2001 From: Sibtain Ali Date: Mon, 4 Sep 2023 00:38:39 +0500 Subject: [PATCH 8/8] tiny refactors for waypoint logic --- src/components/DistanceRequest.js | 4 ++-- src/libs/TransactionUtils.js | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/components/DistanceRequest.js b/src/components/DistanceRequest.js index dbab56732086..453261f8607b 100644 --- a/src/components/DistanceRequest.js +++ b/src/components/DistanceRequest.js @@ -91,7 +91,7 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) const haveWaypointsChanged = !_.isEqual(previousWaypoints, waypoints); const doesRouteExist = lodashHas(transaction, 'routes.route0.geometry.coordinates'); const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints); - const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && _.keys(validatedWaypoints).length > 1; + const shouldFetchRoute = (!doesRouteExist || haveWaypointsChanged) && !isLoadingRoute && _.size(validatedWaypoints) > 1; const waypointMarkers = useMemo( () => _.filter( @@ -254,7 +254,7 @@ function DistanceRequest({iou, iouType, report, transaction, mapboxAccessToken}) success style={[styles.w100, styles.mb4, styles.ph4, styles.flexShrink0]} onPress={() => IOU.navigateToNextPage(iou, iouType, reportID, report)} - isDisabled={_.keys(validatedWaypoints).length < 2} + isDisabled={_.size(validatedWaypoints) < 2} text={translate('common.next')} /> diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index f6eec9466a5c..b31ebb0757c9 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -284,6 +284,18 @@ function getAllReportTransactions(reportID) { return _.filter(allTransactions, (transaction) => `${transaction.reportID}` === `${reportID}`); } +/** + * Checks if a waypoint has a valid address + * @param {Object} waypoint + * @returns {Boolean} Returns true if the address is valid + */ +function waypointHasValidAddress(waypoint) { + if (!waypoint || !waypoint.address || typeof waypoint.address !== 'string' || waypoint.address.trim() === '') { + return false; + } + return true; +} + /** * Filters the waypoints which are valid and returns those * @param {Object} waypoints @@ -304,7 +316,7 @@ function getValidWaypoints(waypoints, reArrangeIndexes = false) { (acc, currentWaypoint, index) => { const previousWaypoint = waypointValues[lastWaypointIndex]; // Check if the waypoint has a valid address - if (!currentWaypoint || !currentWaypoint.address || typeof currentWaypoint.address !== 'string' || currentWaypoint.address.trim() === '') { + if (!waypointHasValidAddress(currentWaypoint)) { return acc; }