Skip to content
Open
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
37 changes: 23 additions & 14 deletions src/components/MoneyRequestConfirmationList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@

/** Show remove expense confirmation modal */
showRemoveExpenseConfirmModal?: () => void;

Copy link
Contributor

Choose a reason for hiding this comment

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

❌ CLEAN-REACT-PATTERNS-1 (docs)

Adding a boolean flag prop shouldHideToSection to conditionally show/hide the "To:" section is the configuration-over-composition anti-pattern (Case 1). This prop introduces if/else branching inside the component body to control feature presence. Each new boolean flag like this increases the component's surface area and coupling.

Instead of adding a boolean flag, consider having MoneyRequestConfirmationList derive this behavior internally. The component already has access to reportID and fetches transactionReport via useOnyx — it could call isMoneyRequestReport itself to determine whether to show the "To:" section, rather than being told what to do from the parent via a prop.


Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

/** When true, hide the "To:" section (e.g. when adding an expense directly to the current report) */
shouldHideToSection?: boolean;
};

type MoneyRequestConfirmationListItem = (Participant & {keyForList: string}) | OptionData;
Expand Down Expand Up @@ -266,6 +269,7 @@
isTimeRequest = false,
iouTimeCount,
iouTimeRate,
shouldHideToSection = false,
}: MoneyRequestConfirmationListProps) {
const [policyCategoriesReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`, {canBeMissing: true});
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {canBeMissing: true});
Expand Down Expand Up @@ -833,19 +837,22 @@
},
);
} else {
const formattedSelectedParticipants = selectedParticipants.map((participant) => ({
...participant,
isSelected: false,
keyForList: `${participant.keyForList ?? participant.accountID ?? participant.reportID}`,
isInteractive: isFromGlobalCreateAndCanEditParticipant && !isTestReceipt && (!isRestrictedToPreferredPolicy || isTypeInvoice),
shouldShowRightCaret: isFromGlobalCreateAndCanEditParticipant && !isTestReceipt && (!isRestrictedToPreferredPolicy || isTypeInvoice),
}));

options.push({
title: translate('common.to'),
data: formattedSelectedParticipants,
sectionIndex: 0,
});
// When adding an expense from within a report, hide the "To:" section since the destination is already the current report
if (!shouldHideToSection) {

Check failure on line 841 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unexpected if as the only statement in an else block

Check failure on line 841 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Unexpected if as the only statement in an else block

Check failure on line 841 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unexpected if as the only statement in an else block

Check failure on line 841 in src/components/MoneyRequestConfirmationList.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Unexpected if as the only statement in an else block
const formattedSelectedParticipants = selectedParticipants.map((participant) => ({
...participant,
isSelected: false,
keyForList: `${participant.keyForList ?? participant.accountID ?? participant.reportID}`,
isInteractive: isFromGlobalCreateAndCanEditParticipant && !isTestReceipt && (!isRestrictedToPreferredPolicy || isTypeInvoice),
shouldShowRightCaret: isFromGlobalCreateAndCanEditParticipant && !isTestReceipt && (!isRestrictedToPreferredPolicy || isTypeInvoice),
}));

options.push({
title: translate('common.to'),
data: formattedSelectedParticipants,
sectionIndex: 0,
});
}
}

return options;
Expand All @@ -860,6 +867,7 @@
isTestReceipt,
isRestrictedToPreferredPolicy,
isTypeInvoice,
shouldHideToSection,
]);

useEffect(() => {
Expand Down Expand Up @@ -1358,5 +1366,6 @@
prevProps.shouldDisplayReceipt === nextProps.shouldDisplayReceipt &&
prevProps.isTimeRequest === nextProps.isTimeRequest &&
prevProps.iouTimeCount === nextProps.iouTimeCount &&
prevProps.iouTimeRate === nextProps.iouTimeRate,
prevProps.iouTimeRate === nextProps.iouTimeRate &&
prevProps.shouldHideToSection === nextProps.shouldHideToSection,
);
3 changes: 3 additions & 0 deletions src/pages/iou/request/step/IOURequestStepConfirmation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
generateReportID,
getReportOrDraftReport,
hasViolations as hasViolationsReportUtils,
isMoneyRequestReport,
isProcessingReport,
isReportOutstanding,
isSelectedManagerMcTest,
Expand Down Expand Up @@ -173,6 +174,7 @@ function IOURequestStepConfirmation({
const canUseReport = !(isProcessingReport(transactionReport) && !policyReal?.harvesting?.enabled) && isReportOutstanding(transactionReport, policyReal?.id, undefined, false);

const shouldUseTransactionReport = !!transactionReport && (canUseReport || !reportWithDraftFallback);
const shouldHideToSection = useMemo(() => isMoneyRequestReport(reportWithDraftFallback), [reportWithDraftFallback]);
const isTransactionReportDifferentFromRoute = useMemo(
() => !!transaction?.reportID && !!reportWithDraftFallback?.reportID && transaction.reportID !== reportWithDraftFallback.reportID,
[reportWithDraftFallback?.reportID, transaction?.reportID],
Expand Down Expand Up @@ -1488,6 +1490,7 @@ function IOURequestStepConfirmation({
isTimeRequest={isTimeRequest}
iouTimeCount={transaction?.comment?.units?.count}
iouTimeRate={transaction?.comment?.units?.rate}
shouldHideToSection={shouldHideToSection}
/>
</View>
</DragAndDropProvider>
Expand Down
Loading