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.

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

Add violations to Search actions #54847

Merged
merged 12 commits into from
Jan 14, 2025
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@
"react-native-launch-arguments": "^4.0.2",
"react-native-localize": "^2.2.6",
"react-native-modal": "^13.0.0",
"react-native-onyx": "2.0.86",
"react-native-onyx": "2.0.87",
"react-native-pager-view": "6.5.1",
"react-native-pdf": "6.7.3",
"react-native-performance": "^5.1.0",
Expand Down
9 changes: 7 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6624,8 +6624,13 @@ function shouldDisplayViolationsRBRInLHN(report: OnyxEntry<Report>, transactionV
/**
* Checks to see if a report contains a violation
*/
function hasViolations(reportID: string | undefined, transactionViolations: OnyxCollection<TransactionViolation[]>, shouldShowInReview?: boolean): boolean {
const transactions = getReportTransactions(reportID);
function hasViolations(
reportID: string | undefined,
transactionViolations: OnyxCollection<TransactionViolation[]>,
shouldShowInReview?: boolean,
reportTransactions?: SearchTransaction[],
): boolean {
const transactions = reportTransactions ?? getReportTransactions(reportID);
return transactions.some((transaction) => hasViolation(transaction.transactionID, transactionViolations, shouldShowInReview));
}

Expand Down
63 changes: 47 additions & 16 deletions src/libs/SearchUIUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type {OnyxCollection} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {SearchColumnType, SearchStatus, SortOrder} from '@components/Search/types';
import ChatListItem from '@components/SelectionList/ChatListItem';
Expand All @@ -18,7 +19,15 @@ import DateUtils from './DateUtils';
import {translateLocal} from './Localize';
import Navigation from './Navigation/Navigation';
import {isAddCommentAction, isDeletedAction} from './ReportActionsUtils';
import {hasOnlyHeldExpenses, isAllowedToApproveExpenseReport as isAllowedToApproveExpenseReportUtils, isClosedReport, isInvoiceReport, isMoneyRequestReport, isSettled} from './ReportUtils';
import {
hasOnlyHeldExpenses,
hasViolations,
isAllowedToApproveExpenseReport as isAllowedToApproveExpenseReportUtils,
isClosedReport,
isInvoiceReport,
isMoneyRequestReport,
isSettled,
} from './ReportUtils';
import {getAmount as getTransactionAmount, getCreated as getTransactionCreatedDate, getMerchant as getTransactionMerchant, isExpensifyCardTransaction, isPending} from './TransactionUtils';

const columnNamesToSortingProperty = {
Expand Down Expand Up @@ -49,6 +58,8 @@ type TransactionKey = `${typeof ONYXKEYS.COLLECTION.TRANSACTION}${string}`;

type ReportActionKey = `${typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS}${string}`;

type ViolationKey = `${typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${string}`;

/**
* @private
*
Expand Down Expand Up @@ -91,6 +102,13 @@ function isTransactionEntry(key: string): key is TransactionKey {
return key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION);
}

/**
* @private
*/
function isViolationEntry(key: string): key is ViolationKey {
return key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
}

/**
* @private
*/
Expand Down Expand Up @@ -243,7 +261,6 @@ function getTransactionsSections(data: OnyxTypes.SearchResults['data'], metadata
}

/**
* @private
* Returns the action that can be taken on a given transaction or report
*
* Do not use directly, use only via `getSections()` facade.
Expand All @@ -262,12 +279,6 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
return CONST.SEARCH.ACTION_TYPES.VIEW;
}

// We need to check both options for a falsy value since the transaction might not have an error but the report associated with it might
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (transaction?.errors || report?.errors) {
return CONST.SEARCH.ACTION_TYPES.REVIEW;
}

luacmartins marked this conversation as resolved.
Show resolved Hide resolved
if (isSettled(report)) {
return CONST.SEARCH.ACTION_TYPES.PAID;
}
Expand All @@ -276,18 +287,17 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
return CONST.SEARCH.ACTION_TYPES.DONE;
}

// We need to check both options for a falsy value since the transaction might not have an error but the report associated with it might. We return early if there are any errors for performance reasons, so we don't need to compute any other possible actions.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (transaction?.errors || report?.errors) {
return CONST.SEARCH.ACTION_TYPES.REVIEW;
}

// We don't need to run the logic if this is not a transaction or iou/expense report, so let's shortcircuit the logic for performance reasons
if (!isMoneyRequestReport(report) || (isTransaction && !data[key].isFromOneTransactionReport)) {
if (!isMoneyRequestReport(report)) {
return CONST.SEARCH.ACTION_TYPES.VIEW;
}

const policy = data[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? {};

const invoiceReceiverPolicy =
isInvoiceReport(report) && report?.invoiceReceiver?.type === CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS
? data[`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver?.policyID}`]
: undefined;

const allReportTransactions = (
isReportEntry(key)
? Object.entries(data)
Expand All @@ -296,6 +306,26 @@ function getAction(data: OnyxTypes.SearchResults['data'], key: string): SearchTr
: [transaction]
) as SearchTransaction[];

const allViolations = Object.fromEntries(Object.entries(data).filter(([itemKey]) => isViolationEntry(itemKey))) as OnyxCollection<OnyxTypes.TransactionViolation[]>;
const shouldShowReview = hasViolations(report.reportID, allViolations, undefined, allReportTransactions);

if (shouldShowReview) {
return CONST.SEARCH.ACTION_TYPES.REVIEW;
}

// Submit/Approve/Pay can only be taken on transactions if the transaction is the only one on the report, otherwise `View` is the only option.
// If this condition is not met, return early for performance reasons
if (isTransaction && !data[key].isFromOneTransactionReport) {
return CONST.SEARCH.ACTION_TYPES.VIEW;
}

const policy = data[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? {};

const invoiceReceiverPolicy =
isInvoiceReport(report) && report?.invoiceReceiver?.type === CONST.REPORT.INVOICE_RECEIVER_TYPE.BUSINESS
? data[`${ONYXKEYS.COLLECTION.POLICY}${report?.invoiceReceiver?.policyID}`]
: undefined;

const chatReport = data[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`] ?? {};
const chatReportRNVP = data[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.chatReportID}`] ?? undefined;

Expand Down Expand Up @@ -605,4 +635,5 @@ export {
getOverflowMenu,
isCorrectSearchUserName,
isReportActionEntry,
getAction,
};
2 changes: 2 additions & 0 deletions src/types/onyx/SearchResults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {ACHAccount, ApprovalRule, ExpenseRule} from './Policy';
import type {InvoiceReceiver, Participants} from './Report';
import type ReportActionName from './ReportActionName';
import type ReportNameValuePairs from './ReportNameValuePairs';
import type {TransactionViolation} from './TransactionViolation';

/** Types of search data */
type SearchDataTypes = ValueOf<typeof CONST.SEARCH.DATA_TYPES>;
Expand Down Expand Up @@ -392,6 +393,7 @@ type SearchResults = {
PrefixedRecord<typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS, Record<string, SearchReportAction>> &
PrefixedRecord<typeof ONYXKEYS.COLLECTION.REPORT, SearchReport> &
PrefixedRecord<typeof ONYXKEYS.COLLECTION.POLICY, SearchPolicy> &
PrefixedRecord<typeof ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, TransactionViolation[]> &
PrefixedRecord<typeof ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, ReportNameValuePairs>;

/** Whether search data is being fetched from server */
Expand Down
79 changes: 79 additions & 0 deletions tests/unit/Search/SearchUIUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import type * as OnyxTypes from '@src/types/onyx';
const accountID = 18439984;
const policyID = 'A1B2C3';
const reportID = '123456789';
const reportID2 = '11111';
const transactionID = '1';
const transactionID2 = '2';

// Given search data results consisting of involved users' personal details, policyID, reportID and transactionID
const searchResults: OnyxTypes.SearchResults = {
Expand Down Expand Up @@ -54,6 +56,27 @@ const searchResults: OnyxTypes.SearchResults = {
type: 'expense',
unheldTotal: -5000,
},
[`report_${reportID2}`]: {
accountID,
action: 'view',
chatReportID: '1706144653204915',
created: '2024-12-21 13:05:20',
currency: 'USD',
isOneTransactionReport: true,
isPolicyExpenseChat: false,
isWaitingOnBankAccount: false,
managerID: accountID,
nonReimbursableTotal: 0,
ownerAccountID: accountID,
policyID,
reportID: reportID2,
reportName: 'Expense Report #123',
stateNum: 1,
statusNum: 1,
total: -5000,
type: 'expense',
unheldTotal: -5000,
},
[`transactions_${transactionID}`]: {
accountID,
action: 'view',
Expand Down Expand Up @@ -86,6 +109,44 @@ const searchResults: OnyxTypes.SearchResults = {
transactionThreadReportID: '456',
transactionType: 'cash',
},
[`transactions_${transactionID2}`]: {
accountID,
action: 'view',
amount: -5000,
canDelete: true,
canHold: true,
canUnhold: false,
category: '',
comment: {
comment: '',
},
created: '2024-12-21',
currency: 'USD',
hasEReceipt: false,
isFromOneTransactionReport: true,
managerID: accountID,
description: '',
hasViolation: false,
merchant: 'Expense',
modifiedAmount: 0,
modifiedCreated: '',
modifiedCurrency: '',
modifiedMerchant: 'Expense',
parentTransactionID: '',
policyID,
reportID: reportID2,
reportType: 'expense',
tag: '',
transactionID: transactionID2,
transactionThreadReportID: '456',
transactionType: 'cash',
},
[`transactionViolations_${transactionID2}`]: [
{
name: CONST.VIOLATIONS.MISSING_CATEGORY,
type: CONST.VIOLATION_TYPES.VIOLATION,
},
],
},
search: {
columnsToShow: {
Expand All @@ -112,3 +173,21 @@ describe('SearchUIUtils', () => {
expect(transactionListItem.merchant).toEqual(expectedMerchant);
});
});

describe('Test getAction', () => {
test('Should return `Pay` action for transaction on policy with no approvals and no violations', () => {
let action = SearchUIUtils.getAction(searchResults.data, `report_${reportID}`);
expect(action).toEqual(CONST.SEARCH.ACTION_TYPES.PAY);

action = SearchUIUtils.getAction(searchResults.data, `transactions_${transactionID}`);
expect(action).toEqual(CONST.SEARCH.ACTION_TYPES.PAY);
});

test('Should return `Review` action for transaction on policy with no approvals and with violations', () => {
let action = SearchUIUtils.getAction(searchResults.data, `report_${reportID2}`);
expect(action).toEqual(CONST.SEARCH.ACTION_TYPES.REVIEW);

action = SearchUIUtils.getAction(searchResults.data, `transactions_${transactionID2}`);
expect(action).toEqual(CONST.SEARCH.ACTION_TYPES.REVIEW);
});
});
Loading