Skip to content

Commit

Permalink
Merge pull request #37875 from tienifr/fix/34189
Browse files Browse the repository at this point in the history
Fix: IOU-Error message remains on IOU report even same error message is dismissed from 1:1
  • Loading branch information
amyevans authored Mar 25, 2024
2 parents 3d78063 + 82bb67f commit b3baa68
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 19 deletions.
8 changes: 4 additions & 4 deletions src/libs/ErrorUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ function getAuthenticateErrorMessage(response: Response): keyof TranslationFlatO
* Method used to get an error object with microsecond as the key.
* @param error - error key or message to be saved
*/
function getMicroSecondOnyxError(error: string | null, isTranslated = false): Errors {
return {[DateUtils.getMicroseconds()]: error && [error, {isTranslated}]};
function getMicroSecondOnyxError(error: string | null, isTranslated = false, errorKey?: number): Errors {
return {[errorKey ?? DateUtils.getMicroseconds()]: error && [error, {isTranslated}]};
}

/**
* Method used to get an error object with microsecond as the key and an object as the value.
* @param error - error key or message to be saved
*/
function getMicroSecondOnyxErrorObject(error: Errors): ErrorFields {
return {[DateUtils.getMicroseconds()]: error};
function getMicroSecondOnyxErrorObject(error: Errors, errorKey?: number): ErrorFields {
return {[errorKey ?? DateUtils.getMicroseconds()]: error};
}

// We can assume that if error is a string, it has already been translated because it is server error
Expand Down
24 changes: 16 additions & 8 deletions src/libs/actions/IOU.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,10 +417,10 @@ function resetMoneyRequestInfo(id = '') {
}

/** Helper function to get the receipt error for money requests, or the generic error if there's no receipt */
function getReceiptError(receipt?: Receipt, filename?: string, isScanRequest = true): Errors | ErrorFields {
function getReceiptError(receipt?: Receipt, filename?: string, isScanRequest = true, errorKey?: number): Errors | ErrorFields {
return isEmptyObject(receipt) || !isScanRequest
? ErrorUtils.getMicroSecondOnyxError('iou.error.genericCreateFailureMessage')
: ErrorUtils.getMicroSecondOnyxErrorObject({error: CONST.IOU.RECEIPT_ERROR, source: receipt.source?.toString() ?? '', filename: filename ?? ''});
? ErrorUtils.getMicroSecondOnyxError('iou.error.genericCreateFailureMessage', false, errorKey)
: ErrorUtils.getMicroSecondOnyxErrorObject({error: CONST.IOU.RECEIPT_ERROR, source: receipt.source?.toString() ?? '', filename: filename ?? ''}, errorKey);
}

function needsToBeManuallySubmitted(iouReport: OnyxTypes.Report) {
Expand Down Expand Up @@ -708,6 +708,8 @@ function buildOnyxDataForMoneyRequest(
},
);

const errorKey = DateUtils.getMicroseconds();

const failureData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
Expand Down Expand Up @@ -763,7 +765,7 @@ function buildOnyxDataForMoneyRequest(
[iouCreatedAction.reportActionID]: {
// Disabling this line since transaction.filename can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, isScanRequest),
errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, isScanRequest, errorKey),
},
[iouAction.reportActionID]: {
errors: ErrorUtils.getMicroSecondOnyxError(null),
Expand All @@ -773,7 +775,7 @@ function buildOnyxDataForMoneyRequest(
[iouAction.reportActionID]: {
// Disabling this line since transaction.filename can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, isScanRequest),
errors: getReceiptError(transaction.receipt, transaction.filename || transaction.receipt?.filename, isScanRequest, errorKey),
},
}),
},
Expand All @@ -783,7 +785,7 @@ function buildOnyxDataForMoneyRequest(
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReport.reportID}`,
value: {
[transactionThreadCreatedReportAction.reportActionID]: {
errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericCreateFailureMessage'),
errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericCreateFailureMessage', false, errorKey),
},
},
},
Expand Down Expand Up @@ -3771,6 +3773,8 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor
});
}

const errorKey = DateUtils.getMicroseconds();

failureData.push(
{
onyxMethod: Onyx.METHOD.MERGE,
Expand All @@ -3779,7 +3783,9 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor
[reportAction.reportActionID]: {
...reportAction,
pendingAction: null,
errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericDeleteFailureMessage'),
errors: {
[errorKey]: ['iou.error.genericDeleteFailureMessage', {isTranslated: false}],
},
},
},
},
Expand All @@ -3801,7 +3807,9 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor
[reportPreviewAction?.reportActionID ?? '']: {
...reportPreviewAction,
pendingAction: null,
errors: ErrorUtils.getMicroSecondOnyxError('iou.error.genericDeleteFailureMessage'),
errors: {
[errorKey]: ['iou.error.genericDeleteFailureMessage', {isTranslated: false}],
},
},
},
},
Expand Down
51 changes: 48 additions & 3 deletions src/libs/actions/ReportActions.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import Onyx from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import * as ReportActionUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type ReportAction from '@src/types/onyx/ReportAction';
import * as Report from './Report';

function clearReportActionErrors(reportID: string, reportAction: OnyxEntry<ReportAction>) {
type IgnoreDirection = 'parent' | 'child';

function clearReportActionErrors(reportID: string, reportAction: ReportAction, keys?: string[]) {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);

if (!reportAction?.reportActionID) {
Expand All @@ -34,14 +35,58 @@ function clearReportActionErrors(reportID: string, reportAction: OnyxEntry<Repor
return;
}

if (keys) {
const errors: Record<string, null> = {};

keys.forEach((key) => {
errors[key] = null;
});

Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`, {
[reportAction.reportActionID]: {
errors,
},
});
return;
}
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`, {
[reportAction.reportActionID]: {
errors: null,
},
});
}

/**
*
ignore: `undefined` means we want to check both parent and children report actions
ignore: `parent` or `child` means we want to ignore checking parent or child report actions because they've been previously checked
*/
function clearAllRelatedReportActionErrors(reportID: string, reportAction: ReportAction | null, ignore?: IgnoreDirection, keys?: string[]) {
const errorKeys = keys ?? Object.keys(reportAction?.errors ?? {});
if (!reportAction || errorKeys.length === 0) {
return;
}

clearReportActionErrors(reportID, reportAction, keys);

const report = ReportUtils.getReport(reportID);
if (report?.parentReportID && report?.parentReportActionID && ignore !== 'parent') {
const parentReportAction = ReportActionUtils.getReportAction(report.parentReportID, report.parentReportActionID);
const parentErrorKeys = Object.keys(parentReportAction?.errors ?? {}).filter((err) => errorKeys.includes(err));

clearAllRelatedReportActionErrors(report.parentReportID, parentReportAction, 'child', parentErrorKeys);
}

if (reportAction.childReportID && ignore !== 'child') {
const childActions = ReportActionUtils.getAllReportActions(reportAction.childReportID);
Object.values(childActions).forEach((action) => {
const childErrorKeys = Object.keys(action.errors ?? {}).filter((err) => errorKeys.includes(err));
clearAllRelatedReportActionErrors(reportAction.childReportID ?? '', action, 'parent', childErrorKeys);
});
}
}

export {
// eslint-disable-next-line import/prefer-default-export
clearReportActionErrors,
clearAllRelatedReportActionErrors,
};
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ function ReportActionItem({
/>
<View style={StyleUtils.getReportActionItemStyle(hovered || isWhisper || isContextMenuActive || !!isEmojiPickerActive || draftMessage !== undefined)}>
<OfflineWithFeedback
onClose={() => ReportActions.clearReportActionErrors(report.reportID, action)}
onClose={() => ReportActions.clearAllRelatedReportActionErrors(report.reportID, action)}
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
pendingAction={
draftMessage !== undefined ? undefined : action.pendingAction ?? (action.isOptimisticAction ? CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD : undefined)
Expand Down
38 changes: 35 additions & 3 deletions tests/actions/IOUTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,13 +786,12 @@ describe('actions/IOU', () => {
.then(
() =>
new Promise<void>((resolve) => {
ReportActions.clearReportActionErrors(iouReportID ?? '', iouAction);
ReportActions.clearReportActionErrors(transactionThreadReport?.reportID ?? '', transactionThreadAction);
ReportActions.clearAllRelatedReportActionErrors(iouReportID ?? '', iouAction);
resolve();
}),
)

// Then the reportAction should be removed from Onyx
// Then the reportAction from chat report should be removed from Onyx
.then(
() =>
new Promise<void>((resolve) => {
Expand All @@ -809,6 +808,39 @@ describe('actions/IOU', () => {
}),
)

// Then the reportAction from iou report should be removed from Onyx
.then(
() =>
new Promise<void>((resolve) => {
const connectionID = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReportID}`,
waitForCollectionCallback: false,
callback: (reportActionsForReport) => {
Onyx.disconnect(connectionID);
iouAction = Object.values(reportActionsForReport ?? {}).find((reportAction) => reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU) ?? null;
expect(iouAction).toBeFalsy();
resolve();
},
});
}),
)

// Then the reportAction from transaction report should be removed from Onyx
.then(
() =>
new Promise<void>((resolve) => {
const connectionID = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transactionThreadReport?.reportID}`,
waitForCollectionCallback: false,
callback: (reportActionsForReport) => {
Onyx.disconnect(connectionID);
expect(reportActionsForReport).toMatchObject({});
resolve();
},
});
}),
)

// Along with the associated transaction
.then(
() =>
Expand Down

0 comments on commit b3baa68

Please sign in to comment.