-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Changes from 11 commits
3c33874
3015ad8
d1bc1d4
afe1cef
bc084f7
38fed63
b0b2b57
377f773
51c2423
fa86169
c361a34
5eea454
79d2392
8b087ce
21ae159
2217f53
5e93b9e
a984951
c620254
ed4d5cf
e667003
b4a3c8b
f67234c
14251ff
25b0a31
9f0aee5
c869bd9
0379ab3
1784122
6ed18c5
ca91b67
015b990
6adbf81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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') | ||
|
@@ -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) { | ||
|
@@ -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) { | ||
|
@@ -540,13 +551,25 @@ function MoneyRequestConfirmationList(props) { | |
<FormHelpMessage | ||
style={[styles.ph1, styles.mb2]} | ||
isError | ||
message={translate(formError)} | ||
message={props.isDistanceRequest ? formError : translate(formError)} | ||
/> | ||
)} | ||
{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) : {}; | ||
|
@@ -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 : ''} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If hasRouteError, it's already shouldDisplayFieldError, isn't it? |
||
interactive={!props.isReadOnly} | ||
/> | ||
)} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
@@ -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; | ||
|
||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we should show There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
/> | ||
</OfflineWithFeedback> | ||
) : ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But not a blocker since this will be handled in #30073