Skip to content

Commit

Permalink
Merge pull request Expensify#54847 from Expensify/cmartins-showReview
Browse files Browse the repository at this point in the history
Add violations to Search actions
  • Loading branch information
lakchote authored Jan 14, 2025
2 parents 0e7af01 + 02e2e93 commit 8d66dab
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 18 deletions.
9 changes: 7 additions & 2 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6632,8 +6632,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;
}

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);
});
});

0 comments on commit 8d66dab

Please sign in to comment.