Skip to content

Stop creating the transaction thread reports optimistically for RequestMoney #62080

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,7 @@ const CONST = {
EMPTY_ARRAY,
EMPTY_OBJECT,
DEFAULT_NUMBER_ID,
FAKE_REPORT_ID: 'FAKE_REPORT_ID',
USE_EXPENSIFY_URL,
EXPENSIFY_URL,
GOOGLE_MEET_URL_ANDROID: 'https://meet.google.com',
Expand Down
34 changes: 29 additions & 5 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ const ROUTES = {
backTo,
moneyRequestReportActionID,
transactionID,
iouReportID,
}: {
reportID: string | undefined;
reportActionID?: string;
backTo?: string;
moneyRequestReportActionID?: string;
transactionID?: string;
iouReportID?: string;
}) => {
if (!reportID) {
Log.warn('Invalid reportID is used to build the SEARCH_REPORT route');
Expand All @@ -105,6 +107,10 @@ const ROUTES = {
queryParams.push(`moneyRequestReportActionID=${moneyRequestReportActionID}`);
}

if (iouReportID) {
queryParams.push(`iouReportID=${iouReportID}`);
}

const queryString = queryParams.length > 0 ? (`${baseRoute}?${queryParams.join('&')}` as const) : baseRoute;
return getUrlWithBackToParam(queryString, backTo);
},
Expand Down Expand Up @@ -376,10 +382,19 @@ const ROUTES = {
REPORT: 'r',
REPORT_WITH_ID: {
route: 'r/:reportID?/:reportActionID?',
getRoute: (reportID: string | undefined, reportActionID?: string, referrer?: string, moneyRequestReportActionID?: string, transactionID?: string, backTo?: string) => {
getRoute: (
reportID: string | undefined,
reportActionID?: string,
referrer?: string,
moneyRequestReportActionID?: string,
transactionID?: string,
backTo?: string,
iouReportID?: string,
) => {
if (!reportID) {
Log.warn('Invalid reportID is used to build the REPORT_WITH_ID route');
}

const baseRoute = reportActionID ? (`r/${reportID}/${reportActionID}` as const) : (`r/${reportID}` as const);

const queryParams: string[] = [];
Expand All @@ -393,6 +408,10 @@ const ROUTES = {
queryParams.push(`transactionID=${transactionID}`);
}

if (iouReportID) {
queryParams.push(`iouReportID=${iouReportID}`);
}

const queryString = queryParams.length > 0 ? `?${queryParams.join('&')}` : '';

return getUrlWithBackToParam(`${baseRoute}${queryString}` as const, backTo);
Expand Down Expand Up @@ -574,10 +593,15 @@ const ROUTES = {
},
MONEY_REQUEST_HOLD_REASON: {
route: ':type/edit/reason/:transactionID?/:searchHash?',
getRoute: (type: ValueOf<typeof CONST.POLICY.TYPE>, transactionID: string, reportID: string, backTo: string, searchHash?: number) => {
const route = searchHash
? (`${type as string}/edit/reason/${transactionID}/${searchHash}/?backTo=${backTo}&reportID=${reportID}` as const)
: (`${type as string}/edit/reason/${transactionID}/?backTo=${backTo}&reportID=${reportID}` as const);
getRoute: (type: ValueOf<typeof CONST.POLICY.TYPE>, transactionID: string, reportID: string | undefined, backTo: string, searchHash?: number) => {
let route = searchHash
? (`${type as string}/edit/reason/${transactionID}/${searchHash}/?backTo=${backTo}` as const)
: (`${type as string}/edit/reason/${transactionID}/?backTo=${backTo}` as const);

if (reportID) {
route = `${route}&reportID=${reportID}` as const;
}

return route;
},
},
Expand Down
15 changes: 11 additions & 4 deletions src/components/MoneyReportHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,18 @@ import useSelectedTransactionsActions from '@hooks/useSelectedTransactionsAction
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import {turnOffMobileSelectionMode} from '@libs/actions/MobileSelectionMode';
import {downloadReportPDF, exportReportToCSV, exportReportToPDF, exportToIntegration, markAsManuallyExported, openUnreportedExpense} from '@libs/actions/Report';
import {downloadReportPDF, exportReportToCSV, exportReportToPDF, exportToIntegration, markAsManuallyExported, openReport, openUnreportedExpense} from '@libs/actions/Report';
import {getThreadReportIDsForTransactions, getTotalAmountForIOUReportPreviewButton} from '@libs/MoneyRequestReportUtils';
import Navigation from '@libs/Navigation/Navigation';
import {buildOptimisticNextStepForPreventSelfApprovalsEnabled} from '@libs/NextStepUtils';
import {getConnectedIntegration} from '@libs/PolicyUtils';
import {getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {getIOUActionForReportID, getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {getReportPrimaryAction} from '@libs/ReportPrimaryActionUtils';
import {getSecondaryReportActions} from '@libs/ReportSecondaryActionUtils';
import {
buildTransactionThread,
changeMoneyRequestHoldStatus,
generateReportID,
getArchiveReason,
getBankAccountRoute,
getIntegrationIcon,
Expand Down Expand Up @@ -547,9 +549,14 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
success
text={translate('iou.reviewDuplicates')}
onPress={() => {
const threadID = transactionThreadReportID ?? getFirstDuplicateThreadID(transactions, reportActions);
let threadID = transactionThreadReportID ?? getFirstDuplicateThreadID(transactions, reportActions);
if (!threadID) {
return;
threadID = generateReportID();
const duplicateTransaction = transactions.find((reportTransaction) => isDuplicate(reportTransaction.transactionID));
const transactionID = duplicateTransaction?.transactionID;
const iouAction = getIOUActionForReportID(moneyRequestReport?.reportID, transactionID);
const optimisticTransactionThread = buildTransactionThread(iouAction, moneyRequestReport, undefined, threadID);
openReport(threadID, undefined, session?.email ? [session?.email] : [], optimisticTransactionThread, iouAction?.reportActionID);
}
Navigation.navigate(ROUTES.TRANSACTION_DUPLICATE_REVIEW_PAGE.getRoute(threadID));
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,17 @@ import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import {getThreadReportIDsForTransactions} from '@libs/MoneyRequestReportUtils';
import {navigationRef} from '@libs/Navigation/Navigation';
import {getIOUActionForTransactionID} from '@libs/ReportActionsUtils';
import {getMoneyRequestSpendBreakdown} from '@libs/ReportUtils';
import {generateReportID, getMoneyRequestSpendBreakdown} from '@libs/ReportUtils';
import {compareValues} from '@libs/SearchUIUtils';
import shouldShowTransactionYear from '@libs/TransactionUtils/shouldShowTransactionYear';
import Navigation from '@navigation/Navigation';
import type {ReportsSplitNavigatorParamList} from '@navigation/types';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import NAVIGATORS from '@src/NAVIGATORS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import {useMoneyRequestReportContext} from './MoneyRequestReportContext';
import MoneyRequestReportTableHeader from './MoneyRequestReportTableHeader';
Expand Down Expand Up @@ -74,6 +77,8 @@ type SortedTransactions = {
sortOrder: SortOrder;
};

type ReportScreenNavigationProps = ReportsSplitNavigatorParamList[typeof SCREENS.REPORT];

const isSortableColumnName = (key: unknown): key is SortableColumnName => !!sortableColumnNames.find((val) => val === key);

const getTransactionKey = (transaction: OnyxTypes.Transaction, key: SortableColumnName) => {
Expand Down Expand Up @@ -151,19 +156,27 @@ function MoneyRequestReportTransactionList({report, transactions, reportActions,
const navigateToTransaction = useCallback(
(activeTransaction: OnyxTypes.Transaction) => {
const iouAction = getIOUActionForTransactionID(reportActions, activeTransaction.transactionID);
const reportIDToNavigate = iouAction?.childReportID;
if (!reportIDToNavigate) {
return;
}
const reportIDToNavigate = iouAction?.childReportID ?? generateReportID();

const backTo = Navigation.getActiveRoute();
const backTo = Navigation.getActiveRoute() as Route;

// Single transaction report will open in RHP, and we need to find every other report ID for the rest of transactions
// to display prev/next arrows in RHP for navigating between transactions
const sortedSiblingTransactionReportIDs = getThreadReportIDsForTransactions(reportActions, sortedTransactions);
setActiveTransactionThreadIDs(sortedSiblingTransactionReportIDs);

Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: reportIDToNavigate, backTo}));
const routeParams = {
reportID: reportIDToNavigate,
backTo,
} as ReportScreenNavigationProps;

if (!iouAction?.childReportID) {
routeParams.moneyRequestReportActionID = iouAction?.reportActionID;
routeParams.transactionID = activeTransaction.transactionID;
routeParams.iouReportID = activeTransaction.reportID;
}

Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute(routeParams));
},
[reportActions, sortedTransactions],
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function MoneyRequestAction({
const transactionID = isMoneyRequestAction(action) ? getOriginalMessage(action)?.IOUTransactionID : CONST.DEFAULT_NUMBER_ID;
if (!action?.childReportID && transactionID && action.reportActionID) {
const optimisticReportID = generateReportID();
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(optimisticReportID, undefined, undefined, action.reportActionID, transactionID, Navigation.getActiveRoute()));
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(optimisticReportID, undefined, undefined, action.reportActionID, transactionID, Navigation.getActiveRoute(), requestReportID));
return;
}

Expand Down
39 changes: 22 additions & 17 deletions src/components/Search/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {canUseTouchScreen} from '@libs/DeviceCapabilities';
import Log from '@libs/Log';
import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
import type {PlatformStackNavigationProp} from '@libs/Navigation/PlatformStackNavigation/types';
import {getOriginalMessage, getReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {generateReportID} from '@libs/ReportUtils';
import {buildSearchQueryString} from '@libs/SearchQueryUtils';
import {
Expand Down Expand Up @@ -379,8 +380,28 @@ function Search({queryJSON, currentSearchResults, lastNonEmptySearchResults, onS

const isFromSelfDM = item.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
const isTransactionItem = isTransactionListItemType(item);
const backTo = Navigation.getActiveRoute();

// If we're trying to open a legacy transaction without a transaction thread, let's create the thread and navigate the user
if (isTransactionItem && item.transactionThreadReportID === CONST.REPORT.UNREPORTED_REPORT_ID) {
const transactionThreadReportID = generateReportID();
const reportAction = getReportAction(item.reportID, item.moneyRequestReportActionID);
const iouReportID = isMoneyRequestAction(reportAction) ? getOriginalMessage(reportAction)?.IOUReportID : undefined;

updateSearchResultsWithTransactionThreadReportID(hash, item.transactionID, transactionThreadReportID);
Navigation.navigate(
ROUTES.SEARCH_REPORT.getRoute({
reportID: transactionThreadReportID,
backTo,
moneyRequestReportActionID: item.moneyRequestReportActionID,
transactionID: item.transactionID,
iouReportID,
}),
);
return;
}

let reportID =
const reportID =
isTransactionItem && (!item.isFromOneTransactionReport || isFromSelfDM) && item.transactionThreadReportID !== CONST.REPORT.UNREPORTED_REPORT_ID
? item.transactionThreadReportID
: item.reportID;
Expand All @@ -389,29 +410,13 @@ function Search({queryJSON, currentSearchResults, lastNonEmptySearchResults, onS
return;
}

const backTo = Navigation.getActiveRoute();
const shouldHandleTransactionAsReport = isReportListItemType(item) || (isTransactionItem && isOpenedAsReport);

if (canUseTableReportView && shouldHandleTransactionAsReport) {
Navigation.navigate(ROUTES.SEARCH_MONEY_REQUEST_REPORT.getRoute({reportID, backTo}));
return;
}

// If we're trying to open a legacy transaction without a transaction thread, let's create the thread and navigate the user
if (isTransactionItem && reportID === CONST.REPORT.UNREPORTED_REPORT_ID) {
reportID = generateReportID();
updateSearchResultsWithTransactionThreadReportID(hash, item.transactionID, reportID);
Navigation.navigate(
ROUTES.SEARCH_REPORT.getRoute({
reportID,
backTo,
moneyRequestReportActionID: item.moneyRequestReportActionID,
transactionID: item.transactionID,
}),
);
return;
}

if (isReportActionListItemType(item)) {
const reportActionID = item.reportActionID;
Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID, reportActionID, backTo}));
Expand Down
2 changes: 1 addition & 1 deletion src/libs/API/parameters/CreatePerDiemRequestParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type CreatePerDiemRequestParams = {
createdChatReportActionID?: string;
createdIOUReportActionID?: string;
reportPreviewReportActionID: string;
transactionThreadReportID: string;
transactionThreadReportID?: string;
createdReportActionIDForThread: string | undefined;
billable?: boolean;
attendees?: string;
Expand Down
2 changes: 2 additions & 0 deletions src/libs/API/parameters/HoldMoneyRequestParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ type HoldMoneyRequestParams = {
comment: string;
reportActionID: string;
commentReportActionID: string;
transactionThreadReportID?: string;
createdReportActionIDForThread?: string;
};

export default HoldMoneyRequestParams;
2 changes: 2 additions & 0 deletions src/libs/API/parameters/MergeDuplicatesParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ type MergeDuplicatesParams = {
receiptID: number;
reportID: string | undefined;
reportActionID?: string | undefined;
transactionThreadReportID?: string;
createdReportActionIDForThread?: string;
};

export default MergeDuplicatesParams;
4 changes: 2 additions & 2 deletions src/libs/API/parameters/RequestMoneyParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ type RequestMoneyParams = {
taxAmount: number;
billable?: boolean;
receiptGpsPoints?: string;
transactionThreadReportID: string;
createdReportActionIDForThread: string | undefined;
transactionThreadReportID?: string;
createdReportActionIDForThread?: string | undefined;
reimbursible?: boolean;
description?: string;
attendees?: string;
Expand Down
1 change: 1 addition & 0 deletions src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1650,6 +1650,7 @@ type ReportsSplitNavigatorParamList = {
backTo?: Routes;
moneyRequestReportActionID?: string;
transactionID?: string;
iouReportID?: string;
};
};

Expand Down
5 changes: 2 additions & 3 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,6 @@ function getOneTransactionThreadReportID(
if (
actionType &&
iouRequestTypesSet.has(actionType) &&
action.childReportID &&
// Include deleted IOU reportActions if:
// - they have an associated IOU transaction ID or
// - they have visible childActions (like comments) that we'd want to display
Expand Down Expand Up @@ -1266,8 +1265,8 @@ function getOneTransactionThreadReportID(
return;
}

// Ensure we have a childReportID associated with the IOU report action
return singleAction?.childReportID;
// Since we don't always create transaction thread optimistically, we return CONST.FAKE_REPORT_ID
return singleAction?.childReportID ?? CONST.FAKE_REPORT_ID;
}

/**
Expand Down
Loading
Loading