Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
72762ce
fix: migrate ReportActionItem to useOnyx for policyTags
marcochavezf Jan 22, 2026
d0ff5e2
fix: improve eslint-disable comments with more specific justification
marcochavezf Jan 22, 2026
58e8860
fix: add currentUserLogin parameter to getForReportActionTemp
marcochavezf Jan 22, 2026
b728cbe
fix: add currentUserLogin to destructuring in copy to clipboard action
marcochavezf Jan 22, 2026
0821bd1
fix: remove unused policyForMovingExpensesID
marcochavezf Jan 22, 2026
f8f7357
chore: trigger CI checks
marcochavezf Jan 22, 2026
128592e
fix: address PR review comments
marcochavezf Feb 4, 2026
14255aa
fix: revert Mobile-Expensify submodule change
marcochavezf Feb 4, 2026
acb30b7
fix: TypeScript error in ContextMenuActions
marcochavezf Feb 4, 2026
be64c7f
test: add unit tests for getForReportActionTemp
marcochavezf Feb 4, 2026
e1ee69b
Merge branch 'main' into marcochavezf/migrate-modified-expense-to-use…
marcochavezf Feb 5, 2026
a2dceaf
Address PR review feedback: simplify eslint comments, revert param re…
marcochavezf Feb 5, 2026
d651e7f
Merge branch 'main' into marcochavezf/migrate-modified-expense-to-use…
marcochavezf Feb 5, 2026
feb6990
Merge branch 'main' into marcochavezf/migrate-modified-expense-to-use…
marcochavezf Feb 6, 2026
0bd9a8d
fix: rename module-level currentUserLogin to storedCurrentUserLogin t…
marcochavezf Feb 6, 2026
cbec8b1
Fix billable/reimbursable localization and add AI-generated fallback …
MelvinBot Feb 10, 2026
0515fda
Fix Prettier formatting in ModifiedExpenseMessage.ts
MelvinBot Feb 10, 2026
7d18ddb
Improve policyID comment and add DEFAULT_TAG_LIST fallback for policy…
MelvinBot Feb 10, 2026
30b188a
Fix: use email instead of login from useCurrentUserPersonalDetails
MelvinBot Feb 11, 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
46 changes: 22 additions & 24 deletions src/libs/ModifiedExpenseMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {getPolicyName, getReportName, getRootParentReport, isPolicyExpenseChat,
import {getFormattedAttendees, getTagArrayFromName} from './TransactionUtils';

let allPolicyTags: OnyxCollection<PolicyTagLists> = {};
// eslint-disable-next-line @typescript-eslint/no-deprecated -- Onyx.connectWithoutView is being removed in https://github.com/Expensify/App/issues/66336
Onyx.connectWithoutView({
key: ONYXKEYS.COLLECTION.POLICY_TAGS,
waitForCollectionCallback: true,
Expand All @@ -43,15 +44,16 @@ Onyx.connectWithoutView({
let environmentURL: string;
getEnvironmentURL().then((url: string) => (environmentURL = url));

let currentUserLogin = '';
let storedCurrentUserLogin = '';
// eslint-disable-next-line @typescript-eslint/no-deprecated -- Onyx.connectWithoutView is being removed in https://github.com/Expensify/App/issues/66336
Onyx.connectWithoutView({
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, value is undefined
if (!value) {
return;
}
currentUserLogin = value?.email ?? '';
storedCurrentUserLogin = value?.email ?? '';
},
});

Expand Down Expand Up @@ -164,7 +166,7 @@ function getForExpenseMovedFromSelfDM(translate: LocalizedTranslate, destination
// In NewDot, the "Move report" flow only supports moving expenses from self-DM to:
// - A policy expense chat
// - A 1:1 DM
const currentUserAccountID = getPersonalDetailByEmail(currentUserLogin)?.accountID;
const currentUserAccountID = getPersonalDetailByEmail(storedCurrentUserLogin)?.accountID;
const reportName = isPolicyExpenseChat(rootParentReport)
? getPolicyExpenseChatName({report: rootParentReport})
: buildReportNameFromParticipantNames({report: rootParentReport, currentUserAccountID});
Expand Down Expand Up @@ -330,7 +332,7 @@ function getForReportAction({
} else if (reportActionOriginalMessage?.source === CONST.CATEGORY_SOURCE.MCC) {
// eslint-disable-next-line @typescript-eslint/no-deprecated
const policy = getPolicy(policyID);
const isAdmin = isPolicyAdmin(policy, currentUserLogin);
const isAdmin = isPolicyAdmin(policy, storedCurrentUserLogin);

// For admins, create a hyperlink to the workspace rules page
if (isAdmin && policy?.id) {
Expand Down Expand Up @@ -514,13 +516,15 @@ function getForReportActionTemp({
movedFromReport,
movedToReport,
policyTags,
currentUserLogin,
}: {
translate: LocalizedTranslate;
reportAction: OnyxEntry<ReportAction>;
policy?: OnyxEntry<Policy>;
movedFromReport?: OnyxEntry<Report>;
movedToReport?: OnyxEntry<Report>;
policyTags: OnyxEntry<PolicyTagLists>;
currentUserLogin: string;
}): string {
if (!isModifiedExpenseAction(reportAction)) {
return '';
Expand Down Expand Up @@ -693,30 +697,17 @@ function getForReportActionTemp({

const hasModifiedBillable = isReportActionOriginalMessageAnObject && 'oldBillable' in reportActionOriginalMessage && 'billable' in reportActionOriginalMessage;
if (hasModifiedBillable) {
buildMessageFragmentForValue(
translate,
reportActionOriginalMessage?.billable ?? '',
reportActionOriginalMessage?.oldBillable ?? '',
translate('iou.expense'),
true,
setFragments,
removalFragments,
changeFragments,
);
const oldBillable = reportActionOriginalMessage?.oldBillable === 'billable' ? translate('common.billable').toLowerCase() : translate('common.nonBillable').toLowerCase();
const newBillable = reportActionOriginalMessage?.billable === 'billable' ? translate('common.billable').toLowerCase() : translate('common.nonBillable').toLowerCase();
buildMessageFragmentForValue(translate, newBillable, oldBillable, translate('iou.expense'), true, setFragments, removalFragments, changeFragments);
}

const hasModifiedReimbursable = isReportActionOriginalMessageAnObject && 'oldReimbursable' in reportActionOriginalMessage && 'reimbursable' in reportActionOriginalMessage;
if (hasModifiedReimbursable) {
buildMessageFragmentForValue(
translate,
reportActionOriginalMessage?.reimbursable ?? '',
reportActionOriginalMessage?.oldReimbursable ?? '',
translate('iou.expense'),
true,
setFragments,
removalFragments,
changeFragments,
);
const oldReimbursable =
reportActionOriginalMessage?.oldReimbursable === 'reimbursable' ? translate('iou.reimbursable').toLowerCase() : translate('iou.nonReimbursable').toLowerCase();
const newReimbursable = reportActionOriginalMessage?.reimbursable === 'reimbursable' ? translate('iou.reimbursable').toLowerCase() : translate('iou.nonReimbursable').toLowerCase();
buildMessageFragmentForValue(translate, newReimbursable, oldReimbursable, translate('iou.expense'), true, setFragments, removalFragments, changeFragments);
}

const hasModifiedAttendees = isReportActionOriginalMessageAnObject && 'oldAttendees' in reportActionOriginalMessage && 'newAttendees' in reportActionOriginalMessage;
Expand All @@ -741,6 +732,13 @@ function getForReportActionTemp({
getMessageLine(translate, `\n${translate('iou.removed')}`, removalFragments);

if (message === '') {
// If we don't have enough structured information to build a detailed message but we
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these changes necessary for this PR? If not, can you please remove it so that it is more straight-forward to review?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, these changes are necessary. Beamanator flagged in his review that getForReportActionTemp was missing the aiGenerated fallback that exists in getForReportAction. Since this PR migrates ReportActionItem to call getForReportActionTemp instead of getForReportAction, without this fallback there would be a behavioral regression: AI-attributed expense modifications would display "changed the expense" instead of "changed the expense based on past activity."

The same reasoning applies to the billable/reimbursable localization fix just above — both were required to maintain parity with the function being replaced.

🤖 This comment was generated with the assistance of an AI tool.

// know the change was AI-generated, fall back to an AI-attributed generic summary so
// users can still understand that Concierge updated the expense automatically.
if (reportActionOriginalMessage?.aiGenerated) {
return `${translate('iou.changedTheExpense')} ${translate('iou.basedOnAI')}`;
}

return translate('iou.changedTheExpense');
}
return `${message.substring(1, message.length)}`;
Expand Down
2 changes: 2 additions & 0 deletions src/pages/inbox/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,7 @@ const ContextMenuActions: ContextMenuAction[] = [
policyTags,
translate,
harvestReport,
currentUserPersonalDetails,
},
) => {
const isReportPreviewAction = isReportPreviewActionReportActionsUtils(reportAction);
Expand All @@ -769,6 +770,7 @@ const ContextMenuActions: ContextMenuAction[] = [
movedFromReport,
movedToReport,
policyTags,
currentUserLogin: currentUserPersonalDetails?.email ?? '',
});
Clipboard.setString(modifyExpenseMessage);
} else if (isReimbursementDeQueuedOrCanceledAction(reportAction)) {
Expand Down
20 changes: 14 additions & 6 deletions src/pages/inbox/report/ReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import useOnyx from '@hooks/useOnyx';
import useOriginalReportID from '@hooks/useOriginalReportID';
import usePolicyForMovingExpenses from '@hooks/usePolicyForMovingExpenses';
import useReportIsArchived from '@hooks/useReportIsArchived';
import {getForReportAction, getMovedReportID} from '@libs/ModifiedExpenseMessage';
import {getForReportActionTemp, getMovedReportID} from '@libs/ModifiedExpenseMessage';
import {getIOUReportIDFromReportActionPreview, getOriginalMessage} from '@libs/ReportActionsUtils';
import {
chatIncludesChronosWithID,
Expand Down Expand Up @@ -99,7 +99,14 @@ function ReportActionItem({
const originalReportID = useOriginalReportID(reportID, action);
const originalReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${originalReportID}`];
const isOriginalReportArchived = useReportIsArchived(originalReportID);
const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();
const {accountID: currentUserAccountID, email: currentUserEmail} = useCurrentUserPersonalDetails();
const {policyForMovingExpensesID} = usePolicyForMovingExpenses();
// When an expense is moved from a self-DM to a workspace, the report's policyID is temporarily
// set to a fake placeholder (CONST.POLICY.OWNER_EMAIL_FAKE). Looking up POLICY_TAGS with that
// fake ID would return nothing, so we fall back to policyForMovingExpensesID (the actual
// destination workspace) to fetch the correct tag list for display.
const policyIDForTags = report?.policyID === CONST.POLICY.OWNER_EMAIL_FAKE && policyForMovingExpensesID ? policyForMovingExpensesID : report?.policyID;
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyIDForTags}`, {canBeMissing: true});
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {canBeMissing: true});
const [allTransactionDrafts] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_DRAFT, {canBeMissing: true});
const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {canBeMissing: true});
Expand All @@ -109,7 +116,6 @@ function ReportActionItem({
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`];
const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST, {canBeMissing: true});
const [personalPolicyID] = useOnyx(ONYXKEYS.PERSONAL_POLICY_ID, {canBeMissing: true});
const {policyForMovingExpensesID} = usePolicyForMovingExpenses();
// The app would crash due to subscribing to the entire report collection if parentReportID is an empty string. So we should have a fallback ID here.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const parentReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.parentReportID || undefined}`];
Expand Down Expand Up @@ -162,12 +168,14 @@ function ReportActionItem({
action as OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_DEQUEUED | typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_ACH_CANCELED>>,
report,
)}
modifiedExpenseMessage={getForReportAction({
modifiedExpenseMessage={getForReportActionTemp({
translate,
reportAction: action,
policyID: report?.policyID,
policy,
movedFromReport,
movedToReport,
policyForMovingExpensesID,
policyTags: policyTags ?? CONST.POLICY.DEFAULT_TAG_LIST,
currentUserLogin: currentUserEmail ?? '',
})}
getTransactionsWithReceipts={getTransactionsWithReceipts}
clearError={clearError}
Expand Down
Loading
Loading