Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Expense details - Violation is not displayed on description field when opening IOU details. #56405

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+106 −73
Open

fix: Expense details - Violation is not displayed on description field when opening IOU details. #56405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1e084f2
fix: Expense details - Violation is not displayed on description fiel…
Krishna2323 Feb 5, 2025
fafe6d4
TS issues.
Krishna2323 Feb 5, 2025
3c2ab26
Merge branch 'Expensify:main' into krishna2323/issue/56310
Krishna2323 Feb 5, 2025
1143243
create useTransactionViolations hook.
Krishna2323 Feb 5, 2025
fc381b2
fix TS fails.
Krishna2323 Feb 5, 2025
acb2a64
fix Jest Unit Tests.
Krishna2323 Feb 5, 2025
6913635
minor fix.
Krishna2323 Feb 5, 2025
083bf1e
update as per requested changes.
Krishna2323 Feb 5, 2025
17fcb63
optimize onyx transaction violations access.
Krishna2323 Feb 6, 2025
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
4 changes: 2 additions & 2 deletions src/components/BrokenConnectionDescription.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import {isInstantSubmitEnabled, isPolicyAdmin as isPolicyAdminPolicyUtils} from '@libs/PolicyUtils';
import {isCurrentUserSubmitter, isProcessingReport, isReportApproved, isReportManuallyReimbursed} from '@libs/ReportUtils';
import {getTransactionViolations} from '@libs/TransactionUtils';
import Navigation from '@navigation/Navigation';
import CONST from '@src/CONST';
import ROUTES from '@src/ROUTES';
Expand All @@ -24,7 +24,7 @@ type BrokenConnectionDescriptionProps = {
function BrokenConnectionDescription({transactionID, policy, report}: BrokenConnectionDescriptionProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const transactionViolations = getTransactionViolations(transactionID);
const transactionViolations = useTransactionViolations(transactionID);

const brokenConnection530Error = transactionViolations?.find((violation) => violation.data?.rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION_530);
const brokenConnectionError = transactionViolations?.find((violation) => violation.data?.rterType === CONST.RTER_VIOLATION_TYPES.BROKEN_CARD_CONNECTION);
Expand Down
8 changes: 6 additions & 2 deletions src/components/MoneyReportHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,12 @@ function MoneyReportHeader({policy, report: moneyRequestReport, transactionThrea
return !!transactions && transactions.length > 0 && transactions.every((t) => isExpensifyCardTransaction(t) && isPending(t));
}, [transactions]);
const transactionIDs = transactions?.map((t) => t.transactionID) ?? [];
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactionIDs);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationTransactionUtils(transactionIDs, moneyRequestReport, policy);
const [violations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {
selector: (allTransactions) =>
Object.fromEntries(Object.entries(allTransactions ?? {}).filter(([key]) => transactionIDs.includes(key.replace(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, '')))),
});
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactionIDs, violations);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationTransactionUtils(transactionIDs, moneyRequestReport, policy, violations);
const hasOnlyHeldExpenses = hasOnlyHeldExpensesReportUtils(moneyRequestReport?.reportID);
const isPayAtEndExpense = isPayAtEndExpenseTransactionUtils(transaction);
const isArchivedReport = isArchivedReportWithID(moneyRequestReport?.reportID);
Expand Down
10 changes: 6 additions & 4 deletions src/components/MoneyRequestHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import {markAsCash as markAsCashUtil} from '@libs/actions/Transaction';
import Navigation from '@libs/Navigation/Navigation';
import {isPolicyAdmin} from '@libs/PolicyUtils';
import {getOriginalMessage, isMoneyRequestAction} from '@libs/ReportActionsUtils';
import {isCurrentUserSubmitter} from '@libs/ReportUtils';
import {
allHavePendingRTERViolation,
getTransactionViolations,
hasPendingRTERViolation,
hasReceipt,
isDuplicate as isDuplicateTransactionUtils,
Expand Down Expand Up @@ -67,6 +67,8 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
isMoneyRequestAction(parentReportAction) ? getOriginalMessage(parentReportAction)?.IOUTransactionID ?? CONST.DEFAULT_NUMBER_ID : CONST.DEFAULT_NUMBER_ID
}`,
);
const transactionViolations = useTransactionViolations(transaction?.transactionID);
const [allTransactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [dismissedHoldUseExplanation, dismissedHoldUseExplanationResult] = useOnyx(ONYXKEYS.NVP_DISMISSED_HOLD_USE_EXPLANATION, {initialValue: true});
const [isLoadingReportData] = useOnyx(ONYXKEYS.IS_LOADING_REPORT_DATA);
const isLoadingHoldUseExplained = isLoadingOnyxValue(dismissedHoldUseExplanationResult);
Expand All @@ -80,9 +82,9 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
const isReportInRHP = route.name === SCREENS.SEARCH.REPORT_RHP;
const shouldDisplaySearchRouter = !isReportInRHP || isSmallScreenWidth;
const transactionIDList = transaction ? [transaction.transactionID] : [];
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactionIDList);
const hasAllPendingRTERViolations = allHavePendingRTERViolation(transactionIDList, allTransactionViolations);

const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationTransactionUtils(transactionIDList, parentReport, policy);
const shouldShowBrokenConnectionViolation = shouldShowBrokenConnectionViolationTransactionUtils(transactionIDList, parentReport, policy, allTransactionViolations);

const shouldShowMarkAsCashButton = hasAllPendingRTERViolations || (shouldShowBrokenConnectionViolation && (!isPolicyAdmin(policy) || isCurrentUserSubmitter(parentReport?.reportID)));

Expand Down Expand Up @@ -121,7 +123,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
),
};
}
if (hasPendingRTERViolation(getTransactionViolations(transaction?.transactionID))) {
if (hasPendingRTERViolation(transactionViolations)) {
return {icon: getStatusIcon(Expensicons.Hourglass), description: translate('iou.pendingMatchWithCreditCardDescription')};
}
if (isScanning) {
Expand Down
17 changes: 10 additions & 7 deletions src/components/ReportActionItem/MoneyRequestPreview/MoneyRequestPreviewContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import useWindowDimensions from '@hooks/useWindowDimensions';
import ControlSelection from '@libs/ControlSelection';
import {convertToDisplayString} from '@libs/CurrencyUtils';
Expand All @@ -48,7 +49,6 @@ import type {TransactionDetails} from '@libs/ReportUtils';
import StringUtils from '@libs/StringUtils';
import {
compareDuplicateTransactionFields,
getTransactionViolations,
hasMissingSmartscanFields,
hasNoticeTypeViolation as hasNoticeTypeViolationTransactionUtils,
hasPendingUI,
Expand Down Expand Up @@ -112,8 +112,11 @@ function MoneyRequestPreviewContent({
const transactionID = isMoneyRequestAction ? getOriginalMessage(action)?.IOUTransactionID : undefined;
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [walletTerms] = useOnyx(ONYXKEYS.WALLET_TERMS);
const [allViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const transactionViolations = getTransactionViolations(transaction?.transactionID);
const [violations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not use transactionViolations instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean the naming?

Copy link
Contributor

Choose a reason for hiding this comment

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

No. I meant reuse const transactionViolations = useTransactionViolations(transaction?.transactionID); instead of violations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rojiphil, we need OnyxCollection for hasViolationTransactionUtils, hasNoticeTypeViolationTransactionUtils & hasWarningTypeViolationTransactionUtils.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Thanks.

selector: (allTransactions) =>
Object.fromEntries(Object.entries(allTransactions ?? {}).filter(([key]) => transaction?.transactionID === key.replace(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, ''))),
});
const transactionViolations = useTransactionViolations(transaction?.transactionID);

const sessionAccountID = session?.accountID;
const managerID = iouReport?.managerID ?? CONST.DEFAULT_NUMBER_ID;
Expand Down Expand Up @@ -146,9 +149,9 @@ function MoneyRequestPreviewContent({
const isOnHold = isOnHoldTransactionUtils(transaction);
const isSettlementOrApprovalPartial = !!iouReport?.pendingFields?.partial;
const isPartialHold = isSettlementOrApprovalPartial && isOnHold;
const hasViolations = hasViolationTransactionUtils(transaction?.transactionID, allViolations, true);
const hasNoticeTypeViolations = hasNoticeTypeViolationTransactionUtils(transaction?.transactionID, allViolations, true) && isPaidGroupPolicy(iouReport);
const hasWarningTypeViolations = hasWarningTypeViolationTransactionUtils(transaction?.transactionID, allViolations, true);
const hasViolations = hasViolationTransactionUtils(transaction, violations, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why didn't we pass transaction?.transactionID?

const hasNoticeTypeViolations = hasNoticeTypeViolationTransactionUtils(transaction?.transactionID, violations, true) && isPaidGroupPolicy(iouReport);
const hasWarningTypeViolations = hasWarningTypeViolationTransactionUtils(transaction?.transactionID, violations, true);
const hasFieldErrors = hasMissingSmartscanFields(transaction);
const isDistanceRequest = isDistanceRequestTransactionUtils(transaction);
const isPerDiemRequest = isPerDiemRequestTransactionUtils(transaction);
Expand Down Expand Up @@ -281,7 +284,7 @@ function MoneyRequestPreviewContent({
if (isPending(transaction)) {
return {shouldShow: true, messageIcon: Expensicons.CreditCardHourglass, messageDescription: translate('iou.transactionPending')};
}
if (shouldShowBrokenConnectionViolation(transaction ? [transaction.transactionID] : [], iouReport, policy)) {
if (shouldShowBrokenConnectionViolation(transaction ? [transaction.transactionID] : [], iouReport, policy, violations)) {
return {shouldShow: true, messageIcon: Expensicons.Hourglass, messageDescription: translate('violations.brokenConnection530Error')};
}
if (hasPendingUI(transaction, transactionViolations)) {
Expand Down
4 changes: 2 additions & 2 deletions src/components/ReportActionItem/MoneyRequestView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import useViolations from '@hooks/useViolations';
import type {ViolationField} from '@hooks/useViolations';
import {convertToDisplayString} from '@libs/CurrencyUtils';
Expand Down Expand Up @@ -49,7 +50,6 @@ import {
getDistanceInMeters,
getTagForDisplay,
getTaxName,
getTransactionViolations,
hasMissingSmartscanFields,
hasReceipt as hasReceiptTransactionUtils,
hasReservationList,
Expand Down Expand Up @@ -134,7 +134,7 @@ function MoneyRequestView({report, shouldShowAnimatedBackground, readonly = fals

const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${linkedTransactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const [transactionBackup] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_BACKUP}${linkedTransactionID ?? CONST.DEFAULT_NUMBER_ID}`);
const transactionViolations = getTransactionViolations(linkedTransactionID);
const transactionViolations = useTransactionViolations(transaction?.transactionID);

