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
35 changes: 17 additions & 18 deletions src/pages/inbox/report/PureReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ import {
getIOUReportIDFromReportActionPreview,
getOriginalMessage,
getPlaidBalanceFailureMessage,
getReimbursedMessage,
getRenamedAction,
getReportActionHtml,
getReportActionMessage,
Expand Down Expand Up @@ -146,8 +145,11 @@ import ConfirmWhisperContent from './actionContents/ConfirmWhisperContent';
import FraudAlertContent from './actionContents/FraudAlertContent';
import JoinRequestContent from './actionContents/JoinRequestContent';
import MentionWhisperContent from './actionContents/MentionWhisperContent';
import ModifiedExpenseContent from './actionContents/ModifiedExpenseContent';
import PaymentContent from './actionContents/PaymentContent';
import PolicyChangeLogContent, {isHandledPolicyChangeLogAction} from './actionContents/PolicyChangeLogContent';
import ReimbursedContent from './actionContents/ReimbursedContent';
import ReimbursementDeQueuedContent from './actionContents/ReimbursementDeQueuedContent';
import ReportMentionWhisperContent from './actionContents/ReportMentionWhisperContent';
import SimpleMessageContent, {isSimpleMessageAction} from './actionContents/SimpleMessageContent';
import {RestrictedReadOnlyContextMenuActions} from './ContextMenu/ContextMenuActions';
Expand All @@ -159,7 +161,6 @@ import ReportActionItemBasicMessage from './ReportActionItemBasicMessage';
import ReportActionItemContentCreated from './ReportActionItemContentCreated';
import ReportActionItemDraft from './ReportActionItemDraft';
import ReportActionItemGrouped from './ReportActionItemGrouped';
import ReportActionItemMessageWithExplain from './ReportActionItemMessageWithExplain';
import ReportActionItemSingle from './ReportActionItemSingle';
import ReportActionItemThread from './ReportActionItemThread';
import TripSummary from './TripSummary';
Expand Down Expand Up @@ -306,12 +307,6 @@ type PureReportActionItemProps = {
/** What missing payment method does this report action indicate, if any? */
missingPaymentMethod?: MissingPaymentMethod | undefined;

/** Returns the preview message for `REIMBURSEMENT_DEQUEUED` or `REIMBURSEMENT_ACH_CANCELED` action */
reimbursementDeQueuedOrCanceledActionMessage?: string;

/** The report action message when expense has been modified. */
modifiedExpenseMessage?: string;

/** Gets all transactions on an IOU report with a receipt */
getTransactionsWithReceipts?: (iouReportID: string | undefined) => OnyxTypes.Transaction[];

Expand Down Expand Up @@ -406,8 +401,6 @@ function PureReportActionItem({
isClosedExpenseReportWithNoExpenses,
isCurrentUserTheOnlyParticipant = () => false,
missingPaymentMethod,
reimbursementDeQueuedOrCanceledActionMessage = '',
modifiedExpenseMessage = '',
getTransactionsWithReceipts = () => [],
clearError = () => {},
clearAllRelatedReportActionErrors = () => {},
Expand Down Expand Up @@ -915,12 +908,17 @@ function PureReportActionItem({
</ReportActionItemBasicMessage>
);
} else if (isReimbursementDeQueuedOrCanceledAction(action)) {
children = <ReportActionItemBasicMessage message={reimbursementDeQueuedOrCanceledActionMessage} />;
children = (
<ReimbursementDeQueuedContent
action={action}
report={report}
/>
);
} else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.MODIFIED_EXPENSE) {
children = (
<ReportActionItemMessageWithExplain
message={modifiedExpenseMessage}
<ModifiedExpenseContent
action={action}
report={report}
childReport={childReport}
originalReport={originalReport}
/>
Expand All @@ -944,7 +942,12 @@ function PureReportActionItem({
/>
);
} else if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.REIMBURSED)) {
children = <ReportActionItemBasicMessage message={getReimbursedMessage(translate, action, report, currentUserAccountID)} />;
children = (
<ReimbursedContent
action={action}
report={report}
/>
);
} else if (isSimpleMessageAction(action)) {
children = <SimpleMessageContent action={action} />;
} else if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.FORWARDED)) {
Expand Down Expand Up @@ -1023,7 +1026,6 @@ function PureReportActionItem({
report={report}
originalReport={originalReport}
policy={policy}
currentUserAccountID={currentUserAccountID}
personalPolicyID={personalPolicyID}
originalReportID={originalReportID}
resolveActionableMentionWhisper={resolveActionableMentionWhisper}
Expand Down Expand Up @@ -1188,7 +1190,6 @@ function PureReportActionItem({
accountIDs={oldestFourAccountIDs}
onSecondaryInteraction={showPopover}
isActive={isReportActionActive && !isContextMenuActive}
currentUserAccountID={currentUserAccountID}
/>
</View>
)}
Expand Down Expand Up @@ -1501,8 +1502,6 @@ export default memo(PureReportActionItem, (prevProps, nextProps) => {
prevProps.isChronosReport === nextProps.isChronosReport &&
prevProps.isClosedExpenseReportWithNoExpenses === nextProps.isClosedExpenseReportWithNoExpenses &&
deepEqual(prevProps.missingPaymentMethod, nextProps.missingPaymentMethod) &&
prevProps.reimbursementDeQueuedOrCanceledActionMessage === nextProps.reimbursementDeQueuedOrCanceledActionMessage &&
prevProps.modifiedExpenseMessage === nextProps.modifiedExpenseMessage &&
prevProps.userBillingFundID === nextProps.userBillingFundID &&
Comment thread
LukasMod marked this conversation as resolved.
Comment on lines 1502 to 1505
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 Invalidate memo when extracted message inputs change

After extracting ModifiedExpenseContent and ReimbursementDeQueuedContent, the memo comparator no longer tracks the derived message props that previously forced updates for these action types. As a result, PureReportActionItem can now skip rerenders when values like currentUserEmail or report fields used by those messages change, leaving stale text/link state (for example, a modified-expense message can keep the help URL instead of switching to workspace rules once user/session data hydrates). Please include the underlying dependencies in the comparator (or keep passing derived message props) so these branches re-render when their inputs change.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The props that bypass the memo (report?.ownerAccountID, report?.policyID, currentUserEmail) are immutable during the component's lifetime, they're set at report creation / login and don't change. The mutable data (policy, policyTags, movedFrom/ToReport) is handled by self-subscription inside the extracted components, which bypasses the memo entirely. The old string comparisons were only catching changes to these self-subscribing values, not to the immutable ones.

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.

Technically there is also another case of change which is the first loading of the data. There can be other cases that could cause a re-render in those circumstances but it might be a good idea to still add the necessary dependencies. WDYT @cristipaval

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Rather than re-add the prop/comparator entry, I pushed the fix to ModifiedExpenseContent itself to read it via useCurrentUserPersonalDetails() , the same pattern we use in other decomposed files. Context updates re-render the child directly when session.email hydrates. It will work well with other PRs too

deepEqual(prevProps.taskReport, nextProps.taskReport) &&
prevProps.shouldHighlight === nextProps.shouldHighlight &&
Expand Down
33 changes: 2 additions & 31 deletions src/pages/inbox/report/ReportActionItem.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import React, {useCallback} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import useOriginalReportID from '@hooks/useOriginalReportID';
import usePolicyForMovingExpenses from '@hooks/usePolicyForMovingExpenses';
import useReportIsArchived from '@hooks/useReportIsArchived';
import useReportTransactions from '@hooks/useReportTransactions';
import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import {getForReportAction, getMovedReportID} from '@libs/ModifiedExpenseMessage';
import {getIOUReportIDFromReportActionPreview, getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {
chatIncludesChronosWithID,
getIndicatedMissingPaymentMethod,
getReimbursementDeQueuedOrCanceledActionMessage,
getTransactionsWithReceipts,
isArchivedNonExpenseReport,
isChatThread,
Expand All @@ -23,9 +19,8 @@ import {
import {clearAllRelatedReportActionErrors} from '@userActions/ClearReportActionErrors';
import {deleteReportActionDraft, resolveActionableMentionWhisper, resolveActionableReportMentionWhisper, toggleEmojiReaction} from '@userActions/Report';
import {clearError} from '@userActions/Transaction';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetailsList, ReportAction, ReportActionReactions, Transaction} from '@src/types/onyx';
import type {PersonalDetailsList, ReportActionReactions, Transaction} from '@src/types/onyx';
import type {PureReportActionItemProps} from './PureReportActionItem';
import PureReportActionItem from './PureReportActionItem';

Expand Down Expand Up @@ -68,27 +63,17 @@ function ReportActionItem({
isTryNewDotNVPDismissed,
...props
}: ReportActionItemProps) {
const {translate} = useLocalize();
const reportID = report?.reportID;
const originalMessage = getOriginalMessage(action);
const originalReportID = useOriginalReportID(reportID, action);
const isOriginalReportArchived = useReportIsArchived(originalReportID);
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}`);
const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [reportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`);
const [originalReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${originalReportID}`);
const [iouReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getIOUReportIDFromReportActionPreview(action)}`);
const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(report?.parentReportID)}`);
const [movedFromReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(action, CONST.REPORT.MOVE_TYPE.FROM)}`);
const [movedToReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(action, CONST.REPORT.MOVE_TYPE.TO)}`);

const taskReportID = originalMessage && 'taskReportID' in originalMessage ? originalMessage.taskReportID : undefined;
const [taskReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${taskReportID}`);
Expand Down Expand Up @@ -149,20 +134,6 @@ function ReportActionItem({
isClosedExpenseReportWithNoExpenses={isClosedExpenseReportWithNoExpenses(iouReport, transactionsOnIOUReport)}
isCurrentUserTheOnlyParticipant={isCurrentUserTheOnlyParticipant}
missingPaymentMethod={missingPaymentMethod}
reimbursementDeQueuedOrCanceledActionMessage={getReimbursementDeQueuedOrCanceledActionMessage(
translate,
action as OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_DEQUEUED | typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_ACH_CANCELED>>,
report,
)}
modifiedExpenseMessage={getForReportAction({
translate,
reportAction: action,
policy,
movedFromReport,
movedToReport,
policyTags: policyTags ?? CONST.POLICY.DEFAULT_TAG_LIST,
currentUserLogin: currentUserEmail ?? '',
})}
getTransactionsWithReceipts={getTransactionsWithReceipts}
clearError={clearError}
clearAllRelatedReportActionErrors={clearAllRelatedReportActionErrors}
Expand Down
18 changes: 3 additions & 15 deletions src/pages/inbox/report/ReportActionItemThread.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type {OnyxEntry} from 'react-native-onyx';
import PressableWithSecondaryInteraction from '@components/PressableWithSecondaryInteraction';
import ReportActionAvatars from '@components/ReportActionAvatars';
import Text from '@components/Text';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -37,24 +38,11 @@ type ReportActionItemThreadProps = {

/** The function that should be called when the thread is LongPressed or right-clicked */
onSecondaryInteraction: (event: GestureResponderEvent | MouseEvent) => void;

/** The accountID of the current user, used for creating optimistic report if needed */
currentUserAccountID: number;
};

function ReportActionItemThread({
numberOfReplies,
accountIDs,
mostRecentReply,
report,
reportAction,
isHovered,
onSecondaryInteraction,
isActive,
currentUserAccountID,
}: ReportActionItemThreadProps) {
function ReportActionItemThread({numberOfReplies, accountIDs, mostRecentReply, report, reportAction, isHovered, onSecondaryInteraction, isActive}: ReportActionItemThreadProps) {
const styles = useThemeStyles();

const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();
const {translate, datetimeToCalendarTime} = useLocalize();
const [childReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportAction.childReportID}`);
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
Expand Down
14 changes: 3 additions & 11 deletions src/pages/inbox/report/actionContents/MentionWhisperContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {ValueOf} from 'type-fest';
import RenderHTML from '@components/RenderHTML';
import type {ActionableItem} from '@components/ReportActionItem/ActionableItemButtons';
import ActionableItemButtons from '@components/ReportActionItem/ActionableItemButtons';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useReportIsArchived from '@hooks/useReportIsArchived';
import {isPolicyAdmin, isPolicyMember, isPolicyOwner} from '@libs/PolicyUtils';
Expand All @@ -17,7 +18,6 @@ type MentionWhisperContentProps = {
report: OnyxEntry<Report>;
originalReport: OnyxEntry<Report>;
policy: OnyxEntry<Policy>;
currentUserAccountID: number;
personalPolicyID: string | undefined;
originalReportID: string | undefined;
resolveActionableMentionWhisper: (
Expand All @@ -28,18 +28,10 @@ type MentionWhisperContentProps = {
) => void;
};

function MentionWhisperContent({
action,
report,
originalReport,
policy,
currentUserAccountID,
personalPolicyID,
originalReportID,
resolveActionableMentionWhisper,
}: MentionWhisperContentProps) {
function MentionWhisperContent({action, report, originalReport, policy, personalPolicyID, originalReportID, resolveActionableMentionWhisper}: MentionWhisperContentProps) {
const {translate} = useLocalize();
const isOriginalReportArchived = useReportIsArchived(originalReportID);
const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();

const reportActionReport = originalReport ?? report;
const reportPolicyID = report?.policyID;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import usePolicyForMovingExpenses from '@hooks/usePolicyForMovingExpenses';
import {getForReportAction, getMovedReportID} from '@libs/ModifiedExpenseMessage';
import ReportActionItemMessageWithExplain from '@pages/inbox/report/ReportActionItemMessageWithExplain';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Report, ReportAction} from '@src/types/onyx';

type ModifiedExpenseContentProps = {
action: ReportAction;
report: OnyxEntry<Report>;
childReport: OnyxEntry<Report>;
originalReport: OnyxEntry<Report>;
};

function ModifiedExpenseContent({action, report, childReport, originalReport}: ModifiedExpenseContentProps) {
const {translate} = useLocalize();
const {email: currentUserEmail} = useCurrentUserPersonalDetails();
const {policyForMovingExpensesID} = usePolicyForMovingExpenses();
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`);

// When expense is moved from self-DM to workspace, policyID is temporarily OWNER_EMAIL_FAKE.
// Fall back to policyForMovingExpensesID (actual destination workspace) for correct tag list.
const policyIDForTags = report?.policyID === CONST.POLICY.OWNER_EMAIL_FAKE && policyForMovingExpensesID ? policyForMovingExpensesID : report?.policyID;
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyIDForTags}`);
const [movedFromReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(action, CONST.REPORT.MOVE_TYPE.FROM)}`);
const [movedToReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(action, CONST.REPORT.MOVE_TYPE.TO)}`);

const modifiedExpenseMessage = getForReportAction({
translate,
reportAction: action,
policy,
movedFromReport,
movedToReport,
policyTags: policyTags ?? CONST.POLICY.DEFAULT_TAG_LIST,
currentUserLogin: currentUserEmail ?? '',
});

return (
<ReportActionItemMessageWithExplain
message={modifiedExpenseMessage}
action={action}
childReport={childReport}
originalReport={originalReport}
/>
);
}

ModifiedExpenseContent.displayName = 'ModifiedExpenseContent';

export default ModifiedExpenseContent;
24 changes: 24 additions & 0 deletions src/pages/inbox/report/actionContents/ReimbursedContent.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import {getReimbursedMessage} from '@libs/ReportActionsUtils';
import ReportActionItemBasicMessage from '@pages/inbox/report/ReportActionItemBasicMessage';
import type {Report, ReportAction} from '@src/types/onyx';

type ReimbursedContentProps = {
action: ReportAction;
report: OnyxEntry<Report>;
};

function ReimbursedContent({action, report}: ReimbursedContentProps) {
const {translate} = useLocalize();
const {accountID: currentUserAccountID} = useCurrentUserPersonalDetails();
const message = getReimbursedMessage(translate, action, report, currentUserAccountID);

return <ReportActionItemBasicMessage message={message} />;
}

ReimbursedContent.displayName = 'ReimbursedContent';

export default ReimbursedContent;
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import {getReimbursementDeQueuedOrCanceledActionMessage} from '@libs/ReportUtils';
import ReportActionItemBasicMessage from '@pages/inbox/report/ReportActionItemBasicMessage';
import type CONST from '@src/CONST';
import type {Report, ReportAction} from '@src/types/onyx';

type ReimbursementDeQueuedContentProps = {
action: ReportAction;
report: OnyxEntry<Report>;
};

function ReimbursementDeQueuedContent({action, report}: ReimbursementDeQueuedContentProps) {
const {translate} = useLocalize();
const message = getReimbursementDeQueuedOrCanceledActionMessage(
translate,
action as OnyxEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_DEQUEUED | typeof CONST.REPORT.ACTIONS.TYPE.REIMBURSEMENT_ACH_CANCELED>>,
report,
);

return <ReportActionItemBasicMessage message={message} />;
}

ReimbursementDeQueuedContent.displayName = 'ReimbursementDeQueuedContent';

export default ReimbursementDeQueuedContent;
Loading
Loading