Skip to content
Merged
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
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
9 changes: 5 additions & 4 deletions src/components/MoneyRequestHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import useOnyx from '@hooks/useOnyx';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import {deleteMoneyRequest, deleteTrackExpense, initSplitExpense} from '@libs/actions/IOU';
import {setupMergeTransactionData} from '@libs/actions/MergeTransaction';
import Navigation from '@libs/Navigation/Navigation';
Expand All @@ -34,7 +33,7 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type {Policy, Report, ReportAction} from '@src/types/onyx';
import type {Policy, Report, ReportAction, TransactionViolation} from '@src/types/onyx';
import type IconAsset from '@src/types/utils/IconAsset';
import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import BrokenConnectionDescription from './BrokenConnectionDescription';
Expand Down Expand Up @@ -65,9 +64,12 @@ type MoneyRequestHeaderProps = {

/** Method to trigger when pressing close button of the header */
onBackButtonPress: () => void;

/** Precomputed violations for the transaction */
transactionViolations?: TransactionViolation[];
};

function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPress}: MoneyRequestHeaderProps) {
function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPress, transactionViolations = []}: MoneyRequestHeaderProps) {
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.

Let's not default to a new, empty array. Maybe we shouldn't default at all and guard later?

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 utility functions already handle undefined gracefully and defaulting to [] is a common React pattern. Without it, we'd need to add ?? [] everywhere we use it. The empty array is reused so no perf impact IMO. Why you think we should not default to an empty array?

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.

Oh it's just this is a new array every time, not a reused reference to an existing one.

// We need to use isSmallScreenWidth instead of shouldUseNarrowLayout to use a correct layout for the hold expense modal https://github.com/Expensify/App/pull/47990#issuecomment-2362382026
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
const {shouldUseNarrowLayout, isSmallScreenWidth} = useResponsiveLayout();
Expand All @@ -81,7 +83,6 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
}`,
{canBeMissing: true},
);
const transactionViolations = useTransactionViolations(transaction?.transactionID);
const {duplicateTransactions, duplicateTransactionViolations} = useDuplicateTransactionsAndViolations(transaction?.transactionID ? [transaction.transactionID] : []);
const [isDeleteModalVisible, setIsDeleteModalVisible] = useState(false);
const [downloadErrorModalVisible, setDownloadErrorModalVisible] = useState(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Text from '@components/Text';
import useCopySelectionHelper from '@hooks/useCopySelectionHelper';
import useLocalize from '@hooks/useLocalize';
import useMobileSelectionMode from '@hooks/useMobileSelectionMode';
import useReportWithTransactionsAndViolations from '@hooks/useReportWithTransactionsAndViolations';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -100,6 +101,7 @@ function MoneyRequestReportTransactionList({
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const {translate, localeCompare} = useLocalize();
const [, , violations] = useReportWithTransactionsAndViolations(report.reportID);
// eslint-disable-next-line rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth
const {shouldUseNarrowLayout, isSmallScreenWidth, isMediumScreenWidth} = useResponsiveLayout();
const [isModalVisible, setIsModalVisible] = useState(false);
Expand Down Expand Up @@ -157,8 +159,9 @@ function MoneyRequestReportTransactionList({
.map((transaction) => ({
...transaction,
shouldBeHighlighted: newTransactions?.includes(transaction),
violations: violations?.[transaction.transactionID] ?? [],
}));
}, [newTransactions, sortBy, sortOrder, transactions, localeCompare]);
}, [newTransactions, sortBy, sortOrder, transactions, localeCompare, violations]);

const navigateToTransaction = useCallback(
(activeTransactionID: string) => {
Expand Down
55 changes: 31 additions & 24 deletions src/components/MoneyRequestReportView/MoneyRequestReportView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import getNonEmptyStringOnyxID from '@libs/getNonEmptyStringOnyxID';
import Log from '@libs/Log';
import {getAllNonDeletedTransactions, shouldDisplayReportTableView, shouldWaitForTransactions as shouldWaitForTransactionsUtil} from '@libs/MoneyRequestReportUtils';
import navigationRef from '@libs/Navigation/navigationRef';
import {getFilteredReportActionsForReportView, getOneTransactionThreadReportID, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {getFilteredReportActionsForReportView, getOneTransactionThreadReportID, getTransactionIDFromReportAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {canEditReportAction, getReportOfflinePendingActionAndErrors, isReportTransactionThread} from '@libs/ReportUtils';
import {buildCannedSearchQuery} from '@libs/SearchQueryUtils';
import Navigation from '@navigation/Navigation';
Expand Down Expand Up @@ -99,7 +99,7 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
const {reportActions: unfilteredReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID);
const reportActions = getFilteredReportActionsForReportView(unfilteredReportActions);

const {transactions: reportTransactions} = useTransactionsAndViolationsForReport(reportID);
const {transactions: reportTransactions, violations: reportViolations} = useTransactionsAndViolationsForReport(reportID);
const transactions = useMemo(() => getAllNonDeletedTransactions(reportTransactions, reportActions), [reportTransactions, reportActions]);

const visibleTransactions = transactions?.filter((transaction) => isOffline || transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE);
Expand Down Expand Up @@ -133,13 +133,18 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
const isEmptyTransactionReport = visibleTransactions && visibleTransactions.length === 0 && transactionThreadReportID === undefined;
const shouldDisplayMoneyRequestActionsList = !!isEmptyTransactionReport || shouldDisplayReportTableView(report, visibleTransactions ?? []);

const reportHeaderView = useMemo(
() =>
isTransactionThreadView ? (
const reportHeaderView = useMemo(() => {
if (isTransactionThreadView) {
// Extract transaction ID from parent report action to get specific violations
const transactionID = getTransactionIDFromReportAction(parentReportAction);
const transactionViolations = transactionID && reportViolations ? (reportViolations as Record<string, OnyxTypes.TransactionViolations>)[transactionID] : undefined;

return (
<MoneyRequestHeader
report={report}
policy={policy}
parentReportAction={parentReportAction}
transactionViolations={transactionViolations}
onBackButtonPress={() => {
if (!backToRoute) {
goBackFromSearchMoneyRequest();
Expand All @@ -148,25 +153,27 @@ function MoneyRequestReportView({report, policy, reportMetadata, shouldDisplayRe
Navigation.goBack(backToRoute);
}}
/>
) : (
<MoneyReportHeader
report={report}
policy={policy}
reportActions={reportActions}
transactionThreadReportID={transactionThreadReportID}
isLoadingInitialReportActions={isLoadingInitialReportActions}
shouldDisplayBackButton
onBackButtonPress={() => {
if (!backToRoute) {
goBackFromSearchMoneyRequest();
return;
}
Navigation.goBack(backToRoute);
}}
/>
),
[backToRoute, isLoadingInitialReportActions, isTransactionThreadView, parentReportAction, policy, report, reportActions, transactionThreadReportID],
);
);
}

return (
<MoneyReportHeader
report={report}
policy={policy}
reportActions={reportActions}
transactionThreadReportID={transactionThreadReportID}
isLoadingInitialReportActions={isLoadingInitialReportActions}
shouldDisplayBackButton
onBackButtonPress={() => {
if (!backToRoute) {
goBackFromSearchMoneyRequest();
return;
}
Navigation.goBack(backToRoute);
}}
/>
);
}, [backToRoute, isLoadingInitialReportActions, isTransactionThreadView, parentReportAction, policy, report, reportActions, reportViolations, transactionThreadReportID]);

if (!!(isLoadingInitialReportActions && reportActions.length === 0 && !isOffline) || shouldWaitForTransactions) {
return <InitialLoadingSkeleton styles={styles} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ import useLocalize from '@hooks/useLocalize';
import useOnyx from '@hooks/useOnyx';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import {getIOUActionForTransactionID} from '@libs/ReportActionsUtils';
import ViolationsUtils from '@libs/Violations/ViolationsUtils';
import variables from '@styles/variables';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Report} from '@src/types/onyx';
import type {Report, TransactionViolation} from '@src/types/onyx';
import type Transaction from '@src/types/onyx/Transaction';

type TransactionItemRowRBRProps = {
Expand All @@ -28,11 +27,13 @@ type TransactionItemRowRBRProps = {

/** Error message for missing required fields in the transaction */
missingFieldError?: string;

/** Precomputed violations for the transaction */
violations?: TransactionViolation[];
};

function TransactionItemRowRBRWithOnyx({transaction, report, containerStyles, missingFieldError}: TransactionItemRowRBRProps) {
function TransactionItemRowRBRWithOnyx({transaction, report, containerStyles, missingFieldError, violations = []}: TransactionItemRowRBRProps) {
const styles = useThemeStyles();
const transactionViolations = useTransactionViolations(transaction?.transactionID, false);
const {translate} = useLocalize();
const theme = useTheme();
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${transaction.reportID}`, {
Expand All @@ -44,7 +45,7 @@ function TransactionItemRowRBRWithOnyx({transaction, report, containerStyles, mi
canBeMissing: true,
});

const RBRMessages = ViolationsUtils.getRBRMessages(transaction, transactionViolations, translate, missingFieldError, Object.values(transactionThreadActions ?? {}), policyTags);
const RBRMessages = ViolationsUtils.getRBRMessages(transaction, violations, translate, missingFieldError, Object.values(transactionThreadActions ?? {}), policyTags);

return (
RBRMessages.length > 0 && (
Expand Down
2 changes: 2 additions & 0 deletions src/components/TransactionItemRow/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ function TransactionItemRow({
report={report}
containerStyles={[styles.mt2, styles.minHeight4]}
missingFieldError={missingFieldError}
violations={transactionItem.violations}
/>
)}
</View>
Expand Down Expand Up @@ -501,6 +502,7 @@ function TransactionItemRow({
transaction={transactionItem}
report={report}
missingFieldError={missingFieldError}
violations={transactionItem.violations}
/>
)}
</View>
Expand Down
8 changes: 8 additions & 0 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
type MemberChangeMessageElement = MessageTextElement | MemberChangeMessageUserMentionElement | MemberChangeMessageRoomReferenceElement;

let allReportActions: OnyxCollection<ReportActions>;
Onyx.connect({

Check warning on line 60 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand All @@ -69,7 +69,7 @@
});

let allReports: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 72 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -78,14 +78,14 @@
});

let isNetworkOffline = false;
Onyx.connect({

Check warning on line 81 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NETWORK,
callback: (val) => (isNetworkOffline = val?.isOffline ?? false),
});

let currentUserAccountID: number | undefined;
let currentEmail = '';
Onyx.connect({

Check warning on line 88 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, value is undefined
Expand All @@ -99,7 +99,7 @@
});

let privatePersonalDetails: PrivatePersonalDetails | undefined;
Onyx.connect({

Check warning on line 102 in src/libs/ReportActionsUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PRIVATE_PERSONAL_DETAILS,
callback: (personalDetails) => {
privatePersonalDetails = personalDetails;
Expand Down Expand Up @@ -267,6 +267,13 @@
return reportAction.originalMessage;
}

/**
* Get the transaction ID from a money request report action
*/
function getTransactionIDFromReportAction(reportAction: OnyxInputOrEntry<ReportAction>): string | undefined {
return isMoneyRequestAction(reportAction) ? getOriginalMessage(reportAction)?.IOUTransactionID : undefined;
}

function isExportIntegrationAction(reportAction: OnyxInputOrEntry<ReportAction>): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.EXPORTED_TO_INTEGRATION> {
return reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.EXPORTED_TO_INTEGRATION;
}
Expand Down Expand Up @@ -3021,6 +3028,7 @@
getSortedReportActionsForDisplay,
getTextFromHtml,
getTrackExpenseActionableWhisper,
getTransactionIDFromReportAction,
getWhisperedTo,
hasRequestFromCurrentAccount,
isActionOfType,
Expand Down
7 changes: 6 additions & 1 deletion src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
getCombinedReportActions,
getFilteredReportActionsForReportView,
getOneTransactionThreadReportID,
getTransactionIDFromReportAction,
isCreatedAction,
isDeletedParentAction,
isMoneyRequestAction,
Expand Down Expand Up @@ -295,7 +296,7 @@ function ReportScreen({route, navigation}: ReportScreenProps) {
// If the count is too high (equal to or exceeds the web pagination size / 50) and there are no cached messages in the report,
// OpenReport will be called each time the user scrolls up the report a bit, clicks on report preview, and then goes back.
const isLinkedMessagePageReady = isLinkedMessageAvailable && (reportActions.length - indexOfLinkedMessage >= CONST.REPORT.MIN_INITIAL_REPORT_ACTION_COUNT || doesCreatedActionExists());
const {transactions: allReportTransactions} = useTransactionsAndViolationsForReport(reportIDFromRoute);
const {transactions: allReportTransactions, violations: allReportViolations} = useTransactionsAndViolationsForReport(reportIDFromRoute);

const reportTransactions = useMemo(() => getAllNonDeletedTransactions(allReportTransactions, reportActions), [allReportTransactions, reportActions]);
// wrapping in useMemo because this is array operation and can cause performance issues
Expand Down Expand Up @@ -370,12 +371,16 @@ function ReportScreen({route, navigation}: ReportScreenProps) {
);

if (isTransactionThreadView) {
const transactionID = getTransactionIDFromReportAction(parentReportAction);
const transactionViolations = transactionID && allReportViolations ? (allReportViolations as Record<string, OnyxTypes.TransactionViolations>)[transactionID] : undefined;

headerView = (
<MoneyRequestHeader
report={report}
policy={policy}
parentReportAction={parentReportAction}
onBackButtonPress={onBackButtonPress}
transactionViolations={transactionViolations}
/>
);
}
Expand Down
Loading