Skip to content
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
32 changes: 26 additions & 6 deletions src/components/ReportActionItem/MovedTransactionAction.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,32 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import RenderHTML from '@components/RenderHTML';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import Parser from '@libs/Parser';
import {getOriginalMessage} from '@libs/ReportActionsUtils';
import {getOriginalMessage, hasReasoning} from '@libs/ReportActionsUtils';
import {getMovedTransactionMessage} from '@libs/ReportUtils';
import ReportActionItemBasicMessage from '@pages/inbox/report/ReportActionItemBasicMessage';
import ReportActionItemMessageWithExplain from '@pages/inbox/report/ReportActionItemMessageWithExplain';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ReportAction} from '@src/types/onyx';
import type {Report, ReportAction} from '@src/types/onyx';

type MovedTransactionActionProps = {
/** The moved transaction action data */
action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION>;

/** The element to render when there is no report that the transaction was moved to or from */
emptyHTML: React.JSX.Element;

/** The child report of the action item */
childReport: OnyxEntry<Report>;

/** Original report from which the given reportAction is first created */
originalReport: OnyxEntry<Report>;
};

function MovedTransactionAction({action, emptyHTML}: MovedTransactionActionProps) {
function MovedTransactionAction({action, emptyHTML, childReport, originalReport}: MovedTransactionActionProps) {
const {translate} = useLocalize();
const movedTransactionOriginalMessage = getOriginalMessage(action);
const toReportID = movedTransactionOriginalMessage?.toReportID;
Expand All @@ -30,9 +38,6 @@ function MovedTransactionAction({action, emptyHTML}: MovedTransactionActionProps
const isPendingDelete = fromReport?.pendingFields?.preview === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
// When the transaction is moved from personal space (unreported), fromReportID will be "0" which doesn't exist in allReports
const hasFromReport = fromReportID === CONST.REPORT.UNREPORTED_REPORT_ID ? true : !!fromReport;
const htmlContent = isPendingDelete
? `<del><comment><muted-text>${Parser.htmlToText(getMovedTransactionMessage(translate, action))}</muted-text></comment></del>`
: `<comment><muted-text>${getMovedTransactionMessage(translate, action)}</muted-text></comment>`;

// When expenses are merged multiple times, the previous fromReportID may reference a deleted report,
// making it impossible to retrieve the report name for display
Expand All @@ -41,6 +46,21 @@ function MovedTransactionAction({action, emptyHTML}: MovedTransactionActionProps
return emptyHTML;
}

const message = getMovedTransactionMessage(translate, action);

if (hasReasoning(action)) {
return (
<ReportActionItemMessageWithExplain
Comment on lines +51 to +53

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep pending-delete formatting when reasoning is present

The new hasReasoning(action) early return bypasses the isPendingDelete branch, so a MOVED_TRANSACTION action with reasoning from a report marked pendingFields.preview=DELETE no longer renders with the deleted (<del>) styling and text-only fallback. In that state, users will see an active-looking message (including links/Explain) for an action that should still appear as deleted, which is a regression from the previous behavior in this component.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I considered that, but I believe the other callers of ReportActionItemMessageWithExplain act the same

message={message}
action={action}
childReport={childReport}
originalReport={originalReport}
/>
);
}

const htmlContent = isPendingDelete ? `<del><comment><muted-text>${Parser.htmlToText(message)}</muted-text></comment></del>` : `<comment><muted-text>${message}</muted-text></comment>`;

return (
<ReportActionItemBasicMessage message="">
<RenderHTML html={htmlContent} />
Expand Down
2 changes: 2 additions & 0 deletions src/pages/inbox/report/PureReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@
clearError(transactionID);
}
clearAllRelatedReportActionErrors(reportID, action, originalReportID);
}, [action, isSendingMoney, reportID, clearAllRelatedReportActionErrors, report, chatReport, clearError]);

Check warning on line 626 in src/pages/inbox/report/PureReportActionItem.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

React Hook useCallback has a missing dependency: 'originalReportID'. Either include it or remove the dependency array

const showDismissReceiptErrorModal = useCallback(async () => {
const result = await showConfirmModal({
Expand Down Expand Up @@ -1465,6 +1465,8 @@
<MovedTransactionAction
action={action as OnyxTypes.ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION>}
emptyHTML={emptyHTML}
childReport={childReport}
originalReport={originalReport}
/>
);
} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.MOVED) {
Expand Down
2 changes: 2 additions & 0 deletions src/types/onyx/OriginalMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,8 @@ type OriginalMessageMovedTransaction = {
toReportID?: string;
/** ID of the original report */
fromReportID: string;
/** Reasoning for the automated move, used by Concierge Explain feature */
reasoning?: string;
};

/** Model of `moved` report action */
Expand Down
54 changes: 54 additions & 0 deletions tests/ui/PureReportActionItemTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -581,4 +581,58 @@ describe('PureReportActionItem', () => {
expect(openLink).toHaveBeenCalledWith(workspaceRulesUrl, expect.any(String));
});
});

describe('MOVED_TRANSACTION action', () => {
const FROM_REPORT_ID = '100';
const TO_REPORT_ID = '200';

beforeEach(async () => {
await act(async () => {
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${FROM_REPORT_ID}`, {
reportID: FROM_REPORT_ID,
reportName: 'Source Report',
type: CONST.REPORT.TYPE.EXPENSE,
});
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${TO_REPORT_ID}`, {
reportID: TO_REPORT_ID,
reportName: 'Destination Report',
type: CONST.REPORT.TYPE.EXPENSE,
});
});
await waitForBatchedUpdatesWithAct();
});

it('renders plain message without Explain link when action has no reasoning', async () => {
const action = createReportAction(CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION, {
fromReportID: FROM_REPORT_ID,
toReportID: TO_REPORT_ID,
});

renderItemWithAction(action);
await waitForBatchedUpdatesWithAct();

// The moved transaction message should be displayed
expect(screen.getByText(/moved this expense/)).toBeOnTheScreen();

// The "Explain" link should NOT be present
expect(screen.queryByText('Explain')).not.toBeOnTheScreen();
});

it('renders Explain link when action has reasoning', async () => {
const action = createReportAction(CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION, {
fromReportID: FROM_REPORT_ID,
toReportID: TO_REPORT_ID,
reasoning: 'This expense violated the max amount rule.',
});

renderItemWithAction(action);
await waitForBatchedUpdatesWithAct();

// The moved transaction message should be displayed
expect(screen.getByText(/moved this expense/)).toBeOnTheScreen();

// The "Explain" link should be present
expect(screen.getByText('Explain')).toBeOnTheScreen();
});
});
});
53 changes: 53 additions & 0 deletions tests/unit/ReportActionsUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
getSortedReportActions,
getSortedReportActionsForDisplay,
getUpdateACHAccountMessage,
hasReasoning,
isIOUActionMatchingTransactionList,
isNewerReportAction,
isReportActionVisibleAsLastAction,
Expand Down Expand Up @@ -3870,6 +3871,7 @@ describe('ReportActionsUtils', () => {
expect(ReportActionsUtils.isOriginalReportDeleted(action, originalReport)).toBe(false);
});
});

