Skip to content

Commit 49e60fa

Browse files
danieldoglasdanieldoglas
authored
Merge pull request Expensify#78254 from DylanDylann/refactor-667
[Part 2]: Remove Onyx.connect() for the key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS in src/libs/TransactionUtils/index.ts
2 parents 77aa31b + 342f2de commit 49e60fa

8 files changed

Lines changed: 105 additions & 23 deletions

File tree

‎src/components/MoneyRequestHeader.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ function MoneyRequestHeader({report, parentReportAction, policy, onBackButtonPre
137137
const defaultExpensePolicy = useDefaultExpensePolicy();
138138
const activePolicyExpenseChat = getPolicyExpenseChat(accountID, defaultExpensePolicy?.id);
139139
const isOnHold = isOnHoldTransactionUtils(transaction);
140-
const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? '', accountID, report, policy);
140+
const isDuplicate = isDuplicateTransactionUtils(transaction, email ?? '', accountID, report, policy, transactionViolations);
141141
const reportID = report?.reportID;
142142
const {removeTransaction, currentSearchHash} = useSearchContext();
143143
const {isExpenseSplit} = getOriginalTransactionWithSplitInfo(transaction, originalTransaction);

‎src/hooks/useSelectedTransactionsActions.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ function useSelectedTransactionsActions({
7171
const [lastVisitedPath] = useOnyx(ONYXKEYS.LAST_VISITED_PATH, {canBeMissing: true});
7272
const [integrationsExportTemplates] = useOnyx(ONYXKEYS.NVP_INTEGRATION_SERVER_EXPORT_TEMPLATES, {canBeMissing: true});
7373
const [csvExportLayouts] = useOnyx(ONYXKEYS.NVP_CSV_EXPORT_LAYOUTS, {canBeMissing: true});
74+
const [allTransactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {canBeMissing: true});
7475

7576
const expensifyIcons = useMemoizedLazyExpensifyIcons(['Stopwatch', 'Trashcan', 'ArrowRight', 'Table', 'DocumentMerge', 'Export', 'ArrowCollapse', 'ArrowSplit', 'ThumbsDown']);
7677
const {duplicateTransactions, duplicateTransactionViolations} = useDuplicateTransactionsAndViolations(selectedTransactionIDs);
@@ -222,7 +223,7 @@ function useSelectedTransactionsActions({
222223
});
223224
}
224225

225-
const hasNoRejectedTransaction = selectedTransactionIDs.every((id) => !hasTransactionBeenRejected(id));
226+
const hasNoRejectedTransaction = selectedTransactionIDs.every((id) => !hasTransactionBeenRejected(allTransactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + id] ?? []));
226227
const canRejectTransactions =
227228
selectedTransactionsList.length > 0 && isMoneyRequestReport && !!session?.email && !!report && canRejectReportAction(session.email, report, policy) && hasNoRejectedTransaction;
228229
if (canRejectTransactions) {
@@ -387,6 +388,7 @@ function useSelectedTransactionsActions({
387388
allReports,
388389
session?.accountID,
389390
showDeleteModal,
391+
allTransactionViolations,
390392
expensifyIcons.Stopwatch,
391393
expensifyIcons.ThumbsDown,
392394
expensifyIcons.Table,

‎src/libs/TransactionUtils/index.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1353,9 +1353,8 @@ function getTransactionViolationsOfTransaction(transactionID: string) {
13531353
/**
13541354
* Check if a transaction has been rejected
13551355
*/
1356-
function hasTransactionBeenRejected(transactionID: string): boolean {
1357-
const transactionViolations = getTransactionViolationsOfTransaction(transactionID);
1358-
return transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE);
1356+
function hasTransactionBeenRejected(transactionViolations: OnyxEntry<TransactionViolations>): boolean {
1357+
return !!transactionViolations && transactionViolations.some((violation) => violation.name === CONST.VIOLATIONS.AUTO_REPORTED_REJECTED_EXPENSE);
13591358
}
13601359

13611360
/**
@@ -1659,14 +1658,12 @@ function isDuplicate(
16591658
currentUserAccountID: number,
16601659
iouReport: OnyxEntry<Report>,
16611660
policy: OnyxEntry<Policy>,
1662-
transactionViolation?: OnyxEntry<TransactionViolations>,
1661+
transactionViolation: OnyxEntry<TransactionViolations>,
16631662
): boolean {
16641663
if (!transaction) {
16651664
return false;
16661665
}
1667-
const duplicatedTransactionViolation = (transactionViolation ?? deprecatedAllTransactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transaction.transactionID}`])?.find(
1668-
(violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
1669-
);
1666+
const duplicatedTransactionViolation = transactionViolation?.find((violation: TransactionViolation) => violation.name === CONST.VIOLATIONS.DUPLICATED_TRANSACTION);
16701667
const hasDuplicatedTransactionViolation = !!duplicatedTransactionViolation;
16711668
const isDuplicatedTransactionViolationDismissed = isViolationDismissed(transaction, duplicatedTransactionViolation, currentUserEmail, currentUserAccountID, iouReport, policy);
16721669

‎src/libs/actions/MergeTransaction.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ import {isPaidGroupPolicy, isPolicyAdmin} from '@libs/PolicyUtils';
2020
import {getIOUActionForReportID} from '@libs/ReportActionsUtils';
2121
import {getReportOrDraftReport, getReportTransactions, getTransactionDetails, isCurrentUserSubmitter, isMoneyRequestReportEligibleForMerge, isReportManager} from '@libs/ReportUtils';
2222
import CONST from '@src/CONST';
23-
import {getTransactionViolationsOfTransaction, isDistanceRequest, isTransactionPendingDelete} from '@src/libs/TransactionUtils';
23+
import {isDistanceRequest, isTransactionPendingDelete} from '@src/libs/TransactionUtils';
2424
import ONYXKEYS from '@src/ONYXKEYS';
2525
import ROUTES from '@src/ROUTES';
26-
import type {CardList, MergeTransaction, Policy, PolicyCategories, PolicyTagLists, Report, Transaction} from '@src/types/onyx';
26+
import type {CardList, MergeTransaction, Policy, PolicyCategories, PolicyTagLists, Report, Transaction, TransactionViolations} from '@src/types/onyx';
2727
import {getUpdateMoneyRequestParams, getUpdateTrackExpenseParams} from './IOU';
2828
import type {UpdateMoneyRequestData} from './IOU';
2929

@@ -183,6 +183,7 @@ function getTransactionsForMerging({
183183

184184
function getOnyxTargetTransactionData(
185185
targetTransaction: Transaction,
186+
targetTransactionViolations: OnyxEntry<TransactionViolations>,
186187
mergeTransaction: MergeTransaction,
187188
policy: OnyxEntry<Policy>,
188189
policyTags: OnyxEntry<PolicyTagLists>,
@@ -194,7 +195,6 @@ function getOnyxTargetTransactionData(
194195
let data: UpdateMoneyRequestData;
195196
const isUnreportedExpense = !mergeTransaction.reportID || mergeTransaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
196197
const transactionThreadReportID = getTransactionThreadReportID(targetTransaction);
197-
const violations = getTransactionViolationsOfTransaction(targetTransaction.transactionID);
198198

199199
// Compare mergeTransaction with targetTransaction and remove fields with same values
200200
const targetTransactionDetails = getTransactionDetails(targetTransaction);
@@ -222,7 +222,7 @@ function getOnyxTargetTransactionData(
222222
policy,
223223
policyTagList: policyTags,
224224
policyCategories,
225-
violations,
225+
violations: targetTransactionViolations ?? [],
226226
shouldBuildOptimisticModifiedExpenseReportAction,
227227
currentUserAccountIDParam,
228228
currentUserEmailParam,
@@ -264,6 +264,7 @@ type MergeTransactionRequestParams = {
264264
mergeTransactionID: string;
265265
mergeTransaction: MergeTransaction;
266266
targetTransaction: Transaction;
267+
allTransactionViolations: OnyxCollection<TransactionViolations>;
267268
sourceTransaction: Transaction;
268269
policy: OnyxEntry<Policy>;
269270
policyTags: OnyxEntry<PolicyTagLists>;
@@ -280,6 +281,7 @@ function mergeTransactionRequest({
280281
mergeTransaction,
281282
targetTransaction,
282283
sourceTransaction,
284+
allTransactionViolations,
283285
policy,
284286
policyTags,
285287
policyCategories,
@@ -317,6 +319,7 @@ function mergeTransactionRequest({
317319

318320
const onyxTargetTransactionData = getOnyxTargetTransactionData(
319321
targetTransaction,
322+
allTransactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + targetTransaction.transactionID] ?? [],
320323
mergeTransaction,
321324
policy,
322325
policyTags,
@@ -411,7 +414,7 @@ function mergeTransactionRequest({
411414

412415
// Optimistic delete duplicated transaction violations
413416
const optimisticTransactionViolations: OnyxUpdate[] = [targetTransaction.transactionID, sourceTransaction.transactionID].map((id) => {
414-
const violations = getTransactionViolationsOfTransaction(id);
417+
const violations = allTransactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + id] ?? [];
415418

416419
return {
417420
onyxMethod: Onyx.METHOD.MERGE,
@@ -421,7 +424,7 @@ function mergeTransactionRequest({
421424
};
422425
});
423426
const failureTransactionViolations: OnyxUpdate[] = [targetTransaction.transactionID, sourceTransaction.transactionID].map((id) => {
424-
const violations = getTransactionViolationsOfTransaction(id);
427+
const violations = allTransactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + id] ?? [];
425428

426429
return {
427430
onyxMethod: Onyx.METHOD.MERGE,

‎src/pages/Search/SearchPage.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ function SearchPage({route}: SearchPageProps) {
125125
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {canBeMissing: true});
126126
const [integrationsExportTemplates] = useOnyx(ONYXKEYS.NVP_INTEGRATION_SERVER_EXPORT_TEMPLATES, {canBeMissing: true});
127127
const [csvExportLayouts] = useOnyx(ONYXKEYS.NVP_CSV_EXPORT_LAYOUTS, {canBeMissing: true});
128+
const [allTransactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {canBeMissing: true});
128129

129130
const [isOfflineModalVisible, setIsOfflineModalVisible] = useState(false);
130131
const [isDownloadErrorModalVisible, setIsDownloadErrorModalVisible] = useState(false);
@@ -581,7 +582,9 @@ function SearchPage({route}: SearchPageProps) {
581582
}
582583

583584
// Check if all selected transactions can be rejected
584-
const hasNoRejectedTransaction = selectedTransactionsKeys.every((id) => !hasTransactionBeenRejected(id));
585+
const hasNoRejectedTransaction = selectedTransactionsKeys.every(
586+
(id) => !hasTransactionBeenRejected(allTransactionViolations?.[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS + id] ?? []),
587+
);
585588

586589
const shouldShowRejectOption =
587590
queryJSON?.type !== CONST.SEARCH.DATA_TYPES.EXPENSE_REPORT &&
@@ -891,6 +894,7 @@ function SearchPage({route}: SearchPageProps) {
891894
dismissedHoldUseExplanation,
892895
dismissedRejectUseExplanation,
893896
areAllTransactionsFromSubmitter,
897+
allTransactionViolations,
894898
currentSearchResults?.data,
895899
isDelegateAccessRestricted,
896900
showDelegateNoAccessModal,

‎src/pages/TransactionMerge/ConfirmationPage.tsx‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ function ConfirmationPage({route}: ConfirmationPageProps) {
4242
const [allReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: false});
4343
const [mergeTransaction, mergeTransactionMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.MERGE_TRANSACTION}${getNonEmptyStringOnyxID(transactionID)}`, {canBeMissing: true});
4444
const {targetTransaction, sourceTransaction, targetTransactionReport} = useMergeTransactions({mergeTransaction});
45+
const [allTransactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {
46+
canBeMissing: false,
47+
});
4548

4649
const policyID = targetTransactionReport?.policyID;
4750
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`, {canBeMissing: true});
@@ -67,6 +70,7 @@ function ConfirmationPage({route}: ConfirmationPageProps) {
6770
mergeTransactionID: transactionID,
6871
mergeTransaction,
6972
targetTransaction,
73+
allTransactionViolations,
7074
sourceTransaction,
7175
policy,
7276
policyTags,

‎tests/actions/MergeTransactionTest.ts‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import Onyx from 'react-native-onyx';
2+
import type {OnyxCollection} from 'react-native-onyx';
23
import {areTransactionsEligibleForMerge, mergeTransactionRequest, setMergeTransactionKey, setupMergeTransactionData} from '@libs/actions/MergeTransaction';
34
import CONST from '@src/CONST';
45
import ONYXKEYS from '@src/ONYXKEYS';
5-
import type {MergeTransaction as MergeTransactionType, Report, Transaction, TransactionViolation} from '@src/types/onyx';
6+
import type {MergeTransaction as MergeTransactionType, Report, Transaction, TransactionViolation, TransactionViolations} from '@src/types/onyx';
67
import createRandomMergeTransaction from '../utils/collections/mergeTransaction';
78
import {createExpenseReport} from '../utils/collections/reports';
89
import createRandomTransaction, {createRandomDistanceRequestTransaction} from '../utils/collections/transaction';
@@ -26,6 +27,23 @@ function createMockViolations(): TransactionViolation[] {
2627
];
2728
}
2829

30+
// Helper function to create allTransactionViolations collection
31+
function createAllTransactionViolations(
32+
targetTransactionID: string,
33+
sourceTransactionID: string,
34+
targetViolations?: TransactionViolation[],
35+
sourceViolations?: TransactionViolation[],
36+
): OnyxCollection<TransactionViolations> {
37+
const allViolations: OnyxCollection<TransactionViolations> = {};
38+
if (targetViolations) {
39+
allViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${targetTransactionID}`] = targetViolations;
40+
}
41+
if (sourceViolations) {
42+
allViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${sourceTransactionID}`] = sourceViolations;
43+
}
44+
return allViolations;
45+
}
46+
2947
describe('mergeTransactionRequest', () => {
3048
let mockFetch: MockFetch;
3149

@@ -76,6 +94,32 @@ describe('mergeTransactionRequest', () => {
7694
};
7795
const mergeTransactionID = 'merge789';
7896

97+
// Sample violations for testing
98+
const targetViolations: TransactionViolation[] = [
99+
{
100+
type: CONST.VIOLATION_TYPES.VIOLATION,
101+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
102+
showInReview: true,
103+
},
104+
{
105+
type: CONST.VIOLATION_TYPES.VIOLATION,
106+
name: CONST.VIOLATIONS.MISSING_TAG,
107+
showInReview: true,
108+
},
109+
];
110+
const sourceViolations: TransactionViolation[] = [
111+
{
112+
type: CONST.VIOLATION_TYPES.VIOLATION,
113+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
114+
showInReview: true,
115+
},
116+
{
117+
type: CONST.VIOLATION_TYPES.VIOLATION,
118+
name: CONST.VIOLATIONS.OVER_LIMIT,
119+
showInReview: true,
120+
},
121+
];
122+
79123
// Set up initial state in Onyx
80124
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction.transactionID}`, targetTransaction);
81125
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${sourceTransaction.transactionID}`, sourceTransaction);
@@ -91,6 +135,7 @@ describe('mergeTransactionRequest', () => {
91135
mergeTransaction,
92136
targetTransaction,
93137
sourceTransaction,
138+
allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, sourceTransaction.transactionID, targetViolations, sourceViolations),
94139
policy: undefined,
95140
policyTags: undefined,
96141
policyCategories: undefined,
@@ -213,6 +258,7 @@ describe('mergeTransactionRequest', () => {
213258
mergeTransaction,
214259
targetTransaction,
215260
sourceTransaction,
261+
allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, sourceTransaction.transactionID, mockViolations, mockViolations),
216262
policy: undefined,
217263
policyTags: undefined,
218264
policyCategories: undefined,
@@ -310,6 +356,7 @@ describe('mergeTransactionRequest', () => {
310356
mergeTransaction,
311357
targetTransaction,
312358
sourceTransaction,
359+
allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, sourceTransaction.transactionID, mockViolations, mockViolations),
313360
policy: undefined,
314361
policyTags: undefined,
315362
policyCategories: undefined,
@@ -374,6 +421,32 @@ describe('mergeTransactionRequest', () => {
374421
};
375422
const mergeTransactionID = 'merge789';
376423

424+
// Sample violations for testing
425+
const targetViolations: TransactionViolation[] = [
426+
{
427+
type: CONST.VIOLATION_TYPES.VIOLATION,
428+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
429+
showInReview: true,
430+
},
431+
{
432+
type: CONST.VIOLATION_TYPES.VIOLATION,
433+
name: CONST.VIOLATIONS.MISSING_COMMENT,
434+
showInReview: true,
435+
},
436+
];
437+
const sourceViolations: TransactionViolation[] = [
438+
{
439+
type: CONST.VIOLATION_TYPES.VIOLATION,
440+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
441+
showInReview: true,
442+
},
443+
{
444+
type: CONST.VIOLATION_TYPES.VIOLATION,
445+
name: CONST.VIOLATIONS.RECEIPT_REQUIRED,
446+
showInReview: true,
447+
},
448+
];
449+
377450
// Set up initial state
378451
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${targetTransaction.transactionID}`, targetTransaction);
379452
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION}${sourceTransaction.transactionID}`, sourceTransaction);
@@ -389,6 +462,7 @@ describe('mergeTransactionRequest', () => {
389462
mergeTransaction,
390463
targetTransaction,
391464
sourceTransaction,
465+
allTransactionViolations: createAllTransactionViolations(targetTransaction.transactionID, sourceTransaction.transactionID, targetViolations, sourceViolations),
392466
policy: undefined,
393467
policyTags: undefined,
394468
policyCategories: undefined,

‎tests/unit/ReportSecondaryActionUtilsTest.ts‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -420,11 +420,9 @@ describe('getSecondaryAction', () => {
420420

421421
await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${TRANSACTION_ID}`, transaction);
422422

423-
await Onyx.set(`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`, [
424-
{
425-
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
426-
} as TransactionViolation,
427-
]);
423+
const violation = {
424+
name: CONST.VIOLATIONS.DUPLICATED_TRANSACTION,
425+
} as TransactionViolation;
428426

429427
await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
430428

@@ -435,7 +433,7 @@ describe('getSecondaryAction', () => {
435433
chatReport,
436434
reportTransactions: [transaction],
437435
originalTransaction: {} as Transaction,
438-
violations: {},
436+
violations: {[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${TRANSACTION_ID}`]: [violation]},
439437
policy,
440438
});
441439
expect(result.includes(CONST.REPORT.SECONDARY_ACTIONS.APPROVE)).toBe(true);

0 commit comments

Comments
 (0)