From c0542d589a974e741d89b1484b0598d21eb30ab5 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Tue, 8 Oct 2024 20:03:34 +0100 Subject: [PATCH 01/10] fix(debug mode): wrong reason for why RBR reports are visible in LHN --- src/languages/en.ts | 1 + src/languages/es.ts | 1 + src/libs/DebugUtils.ts | 9 ++- src/libs/OptionsListUtils.ts | 119 +--------------------------- src/libs/ReportUtils.ts | 115 ++++++++++++++++++++++++++- src/libs/SidebarUtils.ts | 20 +---- src/libs/WorkspacesSettingsUtils.ts | 2 +- 7 files changed, 129 insertions(+), 138 deletions(-) diff --git a/src/languages/en.ts b/src/languages/en.ts index c44ac6f2cedf..eb68cf9b23c2 100755 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -4931,6 +4931,7 @@ const translations = { reasonVisibleInLHN: { hasDraftComment: 'Has draft comment', hasGBR: 'Has GBR', + hasRBR: 'Has RBR', pinnedByUser: 'Pinned by user', hasIOUViolations: 'Has IOU violations', hasAddWorkspaceRoomErrors: 'Has add workspace room errors', diff --git a/src/languages/es.ts b/src/languages/es.ts index 76cb22772f88..7aec0d6eef50 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -5443,6 +5443,7 @@ const translations = { reasonVisibleInLHN: { hasDraftComment: 'Tiene comentario en borrador', hasGBR: 'Tiene GBR', + hasRBR: 'Tiene RBR', pinnedByUser: 'Fijado por el usuario', hasIOUViolations: 'Tiene violaciones de IOU', hasAddWorkspaceRoomErrors: 'Tiene errores al agregar sala de espacio de trabajo', diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index ed8e486517ff..af682e9cdc6a 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -7,7 +7,6 @@ import CONST from '@src/CONST'; import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Beta, Policy, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; -import * as OptionsListUtils from './OptionsListUtils'; import * as ReportUtils from './ReportUtils'; class NumberError extends SyntaxError { @@ -598,7 +597,11 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath return null; } - const doesReportHaveViolations = OptionsListUtils.shouldShowViolations(report, transactionViolations); + const doesReportHaveViolations = ReportUtils.shouldShowViolations(report, transactionViolations); + + if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { + return `debug.reasonVisibleInLHN.hasRBR`; + } const reason = ReportUtils.reasonForReportToBeInOptionList({ report, @@ -646,7 +649,7 @@ function getReasonAndReportActionForGBRInLHNRow(report: OnyxEntry): GBRR * Gets the report action that is causing the RBR to show up in LHN */ function getRBRReportAction(report: OnyxEntry, reportActions: OnyxEntry): OnyxEntry { - const {reportAction} = OptionsListUtils.getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions); + const {reportAction} = ReportUtils.getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions); return reportAction; } diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index a004974b88e4..2bb52b461f55 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -41,7 +41,6 @@ import type DeepValueOf from '@src/types/utils/DeepValueOf'; import {isEmptyObject} from '@src/types/utils/EmptyObject'; import times from '@src/utils/times'; import Timing from './actions/Timing'; -import * as ErrorUtils from './ErrorUtils'; import filterArrayByMatch from './filterArrayByMatch'; import localeCompare from './LocaleCompare'; import * as LocalePhoneNumber from './LocalePhoneNumber'; @@ -342,26 +341,6 @@ Onyx.connect({ }, }); -let allTransactions: OnyxCollection = {}; -Onyx.connect({ - key: ONYXKEYS.COLLECTION.TRANSACTION, - waitForCollectionCallback: true, - callback: (value) => { - if (!value) { - return; - } - - allTransactions = Object.keys(value) - .filter((key) => !!value[key]) - .reduce((result: OnyxCollection, key) => { - if (result) { - // eslint-disable-next-line no-param-reassign - result[key] = value[key]; - } - return result; - }, {}); - }, -}); let activePolicyID: OnyxEntry; Onyx.connect({ key: ONYXKEYS.NVP_ACTIVE_POLICY_ID, @@ -480,78 +459,6 @@ function uniqFast(items: string[]): string[] { return result; } -type ReportErrorsAndReportActionThatRequiresAttention = { - errors: OnyxCommon.ErrorFields; - reportAction?: OnyxEntry; -}; - -function getAllReportActionsErrorsAndReportActionThatRequiresAttention(report: OnyxEntry, reportActions: OnyxEntry): ReportErrorsAndReportActionThatRequiresAttention { - const reportActionsArray = Object.values(reportActions ?? {}); - const reportActionErrors: OnyxCommon.ErrorFields = {}; - let reportAction: OnyxEntry; - - for (const action of reportActionsArray) { - if (action && !isEmptyObject(action.errors)) { - Object.assign(reportActionErrors, action.errors); - - if (!reportAction) { - reportAction = action; - } - } - } - const parentReportAction: OnyxEntry = - !report?.parentReportID || !report?.parentReportActionID ? undefined : allReportActions?.[report.parentReportID ?? '-1']?.[report.parentReportActionID ?? '-1']; - - if (ReportActionUtils.wasActionTakenByCurrentUser(parentReportAction) && ReportActionUtils.isTransactionThread(parentReportAction)) { - const transactionID = ReportActionUtils.isMoneyRequestAction(parentReportAction) ? ReportActionUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID : null; - const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; - if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) { - reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); - reportAction = undefined; - } - } else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) { - if (ReportUtils.shouldShowRBRForMissingSmartscanFields(report?.reportID ?? '-1') && !ReportUtils.isSettled(report?.reportID)) { - reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); - reportAction = ReportUtils.getReportActionWithMissingSmartscanFields(report?.reportID ?? '-1'); - } - } else if (ReportUtils.hasSmartscanError(reportActionsArray)) { - reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); - reportAction = ReportUtils.getReportActionWithSmartscanError(reportActionsArray); - } - - return { - errors: reportActionErrors, - reportAction, - }; -} - -/** - * Get an object of error messages keyed by microtime by combining all error objects related to the report. - */ -function getAllReportErrors(report: OnyxEntry, reportActions: OnyxEntry): OnyxCommon.Errors { - const reportErrors = report?.errors ?? {}; - const reportErrorFields = report?.errorFields ?? {}; - const {errors: reportActionErrors} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions); - - // All error objects related to the report. Each object in the sources contains error messages keyed by microtime - const errorSources = { - reportErrors, - ...reportErrorFields, - ...reportActionErrors, - }; - - // Combine all error messages keyed by microtime into one object - const errorSourcesArray = Object.values(errorSources ?? {}); - const allReportErrors = {}; - - for (const errors of errorSourcesArray) { - if (!isEmptyObject(errors)) { - Object.assign(allReportErrors, errors); - } - } - return allReportErrors; -} - /** * Get the last actor display name from last actor details. */ @@ -745,7 +652,7 @@ function getLastMessageTextForReport(report: OnyxEntry, lastActorDetails } function hasReportErrors(report: Report, reportActions: OnyxEntry) { - return !isEmptyObject(getAllReportErrors(report, reportActions)); + return !isEmptyObject(ReportUtils.getAllReportErrors(report, reportActions)); } /** @@ -813,7 +720,7 @@ function createOption( result.shouldShowSubscript = ReportUtils.shouldReportShowSubscript(report); result.isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); result.isOwnPolicyExpenseChat = report.isOwnPolicyExpenseChat ?? false; - result.allReportErrors = getAllReportErrors(report, reportActions); + result.allReportErrors = ReportUtils.getAllReportErrors(report, reportActions); result.brickRoadIndicator = hasReportErrors(report, reportActions) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''; result.pendingAction = report.pendingFields ? report.pendingFields.addWorkspaceRoom ?? report.pendingFields.createChat : undefined; result.ownerAccountID = report.ownerAccountID; @@ -1767,23 +1674,6 @@ function getUserToInviteOption({ return userToInvite; } -/** - * Check whether report has violations - */ -function shouldShowViolations(report: Report, transactionViolations: OnyxCollection) { - const {parentReportID, parentReportActionID} = report ?? {}; - const canGetParentReport = parentReportID && parentReportActionID && allReportActions; - if (!canGetParentReport) { - return false; - } - const parentReportActions = allReportActions ? allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? {} : {}; - const parentReportAction = parentReportActions[parentReportActionID] ?? null; - if (!parentReportAction) { - return false; - } - return ReportUtils.shouldDisplayTransactionThreadViolations(report, transactionViolations, parentReportAction); -} - /** * filter options based on specific conditions */ @@ -1894,7 +1784,7 @@ function getOptions( // Filter out all the reports that shouldn't be displayed const filteredReportOptions = options.reports.filter((option) => { const report = option.item; - const doesReportHaveViolations = shouldShowViolations(report, transactionViolations); + const doesReportHaveViolations = ReportUtils.shouldShowViolations(report, transactionViolations); return ReportUtils.shouldReportBeInOptionList({ report, @@ -2625,7 +2515,6 @@ export { getPersonalDetailsForAccountIDs, getIOUConfirmationOptionsFromPayeePersonalDetail, isSearchStringMatchUserDetails, - getAllReportErrors, getPolicyExpenseReportOption, getIOUReportIDOfLastAction, getParticipantsOption, @@ -2651,13 +2540,11 @@ export { getFirstKeyForList, canCreateOptimisticPersonalDetailOption, getUserToInviteOption, - shouldShowViolations, getPersonalDetailSearchTerms, getCurrentUserSearchTerms, getEmptyOptions, shouldUseBoldText, getAlternateText, - getAllReportActionsErrorsAndReportActionThatRequiresAttention, hasReportErrors, }; diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 4a8d48d4d81a..8bf7984e3e34 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -48,7 +48,7 @@ import type {Participant} from '@src/types/onyx/IOU'; import type {SelectedParticipant} from '@src/types/onyx/NewGroupChatDraft'; import type {OriginalMessageExportedToIntegration} from '@src/types/onyx/OldDotAction'; import type Onboarding from '@src/types/onyx/Onboarding'; -import type {Errors, Icon, PendingAction} from '@src/types/onyx/OnyxCommon'; +import type {ErrorFields, Errors, Icon, PendingAction} from '@src/types/onyx/OnyxCommon'; import type {OriginalMessageChangeLog, PaymentMethodType} from '@src/types/onyx/OriginalMessage'; import type {Status} from '@src/types/onyx/PersonalDetails'; import type {ConnectionName} from '@src/types/onyx/Policy'; @@ -64,6 +64,7 @@ import * as SessionUtils from './actions/Session'; import * as CurrencyUtils from './CurrencyUtils'; import DateUtils from './DateUtils'; import {hasValidDraftComment} from './DraftCommentUtils'; +import * as ErrorUtils from './ErrorUtils'; import getAttachmentDetails from './fileDownload/getAttachmentDetails'; import getIsSmallScreenWidth from './getIsSmallScreenWidth'; import isReportMessageAttachment from './isReportMessageAttachment'; @@ -1375,7 +1376,7 @@ function findLastAccessedReport(ignoreDomainRooms: boolean, openOnAdminRoom = fa } // We allow public announce rooms, admins, and announce rooms through since we bypass the default rooms beta for them. - // Check where ReportUtils.findLastAccessedReport is called in MainDrawerNavigator.js for more context. + // Check where findLastAccessedReport is called in MainDrawerNavigator.js for more context. // Domain rooms are now the only type of default room that are on the defaultRooms beta. if (ignoreDomainRooms && isDomainRoom(report) && !hasExpensifyGuidesEmails(Object.keys(report?.participants ?? {}).map(Number))) { return false; @@ -6217,6 +6218,112 @@ function shouldAdminsRoomBeVisible(report: OnyxEntry): boolean { return true; } +/** + * Check whether report has violations + */ +function shouldShowViolations(report: Report, transactionViolations: OnyxCollection) { + const {parentReportID, parentReportActionID} = report ?? {}; + const canGetParentReport = parentReportID && parentReportActionID && allReportActions; + if (!canGetParentReport) { + return false; + } + const parentReportActions = allReportActions ? allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? {} : {}; + const parentReportAction = parentReportActions[parentReportActionID] ?? null; + if (!parentReportAction) { + return false; + } + return shouldDisplayTransactionThreadViolations(report, transactionViolations, parentReportAction); +} + +type ReportErrorsAndReportActionThatRequiresAttention = { + errors: ErrorFields; + reportAction?: OnyxEntry; +}; + +function getAllReportActionsErrorsAndReportActionThatRequiresAttention(report: OnyxEntry, reportActions: OnyxEntry): ReportErrorsAndReportActionThatRequiresAttention { + const reportActionsArray = Object.values(reportActions ?? {}); + const reportActionErrors: ErrorFields = {}; + let reportAction: OnyxEntry; + + for (const action of reportActionsArray) { + if (action && !isEmptyObject(action.errors)) { + Object.assign(reportActionErrors, action.errors); + + if (!reportAction) { + reportAction = action; + } + } + } + const parentReportAction: OnyxEntry = + !report?.parentReportID || !report?.parentReportActionID ? undefined : allReportActions?.[report.parentReportID ?? '-1']?.[report.parentReportActionID ?? '-1']; + + if (ReportActionsUtils.wasActionTakenByCurrentUser(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction)) { + const transactionID = ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID : null; + const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; + if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !isSettled(transaction?.reportID)) { + reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); + reportAction = undefined; + } + } else if ((isIOUReport(report) || isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) { + if (shouldShowRBRForMissingSmartscanFields(report?.reportID ?? '-1') && !isSettled(report?.reportID)) { + reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); + reportAction = getReportActionWithMissingSmartscanFields(report?.reportID ?? '-1'); + } + } else if (hasSmartscanError(reportActionsArray)) { + reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage'); + reportAction = getReportActionWithSmartscanError(reportActionsArray); + } + + return { + errors: reportActionErrors, + reportAction, + }; +} + +/** + * Get an object of error messages keyed by microtime by combining all error objects related to the report. + */ +function getAllReportErrors(report: OnyxEntry, reportActions: OnyxEntry): Errors { + const reportErrors = report?.errors ?? {}; + const reportErrorFields = report?.errorFields ?? {}; + const {errors: reportActionErrors} = getAllReportActionsErrorsAndReportActionThatRequiresAttention(report, reportActions); + + // All error objects related to the report. Each object in the sources contains error messages keyed by microtime + const errorSources = { + reportErrors, + ...reportErrorFields, + ...reportActionErrors, + }; + + // Combine all error messages keyed by microtime into one object + const errorSourcesArray = Object.values(errorSources ?? {}); + const allReportErrors = {}; + + for (const errors of errorSourcesArray) { + if (!isEmptyObject(errors)) { + Object.assign(allReportErrors, errors); + } + } + return allReportErrors; +} + +function hasReportErrorsOtherThanFailedReceipt(report: Report, doesReportHaveViolations: boolean, transactionViolations: OnyxCollection) { + const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {}; + const allReportErrors = getAllReportErrors(report, reportActions) ?? {}; + const transactionReportActions = ReportActionsUtils.getAllReportActions(report.reportID); + const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, transactionReportActions, undefined); + let doesTransactionThreadReportHasViolations = false; + if (oneTransactionThreadReportID) { + const transactionReport = getReport(oneTransactionThreadReportID); + doesTransactionThreadReportHasViolations = !!transactionReport && shouldShowViolations(transactionReport, transactionViolations); + } + return ( + doesTransactionThreadReportHasViolations || + doesReportHaveViolations || + Object.values(allReportErrors).some((error) => error?.[0] !== Localize.translateLocal('iou.error.genericSmartscanFailureMessage')) + ); +} + type ShouldReportBeInOptionListParams = { report: OnyxEntry; currentReportId: string; @@ -8428,6 +8535,10 @@ export { hasMissingInvoiceBankAccount, reasonForReportToBeInOptionList, getReasonAndReportActionThatRequiresAttention, + hasReportErrorsOtherThanFailedReceipt, + shouldShowViolations, + getAllReportErrors, + getAllReportActionsErrorsAndReportActionThatRequiresAttention, }; export type { diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 99811645a6ea..dd62b40a2493 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -105,23 +105,11 @@ function getOrderedReportIDs( if ((Object.values(CONST.REPORT.UNSUPPORTED_TYPE) as string[]).includes(report?.type ?? '')) { return; } - const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {}; const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '-1', report?.parentReportActionID ?? '-1'); - const doesReportHaveViolations = OptionsListUtils.shouldShowViolations(report, transactionViolations); + const doesReportHaveViolations = ReportUtils.shouldShowViolations(report, transactionViolations); const isHidden = ReportUtils.getReportNotificationPreference(report) === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN; const isFocused = report.reportID === currentReportId; - const allReportErrors = OptionsListUtils.getAllReportErrors(report, reportActions) ?? {}; - const transactionReportActions = ReportActionsUtils.getAllReportActions(report.reportID); - const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, transactionReportActions, undefined); - let doesTransactionThreadReportHasViolations = false; - if (oneTransactionThreadReportID) { - const transactionReport = ReportUtils.getReport(oneTransactionThreadReportID); - doesTransactionThreadReportHasViolations = !!transactionReport && OptionsListUtils.shouldShowViolations(transactionReport, transactionViolations); - } - const hasErrorsOtherThanFailedReceipt = - doesTransactionThreadReportHasViolations || - doesReportHaveViolations || - Object.values(allReportErrors).some((error) => error?.[0] !== Localize.translateLocal('iou.error.genericSmartscanFailureMessage')); + const hasErrorsOtherThanFailedReceipt = ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations); const isReportInAccessible = report?.errorFields?.notFound; if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0', parentReportAction)) { return; @@ -235,7 +223,7 @@ function getOrderedReportIDs( } function shouldShowRedBrickRoad(report: Report, reportActions: OnyxEntry, hasViolations: boolean, transactionViolations?: OnyxCollection) { - const hasErrors = Object.keys(OptionsListUtils.getAllReportErrors(report, reportActions)).length !== 0; + const hasErrors = Object.keys(ReportUtils.getAllReportErrors(report, reportActions)).length !== 0; const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, ReportActionsUtils.getAllReportActions(report.reportID)); if (oneTransactionThreadReportID) { @@ -291,7 +279,7 @@ function getOptionData({ const result: ReportUtils.OptionData = { text: '', alternateText: undefined, - allReportErrors: OptionsListUtils.getAllReportErrors(report, reportActions), + allReportErrors: ReportUtils.getAllReportErrors(report, reportActions), brickRoadIndicator: null, tooltipText: null, subtitle: undefined, diff --git a/src/libs/WorkspacesSettingsUtils.ts b/src/libs/WorkspacesSettingsUtils.ts index d8cd2ff00828..b8365d9e89d6 100644 --- a/src/libs/WorkspacesSettingsUtils.ts +++ b/src/libs/WorkspacesSettingsUtils.ts @@ -60,7 +60,7 @@ Onyx.connect({ */ const getBrickRoadForPolicy = (report: Report, altReportActions?: OnyxCollection): BrickRoad => { const reportActions = (altReportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {}; - const reportErrors = OptionsListUtils.getAllReportErrors(report, reportActions); + const reportErrors = ReportUtils.getAllReportErrors(report, reportActions); const oneTransactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(report.reportID, reportActions); let doesReportContainErrors = Object.keys(reportErrors ?? {}).length !== 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined; From 39bdf3309c65663506eedcab50cc50b58c7b81e2 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Tue, 8 Oct 2024 20:53:29 +0100 Subject: [PATCH 02/10] chore: fix test cases failing for DebugUtils --- src/libs/DebugUtils.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index af682e9cdc6a..6e95dca7114a 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -599,10 +599,6 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath const doesReportHaveViolations = ReportUtils.shouldShowViolations(report, transactionViolations); - if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { - return `debug.reasonVisibleInLHN.hasRBR`; - } - const reason = ReportUtils.reasonForReportToBeInOptionList({ report, // We can't pass report.reportID because it will cause reason to always be isFocused @@ -615,8 +611,12 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath includeSelfDM: true, }); - // When there's no specific reason, we default to isFocused since the report is only showing because we're viewing it + // When there's no specific reason, we default to isFocused if the report is only showing because we're viewing it + // Otherwise we return hasRBR if the report has errors other that failed receipt if (reason === null || reason === CONST.REPORT_IN_LHN_REASONS.DEFAULT) { + if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { + return `debug.reasonVisibleInLHN.hasRBR`; + } return 'debug.reasonVisibleInLHN.isFocused'; } From f0dc3c16dfbee852a5eb5d026ece715568b3ba02 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Tue, 8 Oct 2024 20:53:43 +0100 Subject: [PATCH 03/10] chore: remove unused dependency --- src/libs/WorkspacesSettingsUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/WorkspacesSettingsUtils.ts b/src/libs/WorkspacesSettingsUtils.ts index b8365d9e89d6..2be641035be7 100644 --- a/src/libs/WorkspacesSettingsUtils.ts +++ b/src/libs/WorkspacesSettingsUtils.ts @@ -9,7 +9,6 @@ import type {Policy, ReimbursementAccount, Report, ReportAction, ReportActions, import type {PolicyConnectionSyncProgress, Unit} from '@src/types/onyx/Policy'; import {isConnectionInProgress} from './actions/connections'; import * as CurrencyUtils from './CurrencyUtils'; -import * as OptionsListUtils from './OptionsListUtils'; import {hasCustomUnitsError, hasEmployeeListError, hasPolicyError, hasSyncError, hasTaxRateError} from './PolicyUtils'; import * as ReportActionsUtils from './ReportActionsUtils'; import * as ReportConnection from './ReportConnection'; From 13807c5f0132562e6569284c0b41261ba3ea8558 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Thu, 10 Oct 2024 09:51:59 +0100 Subject: [PATCH 04/10] fix(debug mode): hasGBR showing instead of hasRBR in debug details page --- src/libs/DebugUtils.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 3b06c8b98540..db84c2b473a7 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -610,12 +610,13 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath includeSelfDM: true, }); + if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { + return `debug.reasonVisibleInLHN.hasRBR`; + } + // When there's no specific reason, we default to isFocused if the report is only showing because we're viewing it // Otherwise we return hasRBR if the report has errors other that failed receipt if (reason === null || reason === CONST.REPORT_IN_LHN_REASONS.DEFAULT) { - if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { - return `debug.reasonVisibleInLHN.hasRBR`; - } return 'debug.reasonVisibleInLHN.isFocused'; } From 9358babd497b9c7f7b857bb32c6adc87660122fd Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Thu, 10 Oct 2024 10:18:17 +0100 Subject: [PATCH 05/10] fix(debug mode): not showing specific RBR reasons for report to be in LHN --- src/libs/DebugUtils.ts | 5 ++++- tests/unit/DebugUtilsTest.ts | 5 ++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index db84c2b473a7..3b6c1c77713f 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -610,7 +610,10 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath includeSelfDM: true, }); - if (ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations)) { + if ( + !([CONST.REPORT_IN_LHN_REASONS.HAS_ADD_WORKSPACE_ROOM_ERRORS, CONST.REPORT_IN_LHN_REASONS.HAS_IOU_VIOLATIONS] as Array).includes(reason) && + ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations) + ) { return `debug.reasonVisibleInLHN.hasRBR`; } diff --git a/tests/unit/DebugUtilsTest.ts b/tests/unit/DebugUtilsTest.ts index 34c2ad2bde73..7ac77fa1b448 100644 --- a/tests/unit/DebugUtilsTest.ts +++ b/tests/unit/DebugUtilsTest.ts @@ -963,8 +963,7 @@ describe('DebugUtils', () => { ); expect(reportAction).toBeUndefined(); }); - // TODO: remove '.failing' once the implementation is fixed - it.failing('returns parentReportAction if it is a transaction thread, the transaction is missing smart scan fields and the report is not settled', async () => { + it('returns undefined if it is a transaction thread, the transaction is missing smart scan fields and the report is not settled', async () => { const MOCK_REPORTS: ReportCollectionDataSet = { [`${ONYXKEYS.COLLECTION.REPORT}1` as const]: { reportID: '1', @@ -1011,7 +1010,7 @@ describe('DebugUtils', () => { MOCK_REPORTS[`${ONYXKEYS.COLLECTION.REPORT}1`] as Report, undefined, ); - expect(reportAction).toBe(1); + expect(reportAction).toBe(undefined); }); describe("Report has missing fields, isn't settled and it's owner is the current user", () => { describe('Report is IOU', () => { From 2fd6a047a5cc45353e016c42d5fbf72ce252347f Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Thu, 10 Oct 2024 16:05:19 +0100 Subject: [PATCH 06/10] chore: add test cases for hasRBR --- tests/unit/DebugUtilsTest.ts | 125 +++++++++++++++++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/tests/unit/DebugUtilsTest.ts b/tests/unit/DebugUtilsTest.ts index 7ac77fa1b448..a87648d08eec 100644 --- a/tests/unit/DebugUtilsTest.ts +++ b/tests/unit/DebugUtilsTest.ts @@ -783,6 +783,131 @@ describe('DebugUtils', () => { const reason = DebugUtils.getReasonForShowingRowInLHN(baseReport); expect(reason).toBe('debug.reasonVisibleInLHN.isFocused'); }); + it('returns correct reason when report has one transaction thread with violations', async () => { + const MOCK_TRANSACTION_REPORT: Report = { + reportID: '1', + ownerAccountID: 12345, + type: CONST.REPORT.TYPE.EXPENSE, + }; + const MOCK_REPORTS: ReportCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT}1` as const]: MOCK_TRANSACTION_REPORT, + [`${ONYXKEYS.COLLECTION.REPORT}2` as const]: { + reportID: '2', + type: CONST.REPORT.TYPE.CHAT, + parentReportID: '1', + parentReportActionID: '1', + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + }, + }; + const MOCK_REPORT_ACTIONS: ReportActionsCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}1` as const]: { + // eslint-disable-next-line @typescript-eslint/naming-convention + '1': { + reportActionID: '1', + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + actorAccountID: 12345, + created: '2024-08-08 18:20:44.171', + childReportID: '2', + message: { + type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + amount: 10, + currency: CONST.CURRENCY.USD, + IOUReportID: '1', + text: 'Vacation expense', + IOUTransactionID: '1', + }, + }, + }, + }; + await Onyx.multiSet({ + ...MOCK_REPORTS, + ...MOCK_REPORT_ACTIONS, + [ONYXKEYS.SESSION]: { + accountID: 12345, + }, + [`${ONYXKEYS.COLLECTION.TRANSACTION}1` as const]: { + transactionID: '1', + amount: 10, + modifiedAmount: 10, + reportID: '1', + }, + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}1` as const]: [ + { + type: CONST.VIOLATION_TYPES.VIOLATION, + name: CONST.VIOLATIONS.MISSING_CATEGORY, + }, + ], + }); + const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_TRANSACTION_REPORT); + expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); + }); + it('returns correct reason when report has violations', async () => { + const MOCK_EXPENSE_REPORT: Report = { + reportID: '1', + chatReportID: '2', + parentReportID: '2', + parentReportActionID: '1', + ownerAccountID: 12345, + stateNum: CONST.REPORT.STATE_NUM.SUBMITTED, + type: CONST.REPORT.TYPE.EXPENSE, + }; + const MOCK_REPORTS: ReportCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT}1` as const]: MOCK_EXPENSE_REPORT, + [`${ONYXKEYS.COLLECTION.REPORT}2` as const]: { + reportID: '2', + chatType: CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT, + }, + }; + const MOCK_REPORT_ACTIONS: ReportActionsCollectionDataSet = { + [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}2` as const]: { + // eslint-disable-next-line @typescript-eslint/naming-convention + '1': { + reportActionID: '1', + actionName: CONST.REPORT.ACTIONS.TYPE.IOU, + actorAccountID: 12345, + created: '2024-08-08 18:20:44.171', + message: { + type: CONST.IOU.REPORT_ACTION_TYPE.CREATE, + amount: 10, + currency: CONST.CURRENCY.USD, + IOUReportID: '1', + text: 'Vacation expense', + IOUTransactionID: '1', + }, + }, + }, + }; + await Onyx.multiSet({ + ...MOCK_REPORTS, + ...MOCK_REPORT_ACTIONS, + [ONYXKEYS.SESSION]: { + accountID: 12345, + }, + [`${ONYXKEYS.COLLECTION.TRANSACTION}1` as const]: { + transactionID: '1', + amount: 10, + modifiedAmount: 10, + reportID: '1', + }, + [`${ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS}1` as const]: [ + { + type: CONST.VIOLATION_TYPES.VIOLATION, + name: CONST.VIOLATIONS.MISSING_CATEGORY, + }, + ], + }); + const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_EXPENSE_REPORT); + expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); + }); + it('returns correct reason when report has errors', () => { + const reason = DebugUtils.getReasonForShowingRowInLHN({ + ...baseReport, + errors: { + error: 'Something went wrong', + }, + }); + expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); + }); }); describe('getReasonAndReportActionForGBRInLHNRow', () => { beforeAll(() => { From de2359ffba4eba811c3f4c19556c5828d1eab4b5 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Fri, 11 Oct 2024 18:33:46 +0100 Subject: [PATCH 07/10] fix(debug mode): is temporary focused reason shown instead of has RBR message --- src/libs/DebugUtils.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 3b6c1c77713f..97469f5f6c74 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -7,6 +7,7 @@ import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Beta, Policy, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; import * as ReportUtils from './ReportUtils'; +import SidebarUtils from './SidebarUtils'; class NumberError extends SyntaxError { constructor() { @@ -127,6 +128,15 @@ Onyx.connect({ }, }); +let reportActions: OnyxCollection; +Onyx.connect({ + key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, + waitForCollectionCallback: true, + callback: (value) => { + reportActions = value; + }, +}); + let transactionViolations: OnyxCollection; Onyx.connect({ key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, @@ -612,7 +622,7 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath if ( !([CONST.REPORT_IN_LHN_REASONS.HAS_ADD_WORKSPACE_ROOM_ERRORS, CONST.REPORT_IN_LHN_REASONS.HAS_IOU_VIOLATIONS] as Array).includes(reason) && - ReportUtils.hasReportErrorsOtherThanFailedReceipt(report, doesReportHaveViolations, transactionViolations) + SidebarUtils.shouldShowRedBrickRoad(report, reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`], doesReportHaveViolations, transactionViolations) ) { return `debug.reasonVisibleInLHN.hasRBR`; } From 6a8b6fd22ecf49d14013c7dff83c3dcba8c2c2a2 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Fri, 11 Oct 2024 18:38:15 +0100 Subject: [PATCH 08/10] chore: fix eslint errors --- src/libs/DebugUtils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 97469f5f6c74..261d2db208d9 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -128,12 +128,12 @@ Onyx.connect({ }, }); -let reportActions: OnyxCollection; +let reportActionsCollection: OnyxCollection; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, waitForCollectionCallback: true, callback: (value) => { - reportActions = value; + reportActionsCollection = value; }, }); @@ -622,7 +622,7 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath if ( !([CONST.REPORT_IN_LHN_REASONS.HAS_ADD_WORKSPACE_ROOM_ERRORS, CONST.REPORT_IN_LHN_REASONS.HAS_IOU_VIOLATIONS] as Array).includes(reason) && - SidebarUtils.shouldShowRedBrickRoad(report, reportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`], doesReportHaveViolations, transactionViolations) + SidebarUtils.shouldShowRedBrickRoad(report, reportActionsCollection?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`], doesReportHaveViolations, transactionViolations) ) { return `debug.reasonVisibleInLHN.hasRBR`; } From 96f59359c35f3d5f2e4cf7f237ee64328e76db20 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Mon, 14 Oct 2024 10:34:46 +0100 Subject: [PATCH 09/10] refactor: apply suggestions --- src/libs/DebugUtils.ts | 17 ++--------------- src/pages/Debug/Report/DebugReportPage.tsx | 6 +++--- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index 261d2db208d9..fee35c50e0dc 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -7,7 +7,6 @@ import type {TranslationPaths} from '@src/languages/types'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Beta, Policy, Report, ReportAction, ReportActions, TransactionViolation} from '@src/types/onyx'; import * as ReportUtils from './ReportUtils'; -import SidebarUtils from './SidebarUtils'; class NumberError extends SyntaxError { constructor() { @@ -128,15 +127,6 @@ Onyx.connect({ }, }); -let reportActionsCollection: OnyxCollection; -Onyx.connect({ - key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, - waitForCollectionCallback: true, - callback: (value) => { - reportActionsCollection = value; - }, -}); - let transactionViolations: OnyxCollection; Onyx.connect({ key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, @@ -601,7 +591,7 @@ function validateReportActionJSON(json: string) { /** * Gets the reason for showing LHN row */ -function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPaths | null { +function getReasonForShowingRowInLHN(report: OnyxEntry, hasRBR: boolean): TranslationPaths | null { if (!report) { return null; } @@ -620,10 +610,7 @@ function getReasonForShowingRowInLHN(report: OnyxEntry): TranslationPath includeSelfDM: true, }); - if ( - !([CONST.REPORT_IN_LHN_REASONS.HAS_ADD_WORKSPACE_ROOM_ERRORS, CONST.REPORT_IN_LHN_REASONS.HAS_IOU_VIOLATIONS] as Array).includes(reason) && - SidebarUtils.shouldShowRedBrickRoad(report, reportActionsCollection?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.reportID}`], doesReportHaveViolations, transactionViolations) - ) { + if (!([CONST.REPORT_IN_LHN_REASONS.HAS_ADD_WORKSPACE_ROOM_ERRORS, CONST.REPORT_IN_LHN_REASONS.HAS_IOU_VIOLATIONS] as Array).includes(reason) && hasRBR) { return `debug.reasonVisibleInLHN.hasRBR`; } diff --git a/src/pages/Debug/Report/DebugReportPage.tsx b/src/pages/Debug/Report/DebugReportPage.tsx index 530b4b5f4aec..28f4ddf3dc34 100644 --- a/src/pages/Debug/Report/DebugReportPage.tsx +++ b/src/pages/Debug/Report/DebugReportPage.tsx @@ -59,12 +59,12 @@ function DebugReportPage({ return []; } - const reasonLHN = DebugUtils.getReasonForShowingRowInLHN(report); - const {reason: reasonGBR, reportAction: reportActionGBR} = DebugUtils.getReasonAndReportActionForGBRInLHNRow(report) ?? {}; - const reportActionRBR = DebugUtils.getRBRReportAction(report, reportActions); const shouldDisplayViolations = ReportUtils.shouldDisplayTransactionThreadViolations(report, transactionViolations, parentReportAction); const shouldDisplayReportViolations = ReportUtils.isReportOwner(report) && ReportUtils.hasReportViolations(reportID); const hasRBR = SidebarUtils.shouldShowRedBrickRoad(report, reportActions, !!shouldDisplayViolations || shouldDisplayReportViolations, transactionViolations); + const reasonLHN = DebugUtils.getReasonForShowingRowInLHN(report, hasRBR); + const {reason: reasonGBR, reportAction: reportActionGBR} = DebugUtils.getReasonAndReportActionForGBRInLHNRow(report) ?? {}; + const reportActionRBR = DebugUtils.getRBRReportAction(report, reportActions); const hasGBR = !hasRBR && !!reasonGBR; return [ From 212f461eeba8e82d7256a7c15b133f7b1b199f37 Mon Sep 17 00:00:00 2001 From: Pedro Guerreiro Date: Mon, 14 Oct 2024 10:52:04 +0100 Subject: [PATCH 10/10] fix: debug utils hasRBR tests --- src/libs/DebugUtils.ts | 2 +- tests/unit/DebugUtilsTest.ts | 17 ++++++++++------- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libs/DebugUtils.ts b/src/libs/DebugUtils.ts index fee35c50e0dc..e7ad63467781 100644 --- a/src/libs/DebugUtils.ts +++ b/src/libs/DebugUtils.ts @@ -591,7 +591,7 @@ function validateReportActionJSON(json: string) { /** * Gets the reason for showing LHN row */ -function getReasonForShowingRowInLHN(report: OnyxEntry, hasRBR: boolean): TranslationPaths | null { +function getReasonForShowingRowInLHN(report: OnyxEntry, hasRBR = false): TranslationPaths | null { if (!report) { return null; } diff --git a/tests/unit/DebugUtilsTest.ts b/tests/unit/DebugUtilsTest.ts index a87648d08eec..fa44b8972cf3 100644 --- a/tests/unit/DebugUtilsTest.ts +++ b/tests/unit/DebugUtilsTest.ts @@ -838,7 +838,7 @@ describe('DebugUtils', () => { }, ], }); - const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_TRANSACTION_REPORT); + const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_TRANSACTION_REPORT, true); expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); }); it('returns correct reason when report has violations', async () => { @@ -896,16 +896,19 @@ describe('DebugUtils', () => { }, ], }); - const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_EXPENSE_REPORT); + const reason = DebugUtils.getReasonForShowingRowInLHN(MOCK_EXPENSE_REPORT, true); expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); }); it('returns correct reason when report has errors', () => { - const reason = DebugUtils.getReasonForShowingRowInLHN({ - ...baseReport, - errors: { - error: 'Something went wrong', + const reason = DebugUtils.getReasonForShowingRowInLHN( + { + ...baseReport, + errors: { + error: 'Something went wrong', + }, }, - }); + true, + ); expect(reason).toBe('debug.reasonVisibleInLHN.hasRBR'); }); });