Comment thread
Beamanator marked this conversation as resolved.
describe('isRejectedAction', () => {
it('should return true for REJECTED action type', () => {
// Given a report action with REJECTED action type
Expand Down Expand Up @@ -3941,4 +3943,55 @@ describe('ReportActionsUtils', () => {
expect(result).toBe(false);
});
});

describe('hasReasoning', () => {
it('should return true when the action has a non-empty reasoning field', () => {
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION> = {
actionName: CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION,
reportActionID: '1',
created: '2025-09-29',
originalMessage: {
fromReportID: '2',
toReportID: '3',
reasoning: 'This expense was moved because it violated the max amount rule.',
},
};

expect(hasReasoning(action)).toBe(true);
});

it('should return false when the action has no reasoning field', () => {
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION> = {
actionName: CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION,
reportActionID: '1',
created: '2025-09-29',
originalMessage: {
fromReportID: '2',
toReportID: '3',
},
};

expect(hasReasoning(action)).toBe(false);
});

it('should return false when reasoning is an empty string', () => {
const action: ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION> = {
actionName: CONST.REPORT.ACTIONS.TYPE.MOVED_TRANSACTION,
reportActionID: '1',
created: '2025-09-29',
originalMessage: {
fromReportID: '2',
toReportID: '3',
reasoning: '',
},
};

expect(hasReasoning(action)).toBe(false);
});

it('should return false when the action is null or undefined', () => {
expect(hasReasoning(null)).toBe(false);
expect(hasReasoning(undefined)).toBe(false);
});
});
});
Loading