From 63639634a441e14888f61d49858a08164383e41f Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 5 Jul 2024 23:41:06 +0800 Subject: [PATCH 01/12] delete all linked report when clearing optimistic chat and transaction error --- .../ReportActionItem/MoneyRequestView.tsx | 14 +++++++++----- src/libs/ReportUtils.ts | 3 ++- src/libs/actions/Report.ts | 13 +++++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index a0d73a1b2844..4301b6e7c340 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -40,6 +40,7 @@ import * as Link from '@userActions/Link'; import * as Transaction from '@userActions/Transaction'; import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; +import * as Report from '@src/libs/actions/Report'; import * as ReportActions from '@src/libs/actions/ReportActions'; import ONYXKEYS from '@src/ONYXKEYS'; import ROUTES from '@src/ROUTES'; @@ -404,11 +405,14 @@ function MoneyRequestView({ if (!transaction?.transactionID) { return; } - if ( - transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && - Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error)) - ) { - deleteTransaction(parentReport, parentReportAction); + if (transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { + if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) { + deleteTransaction(parentReport, parentReportAction); + } + const chatReportID = parentReport?.parentReportID ?? ''; + if (ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReportID)) { + Report.navigateToConciergeChatAndDeleteReport(chatReportID, true); + } } Transaction.clearError(transaction.transactionID); ReportActions.clearAllRelatedReportActionErrors(report.reportID, parentReportAction); diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 342e2439ed66..955bbe18a2a3 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -6066,7 +6066,8 @@ function isValidReportIDFromPath(reportIDFromPath: string): boolean { /** * Return the errors we have when creating a chat or a workspace room */ -function getAddWorkspaceRoomOrChatReportErrors(report: OnyxEntry): Errors | null | undefined { +function getAddWorkspaceRoomOrChatReportErrors(reportOrID: OnyxEntry | string): Errors | null | undefined { + const report = typeof reportOrID === 'string' ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] : reportOrID; // We are either adding a workspace room, or we're creating a chat, it isn't possible for both of these to have errors for the same report at the same time, so // simply looking up the first truthy value will get the relevant property if it's set. return report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat; diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index a1a52b64d49f..c1157a6c4f53 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2189,6 +2189,12 @@ function deleteReport(reportID: string) { Onyx.multiSet(onyxData); + Object.values(reportActionsForReport ?? {}).forEach((reportAction) => { + if (reportAction.childReportID) { + deleteReport(reportAction.childReportID); + } + }); + // Delete linked IOU report if (report?.iouReportID) { deleteReport(report.iouReportID); @@ -2198,9 +2204,12 @@ function deleteReport(reportID: string) { /** * @param reportID The reportID of the policy report (workspace room) */ -function navigateToConciergeChatAndDeleteReport(reportID: string) { +function navigateToConciergeChatAndDeleteReport(reportID: string, shouldPopToTop?: boolean) { // Dismiss the current report screen and replace it with Concierge Chat - Navigation.goBack(); + if (shouldPopToTop) { + Navigation.setShouldPopAllStateOnUP(); + } + Navigation.goBack(undefined, undefined, shouldPopToTop); navigateToConciergeChat(); deleteReport(reportID); } From af16ea635ba314c4687099198509408793976fcb Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 10 Jul 2024 11:39:35 +0800 Subject: [PATCH 02/12] lint --- src/libs/actions/Report.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index f9b38de4f54c..2147f2971f72 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2169,9 +2169,10 @@ function deleteReport(reportID: string) { Onyx.multiSet(onyxData); Object.values(reportActionsForReport ?? {}).forEach((reportAction) => { - if (reportAction.childReportID) { - deleteReport(reportAction.childReportID); + if (!reportAction.childReportID) { + return; } + deleteReport(reportAction.childReportID); }); // Delete linked IOU report From 35cd311bbffa44a0b26a357eafa6e0fe4b030b87 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 10 Jul 2024 11:51:09 +0800 Subject: [PATCH 03/12] access the report from ReportConnection --- src/libs/ReportUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 13db0072c642..236a5486c8ef 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -6107,7 +6107,7 @@ function isValidReportIDFromPath(reportIDFromPath: string): boolean { * Return the errors we have when creating a chat or a workspace room */ function getAddWorkspaceRoomOrChatReportErrors(reportOrID: OnyxEntry | string): Errors | null | undefined { - const report = typeof reportOrID === 'string' ? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] : reportOrID; + const report = typeof reportOrID === 'string' ? ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] : reportOrID; // We are either adding a workspace room, or we're creating a chat, it isn't possible for both of these to have errors for the same report at the same time, so // simply looking up the first truthy value will get the relevant property if it's set. return report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat; From 51cf9e9fb933187ab2009ad9b2992f072022ae9b Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 10 Jul 2024 11:56:25 +0800 Subject: [PATCH 04/12] pass missing argument --- src/libs/actions/Report.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 2147f2971f72..50146c0368d9 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2187,7 +2187,7 @@ function deleteReport(reportID: string) { function navigateToConciergeChatAndDeleteReport(reportID: string, shouldPopToTop?: boolean) { // Dismiss the current report screen and replace it with Concierge Chat if (shouldPopToTop) { - Navigation.setShouldPopAllStateOnUP(); + Navigation.setShouldPopAllStateOnUP(true); } Navigation.goBack(undefined, undefined, shouldPopToTop); navigateToConciergeChat(); From 1a0c0c3b10ff4195243d75929e7b9ba265fc7ac6 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 11 Jul 2024 11:05:02 +0800 Subject: [PATCH 05/12] early return when deleting the error report --- src/components/ReportActionItem/MoneyRequestView.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 4301b6e7c340..93068048b761 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -406,12 +406,13 @@ function MoneyRequestView({ return; } if (transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { - if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) { - deleteTransaction(parentReport, parentReportAction); - } const chatReportID = parentReport?.parentReportID ?? ''; if (ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReportID)) { Report.navigateToConciergeChatAndDeleteReport(chatReportID, true); + return; + } + if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) { + deleteTransaction(parentReport, parentReportAction); } } Transaction.clearError(transaction.transactionID); From 14aceb5ea22294d528b42f929700581e290b641f Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 11 Jul 2024 11:16:13 +0800 Subject: [PATCH 06/12] filter out falsey value --- src/libs/TransactionUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 89de7d1e55ec..8c78254e8aab 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -581,7 +581,7 @@ function getAllReportTransactions(reportID?: string, transactions?: OnyxCollecti // `reportID` from the `/CreateDistanceRequest` endpoint return's number instead of string for created `transaction`. // For reference, https://github.com/Expensify/App/pull/26536#issuecomment-1703573277. // We will update this in a follow-up Issue. According to this comment: https://github.com/Expensify/App/pull/26536#issuecomment-1703591019. - const nonNullableTransactions: Transaction[] = Object.values(transactions ?? allTransactions ?? {}).filter((transaction): transaction is Transaction => transaction !== null); + const nonNullableTransactions = Object.values(transactions ?? allTransactions ?? {}).filter(transaction => !!transaction); return nonNullableTransactions.filter((transaction) => `${transaction.reportID}` === `${reportID}`); } From 360f07c8eec4dc01287c731cd707de8ab9156ac0 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 11 Jul 2024 11:20:29 +0800 Subject: [PATCH 07/12] prettier --- src/libs/TransactionUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 8c78254e8aab..3e1c790913e9 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -581,7 +581,7 @@ function getAllReportTransactions(reportID?: string, transactions?: OnyxCollecti // `reportID` from the `/CreateDistanceRequest` endpoint return's number instead of string for created `transaction`. // For reference, https://github.com/Expensify/App/pull/26536#issuecomment-1703573277. // We will update this in a follow-up Issue. According to this comment: https://github.com/Expensify/App/pull/26536#issuecomment-1703591019. - const nonNullableTransactions = Object.values(transactions ?? allTransactions ?? {}).filter(transaction => !!transaction); + const nonNullableTransactions = Object.values(transactions ?? allTransactions ?? {}).filter((transaction) => !!transaction); return nonNullableTransactions.filter((transaction) => `${transaction.reportID}` === `${reportID}`); } From 5fd50f09cb363f06397e584b50fe6c08dad32e99 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 11 Jul 2024 11:25:58 +0800 Subject: [PATCH 08/12] fix typing --- src/libs/TransactionUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 3e1c790913e9..69b085d054ff 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -582,7 +582,7 @@ function getAllReportTransactions(reportID?: string, transactions?: OnyxCollecti // For reference, https://github.com/Expensify/App/pull/26536#issuecomment-1703573277. // We will update this in a follow-up Issue. According to this comment: https://github.com/Expensify/App/pull/26536#issuecomment-1703591019. const nonNullableTransactions = Object.values(transactions ?? allTransactions ?? {}).filter((transaction) => !!transaction); - return nonNullableTransactions.filter((transaction) => `${transaction.reportID}` === `${reportID}`); + return nonNullableTransactions.filter((transaction) => `${transaction?.reportID}` === `${reportID}`); } function waypointHasValidAddress(waypoint: RecentWaypoint | Waypoint): boolean { From fccb7cb48a318567842ff5f6d6628a4cc265c25a Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 11 Jul 2024 11:38:35 +0800 Subject: [PATCH 09/12] fix typing --- src/libs/TransactionUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libs/TransactionUtils.ts b/src/libs/TransactionUtils.ts index 69b085d054ff..473cd47b9939 100644 --- a/src/libs/TransactionUtils.ts +++ b/src/libs/TransactionUtils.ts @@ -581,8 +581,8 @@ function getAllReportTransactions(reportID?: string, transactions?: OnyxCollecti // `reportID` from the `/CreateDistanceRequest` endpoint return's number instead of string for created `transaction`. // For reference, https://github.com/Expensify/App/pull/26536#issuecomment-1703573277. // We will update this in a follow-up Issue. According to this comment: https://github.com/Expensify/App/pull/26536#issuecomment-1703591019. - const nonNullableTransactions = Object.values(transactions ?? allTransactions ?? {}).filter((transaction) => !!transaction); - return nonNullableTransactions.filter((transaction) => `${transaction?.reportID}` === `${reportID}`); + const nonNullableTransactions: Transaction[] = Object.values(transactions ?? allTransactions ?? {}).filter((transaction): transaction is Transaction => !!transaction); + return nonNullableTransactions.filter((transaction) => `${transaction.reportID}` === `${reportID}`); } function waypointHasValidAddress(waypoint: RecentWaypoint | Waypoint): boolean { From 81241ac24f6e3723b85cc654c81fb4adf1a83550 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 13 Jul 2024 10:20:03 +0800 Subject: [PATCH 10/12] only accept report object --- src/libs/ReportUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index fccd44fe46d5..f7dca71fd15f 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -6091,8 +6091,7 @@ function isValidReportIDFromPath(reportIDFromPath: string): boolean { /** * Return the errors we have when creating a chat or a workspace room */ -function getAddWorkspaceRoomOrChatReportErrors(reportOrID: OnyxEntry | string): Errors | null | undefined { - const report = typeof reportOrID === 'string' ? ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportOrID}`] : reportOrID; +function getAddWorkspaceRoomOrChatReportErrors(report: OnyxEntry): Errors | null | undefined { // We are either adding a workspace room, or we're creating a chat, it isn't possible for both of these to have errors for the same report at the same time, so // simply looking up the first truthy value will get the relevant property if it's set. return report?.errorFields?.addWorkspaceRoom ?? report?.errorFields?.createChat; From ba952923288dff0461303adab3152240211b24f3 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 13 Jul 2024 10:26:28 +0800 Subject: [PATCH 11/12] make delete all child report optional and pass the chat report instead of its id --- .../ReportActionItem/MoneyRequestView.tsx | 8 ++++--- src/libs/actions/Report.ts | 22 ++++++++++--------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index 4073aa3df846..c39887666645 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -116,6 +116,9 @@ function MoneyRequestView({ const {isOffline} = useNetwork(); const {translate, toLocaleDigit} = useLocalize(); const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID); + const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReport?.parentReportID}`, { + selector: (report) => report && {reportID: report.reportID, errorFields: report.errorFields}, + }); const parentReportAction = parentReportActions?.[report.parentReportActionID ?? '-1']; const isTrackExpense = ReportUtils.isTrackExpenseReport(report); @@ -405,9 +408,8 @@ function MoneyRequestView({ return; } if (transaction.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { - const chatReportID = parentReport?.parentReportID ?? ''; - if (ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReportID)) { - Report.navigateToConciergeChatAndDeleteReport(chatReportID, true); + if (chatReport?.reportID && ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReport)) { + Report.navigateToConciergeChatAndDeleteReport(chatReport.reportID, true, true); return; } if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) { diff --git a/src/libs/actions/Report.ts b/src/libs/actions/Report.ts index 50de3f519ce1..0b603416b72d 100644 --- a/src/libs/actions/Report.ts +++ b/src/libs/actions/Report.ts @@ -2145,7 +2145,7 @@ function addPolicyReport(policyReport: ReportUtils.OptimisticChatReport) { } /** Deletes a report, along with its reportActions, any linked reports, and any linked IOU report. */ -function deleteReport(reportID: string) { +function deleteReport(reportID: string, shouldDeleteChildReports = false) { const report = ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; const onyxData: Record = { [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]: null, @@ -2165,30 +2165,32 @@ function deleteReport(reportID: string) { Onyx.multiSet(onyxData); - Object.values(reportActionsForReport ?? {}).forEach((reportAction) => { - if (!reportAction.childReportID) { - return; - } - deleteReport(reportAction.childReportID); - }); + if (shouldDeleteChildReports) { + Object.values(reportActionsForReport ?? {}).forEach((reportAction) => { + if (!reportAction.childReportID) { + return; + } + deleteReport(reportAction.childReportID, shouldDeleteChildReports); + }); + } // Delete linked IOU report if (report?.iouReportID) { - deleteReport(report.iouReportID); + deleteReport(report.iouReportID, shouldDeleteChildReports); } } /** * @param reportID The reportID of the policy report (workspace room) */ -function navigateToConciergeChatAndDeleteReport(reportID: string, shouldPopToTop?: boolean) { +function navigateToConciergeChatAndDeleteReport(reportID: string, shouldPopToTop = false, shouldDeleteChildReports = false) { // Dismiss the current report screen and replace it with Concierge Chat if (shouldPopToTop) { Navigation.setShouldPopAllStateOnUP(true); } Navigation.goBack(undefined, undefined, shouldPopToTop); navigateToConciergeChat(); - deleteReport(reportID); + deleteReport(reportID, shouldDeleteChildReports); } /** From 1e6c518ba61ae52b1ef4b6ea3ca6e5c63acab3a7 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Sat, 13 Jul 2024 10:47:05 +0800 Subject: [PATCH 12/12] lint --- src/components/ReportActionItem/MoneyRequestView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/ReportActionItem/MoneyRequestView.tsx b/src/components/ReportActionItem/MoneyRequestView.tsx index c39887666645..29c480dcce7d 100644 --- a/src/components/ReportActionItem/MoneyRequestView.tsx +++ b/src/components/ReportActionItem/MoneyRequestView.tsx @@ -117,7 +117,7 @@ function MoneyRequestView({ const {translate, toLocaleDigit} = useLocalize(); const [activePolicyID] = useOnyx(ONYXKEYS.NVP_ACTIVE_POLICY_ID); const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReport?.parentReportID}`, { - selector: (report) => report && {reportID: report.reportID, errorFields: report.errorFields}, + selector: (chatReportValue) => chatReportValue && {reportID: chatReportValue.reportID, errorFields: chatReportValue.errorFields}, }); const parentReportAction = parentReportActions?.[report.parentReportActionID ?? '-1'];