Skip to content
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.

Sign up for GitHub

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

Fix infinite get route request #45579

Merged
merged 2 commits into from
Jul 18, 2024
Merged

Fix infinite get route request #45579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
afb9f1b
make it optional
bernhardoj Jul 17, 2024
c3a799f
only refetch route if the route is empty and there is no route error
bernhardoj Jul 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/libs/TransactionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,8 @@ function hasPendingUI(transaction: OnyxEntry<Transaction>, transactionViolations
/**
* Check if the transaction has a defined route
*/
function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType: boolean): boolean {
return !!transaction?.routes?.route0?.geometry?.coordinates || (isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);
function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType?: boolean): boolean {
return !!transaction?.routes?.route0?.geometry?.coordinates || (!!isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);
}

function getAllReportTransactions(reportID?: string, transactions?: OnyxCollection<Transaction>): Transaction[] {
Expand Down
5 changes: 2 additions & 3 deletions src/pages/iou/request/step/IOURequestStepDistance.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,12 @@ function IOURequestStepDistance({
const isLoadingRoute = transaction?.comment?.isLoading ?? false;
const isLoading = transaction?.isLoading ?? false;
const hasRouteError = !!transaction?.errorFields?.route;
const hasRoute = TransactionUtils.hasRoute(transaction, true);
const hasRoute = TransactionUtils.hasRoute(transaction);
const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints);
const previousValidatedWaypoints = usePrevious(validatedWaypoints);
const haveValidatedWaypointsChanged = !isEqual(previousValidatedWaypoints, validatedWaypoints);
const isRouteAbsentWithoutErrors = !hasRoute && !hasRouteError;
const isEmptyCoordinates = !transaction?.routes?.route0?.geometry?.coordinates?.length;
const shouldFetchRoute = (isEmptyCoordinates || isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && Object.keys(validatedWaypoints).length > 1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thesahindia @neil-marcellini While working on this other PR, I found an issue from our PR #45139. When the selected waypoints are invalid (gives an error), it will result in an infinite request of GetRoute.

web_error.mp4

So, my first thought is to only re-fetch the route if there is no error. Then, I saw that we already have that kind of logic, the isRouteAbsentWithoutErrors.

hasRoute also checks for the route coordinate, but it also checks for customUnit.quantity if isDistanceRequestType.

App/src/libs/TransactionUtils.ts

Lines 576 to 578 in 52b8fdd

function hasRoute(transaction: OnyxEntry<Transaction>, isDistanceRequestType: boolean): boolean {
return !!transaction?.routes?.route0?.geometry?.coordinates || (isDistanceRequestType && !!transaction?.comment?.customUnit?.quantity);
}

Because hasRoute is only used to re-fetch the routes, I decided to remove isEmptyCoordinates and pass nothing to isDistanceRequestType param.

const shouldFetchRoute = (isRouteAbsentWithoutErrors || haveValidatedWaypointsChanged) && !isLoadingRoute && Object.keys(validatedWaypoints).length > 1;
const [shouldShowAtLeastTwoDifferentWaypointsError, setShouldShowAtLeastTwoDifferentWaypointsError] = useState(false);
const isWaypointEmpty = (waypoint?: Waypoint) => {
if (!waypoint) {
Expand Down
Loading