-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add bulk export to accounting integration #81421
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
base: main
Are you sure you want to change the base?
Changes from all commits
f84ffac
68357b5
c5fc2ba
a5fb04c
ab27bca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -725,6 +725,105 @@ function exportToIntegrationOnSearch(hash: number, reportID: string | undefined, | |
| API.write(WRITE_COMMANDS.REPORT_EXPORT, params, {optimisticData, failureData, successData}); | ||
| } | ||
|
|
||
| function exportMultipleReportsToIntegration(hash: number, reportIDs: string[], connectionName: ConnectionName, currentSearchKey?: SearchKey) { | ||
| if (!reportIDs.length) { | ||
| return; | ||
| } | ||
|
|
||
| const optimisticActions: Record<string, OptimisticExportIntegrationAction> = {}; | ||
| const successActions: Record<string, OptimisticExportIntegrationAction> = {}; | ||
| const optimisticReportActions: Record<string, string> = {}; | ||
|
|
||
| for (const reportID of reportIDs) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ PERF-13 (docs)The function Hoist the function call outside the loop to eliminate O(n) redundant computations: function exportMultipleReportsToIntegration(hash: number, reportIDs: string[], connectionName: ConnectionName, currentSearchKey?: SearchKey) {
if (!reportIDs.length) {
return;
}
const baseOptimisticAction = buildOptimisticExportIntegrationAction(connectionName);
const optimisticActions: Record<string, OptimisticExportIntegrationAction> = {};
const successActions: Record<string, OptimisticExportIntegrationAction> = {};
const optimisticReportActions: Record<string, string> = {};
for (const reportID of reportIDs) {
const optimisticAction = {...baseOptimisticAction, reportActionID: baseOptimisticAction.reportActionID + reportID}; // Ensure unique IDs
const successAction: OptimisticExportIntegrationAction = {...optimisticAction, pendingAction: null};
const optimisticReportActionID = optimisticAction.reportActionID;
optimisticActions[reportID] = optimisticAction;
successActions[reportID] = successAction;
optimisticReportActions[reportID] = optimisticReportActionID;
}
// ... rest of function
}Note: If Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency. |
||
| const optimisticAction = buildOptimisticExportIntegrationAction(connectionName); | ||
| const successAction: OptimisticExportIntegrationAction = {...optimisticAction, pendingAction: null}; | ||
| const optimisticReportActionID = optimisticAction.reportActionID; | ||
|
|
||
| optimisticActions[reportID] = optimisticAction; | ||
| successActions[reportID] = successAction; | ||
| optimisticReportActions[reportID] = optimisticReportActionID; | ||
| } | ||
|
|
||
| const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.REPORT_METADATA | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS>> = [ | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE_COLLECTION, | ||
| key: ONYXKEYS.COLLECTION.REPORT_METADATA, | ||
| value: Object.fromEntries(reportIDs.map((reportID) => [`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {isActionLoading: true}])), | ||
| }, | ||
| ]; | ||
|
|
||
| for (const reportID of reportIDs) { | ||
| optimisticData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, | ||
| value: { | ||
| [optimisticReportActions[reportID]]: optimisticActions[reportID], | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| const successData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.REPORT_METADATA | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.SNAPSHOT>> = [ | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE_COLLECTION, | ||
| key: ONYXKEYS.COLLECTION.REPORT_METADATA, | ||
| value: Object.fromEntries(reportIDs.map((reportID) => [`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {isActionLoading: false}])), | ||
| }, | ||
| ]; | ||
|
|
||
| for (const reportID of reportIDs) { | ||
| successData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, | ||
| value: { | ||
| [optimisticReportActions[reportID]]: successActions[reportID], | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| if (currentSearchKey === CONST.SEARCH.SEARCH_KEYS.EXPORT) { | ||
| successData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.SNAPSHOT}${hash}`, | ||
| value: { | ||
| data: Object.fromEntries(reportIDs.map((reportID) => [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, null])), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| const failureData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.REPORT_METADATA | typeof ONYXKEYS.COLLECTION.REPORT_ACTIONS | typeof ONYXKEYS.COLLECTION.REPORT>> = [ | ||
| { | ||
| onyxMethod: Onyx.METHOD.MERGE_COLLECTION, | ||
| key: ONYXKEYS.COLLECTION.REPORT_METADATA, | ||
| value: Object.fromEntries(reportIDs.map((reportID) => [`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportID}`, {isActionLoading: false}])), | ||
| }, | ||
| ]; | ||
|
|
||
| for (const reportID of reportIDs) { | ||
| failureData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, | ||
| value: { | ||
| [optimisticReportActions[reportID]]: null, | ||
| }, | ||
| }); | ||
|
|
||
| failureData.push({ | ||
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, | ||
| value: {errors: getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage')}, | ||
| }); | ||
| } | ||
|
|
||
| const params = { | ||
| reportIDList: reportIDs, | ||
| connectionName, | ||
| type: 'MANUAL', | ||
| optimisticReportActions: JSON.stringify(optimisticReportActions), | ||
| } satisfies ReportExportParams; | ||
|
|
||
| API.write(WRITE_COMMANDS.REPORT_EXPORT, params, {optimisticData, failureData, successData}); | ||
| } | ||
|
|
||
| function payMoneyRequestOnSearch(hash: number, paymentData: PaymentData[], currentSearchKey?: SearchKey) { | ||
| const optimisticData: Array<OnyxUpdate<typeof ONYXKEYS.COLLECTION.REPORT_METADATA>> = [ | ||
| { | ||
|
|
@@ -1420,6 +1519,7 @@ export { | |
| getLastPolicyPaymentMethod, | ||
| getLastPolicyBankAccountID, | ||
| exportToIntegrationOnSearch, | ||
| exportMultipleReportsToIntegration, | ||
| getPayOption, | ||
| isValidBulkPayOption, | ||
| handleBulkPayItemSelected, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ PERF-13 (docs)
The function
buildOptimisticExportIntegrationAction(connectionName, true)is called inside the for loop but does not use the iterator variablereportID. This results in redundant computation - the function produces the same result for each iteration based only onconnectionNameand the booleantrue, both of which are constant throughout the loop.The constant
labelis already hoisted outside the loop correctly. Similarly, the result ofbuildOptimisticExportIntegrationActionshould be computed once and reused:Note: If
buildOptimisticExportIntegrationActiongenerates unique IDs internally (e.g., timestamps), you may need to adjust the implementation to ensure uniqueness while still avoiding redundant computation.Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.