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

fix: show error message for invalid receipt #34087

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
10 changes: 9 additions & 1 deletion src/components/ReportActionItem/MoneyRequestView.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import iouReportPropTypes from '@pages/iouReportPropTypes';
import reportPropTypes from '@pages/reportPropTypes';
import {policyDefaultProps, policyPropTypes} from '@pages/workspace/withPolicy';
import * as IOU from '@userActions/IOU';
import * as Transaction from '@userActions/Transaction';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -220,7 +221,14 @@ function MoneyRequestView({report, parentReport, parentReportActions, policyCate
<AnimatedEmptyStateBackground />
<View style={[StyleUtils.getReportWelcomeTopMarginStyle(isSmallScreenWidth)]}>
{hasReceipt && (
<OfflineWithFeedback pendingAction={pendingAction}>
<OfflineWithFeedback
pendingAction={pendingAction}
errors={transaction.errors}
errorRowStyles={[styles.ml4]}
onClose={() => {
Transaction.clearError(transaction.transactionID);
Copy link
Contributor

Choose a reason for hiding this comment

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

We missed handling the case of createChat errorfields which would happen when the creation of the chat itself fails thereby causing #43481.

}}
>
<View style={styles.moneyRequestViewImage}>
<ReportActionItemImage
thumbnail={receiptURIs.thumbnail}
Expand Down
7 changes: 5 additions & 2 deletions src/libs/ReceiptUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Str from 'expensify-common/lib/str';
import _ from 'lodash';
import type {ImageSourcePropType} from 'react-native';
import ReceiptDoc from '@assets/images/receipt-doc.png';
import ReceiptGeneric from '@assets/images/receipt-generic.png';
Expand All @@ -7,6 +8,7 @@ import ReceiptSVG from '@assets/images/receipt-svg.png';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
import type {Transaction} from '@src/types/onyx';
import type {ReceiptError} from '@src/types/onyx/Transaction';
import * as FileUtils from './fileDownload/FileUtils';

type ThumbnailAndImageURI = {
Expand All @@ -30,9 +32,10 @@ type FileNameAndExtension = {
*/
function getThumbnailAndImageURIs(transaction: Transaction, receiptPath: string | null = null, receiptFileName: string | null = null): ThumbnailAndImageURI {
// URI to image, i.e. blob:new.expensify.com/9ef3a018-4067-47c6-b29f-5f1bd35f213d or expensify.com/receipts/w_e616108497ef940b7210ec6beb5a462d01a878f4.jpg
const path = transaction?.receipt?.source ?? receiptPath ?? '';
const errors = _.findLast(transaction.errors) as ReceiptError | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need union type here ReceiptError | null, can it be ReceiptError only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

error can be undefined (receipt without error), so I think it should be ReceiptError | undefined

const path = errors?.source ?? transaction?.receipt?.source ?? receiptPath ?? '';
tienifr marked this conversation as resolved.
Show resolved Hide resolved
// filename of uploaded image or last part of remote URI
const filename = transaction?.filename ?? receiptFileName ?? '';
const filename = errors?.filename ?? transaction?.filename ?? receiptFileName ?? '';
const isReceiptImage = Str.isImage(filename);

const hasEReceipt = transaction?.hasEReceipt;
Expand Down
13 changes: 7 additions & 6 deletions src/libs/actions/IOU.js
Original file line number Diff line number Diff line change
Expand Up @@ -3304,18 +3304,18 @@ function detachReceipt(transactionID) {
* @param {String} filePath
*/
function replaceReceipt(transactionID, receipt, filePath) {
const transaction = lodashGet(allTransactions, 'transactionID', {});
const transaction = allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] || {};
const oldReceipt = lodashGet(transaction, 'receipt', {});

const receiptOptimistic = {
source: filePath,
state: CONST.IOU.RECEIPT_STATE.OPEN,
};
const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`,
value: {
receipt: {
source: filePath,
state: CONST.IOU.RECEIPT_STATE.OPEN,
},
receipt: receiptOptimistic,
filename: receipt.name,
},
},
Expand All @@ -3328,6 +3328,7 @@ function replaceReceipt(transactionID, receipt, filePath) {
value: {
receipt: oldReceipt,
filename: transaction.filename,
errors: getReceiptError(receiptOptimistic, receipt.name),
},
},
];
Expand Down
6 changes: 5 additions & 1 deletion src/libs/actions/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,8 @@ function updateWaypoints(transactionID: string, waypoints: WaypointCollection, i
});
}

export {addStop, createInitialWaypoints, saveWaypoint, removeWaypoint, getRoute, getRouteForDraft, updateWaypoints};
function clearError(transactionID: string) {
Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, {errors: null});
}

export {addStop, createInitialWaypoints, saveWaypoint, removeWaypoint, getRoute, getRouteForDraft, updateWaypoints, clearError};
8 changes: 6 additions & 2 deletions src/types/onyx/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,18 @@ type Route = {

type Routes = Record<string, Route>;

type ReceiptError = {error?: string; source: string; filename: string};

type ReceiptErrors = Record<string, ReceiptError>;

type Transaction = {
amount: number;
billable: boolean;
category: string;
comment: Comment;
created: string;
currency: string;
errors?: OnyxCommon.Errors;
errors?: OnyxCommon.Errors | ReceiptErrors;
errorFields?: OnyxCommon.ErrorFields<'route'>;
// The name of the file used for a receipt (formerly receiptFilename)
filename?: string;
Expand Down Expand Up @@ -97,4 +101,4 @@ type Transaction = {
};

export default Transaction;
export type {WaypointCollection, Comment, Receipt, Waypoint};
export type {WaypointCollection, Comment, Receipt, Waypoint, ReceiptError};
Loading