Skip to content

Commit c408634

Browse files
authored
Merge pull request #99279 from Expensify/claude-refactorInlineEditConnectWithoutView
Refactor TransactionInlineEdit actions into pure functions (remove connectWithoutView)
2 parents de93ee5 + 052c2d2 commit c408634

5 files changed

Lines changed: 129 additions & 120 deletions

File tree

‎src/hooks/useDuplicateTransactionsAndViolations.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import type {Transaction, TransactionViolation, TransactionViolations} from '@sr
55
import type {OnyxCollection} from 'react-native-onyx';
66

77
import {useMemo} from 'react';
8+
// Use the original useOnyx hook to get the real-time data from Onyx and not from the snapshot
9+
// eslint-disable-next-line no-restricted-imports
10+
import {useOnyx as useOnyxWithoutSnapshots} from 'react-native-onyx';
811

912
import useOnyx from './useOnyx';
1013

@@ -143,4 +146,38 @@ function useDuplicateTransactionsAndViolations(transactionIDs: string[]): Duplic
143146
);
144147
}
145148

149+
/**
150+
* Same as {@link useDuplicateTransactionsAndViolations}, but reads the transaction and violation collections from live
151+
* Onyx rather than the Search snapshot.
152+
* More info: https://github.com/Expensify/App/pull/99279#discussion_r3892786625
153+
* @param transactionIDs - Array of transaction IDs to check for duplicates.
154+
* @returns - An object containing duplicate transactions and their violations.
155+
*/
156+
function useLiveDuplicateTransactionsAndViolations(transactionIDs: string[]): DuplicateTransactionsAndViolations {
157+
const violationsSelectorMemo = useMemo(() => {
158+
return (allTransactionsViolations: OnyxCollection<TransactionViolations>) => selectViolationsWithDuplicates(transactionIDs, allTransactionsViolations);
159+
}, [transactionIDs]);
160+
161+
const [duplicateTransactionViolations] = useOnyxWithoutSnapshots(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, {
162+
selector: violationsSelectorMemo,
163+
});
164+
165+
const transactionSelector = useMemo(() => {
166+
return (allTransactions: OnyxCollection<Transaction>) => selectTransactionsWithDuplicates(transactionIDs, allTransactions, duplicateTransactionViolations);
167+
}, [transactionIDs, duplicateTransactionViolations]);
168+
169+
const [duplicateTransactions] = useOnyxWithoutSnapshots(ONYXKEYS.COLLECTION.TRANSACTION, {
170+
selector: transactionSelector,
171+
});
172+
173+
return useMemo(
174+
() => ({
175+
duplicateTransactions,
176+
duplicateTransactionViolations,
177+
}),
178+
[duplicateTransactions, duplicateTransactionViolations],
179+
);
180+
}
181+
182+
export {useLiveDuplicateTransactionsAndViolations};
146183
export default useDuplicateTransactionsAndViolations;

‎src/hooks/useTransactionInlineEdit.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {useOnyx as useOnyxWithoutSnapshots} from 'react-native-onyx';
3535
import {useCurrencyListActions} from './useCurrencyList';
3636
import useDelegateAccountID from './useDelegateAccountID';
3737
import useDistanceRateOriginalPolicy from './useDistanceRateOriginalPolicy';
38+
import {useLiveDuplicateTransactionsAndViolations} from './useDuplicateTransactionsAndViolations';
3839
import useNetwork from './useNetwork';
3940
import useOnyx from './useOnyx';
4041
import usePersonalPolicy from './usePersonalPolicy';
@@ -94,6 +95,11 @@ function useTransactionInlineEdit({transactionID, hash, linkedReportAction}: Use
9495
const effectiveParentReport = isUnreported ? selfDMReport : parentReport;
9596
const effectiveParentReportID = effectiveParentReport?.reportID;
9697

98+
const [liveParentReport] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(reportID)}`);
99+
const [selfDMReportID] = useOnyx(ONYXKEYS.SELF_DM_REPORT_ID);
100+
const [liveSelfDMReport] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(selfDMReportID)}`);
101+
const parentReportForAction = isUnreported ? (liveSelfDMReport ?? selfDMReport) : (effectiveParentReport ?? liveParentReport);
102+
97103
const linkedReportActionID = linkedReportAction?.reportActionID;
98104

