From 03f6abee4691333e1da39634ce277f4627e04838 Mon Sep 17 00:00:00 2001 From: Alberto Date: Thu, 24 Aug 2023 11:51:30 +0200 Subject: [PATCH 01/15] Show red dot for missing fields --- src/CONST.js | 1 + src/components/MenuItem.js | 11 ++++++++++- src/components/ReportActionItem/MoneyRequestView.js | 13 +++++++++++++ src/components/menuItemPropTypes.js | 3 +++ src/languages/en.js | 3 +++ src/styles/styles.js | 6 ++++++ 6 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/CONST.js b/src/CONST.js index 878d3f858dfb..f6b2968efc94 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -664,6 +664,7 @@ const CONST = { }, TRANSACTION: { DEFAULT_MERCHANT: 'Request', + UNKNOWN_MERCHANT: 'Unknown Merchant', TYPE: { CUSTOM_UNIT: 'customUnit', }, diff --git a/src/components/MenuItem.js b/src/components/MenuItem.js index e284244f694c..109bd1727c12 100644 --- a/src/components/MenuItem.js +++ b/src/components/MenuItem.js @@ -54,6 +54,7 @@ const defaultProps = { disabled: false, isSelected: false, subtitle: undefined, + subtitleTextStyle: {}, iconType: CONST.ICON_TYPE_ICON, onPress: () => {}, onSecondaryInteraction: undefined, @@ -102,6 +103,14 @@ const MenuItem = React.forwardRef((props, ref) => { isDeleted ? styles.offlineFeedback.deleted : undefined, ]); + const subtitleTextStyle = StyleUtils.combineStyles([ + styles.textLabelSupporting, + props.icon && !_.isArray(props.icon) ? styles.ml3 : undefined, + props.title ? descriptionVerticalMargin : StyleUtils.getFontSizeStyle(variables.fontSizeNormal), + props.descriptionTextStyle, + isDeleted ? styles.offlineFeedback.deleted : undefined, + ]); + const fallbackAvatarSize = props.viewMode === CONST.OPTION_MODE.COMPACT ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT; return ( @@ -278,7 +287,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) && ( - {props.subtitle} + {props.subtitle} )} {!_.isEmpty(props.floatRightAvatars) && ( diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index a22eaf65412e..67ce80068fe0 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -108,11 +108,15 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, polic const hasReceipt = TransactionUtils.hasReceipt(transaction); let receiptURIs; + let hasErrors = false; if (hasReceipt) { receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction.receipt.source, transaction.filename); + hasErrors = _.has(transaction, 'errors'); } const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); + console.log('eeee'); + console.log(transaction); return ( @@ -142,6 +146,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, polic disabled={isSettled || !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} /> @@ -162,6 +169,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, polic 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} /> @@ -172,6 +182,9 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, polic 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} /> {shouldShowHorizontalRule && } diff --git a/src/components/menuItemPropTypes.js b/src/components/menuItemPropTypes.js index a6424518478c..29bf973ba003 100644 --- a/src/components/menuItemPropTypes.js +++ b/src/components/menuItemPropTypes.js @@ -87,6 +87,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]), diff --git a/src/languages/en.js b/src/languages/en.js index 84b0561b3c38..30093f7cfdc0 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -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', diff --git a/src/styles/styles.js b/src/styles/styles.js index 48628993cdf8..0a9760372999 100644 --- a/src/styles/styles.js +++ b/src/styles/styles.js @@ -1113,6 +1113,12 @@ const styles = { color: themeColors.textSupporting, }, + textLabelError: { + fontFamily: fontFamily.EXP_NEUE, + fontSize: variables.fontSizeLabel, + color: themeColors.textError, + }, + textReceiptUpload: { ...headlineFont, fontSize: variables.fontSizeXLarge, From 1f4254441207eb99e9202e90b4967e56e5b8a41f Mon Sep 17 00:00:00 2001 From: Alberto Date: Fri, 25 Aug 2023 10:45:59 +0200 Subject: [PATCH 02/15] change branch --- src/libs/TransactionUtils.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index b72c6e03217d..b5122e219f65 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -123,6 +123,11 @@ 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; } + + if (_.has(transaction, 'receipt') && _.has(transaction, 'errors') && areRequiredFieldsPopulated) { + updatedTransaction.errors = {}; + } + updatedTransaction.pendingFields = { ...(_.has(transactionChanges, 'comment') && {comment: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}), ...(_.has(transactionChanges, 'created') && {created: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}), @@ -134,6 +139,12 @@ function getUpdatedTransaction(transaction, transactionChanges, isFromExpenseRep return updatedTransaction; } +function areRequiredFieldsPopulated(transaction) { + return transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT + && transaction.amount !== 0 && transaction.modifiedAmount !== 0 + && transaction.created !== '' && transaction.modifiedCreated !== ''; +} + /** * Retrieve the particular transaction object given its ID. * From 6bbf2e1ae781bdd0ae2dfb7bfc7d05e77ea3cb01 Mon Sep 17 00:00:00 2001 From: Alberto Date: Tue, 29 Aug 2023 14:51:31 +0200 Subject: [PATCH 03/15] Add the red dot everywhere --- .../ReportActionItem/MoneyRequestPreview.js | 9 ++++++++- .../ReportActionItem/ReportPreview.js | 8 ++++++++ src/languages/en.js | 1 + src/libs/OptionsListUtils.js | 20 ++++++++++++++----- src/libs/ReportUtils.js | 6 ++++++ src/libs/TransactionUtils.js | 10 ++++++++++ 6 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview.js b/src/components/ReportActionItem/MoneyRequestPreview.js index 1c536ab13b28..b7a1fcfe50a2 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview.js +++ b/src/components/ReportActionItem/MoneyRequestPreview.js @@ -32,6 +32,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 */ @@ -244,7 +245,13 @@ function MoneyRequestPreview(props) { )} - + {TransactionUtils.hasFieldErrors(props.transaction) && ( + + )} + diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index 9611ebb30d30..d32daf27fdc3 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -30,6 +30,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 */ @@ -113,6 +114,7 @@ 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.hasFieldErrors(props.iouReportID, props.action); const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action); const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts; @@ -186,6 +188,12 @@ function ReportPreview(props) { {getPreviewMessage()} + {hasErrors && ( + + )} diff --git a/src/languages/en.js b/src/languages/en.js index 2a4814d120cc..e5e96c0571b0 100755 --- a/src/languages/en.js +++ b/src/languages/en.js @@ -448,6 +448,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: { diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 72adae70e874..3778128badbf 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -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 @@ -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)), - {}, - ); + let 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.hasFieldErrors(iouReportID, action)) { + _.extend(reportActionErrors, {createChat: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')}); + } + } + }); // All error objects related to the report. Each object in the sources contains error messages keyed by microtime const errorSources = { diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index d41a31c5bae7..33c2eea4a7cc 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1358,6 +1358,11 @@ function areAllRequestsBeingSmartScanned(iouReportID, reportPreviewAction) { return _.all(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)); } +function hasFieldErrors(iouReportID, reportPreviewAction) { + const transactionsWithReceipts = getTransactionsWithReceipts(iouReportID); + return _.some(transactionsWithReceipts, (transaction) => TransactionUtils.hasFieldErrors(transaction)); +} + /** * Given a parent IOU report action get report name for the LHN. * @@ -3578,4 +3583,5 @@ export { areAllRequestsBeingSmartScanned, getReportPreviewDisplayTransactions, getTransactionsWithReceipts, + hasFieldErrors, }; diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 9125daf7751f..af542c687f55 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -239,6 +239,15 @@ function getCreated(transaction) { return ''; } +function hasFieldErrors(transaction) { + if (!_.has(transaction, 'errors')) { + return false; + } + return getAmount(transaction, false) === 0 + || getMerchant(transaction) === CONST.TRANSACTION.UNKNOWN_MERCHANT + || getCreated(transaction) === ''; +} + /** * Get the transactions related to a report preview with receipts * Get the details linked to the IOU reportAction @@ -319,4 +328,5 @@ export { isReceiptBeingScanned, validateWaypoints, isDistanceRequest, + hasFieldErrors, }; From 6013eb89092efe28c40cf5a532611d3bb8ca6273 Mon Sep 17 00:00:00 2001 From: Alberto Date: Tue, 29 Aug 2023 14:53:49 +0200 Subject: [PATCH 04/15] style fixes --- src/components/ReportActionItem/MoneyRequestPreview.js | 2 +- src/components/ReportActionItem/MoneyRequestView.js | 2 -- src/components/ReportActionItem/ReportPreview.js | 2 +- src/libs/OptionsListUtils.js | 2 +- 4 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview.js b/src/components/ReportActionItem/MoneyRequestPreview.js index b7a1fcfe50a2..eb87cc2ec6e3 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview.js +++ b/src/components/ReportActionItem/MoneyRequestPreview.js @@ -32,7 +32,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"; +import colors from '../../styles/colors'; const propTypes = { /** The active IOUReport, used for Onyx subscription */ diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index f022ce129d4f..e962dd5702cd 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -93,8 +93,6 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans } const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); - console.log('eeee'); - console.log(transaction); return ( diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index d32daf27fdc3..8da8740b32ba 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -30,7 +30,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"; +import colors from '../../styles/colors'; const propTypes = { /** All the data of the action */ diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 3778128badbf..57a9e5efe171 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -17,7 +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"; +import * as ErrorUtils from './ErrorUtils'; /** * OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can From 907f084400069af556ba42aa09af9f4d22a96e98 Mon Sep 17 00:00:00 2001 From: Alberto Date: Tue, 29 Aug 2023 14:56:36 +0200 Subject: [PATCH 05/15] improve consistency --- src/components/ReportActionItem/MoneyRequestPreview.js | 3 ++- src/components/ReportActionItem/MoneyRequestView.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview.js b/src/components/ReportActionItem/MoneyRequestPreview.js index eb87cc2ec6e3..51fa2e08b308 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview.js +++ b/src/components/ReportActionItem/MoneyRequestPreview.js @@ -151,6 +151,7 @@ function MoneyRequestPreview(props) { let description = requestComment; const hasReceipt = TransactionUtils.hasReceipt(props.transaction); const isScanning = hasReceipt && TransactionUtils.isReceiptBeingScanned(props.transaction); + const hasFieldErrors = hasReceipt && !isScanning && TransactionUtils.hasFieldErrors(props.transaction); const isDistanceRequest = TransactionUtils.isDistanceRequest(props.transaction); // On a distance request the merchant of the transaction will be used for the description since that's where it's stored in the database @@ -245,7 +246,7 @@ function MoneyRequestPreview(props) { )} - {TransactionUtils.hasFieldErrors(props.transaction) && ( + {hasFieldErrors && ( Date: Tue, 29 Aug 2023 15:03:25 +0200 Subject: [PATCH 06/15] spanish --- src/languages/es.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/languages/es.js b/src/languages/es.js index fd6fcd9b767f..8b71e56bb258 100644 --- a/src/languages/es.js +++ b/src/languages/es.js @@ -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', @@ -444,6 +447,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: { From a8dac545afa438af43765e04ee0c8db573581141 Mon Sep 17 00:00:00 2001 From: Alberto Date: Tue, 29 Aug 2023 15:15:50 +0200 Subject: [PATCH 07/15] lint --- src/components/MenuItem.js | 8 -------- src/components/ReportActionItem/ReportPreview.js | 2 +- src/libs/OptionsListUtils.js | 4 ++-- src/libs/ReportUtils.js | 2 +- src/libs/TransactionUtils.js | 12 ++++++------ 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/components/MenuItem.js b/src/components/MenuItem.js index 109bd1727c12..adf3fa0cdd80 100644 --- a/src/components/MenuItem.js +++ b/src/components/MenuItem.js @@ -103,14 +103,6 @@ const MenuItem = React.forwardRef((props, ref) => { isDeleted ? styles.offlineFeedback.deleted : undefined, ]); - const subtitleTextStyle = StyleUtils.combineStyles([ - styles.textLabelSupporting, - props.icon && !_.isArray(props.icon) ? styles.ml3 : undefined, - props.title ? descriptionVerticalMargin : StyleUtils.getFontSizeStyle(variables.fontSizeNormal), - props.descriptionTextStyle, - isDeleted ? styles.offlineFeedback.deleted : undefined, - ]); - const fallbackAvatarSize = props.viewMode === CONST.OPTION_MODE.COMPACT ? CONST.AVATAR_SIZE.SMALL : CONST.AVATAR_SIZE.DEFAULT; return ( diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index 8da8740b32ba..cc5273bd6832 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -114,7 +114,7 @@ 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.hasFieldErrors(props.iouReportID, props.action); + const hasErrors = hasReceipts && ReportUtils.hasFieldErrors(props.iouReportID); const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action); const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts; diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 57a9e5efe171..664816595adc 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -351,7 +351,7 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic function getAllReportErrors(report, reportActions) { const reportErrors = report.errors || {}; const reportErrorFields = report.errorFields || {}; - let reportActionErrors = {}; + const reportActionErrors = {}; _.each(reportActions, (action) => { if (action && !_.isEmpty(action.errors)) { _.extend(reportActionErrors, action.errors); @@ -360,7 +360,7 @@ function getAllReportErrors(report, reportActions) { // 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.hasFieldErrors(iouReportID, action)) { + if (ReportUtils.hasFieldErrors(iouReportID)) { _.extend(reportActionErrors, {createChat: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')}); } } diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index 33c2eea4a7cc..d0844df29401 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1358,7 +1358,7 @@ function areAllRequestsBeingSmartScanned(iouReportID, reportPreviewAction) { return _.all(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)); } -function hasFieldErrors(iouReportID, reportPreviewAction) { +function hasFieldErrors(iouReportID) { const transactionsWithReceipts = getTransactionsWithReceipts(iouReportID); return _.some(transactionsWithReceipts, (transaction) => TransactionUtils.hasFieldErrors(transaction)); } diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index af542c687f55..f34796c8c5cb 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -85,6 +85,12 @@ function hasReceipt(transaction) { return lodashGet(transaction, 'receipt.state', '') !== ''; } +function areRequiredFieldsPopulated(transaction) { + return transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT + && transaction.amount !== 0 && transaction.modifiedAmount !== 0 + && transaction.created !== '' && transaction.modifiedCreated !== ''; +} + /** * Given the edit made to the money request, return an updated transaction object. * @@ -142,12 +148,6 @@ function getUpdatedTransaction(transaction, transactionChanges, isFromExpenseRep return updatedTransaction; } -function areRequiredFieldsPopulated(transaction) { - return transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT - && transaction.amount !== 0 && transaction.modifiedAmount !== 0 - && transaction.created !== '' && transaction.modifiedCreated !== ''; -} - /** * Retrieve the particular transaction object given its ID. * From e20f91d2f886293d78a7806af507ee3018b02fae Mon Sep 17 00:00:00 2001 From: Alberto Date: Tue, 29 Aug 2023 15:26:32 +0200 Subject: [PATCH 08/15] prettier --- .../ReportActionItem/MoneyRequestPreview.js | 2 +- src/libs/TransactionUtils.js | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview.js b/src/components/ReportActionItem/MoneyRequestPreview.js index 51fa2e08b308..50276c4f31ca 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview.js +++ b/src/components/ReportActionItem/MoneyRequestPreview.js @@ -252,7 +252,7 @@ function MoneyRequestPreview(props) { fill={colors.red} /> )} - + diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index f34796c8c5cb..720a8594aa5c 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -86,9 +86,14 @@ function hasReceipt(transaction) { } function areRequiredFieldsPopulated(transaction) { - return transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT - && transaction.amount !== 0 && transaction.modifiedAmount !== 0 - && transaction.created !== '' && transaction.modifiedCreated !== ''; + return ( + transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && + transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && + transaction.amount !== 0 && + transaction.modifiedAmount !== 0 && + transaction.created !== '' && + transaction.modifiedCreated !== '' + ); } /** @@ -243,9 +248,7 @@ function hasFieldErrors(transaction) { if (!_.has(transaction, 'errors')) { return false; } - return getAmount(transaction, false) === 0 - || getMerchant(transaction) === CONST.TRANSACTION.UNKNOWN_MERCHANT - || getCreated(transaction) === ''; + return getAmount(transaction, false) === 0 || getMerchant(transaction) === CONST.TRANSACTION.UNKNOWN_MERCHANT || getCreated(transaction) === ''; } /** From f0d2fc3268b6d6c259e2d071e5b7dc1f0377769d Mon Sep 17 00:00:00 2001 From: Alberto Date: Wed, 30 Aug 2023 10:59:06 +0200 Subject: [PATCH 09/15] do not use errors field --- src/components/ReportActionItem/MoneyRequestPreview.js | 2 +- src/components/ReportActionItem/MoneyRequestView.js | 2 +- src/components/ReportActionItem/ReportPreview.js | 2 +- src/libs/OptionsListUtils.js | 2 +- src/libs/ReportUtils.js | 6 +++--- src/libs/TransactionUtils.js | 9 +++------ 6 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestPreview.js b/src/components/ReportActionItem/MoneyRequestPreview.js index 0f44d16d9524..9a3d91b35208 100644 --- a/src/components/ReportActionItem/MoneyRequestPreview.js +++ b/src/components/ReportActionItem/MoneyRequestPreview.js @@ -153,7 +153,7 @@ function MoneyRequestPreview(props) { let description = requestComment; const hasReceipt = TransactionUtils.hasReceipt(props.transaction); const isScanning = hasReceipt && TransactionUtils.isReceiptBeingScanned(props.transaction); - const hasFieldErrors = hasReceipt && !isScanning && TransactionUtils.hasFieldErrors(props.transaction); + const hasFieldErrors = TransactionUtils.hasMissingSmartscanFields(props.transaction); const isDistanceRequest = TransactionUtils.isDistanceRequest(props.transaction); // On a distance request the merchant of the transaction will be used for the description since that's where it's stored in the database diff --git a/src/components/ReportActionItem/MoneyRequestView.js b/src/components/ReportActionItem/MoneyRequestView.js index d7896d3d502a..b549df7fc55f 100644 --- a/src/components/ReportActionItem/MoneyRequestView.js +++ b/src/components/ReportActionItem/MoneyRequestView.js @@ -89,7 +89,7 @@ function MoneyRequestView({report, parentReport, shouldShowHorizontalRule, trans let hasErrors = false; if (hasReceipt) { receiptURIs = ReceiptUtils.getThumbnailAndImageURIs(transaction.receipt.source, transaction.filename); - hasErrors = TransactionUtils.hasFieldErrors(transaction); + hasErrors = TransactionUtils.hasMissingSmartscanFields(transaction); } const isDistanceRequest = TransactionUtils.isDistanceRequest(transaction); diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index c8714d6f0e11..1ccab7d6f3ff 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -116,7 +116,7 @@ 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.hasFieldErrors(props.iouReportID); + const hasErrors = hasReceipts && ReportUtils.hasMissingSmartscanFields(props.iouReportID); const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action); const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts; diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 1b2eee10387a..2b75dcd377d0 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -360,7 +360,7 @@ function getAllReportErrors(report, reportActions) { // 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.hasFieldErrors(iouReportID)) { + if (ReportUtils.hasMissingSmartscanFields(iouReportID)) { _.extend(reportActionErrors, {createChat: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')}); } } diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index a2473ced633b..b57ddefaabb4 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1359,9 +1359,9 @@ function areAllRequestsBeingSmartScanned(iouReportID, reportPreviewAction) { return _.all(transactionsWithReceipts, (transaction) => TransactionUtils.isReceiptBeingScanned(transaction)); } -function hasFieldErrors(iouReportID) { +function hasMissingSmartscanFields(iouReportID) { const transactionsWithReceipts = getTransactionsWithReceipts(iouReportID); - return _.some(transactionsWithReceipts, (transaction) => TransactionUtils.hasFieldErrors(transaction)); + return _.some(transactionsWithReceipts, (transaction) => TransactionUtils.hasMissingSmartscanFields(transaction)); } /** @@ -3584,5 +3584,5 @@ export { areAllRequestsBeingSmartScanned, getReportPreviewDisplayTransactions, getTransactionsWithReceipts, - hasFieldErrors, + hasMissingSmartscanFields, }; diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 720a8594aa5c..3fdcde9a6ee1 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -244,11 +244,8 @@ function getCreated(transaction) { return ''; } -function hasFieldErrors(transaction) { - if (!_.has(transaction, 'errors')) { - return false; - } - return getAmount(transaction, false) === 0 || getMerchant(transaction) === CONST.TRANSACTION.UNKNOWN_MERCHANT || getCreated(transaction) === ''; +function hasMissingSmartscanFields(transaction) { + return hasReceipt(transaction) && !isReceiptBeingScanned(transaction) && !areRequiredFieldsPopulated(transaction); } /** @@ -331,5 +328,5 @@ export { isReceiptBeingScanned, validateWaypoints, isDistanceRequest, - hasFieldErrors, + hasMissingSmartscanFields, }; From b46334601e1706c5428768a785d2929fb8a87389 Mon Sep 17 00:00:00 2001 From: Alberto Date: Wed, 30 Aug 2023 11:03:29 +0200 Subject: [PATCH 10/15] no need to clear errors anymore --- src/libs/TransactionUtils.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 3fdcde9a6ee1..ab6831942495 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -138,10 +138,6 @@ function getUpdatedTransaction(transaction, transactionChanges, isFromExpenseRep updatedTransaction.receipt.state = CONST.IOU.RECEIPT_STATE.OPEN; } - if (_.has(transaction, 'receipt') && _.has(transaction, 'errors') && areRequiredFieldsPopulated) { - updatedTransaction.errors = {}; - } - updatedTransaction.pendingFields = { ...(_.has(transactionChanges, 'comment') && {comment: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}), ...(_.has(transactionChanges, 'created') && {created: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}), From fa60dd75e197586f94d9c1ea5a9e1650286a627d Mon Sep 17 00:00:00 2001 From: Alberto Date: Wed, 30 Aug 2023 11:06:35 +0200 Subject: [PATCH 11/15] comments --- src/libs/ReportUtils.js | 6 ++++++ src/libs/TransactionUtils.js | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/libs/ReportUtils.js b/src/libs/ReportUtils.js index b57ddefaabb4..cafa38c3c998 100644 --- a/src/libs/ReportUtils.js +++ b/src/libs/ReportUtils.js @@ -1359,6 +1359,12 @@ 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)); diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index ab6831942495..69e0a16bfbad 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -85,6 +85,10 @@ function hasReceipt(transaction) { return lodashGet(transaction, 'receipt.state', '') !== ''; } +/** + * @param {Object} transaction + * @returns {Boolean} + */ function areRequiredFieldsPopulated(transaction) { return ( transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && @@ -240,6 +244,12 @@ function getCreated(transaction) { return ''; } +/** + * 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) && !areRequiredFieldsPopulated(transaction); } From afc5d1d116b0e3e359865a8decc74838b4e18a87 Mon Sep 17 00:00:00 2001 From: Alberto Date: Wed, 30 Aug 2023 11:33:33 +0200 Subject: [PATCH 12/15] lint --- src/libs/TransactionUtils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 69e0a16bfbad..295b2bb4fd2a 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -244,6 +244,10 @@ function getCreated(transaction) { return ''; } +function isReceiptBeingScanned(transaction) { + return transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANREADY || transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANNING; +} + /** * Check if the transaction has a non-smartscanning receipt and is missing required fields * @@ -270,10 +274,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 From 46835ebfa9db734111b4e5145df6ad4294fc8de3 Mon Sep 17 00:00:00 2001 From: Alberto Date: Fri, 1 Sep 2023 11:05:38 +0200 Subject: [PATCH 13/15] check fileds correctly --- src/libs/TransactionUtils.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 295b2bb4fd2a..9b9a56ec7ef0 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -89,15 +89,8 @@ function hasReceipt(transaction) { * @param {Object} transaction * @returns {Boolean} */ -function areRequiredFieldsPopulated(transaction) { - return ( - transaction.merchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && - transaction.modifiedMerchant !== CONST.TRANSACTION.DEFAULT_MERCHANT && - transaction.amount !== 0 && - transaction.modifiedAmount !== 0 && - transaction.created !== '' && - transaction.modifiedCreated !== '' - ); +function areModifiedFieldsPopulated(transaction) { + return transaction.modifiedMerchant !== CONST.TRANSACTION.UNKNOWN_MERCHANT && transaction.modifiedAmount !== 0 && transaction.modifiedCreated !== ''; } /** From aaa345b09ee74d69f5fb5a4c50662cc47a5705ae Mon Sep 17 00:00:00 2001 From: Alberto Date: Fri, 1 Sep 2023 11:33:00 +0200 Subject: [PATCH 14/15] show correct merchant --- src/components/ReportActionItem/ReportPreview.js | 15 +++++++++------ src/libs/OptionsListUtils.js | 2 +- src/libs/TransactionUtils.js | 2 +- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index 1ccab7d6f3ff..fafbb63381df 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -120,12 +120,15 @@ function ReportPreview(props) { const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action); const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts; - const previewSubtitle = hasOnlyOneReceiptRequest - ? transactionsWithReceipts[0].merchant - : props.translate('iou.requestCount', { - count: numberOfRequests, - scanningReceipts: numberOfScanningReceipts, - }); + let previewSubtitle = ''; + if (hasOnlyOneReceiptRequest) { + previewSubtitle = transactionsWithReceipts[0].modifiedMerchant || transactionsWithReceipts[0].merchant; + } else { + previewSubtitle = props.translate('iou.requestCount', { + count: numberOfRequests, + scanningReceipts: numberOfScanningReceipts, + }); + } const getDisplayAmount = () => { if (reportTotal) { diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 0143322e3515..4aa1ce14cf5c 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -361,7 +361,7 @@ function getAllReportErrors(report, reportActions) { // 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, {createChat: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')}); + _.extend(reportActionErrors, {smartscan: ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage')}); } } }); diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index 9b9a56ec7ef0..e3faa494091c 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -248,7 +248,7 @@ function isReceiptBeingScanned(transaction) { * @returns {Boolean} */ function hasMissingSmartscanFields(transaction) { - return hasReceipt(transaction) && !isReceiptBeingScanned(transaction) && !areRequiredFieldsPopulated(transaction); + return hasReceipt(transaction) && !isReceiptBeingScanned(transaction) && !areModifiedFieldsPopulated(transaction); } /** From 4654843aa0384ef364b6594603c9d11f777601b1 Mon Sep 17 00:00:00 2001 From: Alberto Date: Fri, 1 Sep 2023 15:41:13 +0200 Subject: [PATCH 15/15] NABs --- src/components/ReportActionItem/ReportPreview.js | 15 ++++++--------- src/libs/TransactionUtils.js | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/components/ReportActionItem/ReportPreview.js b/src/components/ReportActionItem/ReportPreview.js index fafbb63381df..4526251d7609 100644 --- a/src/components/ReportActionItem/ReportPreview.js +++ b/src/components/ReportActionItem/ReportPreview.js @@ -120,15 +120,12 @@ function ReportPreview(props) { const lastThreeTransactionsWithReceipts = ReportUtils.getReportPreviewDisplayTransactions(props.action); const hasOnlyOneReceiptRequest = numberOfRequests === 1 && hasReceipts; - let previewSubtitle = ''; - if (hasOnlyOneReceiptRequest) { - previewSubtitle = transactionsWithReceipts[0].modifiedMerchant || transactionsWithReceipts[0].merchant; - } else { - previewSubtitle = props.translate('iou.requestCount', { - count: numberOfRequests, - scanningReceipts: numberOfScanningReceipts, - }); - } + const previewSubtitle = hasOnlyOneReceiptRequest + ? TransactionUtils.getMerchant(transactionsWithReceipts[0]) + : props.translate('iou.requestCount', { + count: numberOfRequests, + scanningReceipts: numberOfScanningReceipts, + }); const getDisplayAmount = () => { if (reportTotal) { diff --git a/src/libs/TransactionUtils.js b/src/libs/TransactionUtils.js index e3faa494091c..39ec2082d84a 100644 --- a/src/libs/TransactionUtils.js +++ b/src/libs/TransactionUtils.js @@ -238,7 +238,7 @@ function getCreated(transaction) { } function isReceiptBeingScanned(transaction) { - return transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANREADY || transaction.receipt.state === CONST.IOU.RECEIPT_STATE.SCANNING; + return _.contains([CONST.IOU.RECEIPT_STATE.SCANREADY, CONST.IOU.RECEIPT_STATE.SCANNING], transaction.receipt.state); } /**