Skip to content

[CP Staging] Revert "Stop creating the transaction thread reports optimistically for RequestMoney" #62024

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

Merged
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: 0 additions & 1 deletion src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,6 @@ 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: 5 additions & 29 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,12 @@ 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 @@ -107,10 +105,6 @@ 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 @@ -382,19 +376,10 @@ 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,
iouReportID?: string,
) => {
getRoute: (reportID: string | undefined, reportActionID?: string, referrer?: string, moneyRequestReportActionID?: string, transactionID?: string, backTo?: 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 @@ -408,10 +393,6 @@ 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 @@ -593,15 +574,10 @@ const ROUTES = {
},
MONEY_REQUEST_HOLD_REASON: {
route: ':type/edit/reason/:transactionID?/:searchHash?',
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;
}

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);
return route;
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@ import {convertToDisplayString} from '@libs/CurrencyUtils';
import {getThreadReportIDsForTransactions} from '@libs/MoneyRequestReportUtils';
import {navigationRef} from '@libs/Navigation/Navigation';
import {getIOUActionForTransactionID} from '@libs/ReportActionsUtils';
import {generateReportID, getMoneyRequestSpendBreakdown} from '@libs/ReportUtils';
import {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 @@ -75,8 +72,6 @@ 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 @@ -155,27 +150,19 @@ function MoneyRequestReportTransactionList({report, transactions, reportActions,
const navigateToTransaction = useCallback(
(activeTransaction: OnyxTypes.Transaction) => {
const iouAction = getIOUActionForTransactionID(reportActions, activeTransaction.transactionID);
const reportIDToNavigate = iouAction?.childReportID ?? generateReportID();
const reportIDToNavigate = iouAction?.childReportID;
if (!reportIDToNavigate) {
return;
}

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

// 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);

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));
Navigation.navigate(ROUTES.SEARCH_REPORT.getRoute({reportID: reportIDToNavigate, backTo}));
},
[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(), requestReportID));
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(optimisticReportID, undefined, undefined, action.reportActionID, transactionID, Navigation.getActiveRoute()));
return;
}

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: 0 additions & 2 deletions src/libs/API/parameters/HoldMoneyRequestParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ type HoldMoneyRequestParams = {
comment: string;
reportActionID: string;
commentReportActionID: string;
transactionThreadReportID?: string;
createdReportActionIDForThread?: string;
};

export default HoldMoneyRequestParams;
2 changes: 0 additions & 2 deletions src/libs/API/parameters/MergeDuplicatesParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ 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: 0 additions & 1 deletion src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1644,7 +1644,6 @@ type ReportsSplitNavigatorParamList = {
backTo?: Routes;
moneyRequestReportActionID?: string;
transactionID?: string;
iouReportID?: string;
};
};

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

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