99105
// Use original Onyx here because the useOnyx wrapper can read the partial Search snapshot report actions, which may miss
@@ -123,6 +129,7 @@ function useTransactionInlineEdit({transactionID, hash, linkedReportAction}: Use
123129
});
124130

125131
const [transactionThreadReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(transactionThreadReportID)}`);
132+
const [liveTransactionThreadReport] = originalUseOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(transactionThreadReportID)}`);
126133
const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${getNonEmptyStringOnyxID(policyID)}`);
127134
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${getNonEmptyStringOnyxID(policyID)}`);
128135
const [reportPolicyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${getNonEmptyStringOnyxID(reportPolicyID)}`);
@@ -141,6 +148,14 @@ function useTransactionInlineEdit({transactionID, hash, linkedReportAction}: Use
141148
const [originalTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.TRANSACTION}${getNonEmptyStringOnyxID(originalTransactionID)}`);
142149
const [personalDetailsList] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
143150
const [isTrackIntentUser] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED, {selector: isTrackIntentUserSelector});
151+
const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
152+
const [session] = useOnyx(ONYXKEYS.SESSION);
153+
const [betas] = useOnyx(ONYXKEYS.BETAS);
154+
155+
// Scoped transaction/violation collections (the edited transaction plus any duplicates) are read here and
156+
// passed into the pure edit actions, which need them to resolve duplicate-transaction violations. This mirrors
157+
// the non-inline edit flow (see DynamicIOURequestStepDate) and avoids subscribing to the full collections.
158+
const {duplicateTransactions, duplicateTransactionViolations} = useLiveDuplicateTransactionsAndViolations([transactionID]);
144159

145160
const {hasSelectedTransactions} = useSearchSelectionContext();
146161

@@ -181,9 +196,9 @@ function useTransactionInlineEdit({transactionID, hash, linkedReportAction}: Use
181196
hash,
182197
transactionID,
183198
transaction,
184-
parentReport: effectiveParentReport,
199+
parentReport: parentReportForAction,
185200
parentReportAction,
186-
transactionThreadReport,
201+
transactionThreadReport: transactionThreadReport ?? liveTransactionThreadReport,
187202
policy: completePolicy ?? policy,
188203
policyForTrackExpense: isTrackExpense ? policyForMovingExpenses : undefined,
189204
policyCategories,
@@ -201,6 +216,12 @@ function useTransactionInlineEdit({transactionID, hash, linkedReportAction}: Use
201216
isTrackIntentUser,
202217
getCurrencyDecimals,
203218
getCurrencySymbol,
219+
transactions: duplicateTransactions,
220+
transactionViolations: duplicateTransactionViolations,
221+
betas,
222+
introSelected,
223+
currentUserAccountID: session?.accountID ?? CONST.DEFAULT_NUMBER_ID,
224+
currentUserEmail: session?.email ?? '',
204225
};
205226
};
206227

‎src/libs/actions/TransactionInlineEdit.ts‎

