From d25a16a8fb4c033a708cdaabcc18d8954a1c8bb6 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 20 Mar 2024 14:29:12 +0100 Subject: [PATCH 1/4] Fix the optimistic GBR for chats with money requests --- src/libs/ReportUtils.ts | 37 ++++++++++++++++++++++ src/libs/actions/IOU.ts | 68 +++++++++-------------------------------- 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 826d58ee6ecc..4884650593e1 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -428,6 +428,10 @@ type AncestorIDs = { type MissingPaymentMethod = 'bankAccount' | 'wallet'; +type OutstandingChildRequest = { + hasOutstandingChildRequest?: boolean; +}; + let currentUserEmail: string | undefined; let currentUserPrivateDomain: string | undefined; let currentUserAccountID: number | undefined; @@ -5331,6 +5335,38 @@ function shouldCreateNewMoneyRequestReport(existingIOUReport: OnyxEntry return !existingIOUReport || hasIOUWaitingOnCurrentUserBankAccount(chatReport) || !canAddOrDeleteTransactions(existingIOUReport); } +/** + * @returns the object to update `report.hasOutstandingChildRequest` + */ +function getOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): OutstandingChildRequest { + if (!iouReport || isEmptyObject(iouReport)) { + return {}; + } + + if (!isPolicyExpenseChat(iouReport)) { + return { + hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0, + }; + } + + const policy = getPolicy(iouReport.policyID); + if (PolicyUtils.isPolicyAdmin(policy)) { + return { + hasOutstandingChildRequest: true, + }; + } + + const shouldBeManuallySubmitted = PolicyUtils.isPaidGroupPolicy(policy) && !policy?.harvesting?.enabled; + if (!shouldBeManuallySubmitted) { + return { + hasOutstandingChildRequest: false, + }; + } + + // We don't need to update hasOutstandingChildRequest in this case + return {}; +} + export { getReportParticipantsTitle, isReportMessageAttachment, @@ -5541,6 +5577,7 @@ export { isJoinRequestInAdminRoom, canAddOrDeleteTransactions, shouldCreateNewMoneyRequestReport, + getOutstandingChildRequest, }; export type { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 91a74c593926..e56d0f0876b1 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -117,10 +117,6 @@ type SendMoneyParamsData = { failureData: OnyxUpdate[]; }; -type OutstandingChildRequest = { - hasOutstandingChildRequest?: boolean; -}; - let betas: OnyxTypes.Beta[] = []; Onyx.connect({ key: ONYXKEYS.BETAS, @@ -410,41 +406,6 @@ function getReceiptError(receipt?: Receipt, filename?: string, isScanRequest = t : ErrorUtils.getMicroSecondOnyxErrorObject({error: CONST.IOU.RECEIPT_ERROR, source: receipt.source?.toString() ?? '', filename: filename ?? ''}); } -function needsToBeManuallySubmitted(iouReport: OnyxTypes.Report) { - const isPolicyExpenseChat = ReportUtils.isExpenseReport(iouReport); - - if (isPolicyExpenseChat) { - const policy = ReportUtils.getPolicy(iouReport.policyID); - const isFromPaidPolicy = PolicyUtils.isPaidGroupPolicy(policy); - - // If the scheduled submit is turned off on the policy, user needs to manually submit the report which is indicated by GBR in LHN - return isFromPaidPolicy && !policy.harvesting?.enabled; - } - - return true; -} - -/** - * Return the object to update hasOutstandingChildRequest - */ -function getOutstandingChildRequest(policy: OnyxEntry | EmptyObject, iouReport: OnyxTypes.Report): OutstandingChildRequest { - if (!needsToBeManuallySubmitted(iouReport)) { - return { - hasOutstandingChildRequest: false, - }; - } - - if (PolicyUtils.isPolicyAdmin(policy)) { - return { - hasOutstandingChildRequest: true, - }; - } - - return { - hasOutstandingChildRequest: iouReport.managerID === userAccountID && iouReport.total !== 0, - }; -} - /** Builds the Onyx data for a money request */ function buildOnyxDataForMoneyRequest( chatReport: OnyxEntry, @@ -468,7 +429,7 @@ function buildOnyxDataForMoneyRequest( isOneOnOneSplit = false, ): [OnyxUpdate[], OnyxUpdate[], OnyxUpdate[]] { const isScanRequest = TransactionUtils.isScanRequest(transaction); - const outstandingChildRequest = getOutstandingChildRequest(policy ?? {}, iouReport); + const outstandingChildRequest = ReportUtils.getOutstandingChildRequest(iouReport); const clearedPendingFields = Object.fromEntries(Object.keys(transaction.pendingFields ?? {}).map((key) => [key, null])); const optimisticData: OnyxUpdate[] = []; let newQuickAction: ValueOf = isScanRequest ? CONST.QUICK_ACTIONS.REQUEST_SCAN : CONST.QUICK_ACTIONS.REQUEST_MANUAL; @@ -1260,17 +1221,21 @@ function getUpdateMoneyRequestParams( } // Step 4: Compute the IOU total and update the report preview message (and report header) so LHN amount owed is correct. - let updatedMoneyRequestReport = {...iouReport}; const diff = calculateDiffAmount(iouReport, updatedTransaction, transaction); - if (ReportUtils.isExpenseReport(iouReport) && typeof updatedMoneyRequestReport.total === 'number') { - // For expense report, the amount is negative so we should subtract total from diff - updatedMoneyRequestReport.total -= diff; + let updatedMoneyRequestReport: OnyxTypes.Report | EmptyObject; + if (!iouReport) { + updatedMoneyRequestReport = {}; + } else if (ReportUtils.isExpenseReport(iouReport) && typeof iouReport.total === 'number') { + // For expense report, the amount is negative, so we should subtract total from diff + updatedMoneyRequestReport = { + ...iouReport, + total: iouReport.total - diff, + }; } else { - updatedMoneyRequestReport = iouReport - ? IOUUtils.updateIOUOwnerAndTotal(iouReport, updatedReportAction.actorAccountID ?? -1, diff, TransactionUtils.getCurrency(transaction), false, true) - : {}; + updatedMoneyRequestReport = IOUUtils.updateIOUOwnerAndTotal(iouReport, updatedReportAction.actorAccountID ?? -1, diff, TransactionUtils.getCurrency(transaction), false, true); } + updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, transactionDetails?.currency); optimisticData.push( @@ -1282,10 +1247,7 @@ function getUpdateMoneyRequestParams( { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`, - value: { - hasOutstandingChildRequest: - iouReport && needsToBeManuallySubmitted(iouReport) && updatedMoneyRequestReport.managerID === userAccountID && updatedMoneyRequestReport.total !== 0, - }, + value: ReportUtils.getOutstandingChildRequest(updatedMoneyRequestReport), }, ); successData.push({ @@ -3100,9 +3062,7 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport?.reportID}`, - value: { - hasOutstandingChildRequest: iouReport && needsToBeManuallySubmitted(iouReport) && updatedIOUReport?.managerID === userAccountID && updatedIOUReport.total !== 0, - }, + value: ReportUtils.getOutstandingChildRequest(updatedIOUReport), }, ); From 2d3f9309137b69736e26c60bed42d70474f52802 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 20 Mar 2024 16:08:53 +0100 Subject: [PATCH 2/4] Fix the optimistic GBR for chats with money requests --- src/libs/ReportUtils.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 8b5ae27c7c25..d16156e3a783 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -5351,23 +5351,17 @@ function getOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): return {}; } - if (!isPolicyExpenseChat(iouReport)) { + if (!isExpenseReport(iouReport)) { return { hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0, }; } const policy = getPolicy(iouReport.policyID); - if (PolicyUtils.isPolicyAdmin(policy)) { - return { - hasOutstandingChildRequest: true, - }; - } - const shouldBeManuallySubmitted = PolicyUtils.isPaidGroupPolicy(policy) && !policy?.harvesting?.enabled; - if (!shouldBeManuallySubmitted) { + if (shouldBeManuallySubmitted || PolicyUtils.isPolicyAdmin(policy)) { return { - hasOutstandingChildRequest: false, + hasOutstandingChildRequest: true, }; } From 365429d6d7d834f0a533cf838a15b8d6e344de45 Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 3 Apr 2024 19:14:14 +0200 Subject: [PATCH 3/4] Simplify getOptimisticOutstandingChildRequest --- src/libs/ReportUtils.ts | 22 +++++++--------------- src/libs/actions/IOU.ts | 10 ++++++---- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 0c79afe1de4c..76b7d532691e 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -438,10 +438,6 @@ type AncestorIDs = { type MissingPaymentMethod = 'bankAccount' | 'wallet'; -type OutstandingChildRequest = { - hasOutstandingChildRequest?: boolean; -}; - let currentUserEmail: string | undefined; let currentUserPrivateDomain: string | undefined; let currentUserAccountID: number | undefined; @@ -5689,29 +5685,25 @@ function hasActionsWithErrors(reportID: string): boolean { } /** - * @returns the object to update `report.hasOutstandingChildRequest` + * @returns the optimistically calculated `hasOutstandingChildRequest` for the report (`null` if the value cannot be calculated). */ -function getOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): OutstandingChildRequest { +function getOptimisticOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): boolean | null { if (!iouReport || isEmptyObject(iouReport)) { - return {}; + return null; } if (!isExpenseReport(iouReport)) { - return { - hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0, - }; + return iouReport.managerID === currentUserAccountID && iouReport.total !== 0; } const policy = getPolicy(iouReport.policyID); const shouldBeManuallySubmitted = PolicyUtils.isPaidGroupPolicy(policy) && !policy?.harvesting?.enabled; if (shouldBeManuallySubmitted || PolicyUtils.isPolicyAdmin(policy)) { - return { - hasOutstandingChildRequest: true, - }; + return true; } // We don't need to update hasOutstandingChildRequest in this case - return {}; + return null; } export { @@ -5939,7 +5931,7 @@ export { isTrackExpenseReport, hasActionsWithErrors, getGroupChatName, - getOutstandingChildRequest, + getOptimisticOutstandingChildRequest, }; export type { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 9b53ad067271..54dce719012d 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -452,7 +452,7 @@ function buildOnyxDataForMoneyRequest( isOneOnOneSplit = false, ): [OnyxUpdate[], OnyxUpdate[], OnyxUpdate[]] { const isScanRequest = TransactionUtils.isScanRequest(transaction); - const outstandingChildRequest = ReportUtils.getOutstandingChildRequest(iouReport); + const outstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(iouReport); const clearedPendingFields = Object.fromEntries(Object.keys(transaction.pendingFields ?? {}).map((key) => [key, null])); const optimisticData: OnyxUpdate[] = []; let newQuickAction: ValueOf = isScanRequest ? CONST.QUICK_ACTIONS.REQUEST_SCAN : CONST.QUICK_ACTIONS.REQUEST_MANUAL; @@ -470,7 +470,7 @@ function buildOnyxDataForMoneyRequest( lastReadTime: DateUtils.getDBTime(), lastMessageTranslationKey: '', iouReportID: iouReport.reportID, - ...outstandingChildRequest, + ...(outstandingChildRequest !== null ? {hasOutstandingChildRequest: outstandingChildRequest} : {}), ...(isNewChatReport ? {pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}} : {}), }, }); @@ -1542,6 +1542,7 @@ function getUpdateMoneyRequestParams( updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, transactionDetails?.currency); + const optimisticOutstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(updatedMoneyRequestReport); optimisticData.push( { onyxMethod: Onyx.METHOD.MERGE, @@ -1551,7 +1552,7 @@ function getUpdateMoneyRequestParams( { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`, - value: ReportUtils.getOutstandingChildRequest(updatedMoneyRequestReport), + value: optimisticOutstandingChildRequest !== null ? {hasOutstandingChildRequest: optimisticOutstandingChildRequest} : {}, }, ); successData.push({ @@ -3714,6 +3715,7 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor ); } + const optimisticOutstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(updatedIOUReport); optimisticData.push( { onyxMethod: Onyx.METHOD.MERGE, @@ -3735,7 +3737,7 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport?.reportID}`, - value: ReportUtils.getOutstandingChildRequest(updatedIOUReport), + value: optimisticOutstandingChildRequest !== null ? {hasOutstandingChildRequest: optimisticOutstandingChildRequest} : {}, }, ); From 6d8bb69f374ea2e33ae57fe0f33f72c7a364be8f Mon Sep 17 00:00:00 2001 From: Pavlo Tsimura Date: Wed, 3 Apr 2024 21:50:13 +0200 Subject: [PATCH 4/4] Revert "Simplify getOptimisticOutstandingChildRequest" This reverts commit 365429d6d7d834f0a533cf838a15b8d6e344de45. --- src/libs/ReportUtils.ts | 22 +++++++++++++++------- src/libs/actions/IOU.ts | 10 ++++------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 76b7d532691e..0c79afe1de4c 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -438,6 +438,10 @@ type AncestorIDs = { type MissingPaymentMethod = 'bankAccount' | 'wallet'; +type OutstandingChildRequest = { + hasOutstandingChildRequest?: boolean; +}; + let currentUserEmail: string | undefined; let currentUserPrivateDomain: string | undefined; let currentUserAccountID: number | undefined; @@ -5685,25 +5689,29 @@ function hasActionsWithErrors(reportID: string): boolean { } /** - * @returns the optimistically calculated `hasOutstandingChildRequest` for the report (`null` if the value cannot be calculated). + * @returns the object to update `report.hasOutstandingChildRequest` */ -function getOptimisticOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): boolean | null { +function getOutstandingChildRequest(iouReport: OnyxEntry | EmptyObject): OutstandingChildRequest { if (!iouReport || isEmptyObject(iouReport)) { - return null; + return {}; } if (!isExpenseReport(iouReport)) { - return iouReport.managerID === currentUserAccountID && iouReport.total !== 0; + return { + hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0, + }; } const policy = getPolicy(iouReport.policyID); const shouldBeManuallySubmitted = PolicyUtils.isPaidGroupPolicy(policy) && !policy?.harvesting?.enabled; if (shouldBeManuallySubmitted || PolicyUtils.isPolicyAdmin(policy)) { - return true; + return { + hasOutstandingChildRequest: true, + }; } // We don't need to update hasOutstandingChildRequest in this case - return null; + return {}; } export { @@ -5931,7 +5939,7 @@ export { isTrackExpenseReport, hasActionsWithErrors, getGroupChatName, - getOptimisticOutstandingChildRequest, + getOutstandingChildRequest, }; export type { diff --git a/src/libs/actions/IOU.ts b/src/libs/actions/IOU.ts index 54dce719012d..9b53ad067271 100644 --- a/src/libs/actions/IOU.ts +++ b/src/libs/actions/IOU.ts @@ -452,7 +452,7 @@ function buildOnyxDataForMoneyRequest( isOneOnOneSplit = false, ): [OnyxUpdate[], OnyxUpdate[], OnyxUpdate[]] { const isScanRequest = TransactionUtils.isScanRequest(transaction); - const outstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(iouReport); + const outstandingChildRequest = ReportUtils.getOutstandingChildRequest(iouReport); const clearedPendingFields = Object.fromEntries(Object.keys(transaction.pendingFields ?? {}).map((key) => [key, null])); const optimisticData: OnyxUpdate[] = []; let newQuickAction: ValueOf = isScanRequest ? CONST.QUICK_ACTIONS.REQUEST_SCAN : CONST.QUICK_ACTIONS.REQUEST_MANUAL; @@ -470,7 +470,7 @@ function buildOnyxDataForMoneyRequest( lastReadTime: DateUtils.getDBTime(), lastMessageTranslationKey: '', iouReportID: iouReport.reportID, - ...(outstandingChildRequest !== null ? {hasOutstandingChildRequest: outstandingChildRequest} : {}), + ...outstandingChildRequest, ...(isNewChatReport ? {pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}} : {}), }, }); @@ -1542,7 +1542,6 @@ function getUpdateMoneyRequestParams( updatedMoneyRequestReport.cachedTotal = CurrencyUtils.convertToDisplayString(updatedMoneyRequestReport.total, transactionDetails?.currency); - const optimisticOutstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(updatedMoneyRequestReport); optimisticData.push( { onyxMethod: Onyx.METHOD.MERGE, @@ -1552,7 +1551,7 @@ function getUpdateMoneyRequestParams( { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`, - value: optimisticOutstandingChildRequest !== null ? {hasOutstandingChildRequest: optimisticOutstandingChildRequest} : {}, + value: ReportUtils.getOutstandingChildRequest(updatedMoneyRequestReport), }, ); successData.push({ @@ -3715,7 +3714,6 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor ); } - const optimisticOutstandingChildRequest = ReportUtils.getOptimisticOutstandingChildRequest(updatedIOUReport); optimisticData.push( { onyxMethod: Onyx.METHOD.MERGE, @@ -3737,7 +3735,7 @@ function deleteMoneyRequest(transactionID: string, reportAction: OnyxTypes.Repor { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport?.reportID}`, - value: optimisticOutstandingChildRequest !== null ? {hasOutstandingChildRequest: optimisticOutstandingChildRequest} : {}, + value: ReportUtils.getOutstandingChildRequest(updatedIOUReport), }, );