const {
created: transactionDate,
Expand Down
19 changes: 10 additions & 9 deletions src/components/ReportActionItem/ReportPreview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import useNetwork from '@hooks/useNetwork';
import usePolicy from '@hooks/usePolicy';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useTransactionViolations from '@hooks/useTransactionViolations';
import ControlSelection from '@libs/ControlSelection';
import {convertToDisplayString} from '@libs/CurrencyUtils';
import {canUseTouchScreen} from '@libs/DeviceCapabilities';
Expand Down Expand Up @@ -71,7 +72,6 @@ import StringUtils from '@libs/StringUtils';
import {
getDescription,
getMerchant,
getTransactionViolations,
hasPendingUI,
isCardTransaction,
isPartialMerchant,
Expand Down Expand Up @@ -147,7 +147,8 @@ function ReportPreview({
const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, {
selector: (_transactions) => reportTransactionsSelector(_transactions, iouReportID),
});
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const lastTransaction = transactions?.at(0);
const [allTransactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
const [userWallet] = useOnyx(ONYXKEYS.USER_WALLET);
const [invoiceReceiverPolicy] = useOnyx(
`${ONYXKEYS.COLLECTION.POLICY}${chatReport?.invoiceReceiver && 'policyID' in chatReport.invoiceReceiver ? chatReport.invoiceReceiver.policyID : CONST.DEFAULT_NUMBER_ID}`,
Expand Down Expand Up @@ -229,17 +230,17 @@ function ReportPreview({
const hasErrors =
(hasMissingSmartscanFields && !iouSettled) ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
hasViolations(iouReportID, transactionViolations, true) ||
hasNoticeTypeViolations(iouReportID, transactionViolations, true) ||
hasWarningTypeViolations(iouReportID, transactionViolations, true) ||
hasViolations(iouReportID, allTransactionViolations, true) ||
hasNoticeTypeViolations(iouReportID, allTransactionViolations, true) ||
hasWarningTypeViolations(iouReportID, allTransactionViolations, true) ||
(isReportOwner(iouReport) && hasReportViolations(iouReportID)) ||
hasActionsWithErrors(iouReportID);
const lastThreeTransactions = transactions?.slice(-3) ?? [];
const lastTransaction = transactions?.at(0);
const lastThreeReceipts = lastThreeTransactions.map((transaction) => ({...getThumbnailAndImageURIs(transaction), transaction}));
const transactionIDList = transactions?.map((reportTransaction) => reportTransaction.transactionID) ?? [];
const showRTERViolationMessage = numberOfRequests === 1 && hasPendingUI(lastTransaction, getTransactionViolations(lastTransaction?.transactionID));
const shouldShowBrokenConnectionViolation = numberOfRequests === 1 && shouldShowBrokenConnectionViolationTransactionUtils(transactionIDList, iouReport, policy);
const lastTransactionViolations = useTransactionViolations(lastTransaction?.transactionID);
const showRTERViolationMessage = numberOfRequests === 1 && hasPendingUI(lastTransaction, lastTransactionViolations);
const shouldShowBrokenConnectionViolation = numberOfRequests === 1 && shouldShowBrokenConnectionViolationTransactionUtils(transactionIDList, iouReport, policy, allTransactionViolations);
let formattedMerchant = numberOfRequests === 1 ? getMerchant(lastTransaction) : null;
const formattedDescription = numberOfRequests === 1 ? getDescription(lastTransaction) : null;

Expand All @@ -250,7 +251,7 @@ function ReportPreview({
const isArchived = isArchivedReportWithID(iouReport?.reportID);
const isAdmin = policy?.role === CONST.POLICY.ROLE.ADMIN;
const filteredTransactions = transactions?.filter((transaction) => transaction) ?? [];
const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions);
const shouldShowSubmitButton = canSubmitReport(iouReport, policy, filteredTransactions, allTransactionViolations);

const shouldDisableSubmitButton = shouldShowSubmitButton && !isAllowedToSubmitDraftExpenseReport(iouReport);

Expand Down
13 changes: 13 additions & 0 deletions src/hooks/useTransactionViolations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import {useMemo} from 'react';
import {useOnyx} from 'react-native-onyx';
import {isViolationDismissed} from '@libs/TransactionUtils';
import ONYXKEYS from '@src/ONYXKEYS';
import type {TransactionViolations} from '@src/types/onyx';

function useTransactionViolations(transactionID?: string): TransactionViolations {
const [transaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`);
const [transactionViolations = []] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`);
return useMemo(() => transactionViolations.filter((violation) => !isViolationDismissed(transaction, violation)), [transaction, transactionViolations]);
}

roryabraham marked this conversation as resolved.
Show resolved Hide resolved
export default useTransactionViolations;
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6920,7 +6920,7 @@ function hasViolations(
reportTransactions?: SearchTransaction[],
): boolean {
const transactions = reportTransactions ?? getReportTransactions(reportID);
return transactions.some((transaction) => hasViolation(transaction.transactionID, transactionViolations, shouldShowInReview));
return transactions.some((transaction) => hasViolation(transaction, transactionViolations, shouldShowInReview));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question. Why not transaction.transactionID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rojiphil, the util function hasViolation requires the transaction object for passing it to isExpensifyCardTransaction, isPending & isViolationDismissed so it's preferred to pass the transaction directly.

}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/libs/SearchUIUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
}

// We check for isAllowedToApproveExpenseReport because if the policy has preventSelfApprovals enabled, we disable the Submit action and in that case we want to show the View action instead
if (canSubmitReport(report, policy, allReportTransactions) && isAllowedToApproveExpenseReport) {
if (canSubmitReport(report, policy, allReportTransactions, allViolations) && isAllowedToApproveExpenseReport) {
return CONST.SEARCH.ACTION_TYPES.SUBMIT;
}

Expand Down
Loading