Skip to content

fix: prevent Move expense from appearing on forwarded reports #83146

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

Open
mavrickdeveloper wants to merge 20 commits into
base: main
Choose a base branch
from
Open

fix: prevent Move expense from appearing on forwarded reports #83146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3396f8d
fix: prevent Move expense from appearing on forwarded reports
mavrickdeveloper Feb 21, 2026
42ba7bd
fix(report): block move expense for forwarded reports
mavrickdeveloper Mar 2, 2026
aa7fc53
Fix forwarded move gating when metadata is missing
mavrickdeveloper Mar 6, 2026
9858fd0
Fix move-expense forwarding fallback with workflow history
mavrickdeveloper Mar 8, 2026
7ba659f
fix(search): align bulk move-to-report gating with route validation
mavrickdeveloper Mar 9, 2026
ad73acd
chore(search): remove unused selection validation type exports
mavrickdeveloper Mar 9, 2026
3c13bf7
Merge upstream/main into fix/79602-move-expense-forwarded-reports (co…
mavrickdeveloper Apr 1, 2026
922cb3c
Hide Move to report for forwarded-like DEW expenses
mavrickdeveloper Apr 2, 2026
884406b
Fix CI test helper calls and workspace fixture typing
mavrickdeveloper Apr 2, 2026
4c38602
fix(search): block forwarded move-to-report from snapshot state
mavrickdeveloper Apr 3, 2026
083b5a4
Merge remote-tracking branch 'upstream/main' into fix/79602-move-expe…
mavrickdeveloper Apr 6, 2026
cb25709
fix(search): revert move-to-report route guard
mavrickdeveloper Apr 7, 2026
474dc73
Merge remote-tracking branch 'upstream/main' into fix/79602-move-expe…
mavrickdeveloper Apr 7, 2026
ac2eb6f
revert: remove report metadata cache sync handling
mavrickdeveloper Apr 10, 2026
d72e2e2
Merge remote-tracking branch 'upstream/main' into fix/79602-move-expe…
mavrickdeveloper Apr 10, 2026
d1a9149
chore: restore main hook dependency shape
mavrickdeveloper Apr 10, 2026
7405c95
docs: clarify forwarded move guards
mavrickdeveloper Apr 25, 2026
e91470f
fix: centralize forwarded move expense eligibility
mavrickdeveloper Apr 26, 2026
2f201b7
Merge remote-tracking branch 'upstream/main' into fix/79602-move-expe…
mavrickdeveloper Apr 26, 2026
9b09039
Consolidate forwarded expense move checks
mavrickdeveloper Apr 29, 2026
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
44 changes: 44 additions & 0 deletions src/components/Search/SearchSelectionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import type {SelectedTransactions} from './types';

type SearchMoveSelectionValidationParams = {
isExpenseReportSearch?: boolean;
getOwnerAccountIDForReportID?: (reportID?: string) => number | undefined;
};

type SearchMoveSelectionValidation = {
canMoveToReport: boolean;
};

function getSearchMoveSelectionValidation(
Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Mar 12, 2026

Choose a reason for hiding this comment

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

@mavrickdeveloper why do we need this function?

Copy link
Copy Markdown
Contributor Author

@mavrickdeveloper mavrickdeveloper Mar 12, 2026

Choose a reason for hiding this comment

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

So i needed getSearchMoveSelectionValidation() because this PR also fixes bulk-move menu/route drift.
During testing, i found the bulk menu and the RHP were applying different checks, so preserved/direct navigation could still expose Move for invalid selections. This helper centralizes the move-selection contract so both surfaces enforce the same rules , only transaction rows, every item must have canChangeReport, all items must belong to one owner, and the search must not be an expense-report search , thoughts?

Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Mar 12, 2026

Choose a reason for hiding this comment

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

I appreciate your effort here. Do you have a recording to visualize this issue, " this PR also fixes bulk-move menu/route drift."? TY

Copy link
Copy Markdown
Contributor Author

@mavrickdeveloper mavrickdeveloper Apr 3, 2026

Choose a reason for hiding this comment

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

here you go , lmk if anything is unclear

2026-04-03.15-13-07.mov

Copy link
Copy Markdown
Contributor Author

@mavrickdeveloper mavrickdeveloper Apr 3, 2026

Choose a reason for hiding this comment

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

Our PR state for Bulk select & route drift :

2026-04-03.16-12-35.mp4

Copy link
Copy Markdown
Contributor Author

@mavrickdeveloper mavrickdeveloper Apr 6, 2026

Choose a reason for hiding this comment

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

@hoangzinh can you please confirm if we should revert the bulk/snapshot fix because that mismatch is mentioned in the issue OP in the actual result (Appears to fail when the BE provides a response to acknowledge that expenses can't be moved off an approved report.)
so the wrong move appearance in bulk select is actually a reported issue , but the route drift is not
lmk what you think

Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

so the wrong move appearance in bulk select is actually a reported issue , but the route drift is not
lmk what you think

Can you elaborate on it, please? The expectation of the original issue is that we shouldn't show "Moving report" in bulk select. Route drift (or direct access) is not what a normal user will do.

Copy link
Copy Markdown
Contributor Author

@mavrickdeveloper mavrickdeveloper Apr 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@hoangzinh yes then we should keep the bulk fix , my last commit fixes 2 part , Move action appearance in action button before BE hydration + route drift

we should revert the route drift issue only , because it is not explicitly mentioned in the OP , but we should keep the bulk menu fix , this :

2026-04-06.16-01-43.mp4

if you look closely staging is wrongly showing move report vs our fix which is correctly blocking it before and after hydration which is included in the issue expected/actual behavior

We should preserve this fix behavior , we are on same page right ?

Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Apr 6, 2026

Choose a reason for hiding this comment

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

ah yes, we should only revert the route drift issue.

Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Apr 7, 2026

Choose a reason for hiding this comment

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

@mavrickdeveloper just wanna confirm, after revert, will we only have code related to your proposal here? #79602 (comment)

selectedTransactions: SelectedTransactions,
{isExpenseReportSearch = false, getOwnerAccountIDForReportID}: SearchMoveSelectionValidationParams = {},
): SearchMoveSelectionValidation {
const selectedTransactionEntries = Object.values(selectedTransactions);
const hasSelections = selectedTransactionEntries.length > 0;
const hasOnlyTransactionSelections = selectedTransactionEntries.every((transaction) => transaction.transaction?.transactionID !== undefined);
Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Apr 28, 2026

Choose a reason for hiding this comment

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

The old logic doesn't have to check this hasOnlyTransactionSelections. Can you explain why do we need to check it in this new util?

const canAllTransactionsBeMoved = hasSelections && selectedTransactionEntries.every((transaction) => transaction.canChangeReport);

const ownerAccountIDs = new Set<number>();
let hasUnknownOwner = false;

for (const transaction of selectedTransactionEntries) {
const ownerAccountID = transaction.ownerAccountID ?? transaction.report?.ownerAccountID ?? getOwnerAccountIDForReportID?.(transaction.reportID);
if (typeof ownerAccountID === 'number') {
ownerAccountIDs.add(ownerAccountID);
if (ownerAccountIDs.size > 1) {
break;
}
continue;
}

hasUnknownOwner = true;
}

const hasMultipleOwners = ownerAccountIDs.size > 1 || (hasUnknownOwner && (ownerAccountIDs.size > 0 || selectedTransactionEntries.length > 1));

return {
canMoveToReport: hasSelections && hasOnlyTransactionSelections && canAllTransactionsBeMoved && !hasMultipleOwners && !isExpenseReportSearch,
};
Comment on lines +39 to +41
Copy link
Copy Markdown
Contributor

@hoangzinh hoangzinh Apr 28, 2026

Choose a reason for hiding this comment

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

We can only export canMoveToReport if we only use this value at the moment

Suggested change
return {
canAllTransactionsBeMoved,
canMoveToReport: hasSelections && hasOnlyTransactionSelections && canAllTransactionsBeMoved && !hasMultipleOwners && !isExpenseReportSearch,
hasMultipleOwners,
hasOnlyTransactionSelections,
hasSelections,
targetOwnerAccountID: hasMultipleOwners ? undefined : targetOwnerAccountID,
};
return {
canMoveToReport: hasSelections && hasOnlyTransactionSelections && canAllTransactionsBeMoved && !hasMultipleOwners && !isExpenseReportSearch,
};

}

export default getSearchMoveSelectionValidation;
59 changes: 53 additions & 6 deletions src/components/Search/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as Sentry from '@sentry/react-native';
import React, {startTransition, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import type {NativeScrollEvent, NativeSyntheticEvent, StyleProp, ViewStyle} from 'react-native';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import FullPageErrorView from '@components/BlockingViews/FullPageErrorView';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
Expand Down Expand Up @@ -69,7 +69,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import {columnsSelector} from '@src/selectors/AdvancedSearchFiltersForm';
import type {OutstandingReportsByPolicyIDDerivedValue, SaveSearch, Transaction} from '@src/types/onyx';
import type {OutstandingReportsByPolicyIDDerivedValue, ReportActions, SaveSearch, Transaction} from '@src/types/onyx';
import type SearchResults from '@src/types/onyx/SearchResults';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import arraysEqual from '@src/utils/arraysEqual';
Expand Down Expand Up @@ -119,6 +119,7 @@ function mapTransactionItemToSelectedEntry(
currentUserLogin: string,
currentUserAccountID: number,
outstandingReportsByPolicyID?: OutstandingReportsByPolicyIDDerivedValue,
reportActions?: OnyxEntry<ReportActions>,
allowNegativeAmount = true,
): [string, SelectedTransactionInfo] {
const {canHoldRequest, canUnholdRequest} = canHoldUnholdReportAction(item.report, item.reportAction, item.holdReportAction, item, item.policy, currentUserAccountID);
Expand All @@ -143,6 +144,7 @@ function mapTransactionItemToSelectedEntry(
transaction: item,
report: item.report,
policy: item.policy,
reportActions,
}),
action: item.action,
groupCurrency: item.groupCurrency,
Expand Down Expand Up @@ -183,6 +185,10 @@ function mapEmptyReportToSelectedEntry(item: TransactionReportGroupListItemType)
];
}

function getReportActionsForTransactionItem(item: TransactionListItemType, reportActions?: OnyxCollection<ReportActions>): OnyxEntry<ReportActions> {
return item.reportID ? reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${item.reportID}`] : undefined;
}

function prepareTransactionsList(
item: TransactionListItemType,
itemTransaction: OnyxEntry<Transaction>,
Expand All @@ -191,6 +197,7 @@ function prepareTransactionsList(
currentUserLogin: string,
currentUserAccountID: number,
outstandingReportsByPolicyID?: OutstandingReportsByPolicyIDDerivedValue,
reportActions?: OnyxCollection<ReportActions>,
) {
if (selectedTransactions[item.keyForList]?.isSelected) {
const {[item.keyForList]: omittedTransaction, ...transactions} = selectedTransactions;
Expand All @@ -205,6 +212,7 @@ function prepareTransactionsList(
currentUserLogin,
currentUserAccountID,
outstandingReportsByPolicyID,
getReportActionsForTransactionItem(item, reportActions),
false,
);

Expand Down Expand Up @@ -807,6 +815,7 @@ function Search({
transaction: transactionItem,
report: transactionItem.report,
policy: transactionItem.policy,
reportActions: getReportActionsForTransactionItem(transactionItem, reportActions),
}),
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
isSelected: areAllMatchingItemsSelected || selectedTransactions[transactionItem.transactionID]?.isSelected || isExpenseReportType,
Expand Down Expand Up @@ -863,6 +872,7 @@ function Search({
transaction: transactionItem,
report: transactionItem.report,
policy: transactionItem.policy,
reportActions: getReportActionsForTransactionItem(transactionItem, reportActions),
}),
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
isSelected: areAllMatchingItemsSelected || selectedTransactions[transactionItem.transactionID].isSelected,
Expand Down Expand Up @@ -997,6 +1007,7 @@ function Search({
email ?? '',
accountID,
outstandingReportsByPolicyID,
reportActions,
);
setSelectedTransactions(updatedTransactions, filteredData);
updateSelectAllMatchingItemsState(updatedTransactions);
Expand Down Expand Up @@ -1062,14 +1073,33 @@ function Search({
const originalItemTransaction =
searchResults?.data?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${itemTransaction?.comment?.originalTransactionID}`] ??
transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${itemTransaction?.comment?.originalTransactionID}`];
return mapTransactionItemToSelectedEntry(transactionItem, itemTransaction, originalItemTransaction, email ?? '', accountID, outstandingReportsByPolicyID);
return mapTransactionItemToSelectedEntry(
transactionItem,
itemTransaction,
originalItemTransaction,
email ?? '',
accountID,
outstandingReportsByPolicyID,
getReportActionsForTransactionItem(transactionItem, reportActions),
);
}),
),
};
setSelectedTransactions(updatedTransactions, filteredData);
updateSelectAllMatchingItemsState(updatedTransactions);
},
[selectedTransactions, setSelectedTransactions, filteredData, updateSelectAllMatchingItemsState, transactions, email, accountID, outstandingReportsByPolicyID, searchResults?.data],
[
selectedTransactions,
setSelectedTransactions,
filteredData,
updateSelectAllMatchingItemsState,
transactions,
email,
accountID,
outstandingReportsByPolicyID,
searchResults?.data,
reportActions,
],
);

const onSelectRow = useCallback(
Expand Down Expand Up @@ -1392,7 +1422,15 @@ function Search({
.map((transactionItem) => {
const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionItem.transactionID}`] as OnyxEntry<Transaction>;
const originalItemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${itemTransaction?.comment?.originalTransactionID}`];
return mapTransactionItemToSelectedEntry(transactionItem, itemTransaction, originalItemTransaction, email ?? '', accountID, outstandingReportsByPolicyID);
return mapTransactionItemToSelectedEntry(
transactionItem,
itemTransaction,
originalItemTransaction,
email ?? '',
accountID,
outstandingReportsByPolicyID,
getReportActionsForTransactionItem(transactionItem, reportActions),
);
});
});
updatedTransactions = Object.fromEntries(allSelections);
Expand All @@ -1404,7 +1442,15 @@ function Search({
.map((transactionItem) => {
const itemTransaction = searchResults?.data?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionItem.transactionID}`] as OnyxEntry<Transaction>;
const originalItemTransaction = searchResults?.data?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${itemTransaction?.comment?.originalTransactionID}`];
return mapTransactionItemToSelectedEntry(transactionItem, itemTransaction, originalItemTransaction, email ?? '', accountID, outstandingReportsByPolicyID);
return mapTransactionItemToSelectedEntry(
transactionItem,
itemTransaction,
originalItemTransaction,
email ?? '',
accountID,
outstandingReportsByPolicyID,
getReportActionsForTransactionItem(transactionItem, reportActions),
);
}),
);
}
Expand All @@ -1423,6 +1469,7 @@ function Search({
accountID,
outstandingReportsByPolicyID,
searchResults?.data,
reportActions,
]);

const onLayout = useCallback(() => {
Expand Down
27 changes: 6 additions & 21 deletions src/hooks/useSearchBulkActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {PaymentMethodType} from '@components/KYCWall/types';
import {ModalActions} from '@components/Modal/Global/ModalContext';
import type {PopoverMenuItem} from '@components/PopoverMenu';
import {useSearchActionsContext, useSearchStateContext} from '@components/Search/SearchContext';
import getSearchMoveSelectionValidation from '@components/Search/SearchSelectionUtils';
import type {BulkPaySelectionData, PaymentData, SearchQueryJSON} from '@components/Search/types';
import {unholdRequest} from '@libs/actions/IOU/Hold';
import {setupMergeTransactionDataAndNavigate} from '@libs/actions/MergeTransaction';
Expand Down Expand Up @@ -1337,28 +1338,12 @@ function useSearchBulkActions({queryJSON}: UseSearchBulkActionsParams) {
}
}

const ownerAccountIDs = new Set<number>();
let hasUnknownOwner = false;
for (const id of selectedTransactionsKeys) {
const transactionEntry = selectedTransactions[id];
if (!transactionEntry) {
continue;
}
const ownerAccountID = transactionEntry.ownerAccountID ?? getReportOrDraftReport(transactionEntry.reportID)?.ownerAccountID;
if (typeof ownerAccountID === 'number') {
ownerAccountIDs.add(ownerAccountID);
if (ownerAccountIDs.size > 1) {
break;
}
} else {
hasUnknownOwner = true;
}
}
const hasMultipleOwners = ownerAccountIDs.size > 1 || (hasUnknownOwner && (ownerAccountIDs.size > 0 || selectedTransactionsKeys.length > 1));

const canAllTransactionsBeMoved = selectedTransactionsKeys.every((id) => selectedTransactions[id].canChangeReport);
const {canMoveToReport} = getSearchMoveSelectionValidation(selectedTransactions, {
isExpenseReportSearch: isExpenseReportType,
getOwnerAccountIDForReportID: (reportID) => getReportOrDraftReport(reportID)?.ownerAccountID,
});

if (canAllTransactionsBeMoved && !hasMultipleOwners && !isExpenseReportType) {
if (canMoveToReport) {
options.push({
text: translate('iou.moveExpenses'),
icon: expensifyIcons.DocumentMerge,
Expand Down
10 changes: 9 additions & 1 deletion src/hooks/useSelectedTransactionsActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,15 @@ function useSelectedTransactionsActions({
}
const iouReportAction = getIOUActionForTransactionID(reportActions, transaction.transactionID);

const canMoveExpense = canEditFieldOfMoneyRequest({reportAction: iouReportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.REPORT, outstandingReportsByPolicyID, transaction});
const canMoveExpense = canEditFieldOfMoneyRequest({
reportAction: iouReportAction,
fieldToEdit: CONST.EDIT_REQUEST_FIELD.REPORT,
outstandingReportsByPolicyID,
transaction,
report,
policy,
reportActions,
});
return canMoveExpense;
});

Expand Down
13 changes: 12 additions & 1 deletion src/libs/ReportSecondaryActionUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -998,6 +998,9 @@ function getSecondaryReportActions({
isChatReportArchived,
outstandingReportsByPolicyID,
transaction,
report,
policy,
reportActions,
});
const canUserPerformWriteAction = canUserPerformWriteActionReportUtils(report, isChatReportArchived);

Expand Down Expand Up @@ -1089,7 +1092,15 @@ function getSecondaryTransactionThreadActions(
if (
reportTransaction?.transactionID &&
reportAction &&
canEditFieldOfMoneyRequest({reportAction, fieldToEdit: CONST.EDIT_REQUEST_FIELD.REPORT, isChatReportArchived, outstandingReportsByPolicyID, transaction: reportTransaction}) &&
canEditFieldOfMoneyRequest({
reportAction,
fieldToEdit: CONST.EDIT_REQUEST_FIELD.REPORT,
isChatReportArchived,
outstandingReportsByPolicyID,
transaction: reportTransaction,
report: parentReport,
policy,
}) &&
canUserPerformWriteActionReportUtils(parentReport, isChatReportArchived)
) {
options.push(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.MOVE_EXPENSE);
Expand Down
Loading
Loading