From a6c76654bda2f68c4986fb9c3523280fce7d0d90 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 14 Aug 2024 12:25:00 +0200 Subject: [PATCH 1/3] optimise getReportOrDraftReport, move same logic to one place --- src/libs/OptionsListUtils.ts | 32 +++++------------------------ src/libs/ReportUtils.ts | 10 ++------- src/libs/actions/IOU.ts | 39 +++++++++--------------------------- 3 files changed, 16 insertions(+), 65 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index b9be3ffac761b..6be86063cacad 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -352,28 +352,6 @@ Onyx.connect({ }, }); -let allReportsDraft: OnyxCollection; -Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT_DRAFT, - waitForCollectionCallback: true, - callback: (value) => (allReportsDraft = value), -}); - -/** - * Get the report or draft report given a reportID - */ -function getReportOrDraftReport(reportID: string | undefined): OnyxEntry { - const allReports = ReportConnection.getAllReports(); - if (!allReports && !allReportsDraft) { - return undefined; - } - - const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; - const draftReport = allReportsDraft?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`]; - - return report ?? draftReport; -} - /** * @param defaultValues {login: accountID} In workspace invite page, when new user is added we pass available data to opt in * @returns Returns avatar data for a list of user accountIDs @@ -572,7 +550,7 @@ function getLastActorDisplayName(lastActorDetails: Partial | nu * Update alternate text for the option when applicable */ function getAlternateText(option: ReportUtils.OptionData, {showChatPreviewLine = false, forcePolicyNamePreview = false}: PreviewConfig) { - const report = getReportOrDraftReport(option.reportID); + const report = ReportUtils.getReportOrDraftReport(option.reportID); const isAdminRoom = ReportUtils.isAdminRoom(report); const isAnnounceRoom = ReportUtils.isAnnounceRoom(report); @@ -628,7 +606,7 @@ function getIOUReportIDOfLastAction(report: OnyxEntry): string | undefin if (!ReportActionUtils.isReportPreviewAction(lastAction)) { return; } - return getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastAction))?.reportID; + return ReportUtils.getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastAction))?.reportID; } /** @@ -665,7 +643,7 @@ function getLastMessageTextForReport(report: OnyxEntry, lastActorDetails const properSchemaForMoneyRequestMessage = ReportUtils.getReportPreviewMessage(report, lastReportAction, true, false, null, true); lastMessageTextFromReport = ReportUtils.formatReportLastMessageText(properSchemaForMoneyRequestMessage); } else if (ReportActionUtils.isReportPreviewAction(lastReportAction)) { - const iouReport = getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction)); + const iouReport = ReportUtils.getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction)); const lastIOUMoneyReportAction = allSortedReportActions[iouReport?.reportID ?? '-1']?.find( (reportAction, key): reportAction is ReportAction => ReportActionUtils.shouldReportActionBeVisible(reportAction, key) && @@ -851,7 +829,7 @@ function createOption( * Get the option for a given report. */ function getReportOption(participant: Participant): ReportUtils.OptionData { - const report = getReportOrDraftReport(participant.reportID); + const report = ReportUtils.getReportOrDraftReport(participant.reportID); const visibleParticipantAccountIDs = ReportUtils.getParticipantsAccountIDsForDisplay(report, true); const option = createOption( @@ -885,7 +863,7 @@ function getReportOption(participant: Participant): ReportUtils.OptionData { * Get the option for a policy expense report. */ function getPolicyExpenseReportOption(participant: Participant | ReportUtils.OptionData): ReportUtils.OptionData { - const expenseReport = ReportUtils.isPolicyExpenseChat(participant) ? getReportOrDraftReport(participant.reportID) : null; + const expenseReport = ReportUtils.isPolicyExpenseChat(participant) ? ReportUtils.getReportOrDraftReport(participant.reportID) : null; const visibleParticipantAccountIDs = Object.entries(expenseReport?.participants ?? {}) .filter(([, reportParticipant]) => reportParticipant && !reportParticipant.hidden) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 62e259b8f05cb..16ff326e67ad9 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -651,14 +651,7 @@ function getChatType(report: OnyxInputOrEntry | Participant): ValueOf { const allReports = ReportConnection.getAllReports(); - if (!allReports && !allReportsDraft) { - return undefined; - } - - const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; - const draftReport = allReportsDraft?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`]; - - return report ?? draftReport; + return allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] ?? allReportsDraft?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`]; } /** @@ -7657,6 +7650,7 @@ export { getReportParticipantsTitle, getReportPreviewMessage, getReportRecipientAccountIDs, + getReportOrDraftReport, getRoom, getRootParentReport, getRouteFromLink, diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index ef5f6d6d61c00..761d29f371c08 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -161,13 +161,6 @@ Onyx.connect({ }, }); -let allReportsDraft: OnyxCollection; -Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT_DRAFT, - waitForCollectionCallback: true, - callback: (value) => (allReportsDraft = value), -}); - let allTransactions: NonNullable> = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.TRANSACTION, @@ -275,21 +268,6 @@ Onyx.connect({ callback: (value) => (primaryPolicyID = value), }); -/** - * Get the report or draft report given a reportID - */ -function getReportOrDraftReport(reportID: string | undefined): OnyxEntry { - const allReports = ReportConnection.getAllReports(); - if (!allReports && !allReportsDraft) { - return undefined; - } - - const report = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; - const draftReport = allReportsDraft?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT}${reportID}`]; - - return report ?? draftReport; -} - /** * Find the report preview action from given chat report and iou report */ @@ -3471,7 +3449,7 @@ function requestMoney( ) { // If the report is iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report); - const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report?.chatReportID) : report; + const currentChatReport = isMoneyRequestReport ? ReportUtils.getReportOrDraftReport(report?.chatReportID) : report; const moneyRequestReportID = isMoneyRequestReport ? report?.reportID : ''; const isMovingTransactionFromTrackExpense = IOUUtils.isMovingTransactionFromTrackExpense(action); @@ -3656,7 +3634,7 @@ function trackExpense( linkedTrackedExpenseReportID?: string, ) { const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report); - const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report.chatReportID) : report; + const currentChatReport = isMoneyRequestReport ? ReportUtils.getReportOrDraftReport(report.chatReportID) : report; const moneyRequestReportID = isMoneyRequestReport ? report.reportID : ''; const isMovingTransactionFromTrackExpense = IOUUtils.isMovingTransactionFromTrackExpense(action); @@ -5029,7 +5007,7 @@ function createDistanceRequest( ) { // If the report is an iou or expense report, we should get the linked chat report to be passed to the getMoneyRequestInformation function const isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report); - const currentChatReport = isMoneyRequestReport ? getReportOrDraftReport(report?.chatReportID) : report; + const currentChatReport = isMoneyRequestReport ? ReportUtils.getReportOrDraftReport(report?.chatReportID) : report; const moneyRequestReportID = isMoneyRequestReport ? report?.reportID : ''; const optimisticReceipt: Receipt = { @@ -6412,7 +6390,7 @@ function getReportFromHoldRequestsOnyxData( pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, }; - const heldReport = getReportOrDraftReport(holdReportAction.childReportID); + const heldReport = ReportUtils.getReportOrDraftReport(holdReportAction.childReportID); if (heldReport) { optimisticHoldReportExpenseActionIDs.push({optimisticReportActionID: reportActionID, oldReportActionID: holdReportAction.reportActionID}); @@ -6844,7 +6822,7 @@ function hasIOUToApproveOrPay(chatReport: OnyxEntry, excludedI const chatReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`] ?? {}; return Object.values(chatReportActions).some((action) => { - const iouReport = getReportOrDraftReport(action.childReportID ?? '-1'); + const iouReport = ReportUtils.getReportOrDraftReport(action.childReportID ?? '-1'); const policy = PolicyUtils.getPolicy(iouReport?.policyID); const shouldShowSettlementButton = canIOUBePaid(iouReport, chatReport, policy) || canApproveIOU(iouReport, chatReport, policy); return action.childReportID?.toString() !== excludedIOUReportID && action.actionName === CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW && shouldShowSettlementButton; @@ -6878,7 +6856,7 @@ function approveMoneyRequest(expenseReport: OnyxEntry, full?: const predictedNextState = isLastApprover(approvalChain) ? CONST.REPORT.STATE_NUM.APPROVED : CONST.REPORT.STATE_NUM.SUBMITTED; const optimisticNextStep = NextStepUtils.buildNextStep(expenseReport, predictedNextStatus); - const chatReport = getReportOrDraftReport(expenseReport?.chatReportID); + const chatReport = ReportUtils.getReportOrDraftReport(expenseReport?.chatReportID); const optimisticReportActionsData: OnyxUpdate = { onyxMethod: Onyx.METHOD.MERGE, @@ -7115,7 +7093,7 @@ function submitReport(expenseReport: OnyxTypes.Report) { } const currentNextStep = allNextSteps[`${ONYXKEYS.COLLECTION.NEXT_STEP}${expenseReport.reportID}`] ?? null; - const parentReport = getReportOrDraftReport(expenseReport.parentReportID); + const parentReport = ReportUtils.getReportOrDraftReport(expenseReport.parentReportID); const policy = PolicyUtils.getPolicy(expenseReport.policyID); const isCurrentUserManager = currentUserPersonalDetails?.accountID === expenseReport.managerID; const isSubmitAndClosePolicy = PolicyUtils.isSubmitAndClose(policy); @@ -7463,7 +7441,8 @@ function replaceReceipt(transactionID: string, file: File, source: string) { */ function setMoneyRequestParticipantsFromReport(transactionID: string, report: OnyxEntry): Participant[] { // If the report is iou or expense report, we should get the chat report to set participant for request money - const chatReport = ReportUtils.isMoneyRequestReport(report) ? getReportOrDraftReport(report?.chatReportID) : report; + console.log('in IOU'); + const chatReport = ReportUtils.isMoneyRequestReport(report) ? ReportUtils.getReportOrDraftReport(report?.chatReportID) : report; const currentUserAccountID = currentUserPersonalDetails?.accountID; const shouldAddAsReport = !isEmptyObject(chatReport) && ReportUtils.isSelfDM(chatReport); let participants: Participant[] = []; From 10ecbc17cd3a2441791f44cb91b8e1b620c24bc5 Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 14 Aug 2024 12:28:09 +0200 Subject: [PATCH 2/3] remove console.logs --- src/libs/actions/IOU.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 761d29f371c08..373170bdb93fe 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -7441,7 +7441,6 @@ function replaceReceipt(transactionID: string, file: File, source: string) { */ function setMoneyRequestParticipantsFromReport(transactionID: string, report: OnyxEntry): Participant[] { // If the report is iou or expense report, we should get the chat report to set participant for request money - console.log('in IOU'); const chatReport = ReportUtils.isMoneyRequestReport(report) ? ReportUtils.getReportOrDraftReport(report?.chatReportID) : report; const currentUserAccountID = currentUserPersonalDetails?.accountID; const shouldAddAsReport = !isEmptyObject(chatReport) && ReportUtils.isSelfDM(chatReport); From c3fcacde49e6f864a43d0a1ca8afcd4e7e449fde Mon Sep 17 00:00:00 2001 From: Tomasz Lesniakiewicz Date: Wed, 14 Aug 2024 14:56:52 +0200 Subject: [PATCH 3/3] fix tests and lint --- src/libs/OptionsListUtils.ts | 1 - tests/actions/EnforceActionExportRestrictions.ts | 5 ----- 2 files changed, 6 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 6be86063cacad..786d7c87db2fd 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -53,7 +53,6 @@ import * as PersonalDetailsUtils from './PersonalDetailsUtils'; import * as PhoneNumber from './PhoneNumber'; import * as PolicyUtils from './PolicyUtils'; import * as ReportActionUtils from './ReportActionsUtils'; -import * as ReportConnection from './ReportConnection'; import * as ReportUtils from './ReportUtils'; import * as TaskUtils from './TaskUtils'; import * as TransactionUtils from './TransactionUtils'; diff --git a/tests/actions/EnforceActionExportRestrictions.ts b/tests/actions/EnforceActionExportRestrictions.ts index bd2a13304078d..9590f878116ef 100644 --- a/tests/actions/EnforceActionExportRestrictions.ts +++ b/tests/actions/EnforceActionExportRestrictions.ts @@ -28,11 +28,6 @@ describe('ReportUtils', () => { // @ts-expect-error the test is asserting that it's undefined, so the TS error is normal expect(ReportUtils.getAllReportActions).toBeUndefined(); }); - - it('does not export getReport', () => { - // @ts-expect-error the test is asserting that it's undefined, so the TS error is normal - expect(ReportUtils.getReportOrDraftReport).toBeUndefined(); - }); }); describe('Policy', () => {