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.

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

[HOLD issue #34615] Display waypoint error in confirm page and detail page #30621

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
3c33874
display waypoint error in confirm page and detail page
dukenv0307 Oct 31, 2023
3015ad8
get string error for distance request
dukenv0307 Oct 31, 2023
d1bc1d4
display red dot in LHN if distance request has error
dukenv0307 Oct 31, 2023
afe1cef
Merge branch 'main' into fix/29783
dukenv0307 Nov 1, 2023
bc084f7
rename function
dukenv0307 Nov 1, 2023
38fed63
Merge branch 'main' into fix/29783
dukenv0307 Nov 2, 2023
b0b2b57
update funtion to be more performant
dukenv0307 Nov 2, 2023
377f773
Merge branch 'main' into fix/29783
dukenv0307 Nov 3, 2023
51c2423
remove un-use function
dukenv0307 Nov 3, 2023
fa86169
Merge branch 'main' into fix/29783
dukenv0307 Nov 6, 2023
c361a34
fix translation
dukenv0307 Nov 6, 2023
5eea454
edit the error
dukenv0307 Nov 8, 2023
79d2392
Merge branch 'main' into fix/29783
dukenv0307 Nov 8, 2023
8b087ce
Merge branch 'main' into fix/29783
dukenv0307 Nov 9, 2023
21ae159
fix lint
dukenv0307 Nov 9, 2023
2217f53
merge main
dukenv0307 Nov 16, 2023
5e93b9e
refactor condition
dukenv0307 Nov 16, 2023
a984951
Merge branch 'main' into fix/29783
dukenv0307 Nov 16, 2023
c620254
fix lint
dukenv0307 Nov 16, 2023
ed4d5cf
fix lint
dukenv0307 Nov 16, 2023
e667003
Merge branch 'main' into fix/29783
dukenv0307 Nov 16, 2023
b4a3c8b
not restore when transaction is loading
dukenv0307 Nov 17, 2023
f67234c
merge main
dukenv0307 Nov 17, 2023
14251ff
merge main
dukenv0307 Nov 17, 2023
25b0a31
Merge branch 'main' into fix/29783
dukenv0307 Nov 23, 2023
9f0aee5
fix offline edit
dukenv0307 Nov 23, 2023
c869bd9
merge main
dukenv0307 Dec 8, 2023
0379ab3
fix ts
dukenv0307 Dec 8, 2023
1784122
fix ts
dukenv0307 Dec 8, 2023
6ed18c5
not check offline in submit function
dukenv0307 Dec 8, 2023
ca91b67
merge main
dukenv0307 Dec 11, 2023
015b990
Merge branch 'main' into fix/29783
dukenv0307 Dec 19, 2023
6adbf81
merge main and copy logic to new confirm page
dukenv0307 Dec 19, 2023
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
2 changes: 1 addition & 1 deletion src/components/DistanceRequest/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function DistanceRequest({transactionID, report, transaction, route, isEditingRe

const isLoadingRoute = lodashGet(transaction, 'comment.isLoading', false);
const isLoading = lodashGet(transaction, 'isLoading', false);
const hasRouteError = !!lodashGet(transaction, 'errorFields.route');
const hasRouteError = TransactionUtils.hasRouteError(transaction);
const hasRoute = TransactionUtils.hasRoute(transaction);
const validatedWaypoints = TransactionUtils.getValidWaypoints(waypoints);
const previousValidatedWaypoints = usePrevious(validatedWaypoints);
Expand Down
36 changes: 30 additions & 6 deletions src/components/MoneyRequestConfirmationList.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import useLocalize from '@hooks/useLocalize';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import DistanceRequestUtils from '@libs/DistanceRequestUtils';
import * as ErrorUtils from '@libs/ErrorUtils';
import * as IOUUtils from '@libs/IOUUtils';
import Log from '@libs/Log';
import * as MoneyRequestUtils from '@libs/MoneyRequestUtils';
Expand Down Expand Up @@ -249,6 +250,7 @@ function MoneyRequestConfirmationList(props) {
const shouldShowBillable = canUseTags && !lodashGet(props.policy, 'disabledFields.defaultBillable', true);

const hasRoute = TransactionUtils.hasRoute(transaction);
const hasRouteError = TransactionUtils.hasRouteError(transaction);
const isDistanceRequestWithoutRoute = props.isDistanceRequest && !hasRoute;
const formattedAmount = isDistanceRequestWithoutRoute
? translate('common.tbd')
Expand All @@ -264,12 +266,16 @@ function MoneyRequestConfirmationList(props) {
const [didConfirmSplit, setDidConfirmSplit] = useState(false);

const shouldDisplayFieldError = useMemo(() => {
if (!props.isEditingSplitBill) {
if (!props.isEditingSplitBill && !props.isDistanceRequest) {
return false;
}

return (props.hasSmartScanFailed && TransactionUtils.hasMissingSmartscanFields(transaction)) || (didConfirmSplit && TransactionUtils.areRequiredFieldsEmpty(transaction));
}, [props.isEditingSplitBill, props.hasSmartScanFailed, transaction, didConfirmSplit]);
return (
(props.hasSmartScanFailed && TransactionUtils.hasMissingSmartscanFields(transaction)) ||
(didConfirmSplit && TransactionUtils.areRequiredFieldsEmpty(transaction)) ||
(props.isDistanceRequest && hasRouteError)
);
}, [props.isEditingSplitBill, props.hasSmartScanFailed, transaction, didConfirmSplit, hasRouteError, props.isDistanceRequest]);

useEffect(() => {
if (shouldDisplayFieldError && props.hasSmartScanFailed) {
Expand All @@ -280,9 +286,14 @@ function MoneyRequestConfirmationList(props) {
setFormError('iou.error.genericSmartscanFailureMessage');
return;
}

if (shouldDisplayFieldError && hasRouteError) {
setFormError(_.values(ErrorUtils.getLatestErrorField(transaction, 'route'))[0]);
return;
}
// reset the form error whenever the screen gains or loses focus
setFormError('');
}, [isFocused, transaction, shouldDisplayFieldError, props.hasSmartScanFailed, didConfirmSplit]);
}, [isFocused, transaction, shouldDisplayFieldError, props.hasSmartScanFailed, didConfirmSplit, hasRouteError]);

useEffect(() => {
if (!shouldCalculateDistanceAmount) {
Expand Down Expand Up @@ -540,13 +551,25 @@ function MoneyRequestConfirmationList(props) {
<FormHelpMessage
style={[styles.ph1, styles.mb2]}
isError
message={translate(formError)}
message={props.isDistanceRequest ? formError : translate(formError)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
message={props.isDistanceRequest ? formError : translate(formError)}
message={formError}

But not a blocker since this will be handled in #30073

/>
)}
{button}
</>
);
}, [confirm, props.bankAccountRoute, props.iouCurrencyCode, props.iouType, props.isReadOnly, props.policyID, selectedParticipants, splitOrRequestOptions, translate, formError]);
}, [
confirm,
props.bankAccountRoute,
props.iouCurrencyCode,
props.iouType,
props.isReadOnly,
props.policyID,
selectedParticipants,
splitOrRequestOptions,
translate,
formError,
props.isDistanceRequest,
]);

const {image: receiptImage, thumbnail: receiptThumbnail} =
props.receiptPath && props.receiptFilename ? ReceiptUtils.getThumbnailAndImageURIs(transaction, props.receiptPath, props.receiptFilename) : {};
Expand Down Expand Up @@ -672,6 +695,7 @@ function MoneyRequestConfirmationList(props) {
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.MONEY_REQUEST_DISTANCE.getRoute(props.iouType, props.reportID))}
disabled={didConfirm || !isTypeRequest}
brickRoadIndicator={shouldDisplayFieldError && hasRouteError ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
Copy link
Contributor

Choose a reason for hiding this comment

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

If hasRouteError, it's already shouldDisplayFieldError, isn't it?
I mean hasRouteError check should be enough.

interactive={!props.isReadOnly}
/>
)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ function MoneyRequestPreview(props) {
const description = requestComment;
const hasReceipt = TransactionUtils.hasReceipt(props.transaction);
const isScanning = hasReceipt && TransactionUtils.isReceiptBeingScanned(props.transaction);
const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(props.transaction);
const isDistanceRequest = TransactionUtils.isDistanceRequest(props.transaction);
const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(props.transaction) || (isDistanceRequest && TransactionUtils.hasRouteError(props.transaction));
const isExpensifyCardTransaction = TransactionUtils.isExpensifyCardTransaction(props.transaction);
const isSettled = ReportUtils.isSettled(props.iouReport.reportID);
const isDeleted = lodashGet(props.action, 'pendingAction', null) === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
Expand Down
9 changes: 9 additions & 0 deletions src/components/ReportActionItem/MoneyRequestView.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions';
import * as CardUtils from '@libs/CardUtils';
import compose from '@libs/compose';
import * as CurrencyUtils from '@libs/CurrencyUtils';
import * as ErrorUtils from '@libs/ErrorUtils';
import Navigation from '@libs/Navigation/Navigation';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import Permissions from '@libs/Permissions';
Expand Down Expand Up @@ -151,11 +152,17 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should
const hasReceipt = TransactionUtils.hasReceipt(transaction);
let receiptURIs;
let hasErrors = false;
const hasRouteError = TransactionUtils.hasRouteError(transaction);

if (hasReceipt) {
receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction);
hasErrors = canEdit && TransactionUtils.hasMissingSmartscanFields(transaction);
}

