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

Jump to bottom

fix: Duplicate waypoint error message is not dimissed after closing distance editor #35910

Merged
merged 3 commits into from
Feb 21, 2024
Merged

fix: Duplicate waypoint error message is not dimissed after closing distance editor #35910

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b39a3a
fix: Duplicate waypoint error message is not dimissed after closing d…
tienifr Feb 6, 2024
218b97a
Merge branch 'main' into fix/34999
tienifr Feb 14, 2024
ba736c8
resolve conflicts
tienifr Feb 20, 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
19 changes: 12 additions & 7 deletions src/components/DistanceRequest/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Report, Transaction} from '@src/types/onyx';
import type {WaypointCollection} from '@src/types/onyx/Transaction';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import DistanceRequestFooter from './DistanceRequestFooter';
import DistanceRequestRenderItem from './DistanceRequestRenderItem';

Expand Down Expand Up @@ -176,7 +177,7 @@ function DistanceRequest({transactionID = '', report, transaction, route, isEdit
);
};

const getError = () => {
const getError = useCallback(() => {
// Get route error if available else show the invalid number of waypoints error.
if (hasRouteError) {
return ErrorUtils.getLatestErrorField((transaction ?? {}) as Transaction, 'route');
Expand All @@ -186,8 +187,12 @@ function DistanceRequest({transactionID = '', report, transaction, route, isEdit
// eslint-disable-next-line @typescript-eslint/naming-convention
return {0: 'iou.error.atLeastTwoDifferentWaypoints'};
}
return {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but I just spotted this now. If this return statement is removed, then I don't think the !isEmptyObject(getError()) check below is going to work. Does this method need to keep returning an empty object at the end of the if statements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's the implementation of isEmptyObject

App/src/types/utils/EmptyObject.ts

Lines 5 to 7 in 6989089

function isEmptyObject<T>(obj: T | EmptyValue): obj is EmptyValue {
return Object.keys(obj ?? {}).length === 0;
}

So I don't think we need to return empty object

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK. Thanks!

};

if (Object.keys(validatedWaypoints).length < Object.keys(waypoints).length) {
// eslint-disable-next-line @typescript-eslint/naming-convention
return {0: translate('iou.error.duplicateWaypointsErrorMessage')};
}
}, [translate, transaction, hasRouteError, validatedWaypoints, waypoints]);

const updateWaypoints = useCallback(
({data}: DraggableListData<string>) => {
Expand All @@ -211,7 +216,7 @@ function DistanceRequest({transactionID = '', report, transaction, route, isEdit

const submitWaypoints = useCallback(() => {
// If there is any error or loading state, don't let user go to next page.
if (Object.keys(validatedWaypoints).length < 2 || hasRouteError || isLoadingRoute || (isLoading && !isOffline)) {
if (!isEmptyObject(getError()) || isLoadingRoute || (isLoading && !isOffline)) {
setHasError(true);
return;
}
Expand All @@ -221,7 +226,7 @@ function DistanceRequest({transactionID = '', report, transaction, route, isEdit
}

onSubmit(waypoints);
}, [onSubmit, setHasError, hasRouteError, isLoadingRoute, isLoading, validatedWaypoints, waypoints, isEditingNewRequest, isEditingRequest, isOffline]);
}, [onSubmit, setHasError, getError, isLoadingRoute, isLoading, waypoints, isEditingNewRequest, isEditingRequest, isOffline]);

const content = (
<>
Expand Down Expand Up @@ -254,10 +259,10 @@ function DistanceRequest({transactionID = '', report, transaction, route, isEdit
</View>
<View style={[styles.w100, styles.pt2]}>
{/* Show error message if there is route error or there are less than 2 routes and user has tried submitting, */}
{((hasError && Object.keys(validatedWaypoints).length < 2) || hasRouteError) && (
{((hasError && !isEmptyObject(getError())) || hasRouteError) && (
<DotIndicatorMessage
style={[styles.mh4, styles.mv3]}
messages={getError()}
messages={getError() ?? {}}
type="error"
/>
)}
Expand Down
Loading