Lines changed: 57 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,8 @@ import {hasEnabledOptions} from '@libs/OptionsListUtils';
77
import Permissions from '@libs/Permissions';
88
import {getLoginByAccountID} from '@libs/PersonalDetailsUtils';
99
import {getTagLists, isGroupPolicy, isMultiLevelTags, resolveCurrentTaxCode} from '@libs/PolicyUtils';
10-
import {getIOUActionForTransactionID, isMoneyRequestAction} from '@libs/ReportActionsUtils';
11-
import {
12-
canEditFieldOfMoneyRequest,
13-
canEditMoneyRequest,
14-
canUserPerformWriteAction,
15-
findSelfDMReportID,
16-
isArchivedReport,
17-
isInvoiceReport,
18-
isIOUReport,
19-
shouldEnableNegative,
20-
} from '@libs/ReportUtils';
10+
import {isMoneyRequestAction} from '@libs/ReportActionsUtils';
11+
import {canEditFieldOfMoneyRequest, canEditMoneyRequest, canUserPerformWriteAction, isArchivedReport, isInvoiceReport, isIOUReport, shouldEnableNegative} from '@libs/ReportUtils';
2112
import {hasEnabledTags} from '@libs/TagsOptionsListUtils';
2213
import {
2314
calculateTaxAmount,
@@ -55,11 +46,12 @@ import type {ValueOf} from 'type-fest';
5546
/**
5647
* Actions for inline editing of transactions from the Search results table and the Expense Report page.
5748
*
58-
* Each function delegates to the corresponding IOU action which owns the canonical Onyx record,
59-
* the API write, failure rollback, and snapshot updates (when a hash is provided).
49+
* These functions are pure: every Onyx value they need (the transaction and violation
50+
* collections, the resolved reports/report action, session, betas, etc.) is passed in by
51+
* the caller (`useTransactionInlineEdit`), which reads it via `useOnyx`. Each function
52+
* delegates to the corresponding IOU action which owns the canonical Onyx record, the API
53+
* write, failure rollback, and snapshot updates (when a hash is provided).
6054
*/
61-
import Onyx from 'react-native-onyx';
62-
6355
import {
6456
updateMoneyRequestAmountAndCurrency,
6557
updateMoneyRequestCategory,
@@ -79,64 +71,6 @@ type TransactionEditPermissions = {
7971
canEditTag: boolean;
8072
};
8173

82-
let allTransactions: NonNullable<OnyxCollection<Transaction>> = {};
83-
Onyx.connectWithoutView({
84-
key: ONYXKEYS.COLLECTION.TRANSACTION,
85-
callback: (value) => {
86-
allTransactions = value ?? {};
87-
},
88-
});
89-
90-
let allTransactionViolations: NonNullable<OnyxCollection<TransactionViolations>> = {};
91-
Onyx.connectWithoutView({
92-
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
93-
callback: (value) => {
94-
allTransactionViolations = value ?? {};
95-
},
96-
});
97-
98-
let allReports: NonNullable<OnyxCollection<Report>> = {};
99-
Onyx.connectWithoutView({
100-
key: ONYXKEYS.COLLECTION.REPORT,
101-
callback: (value) => {
102-
allReports = value ?? {};
103-
},
104-
});
105-
106-
let allReportActions: NonNullable<OnyxCollection<ReportActions>> = {};
107-
Onyx.connectWithoutView({
108-
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
109-
callback: (value) => {
110-
allReportActions = value ?? {};
111-
},
112-
});
113-
114-
let currentUserAccountID: number = CONST.DEFAULT_NUMBER_ID;
115-
let currentUserEmail = '';
116-
Onyx.connectWithoutView({
117-
key: ONYXKEYS.SESSION,
118-
callback: (value) => {
119-
currentUserEmail = value?.email ?? '';
120-
currentUserAccountID = value?.accountID ?? CONST.DEFAULT_NUMBER_ID;
121-
},
122-
});
123-
124-
let allBetas: Beta[] | undefined;
125-
Onyx.connectWithoutView({
126-
key: ONYXKEYS.BETAS,
127-
callback: (value) => {
128-
allBetas = value ?? undefined;
129-
},
130-
});
131-
132-
let introSelected: OnyxEntry<IntroSelected>;
133-
Onyx.connectWithoutView({
134-
key: ONYXKEYS.NVP_INTRO_SELECTED,
135-
callback: (value) => {
136-
introSelected = value;
137-
},
138-
});
139-
14074
const NO_EDIT: Readonly<TransactionEditPermissions> = Object.freeze({
14175
canEditDate: false,
14276
canEditMerchant: false,
@@ -201,6 +135,24 @@ type GetIouParamsInput = {
201135
isTrackIntentUser: boolean | undefined;
202136
getCurrencyDecimals: CurrencyListActionsContextType['getCurrencyDecimals'];
203137
getCurrencySymbol: CurrencyListActionsContextType['getCurrencySymbol'];
138+
139+
/** The transaction being edited plus any of its duplicates, scoped by the caller. */
140+
transactions: OnyxCollection<Transaction>;
141+
142+
/** Violations for the transaction being edited plus any of its duplicates, scoped by the caller. */
143+
transactionViolations: OnyxCollection<TransactionViolations>;
144+
145+
/** Betas the current user has access to, used to gate ASAP submit behavior. */
146+
betas: Beta[] | undefined;
147+
148+
/** Onboarding intro data, needed to build a transaction thread report when one doesn't exist yet. */
149+
introSelected: OnyxEntry<IntroSelected>;
150+
151+
/** The current user's account ID. */
152+
currentUserAccountID: number;
153+
154+
/** The current user's email/login. */
155+
currentUserEmail: string;
204156
};
205157

206158
type TransactionInlineEditParams = GetIouParamsInput & {
@@ -236,51 +188,31 @@ function getIouParamsForTransaction({
236188
isTrackIntentUser,
237189
getCurrencyDecimals,
238190
getCurrencySymbol,
191+
transactionViolations,
192+
betas,
193+
introSelected,
194+
currentUserAccountID,
195+
currentUserEmail,
239196
}: GetIouParamsInput) {
240-
const transactionViolations = allTransactionViolations[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`];
241-
const isUnreportedExpense = !transaction?.reportID || transaction.reportID === CONST.REPORT.UNREPORTED_REPORT_ID;
242-
243-
let resolvedParentReport = parentReport;
244-
if (!resolvedParentReport?.reportID && transaction?.reportID && transaction.reportID !== CONST.REPORT.UNREPORTED_REPORT_ID) {
245-
resolvedParentReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${transaction.reportID}`];
246-
}
247-
248-
let resolvedParentReportAction = parentReportAction;
249-
if (!resolvedParentReportAction && resolvedParentReport?.reportID) {
250-
const reportActions = allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${resolvedParentReport.reportID}`] ?? {};
251-
resolvedParentReportAction = getIOUActionForTransactionID(Object.values(reportActions), transactionID);
252-
}
253-
254-
if (isUnreportedExpense) {
255-
const selfDMReportID = findSelfDMReportID(allReports);
256-
if (selfDMReportID) {
257-
resolvedParentReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${selfDMReportID}`] ?? resolvedParentReport;
258-
259-
if (!resolvedParentReportAction) {
260-
const selfDMReportActions = allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${selfDMReportID}`] ?? {};
261-
resolvedParentReportAction = getIOUActionForTransactionID(Object.values(selfDMReportActions), transactionID);
262-
}
263-
}
264-
}
197+
// transaction is passed in by the caller; only the scoped violations are derived here for the thread-report build.
198+
const transactionViolationsForTransaction = transactionViolations?.[`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}${transactionID}`];
265199

200+
// parentReport (already resolved to the self DM for unreported expenses), parentReportAction, and
201+
// transactionThreadReport are resolved by the caller (useTransactionInlineEdit) via useOnyx, so they
202+
// are used directly here. The only remaining resolution is building a transaction thread report when
203+
// one doesn't exist in Onyx yet.
266204
let resolvedTransactionThreadReport = transactionThreadReport;
267-
const transactionThreadReportID = resolvedTransactionThreadReport?.reportID ?? transaction?.transactionThreadReportID ?? resolvedParentReportAction?.childReportID;
268-
269-
if (!resolvedTransactionThreadReport && transactionThreadReportID) {
270-
resolvedTransactionThreadReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`];
271-
}
272-
273-
if (!resolvedTransactionThreadReport && resolvedParentReportAction && transaction) {
205+
if (!resolvedTransactionThreadReport && parentReportAction && transaction) {
274206
resolvedTransactionThreadReport = createTransactionThreadReport({
275207
introSelected,
276208
conciergeChat,
277209
currentUserLogin: currentUserEmail,
278210
currentUserAccountID,
279-
betas: allBetas,
280-
iouReport: resolvedParentReport,
281-
iouReportAction: resolvedParentReportAction,
211+
betas,
212+
iouReport: parentReport,
213+
iouReportAction: parentReportAction,
282214
transaction,
283-
transactionViolations: transactionViolations ?? undefined,
215+
transactionViolations: transactionViolationsForTransaction ?? undefined,
284216
personalDetails: personalDetailsList,
285217
isSelfTourViewed,
286218
hasCompletedGuidedSetupFlow,
@@ -290,14 +222,14 @@ function getIouParamsForTransaction({
290222
return {
291223
transactionID,
292224
transactionThreadReport: resolvedTransactionThreadReport,
293-
parentReport: resolvedParentReport,
294-
iouReportOwnerLogin: getLoginByAccountID(resolvedParentReport?.ownerAccountID, personalDetailsList),
225+
parentReport,
226+
iouReportOwnerLogin: getLoginByAccountID(parentReport?.ownerAccountID, personalDetailsList),
295227
policy,
296228
policyForTrackExpense,
297229
policyCategories,
298230
currentUserAccountIDParam: currentUserAccountID,
299231
currentUserEmailParam: currentUserEmail,
300-
isASAPSubmitBetaEnabled: Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, allBetas),
232+
isASAPSubmitBetaEnabled: Permissions.isBetaEnabled(CONST.BETAS.ASAP_SUBMIT, betas),
301233
delegateAccountID,
302234
isTrackIntentUser,
303235
getCurrencyDecimals,
@@ -320,8 +252,8 @@ function editTransactionDateInline(params: TransactionInlineEditParams, newDate:
320252
// updateMoneyRequestDate uses 'policyTags' (not policyTagList)
321253
policyTags: iouParams.policyTagList,
322254
value: newDate,
323-
transactions: allTransactions,
324-
transactionViolations: allTransactionViolations,
255+
transactions: params.transactions,
256+
transactionViolations: params.transactionViolations,
325257
isOffline: params.isOffline,
326258
hash: params.hash,
327259
distanceOriginalPolicy: params.distanceOriginalPolicy,
@@ -331,6 +263,9 @@ function editTransactionDateInline(params: TransactionInlineEditParams, newDate:
331263

332264
/** Updates the merchant of an expense from the Search results table or the Expense Report page. */
333265
function editTransactionMerchantInline(params: TransactionInlineEditParams, newMerchant: string) {
266+
// Validate before building iouParams: getIouParamsForTransaction can call createTransactionThreadReport,
267+
// which fires Onyx.merge + openReport (an API call). Building params first would optimistically create a
268+
// thread report for an edit we're about to discard, so validate first.
334269
if (!isValidMerchant(newMerchant, params.transaction, params.parentReport)) {
335270
return;
336271
}
@@ -367,15 +302,19 @@ function editTransactionCategoryInline(params: TransactionInlineEditParams, newC
367302

368303
/** Updates the amount and currency of an expense from the Search results table or the Expense Report page. */
369304
function editTransactionAmountInline(params: TransactionInlineEditParams, newAmount: number) {
370-
const iouParams = getIouParamsForTransaction(params);
305+
// Validate before building iouParams: getIouParamsForTransaction can call createTransactionThreadReport,
306+
// which fires Onyx.merge + openReport (an API call). Building params first would optimistically create a
307+
// thread report for an edit we're about to discard, so validate against params directly first.
371308
const iouType = isInvoiceReport(params.parentReport) ? CONST.IOU.TYPE.INVOICE : CONST.IOU.TYPE.SUBMIT;
372-
const allowNegative = shouldEnableNegative(params.parentReport, iouParams.policy, iouType);
309+
const allowNegative = shouldEnableNegative(params.parentReport, params.policy, iouType);
373310
const isP2P = isIOUReport(params.parentReport);
374311

375312
if (!isValidMoneyRequestAmount(newAmount, iouType, allowNegative, isP2P)) {
376313
return;
377314
}
378315

316+
const iouParams = getIouParamsForTransaction(params);
317+
379318
// Keep the existing currency — only the amount is changing from the search table
380319
const currency = iouParams.transaction?.modifiedCurrency ?? iouParams.transaction?.currency ?? CONST.CURRENCY.USD;
381320
// Recalculate tax from the existing tax code and the new amount
@@ -391,8 +330,8 @@ function editTransactionAmountInline(params: TransactionInlineEditParams, newAmo
391330
taxCode,
392331
taxValue: taxPercentage,
393332
allowNegative,
394-
transactions: allTransactions,
395-
transactionViolations: allTransactionViolations,
333+
transactions: params.transactions,
334+
transactionViolations: params.transactionViolations,
396335
policyRecentlyUsedCurrencies: [],
397336
hash: params.hash,
398337
});

‎tests/actions/IOUTest/UpdateMoneyRequestTest.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2799,6 +2799,12 @@ describe('actions/IOU/UpdateMoneyRequest', () => {
27992799
personalDetailsList: undefined,
28002800
getCurrencyDecimals: getCurrencyDecimalsLocal,
28012801
getCurrencySymbol: getCurrencySymbolLocal,
2802+
transactions: {[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]: fakeTransaction},
2803+
transactionViolations: {},
2804+
betas: undefined,
2805+
introSelected: undefined,
2806+
currentUserAccountID: RORY_ACCOUNT_ID,
2807+
currentUserEmail: RORY_EMAIL,
28022808
},
28032809
newMerchant,
28042810
);

0 commit comments

Comments
 (0)