if (isDistanceRequest) {
hasErrors = canEdit && hasRouteError;
}

const pendingAction = lodashGet(transaction, 'pendingAction');
const getPendingFieldAction = (fieldPath) => lodashGet(transaction, fieldPath) || pendingAction;

Expand Down Expand Up @@ -211,6 +218,8 @@ function MoneyRequestView({report, betas, parentReport, policyCategories, should
shouldShowRightIcon={canEdit && !isSettled}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DISTANCE))}
brickRoadIndicator={hasErrors && hasRouteError ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
error={hasErrors && hasRouteError ? lodashValues(ErrorUtils.getLatestErrorField(transaction, 'route'))[0] : ''}
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we should show Please enter a valid address

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This display for special request so I think we should display correctly the route error that is returned from BE. We also display this in MoneyRequestConfirmList.

/>
</OfflineWithFeedback>
) : (
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/ReportPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function ReportPreview(props) {
const hasReceipts = transactionsWithReceipts.length > 0;
const hasOnlyDistanceRequests = ReportUtils.hasOnlyDistanceRequestTransactions(props.iouReportID);
const isScanning = hasReceipts && ReportUtils.areAllRequestsBeingSmartScanned(props.iouReportID, props.action);
const hasErrors = hasReceipts && ReportUtils.hasMissingSmartscanFields(props.iouReportID);
const hasErrors = (hasReceipts && ReportUtils.hasMissingSmartscanFields(props.iouReportID)) || ReportUtils.hasRouteError(props.iouReportID);
const lastThreeTransactionsWithReceipts = transactionsWithReceipts.slice(-3);
const lastThreeReceipts = _.map(lastThreeTransactionsWithReceipts, (transaction) => ReceiptUtils.getThumbnailAndImageURIs(transaction));
const hasNonReimbursableTransactions = ReportUtils.hasNonReimbursableTransactions(props.iouReportID);
Expand Down
8 changes: 8 additions & 0 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,18 @@ function getAllReportErrors(report, reportActions) {
if (TransactionUtils.hasMissingSmartscanFields(transaction)) {
_.extend(reportActionErrors, {smartscan: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')});
}

if (TransactionUtils.isDistanceRequest(transaction) && TransactionUtils.hasRouteError(transaction)) {
_.extend(reportActionErrors, {waypoint: ErrorUtils.getLatestErrorField(transaction, 'route')});
}
} else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report.ownerAccountID === currentUserAccountID) {
if (ReportUtils.hasMissingSmartscanFields(report.reportID)) {
_.extend(reportActionErrors, {smartscan: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')});
}

if (ReportUtils.hasRouteError(report.reportID)) {
neil-marcellini marked this conversation as resolved.
Show resolved Hide resolved
_.extend(reportActionErrors, {waypoint: ErrorUtils.getMicroSecondOnyxError('bankAccount.error.address')});
}
}

// All error objects related to the report. Each object in the sources contains error messages keyed by microtime
Expand Down
20 changes: 16 additions & 4 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1704,7 +1704,7 @@ function canEditReportAction(reportAction) {
/**
* Gets all transactions on an IOU report with a receipt
*
* @param {string|null} iouReportID
* @param {String} iouReportID
* @returns {[Object]}
*/
function getTransactionsWithReceipts(iouReportID) {
Expand Down Expand Up @@ -1732,15 +1732,26 @@ function areAllRequestsBeingSmartScanned(iouReportID, reportPreviewAction) {
return _.all(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction));
}

/**
* Check if any of the distance transactions in the report has invalid route
*
* @param {String|undefined} iouReportID
* @returns {Boolean}
*/
function hasRouteError(iouReportID) {
const allTransactions = TransactionUtils.getAllReportTransactions(iouReportID);
return _.some(allTransactions, (transaction) => TransactionUtils.hasRouteError(transaction));
}

/**
* Check if any of the transactions in the report has required missing fields
*
* @param {Object|null} iouReportID
* @param {String} iouReportID
* @returns {Boolean}
*/
function hasMissingSmartscanFields(iouReportID) {
const transactionsWithReceipts = getTransactionsWithReceipts(iouReportID);
return _.some(transactionsWithReceipts, (transaction) => TransactionUtils.hasMissingSmartscanFields(transaction));
const allTransactions = TransactionUtils.getAllReportTransactions(iouReportID);
return _.some(allTransactions, (transaction) => TransactionUtils.hasMissingSmartscanFields(transaction));
}

/**
Expand Down Expand Up @@ -4307,6 +4318,7 @@ export {
hasOnlyDistanceRequestTransactions,
hasNonReimbursableTransactions,
hasMissingSmartscanFields,
hasRouteError,
getIOUReportActionDisplayMessage,
isWaitingForAssigneeToCompleteTask,
isGroupChat,
Expand Down
8 changes: 8 additions & 0 deletions src/libs/TransactionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,13 @@ function hasMissingSmartscanFields(transaction: Transaction): boolean {
return hasReceipt(transaction) && !isDistanceRequest(transaction) && !isReceiptBeingScanned(transaction) && areRequiredFieldsEmpty(transaction);
}

/**
* Check if the transaction has route error
*/
function hasRouteError(transaction: Transaction): boolean {
return !!transaction?.errorFields?.route;
}

/**
* Check if the transaction has a defined route
*/
Expand Down Expand Up @@ -484,4 +491,5 @@ export {
getWaypointIndex,
waypointHasValidAddress,
getRecentTransactions,
hasRouteError,
};
Loading