Skip to content

Commit

Permalink
Merge pull request Expensify#26155 from Expensify/alberto-smartErrors
Browse files Browse the repository at this point in the history
Display Smartscan errors
  • Loading branch information
tylerkaraszewski authored Sep 1, 2023
2 parents afbb90b + 4654843 commit 28a31b8
Show file tree
Hide file tree
Showing 12 changed files with 99 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,7 @@ const CONST = {
},
TRANSACTION: {
DEFAULT_MERCHANT: 'Request',
UNKNOWN_MERCHANT: 'Unknown Merchant',
TYPE: {
CUSTOM_UNIT: 'customUnit',
},
Expand Down
3 changes: 2 additions & 1 deletion src/components/MenuItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const defaultProps = {
disabled: false,
isSelected: false,
subtitle: undefined,
subtitleTextStyle: {},
iconType: CONST.ICON_TYPE_ICON,
onPress: () => {},
onSecondaryInteraction: undefined,
Expand Down Expand Up @@ -278,7 +279,7 @@ const MenuItem = React.forwardRef((props, ref) => {
{/* Since subtitle can be of type number, we should allow 0 to be shown */}
{(props.subtitle || props.subtitle === 0) && (
<View style={[styles.justifyContentCenter, styles.mr1]}>
<Text style={[styles.textLabelSupporting, props.style]}>{props.subtitle}</Text>
<Text style={[props.subtitleTextStyle || styles.textLabelSupporting, props.style]}>{props.subtitle}</Text>
</View>
)}
{!_.isEmpty(props.floatRightAvatars) && (
Expand Down
8 changes: 8 additions & 0 deletions src/components/ReportActionItem/MoneyRequestPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import PressableWithFeedback from '../Pressable/PressableWithoutFeedback';
import * as ReceiptUtils from '../../libs/ReceiptUtils';
import ReportActionItemImages from './ReportActionItemImages';
import transactionPropTypes from '../transactionPropTypes';
import colors from '../../styles/colors';

const propTypes = {
/** The active IOUReport, used for Onyx subscription */
Expand Down Expand Up @@ -152,6 +153,7 @@ 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 getSettledMessage = () => {
Expand Down Expand Up @@ -241,6 +243,12 @@ function MoneyRequestPreview(props) {
</>
)}
</View>
{hasFieldErrors && (
<Icon
src={Expensicons.DotIndicator}
fill={colors.red}
/>
)}
<Icon
fill={StyleUtils.getIconFillColor(getButtonState(props.isHovered))}
src={Expensicons.ArrowRight}
Expand Down
11 changes: 11 additions & 0 deletions src/components/ReportActionItem/MoneyRequestView.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,10 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans

const hasReceipt = TransactionUtils.hasReceipt(transaction);
let receiptURIs;
let hasErrors = false;
if (hasReceipt) {
receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction.receipt.source, transaction.filename);
hasErrors = TransactionUtils.hasMissingSmartscanFields(transaction);
}

const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction);
Expand Down Expand Up @@ -120,6 +122,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans
interactive={canEdit}
shouldShowRightIcon={canEdit}
onPress={() => Navigation.navigate(ROUTES.getEditRequestRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.AMOUNT))}
brickRoadIndicator={hasErrors && transactionAmount === 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
subtitle={hasErrors && transactionAmount === 0 ? translate('common.error.enterAmount') : ''}
subtitleTextStyle={styles.textLabelError}
/>
</OfflineWithFeedback>
<OfflineWithFeedback pendingAction={lodashGet(transaction, 'pendingFields.comment') || lodashGet(transaction, 'pendingAction')}>
Expand All @@ -140,6 +145,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans
shouldShowRightIcon={canEdit}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.getEditRequestRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.DATE))}
brickRoadIndicator={hasErrors && transactionDate === '' ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
subtitle={hasErrors && transactionDate === '' ? translate('common.error.enterDate') : ''}
subtitleTextStyle={styles.textLabelError}
/>
</OfflineWithFeedback>
<OfflineWithFeedback pendingAction={lodashGet(transaction, 'pendingFields.merchant') || lodashGet(transaction, 'pendingAction')}>
Expand All @@ -150,6 +158,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans
shouldShowRightIcon={canEdit}
titleStyle={styles.flex1}
onPress={() => Navigation.navigate(ROUTES.getEditRequestRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.MERCHANT))}
brickRoadIndicator={hasErrors && transactionMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''}
subtitle={hasErrors && transactionMerchant === CONST.TRANSACTION.UNKNOWN_MERCHANT ? translate('common.error.enterMerchant') : ''}
subtitleTextStyle={styles.textLabelError}
/>
</OfflineWithFeedback>
{shouldShowHorizontalRule && <View style={styles.reportHorizontalRule} />}
Expand Down
10 changes: 9 additions & 1 deletion src/components/ReportActionItem/ReportPreview.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import * as ReceiptUtils from '../../libs/ReceiptUtils';
import * as ReportActionUtils from '../../libs/ReportActionsUtils';
import * as TransactionUtils from '../../libs/TransactionUtils';
import ReportActionItemImages from './ReportActionItemImages';
import colors from '../../styles/colors';

const propTypes = {
/** All the data of the action */
Expand Down Expand Up @@ -115,11 +116,12 @@ function ReportPreview(props) {
const numberOfScanningReceipts = _.filter(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)).length;
const hasReceipts = transactionsWithReceipts.length > 0;
const isScanning = hasReceipts && ReportUtils.areAllRequestsBeingSmartScanned(props.iouReportID, props.action);
const hasErrors = hasReceipts && ReportUtils.hasMissingSmartscanFields(props.iouReportID);
const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action);

const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts;
const previewSubtitle = hasOnlyOneReceiptRequest
? transactionsWithReceipts[0].merchant
? TransactionUtils.getMerchant(transactionsWithReceipts[0])
: props.translate('iou.requestCount', {
count: numberOfRequests,
scanningReceipts: numberOfScanningReceipts,
Expand Down Expand Up @@ -188,6 +190,12 @@ function ReportPreview(props) {
<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
<Text style={[styles.textLabelSupporting, styles.mb1, styles.lh20]}>{getPreviewMessage()}</Text>
</View>
{hasErrors && (
<Icon
src={Expensicons.DotIndicator}
fill={colors.red}
/>
)}
<Icon
fill={StyleUtils.getIconFillColor(getButtonState(props.isHovered))}
src={Expensicons.ArrowRight}
Expand Down
3 changes: 3 additions & 0 deletions src/components/menuItemPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ const propTypes = {
/** A right-aligned subtitle for this menu option */
subtitle: PropTypes.oneOfType([PropTypes.string, PropTypes.number]),

/** Style for the subtitle */
subtitleTextStyle: stylePropTypes,

/** Flag to choose between avatar image or an icon */
iconType: PropTypes.oneOf([CONST.ICON_TYPE_AVATAR, CONST.ICON_TYPE_ICON, CONST.ICON_TYPE_WORKSPACE]),

Expand Down
4 changes: 4 additions & 0 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ export default {
characterLimit: ({limit}) => `Exceeds the maximum length of ${limit} characters`,
dateInvalid: 'Please select a valid date',
invalidCharacter: 'Invalid character',
enterMerchant: 'Enter a merchant name',
enterAmount: 'Enter an amount',
enterDate: 'Enter a date',
},
comma: 'comma',
semicolon: 'semicolon',
Expand Down Expand Up @@ -454,6 +457,7 @@ export default {
genericCreateFailureMessage: 'Unexpected error requesting money, please try again later',
genericDeleteFailureMessage: 'Unexpected error deleting the money request, please try again later',
genericEditFailureMessage: 'Unexpected error editing the money request, please try again later',
genericSmartscanFailureMessage: 'Transaction is missing fields',
},
},
notificationPreferencesPage: {
Expand Down
4 changes: 4 additions & 0 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ export default {
characterLimit: ({limit}) => `Supera el límite de ${limit} caracteres`,
dateInvalid: 'Por favor, selecciona una fecha válida',
invalidCharacter: 'Carácter invalido',
enterMerchant: 'Introduce un comerciante',
enterAmount: 'Introduce un importe',
enterDate: 'Introduce una fecha',
},
comma: 'la coma',
semicolon: 'el punto y coma',
Expand Down Expand Up @@ -453,6 +456,7 @@ export default {
genericCreateFailureMessage: 'Error inesperado solicitando dinero, Por favor, inténtalo más tarde',
genericDeleteFailureMessage: 'Error inesperado eliminando la solicitud de dinero. Por favor, inténtalo más tarde',
genericEditFailureMessage: 'Error inesperado al guardar la solicitud de dinero. Por favor, inténtalo más tarde',
genericSmartscanFailureMessage: 'La transacción tiene campos vacíos',
},
},
notificationPreferencesPage: {
Expand Down
20 changes: 15 additions & 5 deletions src/libs/OptionsListUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import * as LocalePhoneNumber from './LocalePhoneNumber';
import * as UserUtils from './UserUtils';
import * as ReportActionUtils from './ReportActionsUtils';
import * as PersonalDetailsUtils from './PersonalDetailsUtils';
import * as ErrorUtils from './ErrorUtils';

/**
* OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can
Expand Down Expand Up @@ -350,11 +351,20 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic
function getAllReportErrors(report, reportActions) {
const reportErrors = report.errors || {};
const reportErrorFields = report.errorFields || {};
const reportActionErrors = _.reduce(
reportActions,
(prevReportActionErrors, action) => (!action || _.isEmpty(action.errors) ? prevReportActionErrors : _.extend(prevReportActionErrors, action.errors)),
{},
);
const reportActionErrors = {};
_.each(reportActions, (action) => {
if (action && !_.isEmpty(action.errors)) {
_.extend(reportActionErrors, action.errors);
} else if (ReportActionUtils.isReportPreviewAction(action)) {
const iouReportID = ReportActionUtils.getIOUReportIDFromReportActionPreview(action);

// Instead of adding all Smartscan errors, let's just add a generic error if there are any. This
// will be more performant and provide the same result in the UI
if (ReportUtils.hasMissingSmartscanFields(iouReportID)) {
_.extend(reportActionErrors, {smartscan: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')});
}
}
});

// All error objects related to the report. Each object in the sources contains error messages keyed by microtime
const errorSources = {
Expand Down
12 changes: 12 additions & 0 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,17 @@ function areAllRequestsBeingSmartScanned(iouReportID, reportPreviewAction) {
return _.all(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction));
}

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

/**
* Given a parent IOU report action get report name for the LHN.
*
Expand Down Expand Up @@ -3593,4 +3604,5 @@ export {
areAllRequestsBeingSmartScanned,
getReportPreviewDisplayTransactions,
getTransactionsWithReceipts,
hasMissingSmartscanFields,
};
28 changes: 24 additions & 4 deletions src/libs/TransactionUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ function hasReceipt(transaction) {
return lodashGet(transaction, 'receipt.state', '') !== '';
}

/**
* @param {Object} transaction
* @returns {Boolean}
*/
function areModifiedFieldsPopulated(transaction) {
return transaction.modifiedMerchant !== CONST.TRANSACTION.UNKNOWN_MERCHANT && transaction.modifiedAmount !== 0 && transaction.modifiedCreated !== '';
}

/**
* Given the edit made to the money request, return an updated transaction object.
*
Expand Down Expand Up @@ -126,6 +134,7 @@ function getUpdatedTransaction(transaction, transactionChanges, isFromExpenseRep
if (shouldStopSmartscan && _.has(transaction, 'receipt') && !_.isEmpty(transaction.receipt) && lodashGet(transaction, 'receipt.state') !== CONST.IOU.RECEIPT_STATE.OPEN) {
updatedTransaction.receipt.state = CONST.IOU.RECEIPT_STATE.OPEN;
}

updatedTransaction.pendingFields = {
...(_.has(transactionChanges, 'comment') && {comment: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
...(_.has(transactionChanges, 'created') && {created: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}),
Expand Down Expand Up @@ -228,6 +237,20 @@ function getCreated(transaction) {
return '';
}

function isReceiptBeingScanned(transaction) {
return _.contains([CONST.IOU.RECEIPT_STATE.SCANREADY, CONST.IOU.RECEIPT_STATE.SCANNING], transaction.receipt.state);
}

/**
* Check if the transaction has a non-smartscanning receipt and is missing required fields
*
* @param {Object} transaction
* @returns {Boolean}
*/
function hasMissingSmartscanFields(transaction) {
return hasReceipt(transaction) && !isReceiptBeingScanned(transaction) && !areModifiedFieldsPopulated(transaction);
}

/**
* Get the transactions related to a report preview with receipts
* Get the details linked to the IOU reportAction
Expand All @@ -244,10 +267,6 @@ function getAllReportTransactions(reportID) {
return _.filter(allTransactions, (transaction) => transaction.reportID === reportID);
}

function isReceiptBeingScanned(transaction) {
return transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANREADY || transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANNING;
}

/**
* Verifies that the provided waypoints are valid
* @param {Object} waypoints
Expand Down Expand Up @@ -308,4 +327,5 @@ export {
isReceiptBeingScanned,
validateWaypoints,
isDistanceRequest,
hasMissingSmartscanFields,
};
6 changes: 6 additions & 0 deletions src/styles/styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,12 @@ const styles = {
color: themeColors.textSupporting,
},

textLabelError: {
fontFamily: fontFamily.EXP_NEUE,
fontSize: variables.fontSizeLabel,
color: themeColors.textError,
},

textReceiptUpload: {
...headlineFont,
fontSize: variables.fontSizeXLarge,
Expand Down

0 comments on commit 28a31b8

Please sign in to comment.