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.

Sign up for GitHub

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

Delete all linked report when clearing optimistic chat and transaction error #44923

Merged
merged 15 commits into from
Jul 15, 2024
Merged

Delete all linked report when clearing optimistic chat and transaction error #44923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
6363963
delete all linked report when clearing optimistic chat and transactio…
bernhardoj Jul 5, 2024
0925b52
Merge branch 'main' into fix/43481-delete-report-when-error
bernhardoj Jul 10, 2024
af16ea6
lint
bernhardoj Jul 10, 2024
35cd311
access the report from ReportConnection
bernhardoj Jul 10, 2024
51cf9e9
pass missing argument
bernhardoj Jul 10, 2024
4e0171d
Merge branch 'main' into fix/43481-delete-report-when-error
bernhardoj Jul 11, 2024
1a0c0c3
early return when deleting the error report
bernhardoj Jul 11, 2024
14aceb5
filter out falsey value
bernhardoj Jul 11, 2024
360f07c
prettier
bernhardoj Jul 11, 2024
5fd50f0
fix typing
bernhardoj Jul 11, 2024
fccb7cb
fix typing
bernhardoj Jul 11, 2024
4b97867
Merge branch 'main' into fix/43481-delete-report-when-error
bernhardoj Jul 13, 2024
81241ac
only accept report object
bernhardoj Jul 13, 2024
ba95292
make delete all child report optional and pass the chat report instea…
bernhardoj Jul 13, 2024
1e6c518
lint
bernhardoj Jul 13, 2024
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
17 changes: 12 additions & 5 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -115,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: (chatReportValue) => chatReportValue && {reportID: chatReportValue.reportID, errorFields: chatReportValue.errorFields},
});

const parentReportAction = parentReportActions?.[report.parentReportActionID ?? '-1'];
const isTrackExpense = ReportUtils.isTrackExpenseReport(report);
Expand Down Expand Up @@ -403,11 +407,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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let us do an early return here to keep possible regressions at bay. Also, I think doing any further steps does not make sense when the chat report and all related reports are deleted. Something like this can be done within the if condition here. What do you think?

if (ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReportID)) {
   Report.navigateToConciergeChatAndDeleteReport(chatReportID, true);
   return;
}
if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) {
   deleteTransaction(parentReport, parentReportAction);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, updated!

if (chatReport?.reportID && ReportUtils.getAddWorkspaceRoomOrChatReportErrors(chatReport)) {
Report.navigateToConciergeChatAndDeleteReport(chatReport.reportID, true, true);
return;
}
if (Object.values(transaction?.errors ?? {})?.find((error) => ErrorUtils.isReceiptError(error))) {
deleteTransaction(parentReport, parentReportAction);
}
}
Transaction.clearError(transaction.transactionID);
ReportActions.clearAllRelatedReportActionErrors(report.reportID, parentReportAction);
Expand Down
2 changes: 1 addition & 1 deletion src/libs/TransactionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: Transaction[] = Object.values(transactions ?? allTransactions ?? {}).filter((transaction): transaction is Transaction => !!transaction);
return nonNullableTransactions.filter((transaction) => `${transaction.reportID}` === `${reportID}`);
}

Expand Down
22 changes: 17 additions & 5 deletions src/libs/actions/Report.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, null> = {
[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]: null,
Expand All @@ -2165,20 +2165,32 @@ function deleteReport(reportID: string) {

Onyx.multiSet(onyxData);

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) {
function navigateToConciergeChatAndDeleteReport(reportID: string, shouldPopToTop = false, shouldDeleteChildReports = false) {
// Dismiss the current report screen and replace it with Concierge Chat
Navigation.goBack();
if (shouldPopToTop) {
Navigation.setShouldPopAllStateOnUP(true);
}
Navigation.goBack(undefined, undefined, shouldPopToTop);
navigateToConciergeChat();
deleteReport(reportID);
deleteReport(reportID, shouldDeleteChildReports);
}

/**
Expand Down
Loading