/**
Expand Down
41 changes: 11 additions & 30 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,7 @@ type OptimisticModifiedExpenseReportAction = Pick<
| 'delegateAccountID'
> & {reportID?: string};

type BaseOptimisticMoneyRequestEntities = {
type OptimisticMoneyRequestEntities = {
iouReport: Report;
type: ValueOf<typeof CONST.IOU.REPORT_ACTION_TYPE>;
amount: number;
Expand All @@ -627,10 +627,6 @@ type BaseOptimisticMoneyRequestEntities = {
linkedTrackedExpenseReportAction?: ReportAction;
};

type OptimisticMoneyRequestEntities = BaseOptimisticMoneyRequestEntities & {shouldGenerateOptimisticTransactionThread?: boolean};
type OptimisticMoneyRequestEntitiesWithTransactionThreadFlag = BaseOptimisticMoneyRequestEntities & {shouldGenerateOptimisticTransactionThread: boolean};
type OptimisticMoneyRequestEntitiesWithoutTransactionThreadFlag = BaseOptimisticMoneyRequestEntities;

type OptimisticTaskReport = SetRequired<
Pick<
Report,
Expand Down Expand Up @@ -3998,16 +3994,16 @@ const changeMoneyRequestHoldStatus = (reportAction: OnyxEntry<ReportAction>, sea

const transactionID = getOriginalMessage(reportAction)?.IOUTransactionID;

if (!transactionID) {
Log.warn('Missing transactionID during the change of the money request hold status');
if (!transactionID || !reportAction.childReportID) {
Log.warn('Missing transactionID and reportAction.childReportID during the change of the money request hold status');
return;
}

const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`] ?? ({} as Transaction);
const isOnHold = isOnHoldTransactionUtils(transaction);
const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${moneyRequestReport.policyID}`] ?? null;

if (isOnHold && reportAction.childReportID) {
if (isOnHold) {
unholdRequest(transactionID, reportAction.childReportID, searchHash);
} else {
const activeRoute = encodeURIComponent(Navigation.getActiveRoute());
Expand Down Expand Up @@ -7348,7 +7344,6 @@ function buildTransactionThread(
reportAction: OnyxEntry<ReportAction | OptimisticIOUReportAction>,
moneyRequestReport: OnyxEntry<Report>,
existingTransactionThreadReportID?: string,
optimisticTransactionThreadReportID?: string,
): OptimisticChatReport {
const participantAccountIDs = [...new Set([currentUserAccountID, Number(reportAction?.actorAccountID)])].filter(Boolean) as number[];
const existingTransactionThreadReport = getReportOrDraftReport(existingTransactionThreadReportID);
Expand All @@ -7371,7 +7366,6 @@ function buildTransactionThread(
notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN,
parentReportActionID: reportAction?.reportActionID,
parentReportID: moneyRequestReport?.reportID,
optimisticReportID: optimisticTransactionThreadReportID,
});
}

Expand All @@ -7384,12 +7378,6 @@ function buildTransactionThread(
* 4. Transaction Thread linked to the IOU action via `parentReportActionID`
* 5. CREATED action for the Transaction Thread
*/
function buildOptimisticMoneyRequestEntities(
optimisticMoneyRequestEntities: OptimisticMoneyRequestEntitiesWithoutTransactionThreadFlag,
): [OptimisticCreatedReportAction, OptimisticCreatedReportAction, OptimisticIOUReportAction, OptimisticChatReport, OptimisticCreatedReportAction | null];
function buildOptimisticMoneyRequestEntities(
optimisticMoneyRequestEntities: OptimisticMoneyRequestEntitiesWithTransactionThreadFlag,
): [OptimisticCreatedReportAction, OptimisticCreatedReportAction, OptimisticIOUReportAction, OptimisticChatReport | undefined, OptimisticCreatedReportAction | null];
function buildOptimisticMoneyRequestEntities({
iouReport,
type,
Expand All @@ -7403,17 +7391,10 @@ function buildOptimisticMoneyRequestEntities({
isSettlingUp = false,
isSendMoneyFlow = false,
isOwnPolicyExpenseChat = false,
shouldGenerateOptimisticTransactionThread = true,
isPersonalTrackingExpense,
existingTransactionThreadReportID,
linkedTrackedExpenseReportAction,
}: OptimisticMoneyRequestEntities): [
OptimisticCreatedReportAction,
OptimisticCreatedReportAction,
OptimisticIOUReportAction,
OptimisticChatReport | undefined,
OptimisticCreatedReportAction | null,
] {
}: OptimisticMoneyRequestEntities): [OptimisticCreatedReportAction, OptimisticCreatedReportAction, OptimisticIOUReportAction, OptimisticChatReport, OptimisticCreatedReportAction | null] {
const createdActionForChat = buildOptimisticCreatedReportAction(payeeEmail);

// The `CREATED` action must be optimistically generated before the IOU action so that it won't appear after the IOU action in the chat.
Expand All @@ -7438,11 +7419,11 @@ function buildOptimisticMoneyRequestEntities({
});

// Create optimistic transactionThread and the `CREATED` action for it, if existingTransactionThreadReportID is undefined
const transactionThread = shouldGenerateOptimisticTransactionThread ? buildTransactionThread(iouAction, iouReport, existingTransactionThreadReportID) : undefined;
const createdActionForTransactionThread = !!existingTransactionThreadReportID || !shouldGenerateOptimisticTransactionThread ? null : buildOptimisticCreatedReportAction(payeeEmail);
const transactionThread = buildTransactionThread(iouAction, iouReport, existingTransactionThreadReportID);
const createdActionForTransactionThread = existingTransactionThreadReportID ? null : buildOptimisticCreatedReportAction(payeeEmail);

// The IOU action and the transactionThread are co-dependent as parent-child, so we need to link them together
iouAction.childReportID = existingTransactionThreadReportID ?? transactionThread?.reportID;
iouAction.childReportID = existingTransactionThreadReportID ?? transactionThread.reportID;

return [createdActionForChat, createdActionForIOUReport, iouAction, transactionThread, createdActionForTransactionThread];
}
Expand Down Expand Up @@ -9231,12 +9212,12 @@ function getAllAncestorReportActionIDs(report: Report | null | undefined, includ

/**
* Get optimistic data of parent report action
* @param reportOrID The reportID of the report that is updated or the optimistic report on its own
* @param reportID The reportID of the report that is updated
* @param lastVisibleActionCreated Last visible action created of the child report
* @param type The type of action in the child report
*/
function getOptimisticDataForParentReportAction(reportOrID: Report | string | undefined, lastVisibleActionCreated: string, type: string): Array<OnyxUpdate | null> {
const report = typeof reportOrID === 'string' ? getReportOrDraftReport(reportOrID) : reportOrID;
function getOptimisticDataForParentReportAction(reportID: string | undefined, lastVisibleActionCreated: string, type: string): Array<OnyxUpdate | null> {
const report = getReportOrDraftReport(reportID);

if (!report || isEmptyObject(report)) {
return [];
Expand Down
Loading
Loading