From 3be886e795fc0aee722f81b265b3cfcad81e0893 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Mon, 25 Aug 2025 12:56:08 +0200 Subject: [PATCH 01/11] feat: Combined optimistic report names functionality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR combines the functionality from three previously reverted PRs: - #65862: Initial optimistic Custom Report Names implementation - #68726: Fix for reassure performance tests - #68737: Extended transaction support for report name computation ### Key Features: - Real-time, formula-based report titles that update as expense data changes - Support for report fields: type, startdate, total, currency, policyname, created - Functions: frontpart, substr, domain for string manipulation - Optimistic updates work offline and provide visible changes without network - Transaction updates trigger report name recalculation - Performance optimized with caching and targeted updates - Beta flag controlled: authAutoReportTitle ### Implementation Details: - Formula.ts: Formula parsing and computation engine - OptimisticReportNames.ts: Main logic for report name updates - OptimisticReportNamesConnectionManager.ts: Onyx data context manager - Enhanced API middleware to process optimistic data - Performance and unit tests included 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- cspell.json | 1 + src/CONST/index.ts | 4 + src/libs/API/index.ts | 26 +- src/libs/Formula.ts | 505 ++++++++++++++++++ src/libs/OptimisticReportNames.ts | 391 ++++++++++++++ .../OptimisticReportNamesConnectionManager.ts | 123 +++++ .../OptimisticReportNames.perf-test.ts | 263 +++++++++ tests/unit/OptimisticReportNamesTest.ts | 445 +++++++++++++++ 8 files changed, 1756 insertions(+), 2 deletions(-) create mode 100644 src/libs/Formula.ts create mode 100644 src/libs/OptimisticReportNames.ts create mode 100644 src/libs/OptimisticReportNamesConnectionManager.ts create mode 100644 tests/perf-test/OptimisticReportNames.perf-test.ts create mode 100644 tests/unit/OptimisticReportNamesTest.ts diff --git a/cspell.json b/cspell.json index cb9892ec87dfe..b599cbf2f3d94 100644 --- a/cspell.json +++ b/cspell.json @@ -239,6 +239,7 @@ "formatjs", "Français", "Frederico", + "freetext", "frontpart", "fullstory", "FWTV", diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 9c99f51274ec6..ddafaa18d06f1 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -666,6 +666,7 @@ const CONST = { BETAS: { ALL: 'all', ASAP_SUBMIT: 'asapSubmit', + AUTH_AUTO_REPORT_TITLE: 'authAutoReportTitle', DEFAULT_ROOMS: 'defaultRooms', P2P_DISTANCE_REQUESTS: 'p2pDistanceRequests', SPOTNANA_TRAVEL: 'spotnanaTravel', @@ -1530,6 +1531,9 @@ const CONST = { APPLY_AIRSHIP_UPDATES: 'apply_airship_updates', APPLY_PUSHER_UPDATES: 'apply_pusher_updates', APPLY_HTTPS_UPDATES: 'apply_https_updates', + COMPUTE_REPORT_NAME: 'compute_report_name', + COMPUTE_REPORT_NAME_FOR_NEW_REPORT: 'compute_report_name_for_new_report', + UPDATE_OPTIMISTIC_REPORT_NAMES: 'update_optimistic_report_names', COLD: 'cold', WARM: 'warm', REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1500, diff --git a/src/libs/API/index.ts b/src/libs/API/index.ts index 50225e8c232ec..4157c20e04902 100644 --- a/src/libs/API/index.ts +++ b/src/libs/API/index.ts @@ -7,6 +7,8 @@ import Log from '@libs/Log'; import {handleDeletedAccount, HandleUnusedOptimisticID, Logging, Pagination, Reauthentication, RecheckConnection, SaveResponseInOnyx} from '@libs/Middleware'; import {isOffline} from '@libs/Network/NetworkStore'; import {push as pushToSequentialQueue, waitForIdle as waitForSequentialQueueIdle} from '@libs/Network/SequentialQueue'; +import * as OptimisticReportNames from '@libs/OptimisticReportNames'; +import {getUpdateContext, initialize as initializeOptimisticReportNamesContext} from '@libs/OptimisticReportNamesConnectionManager'; import Pusher from '@libs/Pusher'; import {processWithMiddleware, use} from '@libs/Request'; import {getAll, getLength as getPersistedRequestsLength} from '@userActions/PersistedRequests'; @@ -15,7 +17,7 @@ import type OnyxRequest from '@src/types/onyx/Request'; import type {PaginatedRequest, PaginationConfig, RequestConflictResolver} from '@src/types/onyx/Request'; import type Response from '@src/types/onyx/Response'; import type {ApiCommand, ApiRequestCommandParameters, ApiRequestType, CommandOfType, ReadCommand, SideEffectRequestCommand, WriteCommand} from './types'; -import {READ_COMMANDS} from './types'; +import {READ_COMMANDS, WRITE_COMMANDS} from './types'; // Setup API middlewares. Each request made will pass through a series of middleware functions that will get called in sequence (each one passing the result of the previous to the next). // Note: The ordering here is intentional as we want to Log, Recheck Connection, Reauthenticate, and Save the Response in Onyx. Errors thrown in one middleware will bubble to the next. @@ -42,6 +44,11 @@ use(Pagination); // middlewares after this, because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state. use(SaveResponseInOnyx); +// Initialize OptimisticReportNames context on module load +initializeOptimisticReportNamesContext().catch(() => { + Log.warn('Failed to initialize OptimisticReportNames context'); +}); + let requestIndex = 0; type OnyxData = { @@ -74,7 +81,22 @@ function prepareRequest( const {optimisticData, ...onyxDataWithoutOptimisticData} = onyxData; if (optimisticData && shouldApplyOptimisticData) { Log.info('[API] Applying optimistic data', false, {command, type}); - Onyx.update(optimisticData); + + // Process optimistic data through report name middleware + // Skip for OpenReport command to avoid unnecessary processing + if (command === WRITE_COMMANDS.OPEN_REPORT) { + Onyx.update(optimisticData); + } else { + try { + const context = getUpdateContext(); + const processedOptimisticData = OptimisticReportNames.updateOptimisticReportNamesFromUpdates(optimisticData, context); + Onyx.update(processedOptimisticData); + } catch (error) { + Log.warn('[API] Failed to process optimistic report names', {error}); + // Fallback to original optimistic data if processing fails + Onyx.update(optimisticData); + } + } } const isWriteRequest = type === CONST.API_REQUEST_TYPE.WRITE; diff --git a/src/libs/Formula.ts b/src/libs/Formula.ts new file mode 100644 index 0000000000000..ae272355253da --- /dev/null +++ b/src/libs/Formula.ts @@ -0,0 +1,505 @@ +import type {OnyxEntry} from 'react-native-onyx'; +import type {ValueOf} from 'type-fest'; +import CONST from '@src/CONST'; +import type {Policy, Report, Transaction} from '@src/types/onyx'; +import {getCurrencySymbol} from './CurrencyUtils'; +import {getAllReportActions} from './ReportActionsUtils'; +import {getReportTransactions} from './ReportUtils'; +import {getCreated, isPartialTransaction} from './TransactionUtils'; + +type FormulaPart = { + /** The original definition from the formula */ + definition: string; + + /** The type of formula part (report, field, user, etc.) */ + type: ValueOf; + + /** The field path for accessing data (e.g., ['type'], ['startdate'], ['total']) */ + fieldPath: string[]; + + /** Functions to apply to the computed value (e.g., ['frontPart']) */ + functions: string[]; +}; + +type FormulaContext = { + report: Report; + policy: OnyxEntry; + transaction?: Transaction; +}; + +const FORMULA_PART_TYPES = { + REPORT: 'report', + FIELD: 'field', + USER: 'user', + FREETEXT: 'freetext', +} as const; + +/** + * Extract formula parts from a formula string, handling nested braces and escapes + * Based on OldDot Formula.extract method + */ +function extract(formula: string, opener = '{', closer = '}'): string[] { + if (!formula || typeof formula !== 'string') { + return []; + } + + const letters = formula.split(''); + const sections: string[] = []; + let nesting = 0; + let start = 0; + + for (let i = 0; i < letters.length; i++) { + // Found an escape character, skip the next character + if (letters.at(i) === '\\') { + i++; + continue; + } + + // Found an opener, save the spot + if (letters.at(i) === opener) { + if (nesting === 0) { + start = i; + } + nesting++; + } + + // Found a closer, decrement the nesting and possibly extract it + if (letters.at(i) === closer && nesting > 0) { + nesting--; + if (nesting === 0) { + sections.push(formula.substring(start, i + 1)); + } + } + } + + return sections; +} + +/** + * Parse a formula string into an array of formula parts + * Based on OldDot Formula.parse method + */ +function parse(formula: string): FormulaPart[] { + if (!formula || typeof formula !== 'string') { + return []; + } + + const parts: FormulaPart[] = []; + const formulaParts = extract(formula); + + // If no formula parts found, treat the entire string as free text + if (formulaParts.length === 0) { + if (formula.trim()) { + parts.push({ + definition: formula, + type: FORMULA_PART_TYPES.FREETEXT, + fieldPath: [], + functions: [], + }); + } + return parts; + } + + // Process the formula by splitting on formula parts to preserve free text + let lastIndex = 0; + + formulaParts.forEach((part) => { + const partIndex = formula.indexOf(part, lastIndex); + + // Add any free text before this formula part + if (partIndex > lastIndex) { + const freeText = formula.substring(lastIndex, partIndex); + if (freeText) { + parts.push({ + definition: freeText, + type: FORMULA_PART_TYPES.FREETEXT, + fieldPath: [], + functions: [], + }); + } + } + + // Add the formula part + parts.push(parsePart(part)); + lastIndex = partIndex + part.length; + }); + + // Add any remaining free text after the last formula part + if (lastIndex < formula.length) { + const freeText = formula.substring(lastIndex); + if (freeText) { + parts.push({ + definition: freeText, + type: FORMULA_PART_TYPES.FREETEXT, + fieldPath: [], + functions: [], + }); + } + } + + return parts; +} + +/** + * Parse a single formula part definition into a FormulaPart object + * Based on OldDot Formula.parsePart method + */ +function parsePart(definition: string): FormulaPart { + const part: FormulaPart = { + definition, + type: FORMULA_PART_TYPES.FREETEXT, + fieldPath: [], + functions: [], + }; + + // If it doesn't start and end with braces, it's free text + if (!definition.startsWith('{') || !definition.endsWith('}')) { + return part; + } + + // Remove the braces and trim + const cleanDefinition = definition.slice(1, -1).trim(); + if (!cleanDefinition) { + return part; + } + + // Split on | to separate functions + const segments = cleanDefinition.split('|'); + const fieldSegment = segments.at(0); + const functions = segments.slice(1); + + // Split the field segment on : to get the field path + const fieldPath = fieldSegment?.split(':'); + const type = fieldPath?.at(0)?.toLowerCase(); + + // Determine the formula part type + if (type === 'report') { + part.type = FORMULA_PART_TYPES.REPORT; + } else if (type === 'field') { + part.type = FORMULA_PART_TYPES.FIELD; + } else if (type === 'user') { + part.type = FORMULA_PART_TYPES.USER; + } + + // Set field path (excluding the type) + part.fieldPath = fieldPath?.slice(1) ?? []; + part.functions = functions; + + return part; +} + +/** + * Compute the value of a formula given a context + */ +function compute(formula: string, context: FormulaContext): string { + if (!formula || typeof formula !== 'string') { + return ''; + } + + const parts = parse(formula); + let result = ''; + + for (const part of parts) { + let value = ''; + + switch (part.type) { + case FORMULA_PART_TYPES.REPORT: + value = computeReportPart(part, context); + value = value === '' ? part.definition : value; + break; + case FORMULA_PART_TYPES.FIELD: + value = computeFieldPart(part); + break; + case FORMULA_PART_TYPES.USER: + value = computeUserPart(part); + break; + case FORMULA_PART_TYPES.FREETEXT: + value = part.definition; + break; + default: + // If we don't recognize the part type, use the original definition + value = part.definition; + } + + // Apply any functions to the computed value + value = applyFunctions(value, part.functions); + result += value; + } + + return result; +} + +/** + * Compute the value of a report formula part + */ +function computeReportPart(part: FormulaPart, context: FormulaContext): string { + const {report, policy} = context; + const [field, format] = part.fieldPath; + + if (!field) { + return part.definition; + } + + switch (field.toLowerCase()) { + case 'type': + return formatType(report.type); + case 'startdate': + return formatDate(getOldestTransactionDate(report.reportID, context), format); + case 'total': + return formatAmount(report.total, getCurrencySymbol(report.currency ?? '') ?? report.currency); + case 'currency': + return report.currency ?? ''; + case 'policyname': + case 'workspacename': + return policy?.name ?? ''; + case 'created': + // Backend will always return at least one report action (of type created) and its date is equal to report's creation date + // We can make it slightly more efficient in the future by ensuring report.created is always present in backend's responses + return formatDate(getOldestReportActionDate(report.reportID), format); + default: + return part.definition; + } +} + +/** + * Compute the value of a field formula part + */ +function computeFieldPart(part: FormulaPart): string { + // Field computation will be implemented later + return part.definition; +} + +/** + * Compute the value of a user formula part + */ +function computeUserPart(part: FormulaPart): string { + // User computation will be implemented later + return part.definition; +} + +/** + * Apply functions to a computed value + */ +function applyFunctions(value: string, functions: string[]): string { + let result = value; + + for (const func of functions) { + const [functionName, ...args] = func.split(':'); + + switch (functionName.toLowerCase()) { + case 'frontpart': + result = getFrontPart(result); + break; + case 'substr': + result = getSubstring(result, args); + break; + case 'domain': + result = getDomainName(result); + break; + default: + // Unknown function, leave value as is + break; + } + } + + return result; +} + +/** + * Get the front part of an email or first word of a string + */ +function getFrontPart(value: string): string { + const trimmed = value.trim(); + + // If it's an email, return the part before @ + if (trimmed.includes('@')) { + return trimmed.split('@').at(0) ?? ''; + } + + // Otherwise, return the first word + return trimmed.split(' ').at(0) ?? ''; +} + +/** + * Get the domain name of an email or URL + */ +function getDomainName(value: string): string { + const trimmed = value.trim(); + + // If it's an email, return the part after @ + if (trimmed.includes('@')) { + return trimmed.split('@').at(1) ?? ''; + } + + return ''; +} + +/** + * Get substring of a value + */ +function getSubstring(value: string, args: string[]): string { + const start = parseInt(args.at(0) ?? '', 10) || 0; + const length = args.at(1) ? parseInt(args.at(1) ?? '', 10) : undefined; + + if (length !== undefined) { + return value.substring(start, start + length); + } + + return value.substring(start); +} + +/** + * Format a date value with support for multiple date formats + */ +function formatDate(dateString: string | undefined, format = 'yyyy-MM-dd'): string { + if (!dateString) { + return ''; + } + + try { + const date = new Date(dateString); + if (Number.isNaN(date.getTime())) { + return ''; + } + + const year = date.getFullYear(); + const month = date.getMonth() + 1; + const day = date.getDate(); + const monthNames = ['January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']; + const shortMonthNames = ['Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']; + + switch (format) { + case 'M/dd/yyyy': + return `${month}/${day.toString().padStart(2, '0')}/${year}`; + case 'MMMM dd, yyyy': + return `${monthNames.at(month - 1)} ${day.toString().padStart(2, '0')}, ${year}`; + case 'dd MMM yyyy': + return `${day.toString().padStart(2, '0')} ${shortMonthNames.at(month - 1)} ${year}`; + case 'yyyy/MM/dd': + return `${year}/${month.toString().padStart(2, '0')}/${day.toString().padStart(2, '0')}`; + case 'MMMM, yyyy': + return `${monthNames.at(month - 1)}, ${year}`; + case 'yy/MM/dd': + return `${year.toString().slice(-2)}/${month.toString().padStart(2, '0')}/${day.toString().padStart(2, '0')}`; + case 'dd/MM/yy': + return `${day.toString().padStart(2, '0')}/${month.toString().padStart(2, '0')}/${year.toString().slice(-2)}`; + case 'yyyy': + return year.toString(); + case 'MM/dd/yyyy': + return `${month.toString().padStart(2, '0')}/${day.toString().padStart(2, '0')}/${year}`; + case 'yyyy-MM-dd': + default: + return `${year}-${month.toString().padStart(2, '0')}-${day.toString().padStart(2, '0')}`; + } + } catch { + return ''; + } +} + +/** + * Format an amount value + */ +function formatAmount(amount: number | undefined, currency: string | undefined): string { + if (amount === undefined) { + return ''; + } + + const absoluteAmount = Math.abs(amount); + const formattedAmount = (absoluteAmount / 100).toFixed(2); + + if (currency) { + return `${currency}${formattedAmount}`; + } + + return formattedAmount; +} + +/** + * Get the date of the oldest report action for a given report + */ +function getOldestReportActionDate(reportID: string): string | undefined { + if (!reportID) { + return undefined; + } + + const reportActions = getAllReportActions(reportID); + if (!reportActions || Object.keys(reportActions).length === 0) { + return undefined; + } + + let oldestDate: string | undefined; + + Object.values(reportActions).forEach((action) => { + if (!action?.created) { + return; + } + + if (oldestDate && action.created > oldestDate) { + return; + } + oldestDate = action.created; + }); + + return oldestDate; +} + +/** + * Format a report type to its human-readable string + */ +function formatType(type: string | undefined): string { + if (!type) { + return ''; + } + + const typeMapping: Record = { + [CONST.REPORT.TYPE.EXPENSE]: 'Expense Report', + [CONST.REPORT.TYPE.INVOICE]: 'Invoice', + [CONST.REPORT.TYPE.CHAT]: 'Chat', + [CONST.REPORT.UNSUPPORTED_TYPE.BILL]: 'Bill', + [CONST.REPORT.UNSUPPORTED_TYPE.PAYCHECK]: 'Paycheck', + [CONST.REPORT.TYPE.IOU]: 'IOU', + [CONST.REPORT.TYPE.TASK]: 'Task', + trip: 'Trip', + }; + + return typeMapping[type.toLowerCase()] || type; +} + +/** + * Get the date of the oldest transaction for a given report + */ +function getOldestTransactionDate(reportID: string, context?: FormulaContext): string | undefined { + if (!reportID) { + return undefined; + } + + const transactions = getReportTransactions(reportID); + if (!transactions || transactions.length === 0) { + return new Date().toISOString(); + } + + let oldestDate: string | undefined; + + transactions.forEach((transaction) => { + // Use updated transaction data if available and matches this transaction + const currentTransaction = context?.transaction && transaction.transactionID === context.transaction.transactionID ? context.transaction : transaction; + + const created = getCreated(currentTransaction); + if (!created) { + return; + } + if (oldestDate && created >= oldestDate) { + return; + } + if (isPartialTransaction(currentTransaction)) { + return; + } + oldestDate = created; + }); + + return oldestDate; +} + +export {FORMULA_PART_TYPES, compute, extract, parse}; + +export type {FormulaContext, FormulaPart}; \ No newline at end of file diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts new file mode 100644 index 0000000000000..5fe92c178909a --- /dev/null +++ b/src/libs/OptimisticReportNames.ts @@ -0,0 +1,391 @@ +import type {OnyxUpdate} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {OnyxKey} from '@src/ONYXKEYS'; +import type {Transaction} from '@src/types/onyx'; +import type Policy from '@src/types/onyx/Policy'; +import type Report from '@src/types/onyx/Report'; +import Timing from './actions/Timing'; +import {compute, FORMULA_PART_TYPES, parse} from './Formula'; +import type {FormulaContext} from './Formula'; +import Log from './Log'; +import {getUpdateContextAsync} from './OptimisticReportNamesConnectionManager'; +import type {UpdateContext} from './OptimisticReportNamesConnectionManager'; +import Performance from './Performance'; +import Permissions from './Permissions'; +import {getTitleReportField, isArchivedReport} from './ReportUtils'; + +/** + * Get the object type from an Onyx key + */ +function determineObjectTypeByKey(key: string): 'report' | 'policy' | 'transaction' | 'unknown' { + if (key.startsWith(ONYXKEYS.COLLECTION.REPORT)) { + return 'report'; + } + if (key.startsWith(ONYXKEYS.COLLECTION.POLICY)) { + return 'policy'; + } + if (key.startsWith(ONYXKEYS.COLLECTION.TRANSACTION)) { + return 'transaction'; + } + return 'unknown'; +} + +/** + * Extract report ID from an Onyx key + */ +function getReportIDFromKey(key: string): string { + return key.replace(ONYXKEYS.COLLECTION.REPORT, ''); +} + +/** + * Extract policy ID from an Onyx key + */ +function getPolicyIDFromKey(key: string): string { + return key.replace(ONYXKEYS.COLLECTION.POLICY, ''); +} + +/** + * Extract transaction ID from an Onyx key + */ +function getTransactionIDFromKey(key: string): string { + return key.replace(ONYXKEYS.COLLECTION.TRANSACTION, ''); +} + +/** + * Get report by ID from the reports collection + */ +function getReportByID(reportID: string, allReports: Record): Report | undefined { + return allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; +} + +/** + * Get policy by ID from the policies collection + */ +function getPolicyByID(policyID: string | undefined, allPolicies: Record): Policy | undefined { + if (!policyID) { + return; + } + return allPolicies[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]; +} + +/** + * Get transaction by ID from the transactions collection + */ +function getTransactionByID(transactionID: string, allTransactions: Record): Transaction | undefined { + return allTransactions[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`]; +} + +/** + * Get all reports associated with a policy ID + */ +function getReportsByPolicyID(policyID: string, allReports: Record, context: UpdateContext): Report[] { + if (policyID === CONST.POLICY.ID_FAKE) { + return []; + } + return Object.values(allReports).filter((report) => { + if (report?.policyID !== policyID) { + return false; + } + + // Filter by type - only reports that support custom names + if (!isValidReportType(report.type)) { + return false; + } + + // Filter by state - exclude reports in high states (like approved or higher) + const stateThreshold = CONST.REPORT.STATE_NUM.APPROVED; + if (report.stateNum && report.stateNum > stateThreshold) { + return false; + } + + // Filter by isArchived - exclude archived reports + const reportNameValuePairs = context.allReportNameValuePairs[`${ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS}${report?.reportID}`]; + if (isArchivedReport(reportNameValuePairs)) { + return false; + } + + return true; + }); +} + +/** + * Get the report associated with a transaction ID + */ +function getReportByTransactionID(transactionID: string, context: UpdateContext): Report | undefined { + if (!transactionID) { + return undefined; + } + + const transaction = getTransactionByID(transactionID, context.allTransactions); + + if (!transaction?.reportID) { + return undefined; + } + + // Get the report using the transaction's reportID from context + return getReportByID(transaction.reportID, context.allReports); +} + +/** + * Generate the Onyx key for a report + */ +function getReportKey(reportID: string): OnyxKey { + return `${ONYXKEYS.COLLECTION.REPORT}${reportID}` as OnyxKey; +} + +/** + * Check if a report should have its name automatically computed + */ +function shouldComputeReportName(report: Report, policy: Policy | undefined): boolean { + // Only compute names for expense reports with policies that have title fields + if (!report || !policy) { + return false; + } + + // Check if the report is an expense report + if (!isValidReportType(report.type)) { + return false; + } + + // Check if the policy has a title field with a formula + const titleField = getTitleReportField(policy.fieldList ?? {}); + if (!titleField?.defaultValue) { + return false; + } + return true; +} + +function isValidReportType(reportType?: string): boolean { + if (!reportType) { + return false; + } + return ( + reportType === CONST.REPORT.TYPE.EXPENSE || + reportType === CONST.REPORT.TYPE.INVOICE || + reportType === CONST.REPORT.UNSUPPORTED_TYPE.BILL || + reportType === CONST.REPORT.UNSUPPORTED_TYPE.PAYCHECK || + reportType === 'trip' + ); +} + +/** + * Compute a new report name if needed based on an optimistic update + */ +function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: OnyxUpdate, context: UpdateContext): string | null { + Performance.markStart(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.start(CONST.TIMING.COMPUTE_REPORT_NAME); + + const {allPolicies} = context; + + // If no report is provided, extract it from the update (for new reports) + const targetReport = report ?? (incomingUpdate.value as Report); + + if (!targetReport) { + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return null; + } + + const policy = getPolicyByID(targetReport.policyID, allPolicies); + + if (!shouldComputeReportName(targetReport, policy)) { + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return null; + } + + const titleField = getTitleReportField(policy?.fieldList ?? {}); + if (!titleField?.defaultValue) { + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return null; + } + + // Quick check: see if the update might affect the report name + const updateType = determineObjectTypeByKey(incomingUpdate.key); + const formula = titleField.defaultValue; + const formulaParts = parse(formula); + + let transaction: Transaction | undefined; + if (updateType === 'transaction') { + transaction = getTransactionByID((incomingUpdate.value as Transaction).transactionID, context.allTransactions); + } + + // Check if any formula part might be affected by this update + const isAffected = formulaParts.some((part) => { + if (part.type === FORMULA_PART_TYPES.REPORT) { + // Checking if the formula part is affected in this manner works, but it could certainly be more precise. + // For example, a policy update only affects the part if the formula in the policy changed, or if the report part references a field on the policy. + // However, if we run into performance problems, this would be a good place to optimize. + return updateType === 'report' || updateType === 'transaction' || updateType === 'policy'; + } + if (part.type === FORMULA_PART_TYPES.FIELD) { + return updateType === 'report'; + } + return false; + }); + + if (!isAffected) { + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return null; + } + + // Build context with the updated data + const updatedReport = + updateType === 'report' && targetReport.reportID === getReportIDFromKey(incomingUpdate.key) ? {...targetReport, ...(incomingUpdate.value as Partial)} : targetReport; + + const updatedPolicy = updateType === 'policy' && targetReport.policyID === getPolicyIDFromKey(incomingUpdate.key) ? {...(policy ?? {}), ...(incomingUpdate.value as Policy)} : policy; + + const updatedTransaction = updateType === 'transaction' ? {...(transaction ?? {}), ...(incomingUpdate.value as Transaction)} : undefined; + + // Compute the new name + const formulaContext: FormulaContext = { + report: updatedReport, + policy: updatedPolicy, + transaction: updatedTransaction, + }; + + const newName = compute(formula, formulaContext); + + // Only return an update if the name actually changed + if (newName && newName !== targetReport.reportName) { + Log.info('[OptimisticReportNames] Report name computed', false, { + reportID: targetReport.reportID, + oldName: targetReport.reportName, + newName, + formula, + updateType, + isNewReport: !report, + }); + + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return newName; + } + + Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); + Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); + return null; +} + +/** + * Update optimistic report names based on incoming updates + * This is the main middleware function that processes optimistic data + */ +function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: UpdateContext): OnyxUpdate[] { + Performance.markStart(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + Timing.start(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + + const {betas, allReports} = context; + + // Check if the feature is enabled + if (!Permissions.canUseCustomReportNames(betas)) { + Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + return updates; + } + + Log.info('[OptimisticReportNames] Processing optimistic updates for report names', false, { + updatesCount: updates.length, + }); + + const additionalUpdates: OnyxUpdate[] = []; + + for (const update of updates) { + const objectType = determineObjectTypeByKey(update.key); + + switch (objectType) { + case 'report': { + const reportID = getReportIDFromKey(update.key); + const report = getReportByID(reportID, allReports); + + // Handle both existing and new reports with the same function + const reportNameUpdate = computeReportNameIfNeeded(report, update, context); + + if (reportNameUpdate) { + additionalUpdates.push({ + key: getReportKey(reportID), + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: reportNameUpdate, + }, + }); + } + break; + } + + case 'policy': { + const policyID = getPolicyIDFromKey(update.key); + const affectedReports = getReportsByPolicyID(policyID, allReports, context); + for (const report of affectedReports) { + const reportNameUpdate = computeReportNameIfNeeded(report, update, context); + + if (reportNameUpdate) { + additionalUpdates.push({ + key: getReportKey(report.reportID), + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: reportNameUpdate, + }, + }); + } + } + break; + } + + case 'transaction': { + let report: Report | undefined; + const transactionUpdate = update.value as Partial; + if (transactionUpdate.reportID) { + report = getReportByID(transactionUpdate.reportID, allReports); + } else { + report = getReportByTransactionID(getTransactionIDFromKey(update.key), context); + } + + if (report) { + const reportNameUpdate = computeReportNameIfNeeded(report, update, context); + + if (reportNameUpdate) { + additionalUpdates.push({ + key: getReportKey(report.reportID), + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: reportNameUpdate, + }, + }); + } + } + break; + } + + default: + continue; + } + } + + const updatedSet = [...updates, ...additionalUpdates]; + + Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); + + Log.info('[OptimisticReportNames] Finished processing report names', false, { + originalUpdatesCount: updates.length, + additionalUpdatesCount: additionalUpdates.length, + totalUpdatesCount: updatedSet.length, + }); + + return updatedSet; +} + +/** + * Create an update context for testing or external use + */ +function createUpdateContext(): Promise { + return getUpdateContextAsync(); +} + +export {updateOptimisticReportNamesFromUpdates, computeReportNameIfNeeded, createUpdateContext, shouldComputeReportName, getReportByTransactionID}; +export type {UpdateContext}; \ No newline at end of file diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts new file mode 100644 index 0000000000000..2176ef567bfd3 --- /dev/null +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -0,0 +1,123 @@ +import type {OnyxEntry} from 'react-native-onyx'; +import Onyx from 'react-native-onyx'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Transaction} from '@src/types/onyx'; +import type Beta from '@src/types/onyx/Beta'; +import type Policy from '@src/types/onyx/Policy'; +import type Report from '@src/types/onyx/Report'; +import type ReportNameValuePairs from '@src/types/onyx/ReportNameValuePairs'; + +type UpdateContext = { + betas: OnyxEntry; + allReports: Record; + allPolicies: Record; + allReportNameValuePairs: Record; + allTransactions: Record; +}; + +let betas: OnyxEntry; +let allReports: Record; +let allPolicies: Record; +let allReportNameValuePairs: Record; +let allTransactions: Record; +let isInitialized = false; +let connectionsInitializedCount = 0; +const totalConnections = 5; +let initializationPromise: Promise | null = null; + +/** + * Initialize the context data + * We use connectWithoutView to prevent the connection manager from affecting React rendering performance + * This is a one-time setup that happens when the module is first loaded + */ +function initialize(): Promise { + if (initializationPromise) { + return initializationPromise; + } + + initializationPromise = new Promise((resolve) => { + function checkAndMarkInitialized() { + connectionsInitializedCount++; + if (connectionsInitializedCount === totalConnections && !isInitialized) { + isInitialized = true; + resolve(); + } + } + + // Connect to user session betas + Onyx.connectWithoutView({ + key: ONYXKEYS.BETAS, + callback: (val) => { + betas = val; + checkAndMarkInitialized(); + }, + }); + + // Connect to all reports + Onyx.connectWithoutView({ + key: ONYXKEYS.COLLECTION.REPORT, + waitForCollectionCallback: true, + callback: (val) => { + allReports = (val as Record) ?? {}; + checkAndMarkInitialized(); + }, + }); + + // Connect to all policies + Onyx.connectWithoutView({ + key: ONYXKEYS.COLLECTION.POLICY, + waitForCollectionCallback: true, + callback: (val) => { + allPolicies = (val as Record) ?? {}; + checkAndMarkInitialized(); + }, + }); + + // Connect to all REPORT_NAME_VALUE_PAIRS + Onyx.connectWithoutView({ + key: ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, + waitForCollectionCallback: true, + callback: (val) => { + allReportNameValuePairs = (val as Record) ?? {}; + checkAndMarkInitialized(); + }, + }); + + // Connect to all TRANSACTIONS + Onyx.connectWithoutView({ + key: ONYXKEYS.COLLECTION.TRANSACTION, + waitForCollectionCallback: true, + callback: (val) => { + allTransactions = (val as Record) ?? {}; + checkAndMarkInitialized(); + }, + }); + }); + + return initializationPromise; +} + +/** + * Asynchronously get the update context + */ +async function getUpdateContextAsync(): Promise { + await initialize(); + return getUpdateContext(); +} + +/** + * Get the current update context synchronously + * Should only be called after initialization is complete + */ +function getUpdateContext(): UpdateContext { + return { + betas: betas ?? null, + allReports: allReports ?? {}, + allPolicies: allPolicies ?? {}, + allReportNameValuePairs: allReportNameValuePairs ?? {}, + allTransactions: allTransactions ?? {}, + }; +} + +export type {UpdateContext}; +export {initialize, getUpdateContext, getUpdateContextAsync}; \ No newline at end of file diff --git a/tests/perf-test/OptimisticReportNames.perf-test.ts b/tests/perf-test/OptimisticReportNames.perf-test.ts new file mode 100644 index 0000000000000..5a437018c7471 --- /dev/null +++ b/tests/perf-test/OptimisticReportNames.perf-test.ts @@ -0,0 +1,263 @@ +import Onyx from 'react-native-onyx'; +import {measurePerformance} from 'reassure'; +import type {UpdateContext} from '@libs/OptimisticReportNames'; +import * as OptimisticReportNames from '@libs/OptimisticReportNames'; +// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportUtils to mock +import * as ReportUtils from '@libs/ReportUtils'; +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type {Policy, Report} from '@src/types/onyx'; +import {createCollection} from '../utils/collections/createCollection'; +import {createRandomPolicy} from '../utils/collections/policies'; +import {createRandomReport} from '../utils/collections/reports'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +// Mock dependencies +jest.mock('@libs/ReportUtils', () => ({ + ...jest.requireActual('@libs/ReportUtils'), + isExpenseReport: jest.fn(), + getTitleReportField: jest.fn(), + getReportTransactions: jest.fn(), +})); +jest.mock('@libs/Performance', () => ({ + markStart: jest.fn(), + markEnd: jest.fn(), +})); +jest.mock('@libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); +jest.mock('@libs/Log', () => ({ + info: jest.fn(), +})); +jest.mock('@libs/Permissions', () => ({ + canUseCustomReportNames: jest.fn(() => true), +})); + +type MockedReportUtils = jest.Mocked; +const mockReportUtils = ReportUtils as MockedReportUtils; + +const mockTitleField = { + defaultValue: 'Report from {report:policyname} on {report:startdate}', + type: CONST.REPORT.REPORT_FIELD_TYPES.TEXT, + key: 'title', +}; + +describe('[OptimisticReportNames] Performance Tests', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + return waitForBatchedUpdates(); + }); + + afterAll(() => { + Onyx.clear(); + }); + + const mockReports = createCollection( + (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, + (index) => + createRandomReport(index, { + type: CONST.REPORT.TYPE.EXPENSE, + }), + 100, + ); + + const mockPolicies = createCollection( + (item) => `${ONYXKEYS.COLLECTION.POLICY}${item.id}`, + (index) => + createRandomPolicy(index, { + fieldList: { + title: mockTitleField, + }, + }), + 10, + ); + + const mockContext: UpdateContext = { + betas: [CONST.BETAS.AUTH_AUTO_REPORT_TITLE], + allReports: mockReports, + allPolicies: mockPolicies, + allReportNameValuePairs: {}, + allTransactions: {}, + }; + + beforeAll(async () => { + mockReportUtils.getTitleReportField.mockReturnValue(mockTitleField); + }); + + describe('updateOptimisticReportNamesFromUpdates', () => { + test('should handle small batches of report updates efficiently', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const updates = Array.from({length: 5}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: 'Updated Report', + total: Math.floor(Math.random() * 10000), + } as Report, + })); + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); + }), + {runs: 20}, + ); + }); + + test('should handle medium batches of report updates efficiently', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const updates = Array.from({length: 25}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: 'Updated Report', + total: Math.floor(Math.random() * 10000), + } as Report, + })); + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); + }), + {runs: 20}, + ); + }); + + test('should handle large batches of report updates efficiently', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const updates = Array.from({length: 100}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: 'Updated Report', + total: Math.floor(Math.random() * 10000), + } as Report, + })); + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); + }), + {runs: 20}, + ); + }); + + test('should handle policy updates that affect many reports efficiently', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const updates = Array.from({length: 3}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.POLICY}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + name: `Updated Policy ${i}`, + fieldList: { + title: mockTitleField, + }, + } as Policy, + })); + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); + }), + {runs: 20}, + ); + }); + + test('should handle mixed update types efficiently', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const reportUpdates = Array.from({length: 15}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: `Updated Report ${i}`, + total: Math.floor(Math.random() * 10000), + } as Report, + })); + + const policyUpdates = Array.from({length: 2}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.POLICY}${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + name: `Updated Policy ${i}`, + fieldList: { + title: mockTitleField, + }, + } as Policy, + })); + + const updates = [...reportUpdates, ...policyUpdates]; + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); + }), + {runs: 20}, + ); + }); + + test('should handle empty updates gracefully', async () => { + await measurePerformance( + (scenario: () => T) => + scenario(() => OptimisticReportNames.updateOptimisticReportNamesFromUpdates([], mockContext)), + {runs: 20}, + ); + }); + + test('should handle updates with no matching data efficiently', async () => { + const emptyContext: UpdateContext = { + betas: [CONST.BETAS.AUTH_AUTO_REPORT_TITLE], + allReports: {}, + allPolicies: {}, + allReportNameValuePairs: {}, + allTransactions: {}, + }; + + await measurePerformance( + (scenario: () => T) => + scenario(() => { + const updates = Array.from({length: 10}, (_, i) => ({ + key: `${ONYXKEYS.COLLECTION.REPORT}nonexistent_${i}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: `Report ${i}`, + } as Report, + })); + + return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, emptyContext); + }), + {runs: 20}, + ); + }); + }); + + describe('computeReportNameIfNeeded', () => { + test('should compute report names efficiently for existing reports', async () => { + const report = Object.values(mockReports)[0]; + const update = { + key: `${ONYXKEYS.COLLECTION.REPORT}${report?.reportID}` as const, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 5000, + } as Report, + }; + + await measurePerformance( + (scenario: () => T) => scenario(() => OptimisticReportNames.computeReportNameIfNeeded(report, update, mockContext)), + {runs: 100}, + ); + }); + + test('should compute report names efficiently for new reports', async () => { + const newReport = createRandomReport(999, { + type: CONST.REPORT.TYPE.EXPENSE, + }); + + const update = { + key: `${ONYXKEYS.COLLECTION.REPORT}${newReport.reportID}` as const, + onyxMethod: Onyx.METHOD.SET, + value: newReport, + }; + + await measurePerformance( + (scenario: () => T) => scenario(() => OptimisticReportNames.computeReportNameIfNeeded(undefined, update, mockContext)), + {runs: 100}, + ); + }); + }); +}); \ No newline at end of file diff --git a/tests/unit/OptimisticReportNamesTest.ts b/tests/unit/OptimisticReportNamesTest.ts new file mode 100644 index 0000000000000..dbaf529e84ec4 --- /dev/null +++ b/tests/unit/OptimisticReportNamesTest.ts @@ -0,0 +1,445 @@ +import Onyx from 'react-native-onyx'; +import type {UpdateContext} from '@libs/OptimisticReportNames'; +import {computeReportNameIfNeeded, getReportByTransactionID, shouldComputeReportName, updateOptimisticReportNamesFromUpdates} from '@libs/OptimisticReportNames'; +// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportUtils to mock +import * as ReportUtils from '@libs/ReportUtils'; +import type {OnyxKey} from '@src/ONYXKEYS'; +import type {Policy, PolicyReportField, Report, Transaction} from '@src/types/onyx'; + +// Mock dependencies +jest.mock('@libs/ReportUtils', () => ({ + ...jest.requireActual('@libs/ReportUtils'), + isExpenseReport: jest.fn(), + getTitleReportField: jest.fn(), + getReportTransactions: jest.fn(), +})); + +jest.mock('@libs/CurrencyUtils', () => ({ + getCurrencySymbol: jest.fn(() => '$'), +})); + +jest.mock('@libs/Performance', () => ({ + markStart: jest.fn(), + markEnd: jest.fn(), +})); + +jest.mock('@libs/actions/Timing', () => ({ + start: jest.fn(), + end: jest.fn(), +})); + +jest.mock('@libs/Log', () => ({ + info: jest.fn(), +})); + +jest.mock('@libs/Permissions', () => ({ + canUseCustomReportNames: jest.fn(() => true), +})); + +const mockReportUtils = ReportUtils as jest.Mocked; + +describe('OptimisticReportNames', () => { + const mockReport: Report = { + reportID: '123', + type: 'expense', + policyID: 'policy123', + reportName: 'Original Report Name', + currency: 'USD', + total: 5000, + }; + + const mockPolicy: Policy = { + id: 'policy123', + name: 'Test Workspace', + fieldList: { + title: { + type: 'text', + key: 'title', + defaultValue: 'Report from {report:policyname}', + } as PolicyReportField, + }, + }; + + const mockContext: UpdateContext = { + betas: ['authAutoReportTitle'], + allReports: { + // eslint-disable-next-line @typescript-eslint/naming-convention + report_123: mockReport, + }, + allPolicies: { + // eslint-disable-next-line @typescript-eslint/naming-convention + policy_policy123: mockPolicy, + }, + allReportNameValuePairs: { + // eslint-disable-next-line @typescript-eslint/naming-convention + reportNameValuePairs_123: { + private_isArchived: '', + }, + }, + allTransactions: {}, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('shouldComputeReportName', () => { + beforeEach(() => { + mockReportUtils.getTitleReportField.mockReturnValue({ + defaultValue: 'Report from {report:policyname}', + } as PolicyReportField); + }); + + test('should return true for valid expense report with policy title field', () => { + const result = shouldComputeReportName(mockReport, mockPolicy); + expect(result).toBe(true); + }); + + test('should return false for report without policy', () => { + const result = shouldComputeReportName(mockReport, undefined); + expect(result).toBe(false); + }); + + test('should return false for unsupported report type', () => { + const chatReport = {...mockReport, type: 'chat'}; + const result = shouldComputeReportName(chatReport, mockPolicy); + expect(result).toBe(false); + }); + + test('should return false when policy has no title field', () => { + mockReportUtils.getTitleReportField.mockReturnValue(undefined); + const result = shouldComputeReportName(mockReport, mockPolicy); + expect(result).toBe(false); + }); + }); + + describe('computeReportNameIfNeeded', () => { + beforeEach(() => { + mockReportUtils.getTitleReportField.mockReturnValue({ + defaultValue: 'Report from {report:policyname}', + } as PolicyReportField); + }); + + test('should compute new report name when formula changes', () => { + const update = { + key: 'report_123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 10000, + }, + }; + + const result = computeReportNameIfNeeded(mockReport, update, mockContext); + + expect(result).toBe('Report from Test Workspace'); + }); + + test('should return null when no computation is needed', () => { + mockReportUtils.getTitleReportField.mockReturnValue(undefined); + + const update = { + key: 'report_123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 10000, + }, + }; + + const result = computeReportNameIfNeeded(mockReport, update, mockContext); + + expect(result).toBeNull(); + }); + + test('should handle new report creation', () => { + const newReport = { + ...mockReport, + reportID: 'new456', + reportName: '', + }; + + const update = { + key: 'report_new456' as OnyxKey, + onyxMethod: Onyx.METHOD.SET, + value: newReport, + }; + + const result = computeReportNameIfNeeded(undefined, update, mockContext); + + expect(result).toBe('Report from Test Workspace'); + }); + + test('should return null when computed name matches current name', () => { + const reportWithComputedName = { + ...mockReport, + reportName: 'Report from Test Workspace', + }; + + const update = { + key: 'report_123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 8000, + }, + }; + + const result = computeReportNameIfNeeded(reportWithComputedName, update, mockContext); + + expect(result).toBeNull(); + }); + }); + + describe('updateOptimisticReportNamesFromUpdates', () => { + beforeEach(() => { + mockReportUtils.getTitleReportField.mockReturnValue({ + defaultValue: 'Report from {report:policyname}', + } as PolicyReportField); + }); + + test('should process report updates and add name updates', () => { + const updates = [ + { + key: 'report_123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 7500, + }, + }, + ]; + + const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); + + expect(result).toHaveLength(2); + expect(result.at(0)).toEqual(updates.at(0)); + expect(result.at(1)?.key).toBe('report_123'); + expect(result.at(1)?.value).toEqual({ + reportName: 'Report from Test Workspace', + }); + }); + + test('should process policy updates and update affected reports', () => { + const updates = [ + { + key: 'policy_policy123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + name: 'Updated Workspace', + }, + }, + ]; + + const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); + + expect(result).toHaveLength(2); + expect(result.at(0)).toEqual(updates.at(0)); + expect(result.at(1)?.key).toBe('report_123'); + }); + + test('should return original updates when no changes needed', () => { + mockReportUtils.getTitleReportField.mockReturnValue(undefined); + + const updates = [ + { + key: 'report_456' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 3000, + }, + }, + ]; + + const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); + + expect(result).toEqual(updates); + }); + + test('should handle empty updates array', () => { + const result = updateOptimisticReportNamesFromUpdates([], mockContext); + + expect(result).toEqual([]); + }); + + test('should return original updates when feature is disabled', () => { + const contextWithDisabledFeature = { + ...mockContext, + betas: [], + }; + + const updates = [ + { + key: 'report_123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + total: 7500, + }, + }, + ]; + + const result = updateOptimisticReportNamesFromUpdates(updates, contextWithDisabledFeature); + + expect(result).toEqual(updates); + }); + }); + + describe('Transaction Updates', () => { + test('should process transaction updates and trigger report name updates', () => { + const contextWithTransaction = { + ...mockContext, + allTransactions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + transactions_txn123: { + transactionID: 'txn123', + reportID: '123', + created: '2024-01-01', + amount: -5000, + currency: 'USD', + merchant: 'Original Merchant', + }, + }, + }; + + const update = { + key: 'transactions_txn123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + created: '2024-02-15', // Updated date + reportID: '123', + }, + }; + + const result = updateOptimisticReportNamesFromUpdates([update], contextWithTransaction); + + // Should include original update + new report name update + expect(result).toHaveLength(2); + expect(result.at(0)).toEqual(update); // Original transaction update + expect(result.at(1)?.key).toBe('report_123'); // New report update + }); + + test('getReportByTransactionID should find report from transaction', () => { + const contextWithTransaction = { + ...mockContext, + allTransactions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + transactions_abc123: { + transactionID: 'abc123', + reportID: '123', + amount: -7500, + created: '2024-01-15', + currency: 'USD', + merchant: 'Test Store', + }, + }, + }; + + const result = getReportByTransactionID('abc123', contextWithTransaction); + + expect(result).toEqual(mockReport); + expect(result?.reportID).toBe('123'); + }); + + test('getReportByTransactionID should return undefined for missing transaction', () => { + const result = getReportByTransactionID('nonexistent', mockContext); + expect(result).toBeUndefined(); + }); + + test('getReportByTransactionID should return undefined for transaction without reportID', () => { + const contextWithIncompleteTransaction = { + ...mockContext, + allTransactions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + transactions_incomplete: { + transactionID: 'incomplete' as OnyxKey, + amount: -1000, + currency: 'USD', + merchant: 'Store', + // Missing reportID + } as unknown as Transaction, + }, + }; + + const result = getReportByTransactionID('incomplete', contextWithIncompleteTransaction); + expect(result).toBeUndefined(); + }); + + test('should handle transaction updates that rely on context lookup', () => { + const contextWithTransaction = { + ...mockContext, + allTransactions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + transactions_xyz789: { + transactionID: 'xyz789', + reportID: '123', + created: '2024-01-01', + amount: -3000, + currency: 'EUR', + merchant: 'Context Store', + }, + }, + }; + + // Transaction update without reportID in the value + const update = { + key: 'transactions_xyz789' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + amount: -4000, // Updated amount + // No reportID provided in update + }, + }; + + const result = updateOptimisticReportNamesFromUpdates([update], contextWithTransaction); + + // Should still find the report through context lookup and generate update + expect(result).toHaveLength(2); + expect(result.at(1)?.key).toBe('report_123'); + }); + + test('should use optimistic transaction data in formula computation', () => { + mockReportUtils.getTitleReportField.mockReturnValue({ + defaultValue: 'Report from {report:startdate}', + } as unknown as PolicyReportField); + + const contextWithTransaction = { + ...mockContext, + allTransactions: { + // eslint-disable-next-line @typescript-eslint/naming-convention + transactions_formula123: { + transactionID: 'formula123', + reportID: '123', + created: '2024-01-01', // Original date + amount: -5000, + currency: 'USD', + merchant: 'Original Store', + }, + }, + }; + + // Mock getReportTransactions to return the original transaction + // eslint-disable-next-line @typescript-eslint/dot-notation + mockReportUtils.getReportTransactions.mockReturnValue([contextWithTransaction.allTransactions['transactions_formula123']]); + + const update = { + key: 'transactions_formula123' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: { + transactionID: 'formula123', + created: '2024-03-15', // Updated date that should be used in formula + modifiedCreated: '2024-03-15', + }, + }; + + const result = updateOptimisticReportNamesFromUpdates([update], contextWithTransaction); + + expect(result).toHaveLength(2); + + // The key test: verify exact report name with optimistic date + const reportUpdate = result.at(1); + expect(reportUpdate).toEqual({ + key: 'report_123', + onyxMethod: Onyx.METHOD.MERGE, + value: { + reportName: 'Report from 2024-03-15', // Exact expected result with updated date + }, + }); + }); + }); +}); \ No newline at end of file From f39ed5e86c497059ff39df4f964c0a5fc6d8b103 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Mon, 25 Aug 2025 13:16:17 +0200 Subject: [PATCH 02/11] fixes --- src/libs/Formula.ts | 2 +- src/libs/OptimisticReportNames.ts | 21 +- .../OptimisticReportNamesConnectionManager.ts | 2 +- .../OptimisticReportNames.perf-test.ts | 406 +++++++++--------- tests/unit/OptimisticReportNamesTest.ts | 308 +++++++------ 5 files changed, 373 insertions(+), 366 deletions(-) diff --git a/src/libs/Formula.ts b/src/libs/Formula.ts index ae272355253da..96627bb284ecc 100644 --- a/src/libs/Formula.ts +++ b/src/libs/Formula.ts @@ -502,4 +502,4 @@ function getOldestTransactionDate(reportID: string, context?: FormulaContext): s export {FORMULA_PART_TYPES, compute, extract, parse}; -export type {FormulaContext, FormulaPart}; \ No newline at end of file +export type {FormulaContext, FormulaPart}; diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index 5fe92c178909a..5d39b33fdb7e7 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -49,6 +49,7 @@ function getPolicyIDFromKey(key: string): string { /** * Extract transaction ID from an Onyx key */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars -- this will be used in near future function getTransactionIDFromKey(key: string): string { return key.replace(ONYXKEYS.COLLECTION.TRANSACTION, ''); } @@ -282,7 +283,7 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: const {betas, allReports} = context; // Check if the feature is enabled - if (!Permissions.canUseCustomReportNames(betas)) { + if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas)) { Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); return updates; @@ -366,26 +367,24 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: } } - const updatedSet = [...updates, ...additionalUpdates]; + Log.info('[OptimisticReportNames] Processing completed', false, { + additionalUpdatesCount: additionalUpdates.length, + totalUpdatesReturned: updates.length + additionalUpdates.length, + }); Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - Log.info('[OptimisticReportNames] Finished processing report names', false, { - originalUpdatesCount: updates.length, - additionalUpdatesCount: additionalUpdates.length, - totalUpdatesCount: updatedSet.length, - }); - - return updatedSet; + return updates.concat(additionalUpdates); } /** - * Create an update context for testing or external use + * Creates update context for optimistic report name processing. + * This should be called before processing optimistic updates */ function createUpdateContext(): Promise { return getUpdateContextAsync(); } export {updateOptimisticReportNamesFromUpdates, computeReportNameIfNeeded, createUpdateContext, shouldComputeReportName, getReportByTransactionID}; -export type {UpdateContext}; \ No newline at end of file +export type {UpdateContext}; diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts index 2176ef567bfd3..1d2fe41311872 100644 --- a/src/libs/OptimisticReportNamesConnectionManager.ts +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -120,4 +120,4 @@ function getUpdateContext(): UpdateContext { } export type {UpdateContext}; -export {initialize, getUpdateContext, getUpdateContextAsync}; \ No newline at end of file +export {initialize, getUpdateContext, getUpdateContextAsync}; diff --git a/tests/perf-test/OptimisticReportNames.perf-test.ts b/tests/perf-test/OptimisticReportNames.perf-test.ts index 5a437018c7471..b6dcdbf87cfb9 100644 --- a/tests/perf-test/OptimisticReportNames.perf-test.ts +++ b/tests/perf-test/OptimisticReportNames.perf-test.ts @@ -1,23 +1,24 @@ import Onyx from 'react-native-onyx'; -import {measurePerformance} from 'reassure'; +import {measureFunction} from 'reassure'; import type {UpdateContext} from '@libs/OptimisticReportNames'; -import * as OptimisticReportNames from '@libs/OptimisticReportNames'; +import {computeReportNameIfNeeded, updateOptimisticReportNamesFromUpdates} from '@libs/OptimisticReportNames'; // eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportUtils to mock import * as ReportUtils from '@libs/ReportUtils'; -import CONST from '@src/CONST'; +import type {OnyxKey} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Policy, Report} from '@src/types/onyx'; -import {createCollection} from '../utils/collections/createCollection'; -import {createRandomPolicy} from '../utils/collections/policies'; +import createCollection from '../utils/collections/createCollection'; import {createRandomReport} from '../utils/collections/reports'; import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; // Mock dependencies jest.mock('@libs/ReportUtils', () => ({ + // jest.requireActual is necessary to include multi-layered module imports (eg. Report.ts has processReportIDDeeplink() which uses parseReportRouteParams() imported from getReportIDFromUrl.ts) + // Without jest.requireActual, parseReportRouteParams would be undefined, causing the test to fail. ...jest.requireActual('@libs/ReportUtils'), + // These methods are mocked below in the beforeAll function to return specific values isExpenseReport: jest.fn(), getTitleReportField: jest.fn(), - getReportTransactions: jest.fn(), })); jest.mock('@libs/Performance', () => ({ markStart: jest.fn(), @@ -30,51 +31,48 @@ jest.mock('@libs/actions/Timing', () => ({ jest.mock('@libs/Log', () => ({ info: jest.fn(), })); -jest.mock('@libs/Permissions', () => ({ - canUseCustomReportNames: jest.fn(() => true), -})); - -type MockedReportUtils = jest.Mocked; -const mockReportUtils = ReportUtils as MockedReportUtils; -const mockTitleField = { - defaultValue: 'Report from {report:policyname} on {report:startdate}', - type: CONST.REPORT.REPORT_FIELD_TYPES.TEXT, - key: 'title', -}; +const mockReportUtils = ReportUtils as jest.Mocked; describe('[OptimisticReportNames] Performance Tests', () => { - beforeAll(() => { - Onyx.init({keys: ONYXKEYS}); - return waitForBatchedUpdates(); - }); + const REPORTS_COUNT = 1000; + const POLICIES_COUNT = 100; - afterAll(() => { - Onyx.clear(); - }); + const mockPolicy = { + id: 'policy1', + name: 'Test Policy', + fieldList: { + // eslint-disable-next-line @typescript-eslint/naming-convention + text_title: { + defaultValue: '{report:type} - {report:startdate} - {report:total} {report:currency}', + }, + }, + } as unknown as Policy; - const mockReports = createCollection( - (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, - (index) => - createRandomReport(index, { - type: CONST.REPORT.TYPE.EXPENSE, - }), - 100, + const mockPolicies = createCollection( + (item) => `policy_${item.id}`, + (index) => ({ + ...mockPolicy, + id: `policy${index}`, + name: `Policy ${index}`, + }), + POLICIES_COUNT, ); - const mockPolicies = createCollection( - (item) => `${ONYXKEYS.COLLECTION.POLICY}${item.id}`, - (index) => - createRandomPolicy(index, { - fieldList: { - title: mockTitleField, - }, - }), - 10, + const mockReports = createCollection( + (item) => `${ONYXKEYS.COLLECTION.REPORT}${item.reportID}`, + (index) => ({ + ...createRandomReport(index), + policyID: `policy${index % POLICIES_COUNT}`, + total: -(Math.random() * 100000), // Random negative amount + currency: 'USD', + lastVisibleActionCreated: new Date().toISOString(), + }), + REPORTS_COUNT, ); const mockContext: UpdateContext = { - betas: [CONST.BETAS.AUTH_AUTO_REPORT_TITLE], + betas: ['authAutoReportTitle'], allReports: mockReports, allPolicies: mockPolicies, allReportNameValuePairs: {}, @@ -82,182 +80,204 @@ describe('[OptimisticReportNames] Performance Tests', () => { }; beforeAll(async () => { - mockReportUtils.getTitleReportField.mockReturnValue(mockTitleField); + Onyx.init({keys: ONYXKEYS}); + mockReportUtils.isExpenseReport.mockReturnValue(true); + mockReportUtils.getTitleReportField.mockReturnValue(mockPolicy.fieldList?.text_title); + await waitForBatchedUpdates(); }); - describe('updateOptimisticReportNamesFromUpdates', () => { - test('should handle small batches of report updates efficiently', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const updates = Array.from({length: 5}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - reportName: 'Updated Report', - total: Math.floor(Math.random() * 10000), - } as Report, - })); - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); - }), - {runs: 20}, - ); - }); + afterAll(() => { + Onyx.clear(); + }); - test('should handle medium batches of report updates efficiently', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const updates = Array.from({length: 25}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - reportName: 'Updated Report', - total: Math.floor(Math.random() * 10000), - } as Report, - })); - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); - }), - {runs: 20}, - ); - }); + describe('Single Report Name Computation', () => { + test('[OptimisticReportNames] computeReportNameIfNeeded() single report', async () => { + const report = Object.values(mockReports).at(0); + const update = { + key: `report_${report?.reportID}` as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: {total: -20000}, + }; - test('should handle large batches of report updates efficiently', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const updates = Array.from({length: 100}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - reportName: 'Updated Report', - total: Math.floor(Math.random() * 10000), - } as Report, - })); - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); - }), - {runs: 20}, - ); + await measureFunction(() => computeReportNameIfNeeded(report, update, mockContext)); }); + }); - test('should handle policy updates that affect many reports efficiently', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const updates = Array.from({length: 3}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.POLICY}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - name: `Updated Policy ${i}`, - fieldList: { - title: mockTitleField, - }, - } as Policy, - })); - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); - }), - {runs: 20}, - ); + describe('Batch Processing Performance', () => { + test('[OptimisticReportNames] updateOptimisticReportNamesFromUpdates() with 10 new reports', async () => { + const updates = Array.from({length: 10}, (_, i) => ({ + key: `report_new${i}` as OnyxKey, + onyxMethod: Onyx.METHOD.SET, + value: { + reportID: `new${i}`, + policyID: `policy${i % 10}`, + total: -(Math.random() * 50000), + currency: 'USD', + lastVisibleActionCreated: new Date().toISOString(), + }, + })); + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(updates, mockContext)); }); - test('should handle mixed update types efficiently', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const reportUpdates = Array.from({length: 15}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.REPORT}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - reportName: `Updated Report ${i}`, - total: Math.floor(Math.random() * 10000), - } as Report, - })); - - const policyUpdates = Array.from({length: 2}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.POLICY}${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - name: `Updated Policy ${i}`, - fieldList: { - title: mockTitleField, - }, - } as Policy, - })); - - const updates = [...reportUpdates, ...policyUpdates]; - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, mockContext); - }), - {runs: 20}, - ); + test('[OptimisticReportNames] updateOptimisticReportNamesFromUpdates() with 50 existing report updates', async () => { + const reportKeys = Object.keys(mockReports).slice(0, 50) as OnyxKey[]; + const updates = reportKeys.map((key) => ({ + key, + onyxMethod: Onyx.METHOD.MERGE, + value: {total: -(Math.random() * 100000)}, + })); + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(updates, mockContext)); }); - test('should handle empty updates gracefully', async () => { - await measurePerformance( - (scenario: () => T) => - scenario(() => OptimisticReportNames.updateOptimisticReportNamesFromUpdates([], mockContext)), - {runs: 20}, - ); + test('[OptimisticReportNames] updateOptimisticReportNamesFromUpdates() with 100 mixed updates', async () => { + const newReportUpdates = Array.from({length: 50}, (_, i) => ({ + key: `report_batch${i}` as OnyxKey, + onyxMethod: Onyx.METHOD.SET, + value: { + reportID: `batch${i}`, + policyID: `policy${i % 20}`, + total: -(Math.random() * 75000), + currency: 'USD', + lastVisibleActionCreated: new Date().toISOString(), + }, + })); + + const existingReportUpdates = Object.keys(mockReports) + .slice(0, 50) + .map((key) => ({ + key: key as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: {total: -(Math.random() * 125000)}, + })); + + const allUpdates = [...newReportUpdates, ...existingReportUpdates]; + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(allUpdates, mockContext)); }); + }); - test('should handle updates with no matching data efficiently', async () => { - const emptyContext: UpdateContext = { - betas: [CONST.BETAS.AUTH_AUTO_REPORT_TITLE], - allReports: {}, - allPolicies: {}, - allReportNameValuePairs: {}, - allTransactions: {}, + describe('Policy Update Impact Performance', () => { + test('[OptimisticReportNames] policy update affecting multiple reports', async () => { + const policyUpdate = { + key: 'policy_policy1' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: {name: 'Updated Policy Name'}, }; - await measurePerformance( - (scenario: () => T) => - scenario(() => { - const updates = Array.from({length: 10}, (_, i) => ({ - key: `${ONYXKEYS.COLLECTION.REPORT}nonexistent_${i}` as const, - onyxMethod: Onyx.METHOD.MERGE, - value: { - reportName: `Report ${i}`, - } as Report, - })); - - return OptimisticReportNames.updateOptimisticReportNamesFromUpdates(updates, emptyContext); - }), - {runs: 20}, - ); + // This should trigger name computation for all reports using policy1 + await measureFunction(() => updateOptimisticReportNamesFromUpdates([policyUpdate], mockContext)); }); - }); - describe('computeReportNameIfNeeded', () => { - test('should compute report names efficiently for existing reports', async () => { - const report = Object.values(mockReports)[0]; - const update = { - key: `${ONYXKEYS.COLLECTION.REPORT}${report?.reportID}` as const, + test('[OptimisticReportNames] multiple policy updates', async () => { + const policyUpdates = Array.from({length: 10}, (_, i) => ({ + key: `policy_policy${i}` as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, + value: {name: `Bulk Updated Policy ${i}`}, + })); + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(policyUpdates, mockContext)); + }); + }); + + describe('Large Dataset Performance', () => { + test('[OptimisticReportNames] processing with large context (1000 reports)', async () => { + const updates = Array.from({length: 1000}, (_, i) => ({ + key: `report_large${i}` as OnyxKey, + onyxMethod: Onyx.METHOD.SET, value: { - total: 5000, - } as Report, - }; + reportID: `large${i}`, + policyID: `policy${i % 50}`, + total: -(Math.random() * 200000), + currency: 'USD', + lastVisibleActionCreated: new Date().toISOString(), + }, + })); - await measurePerformance( - (scenario: () => T) => scenario(() => OptimisticReportNames.computeReportNameIfNeeded(report, update, mockContext)), - {runs: 100}, - ); + await measureFunction(() => updateOptimisticReportNamesFromUpdates(updates, mockContext)); }); - test('should compute report names efficiently for new reports', async () => { - const newReport = createRandomReport(999, { - type: CONST.REPORT.TYPE.EXPENSE, - }); + test('[OptimisticReportNames] worst case: many irrelevant updates', async () => { + // Create updates that won't trigger name computation to test filtering performance + const irrelevantUpdates = Array.from({length: 100}, (_, i) => ({ + key: `transaction_${i}` as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: {description: `Updated transaction ${i}`}, + })); - const update = { - key: `${ONYXKEYS.COLLECTION.REPORT}${newReport.reportID}` as const, + await measureFunction(() => updateOptimisticReportNamesFromUpdates(irrelevantUpdates, mockContext)); + }); + }); + + describe('Edge Cases Performance', () => { + test('[OptimisticReportNames] reports without formulas', async () => { + // Mock reports with policies that don't have formulas + const contextWithoutFormulas: UpdateContext = { + ...mockContext, + allPolicies: createCollection( + (item) => `policy_${item.id}`, + (index) => + ({ + id: `policy${index}`, + name: `Policy ${index}`, + fieldList: { + // eslint-disable-next-line @typescript-eslint/naming-convention + text_title: { + name: 'Title', + defaultValue: 'Static Title', + fieldID: 'text_title', + orderWeight: 0, + type: 'text' as const, + deletable: true, + values: [], + keys: [], + externalIDs: [], + disabledOptions: [], + isTax: false, + }, + }, + }) as unknown as Policy, + 50, + ), + allReportNameValuePairs: {}, + }; + + const updates = Array.from({length: 20}, (_, i) => ({ + key: `report_static${i}` as OnyxKey, onyxMethod: Onyx.METHOD.SET, - value: newReport, + value: { + reportID: `static${i}`, + policyID: `policy${i % 10}`, + total: -10000, + currency: 'USD', + }, + })); + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(updates, contextWithoutFormulas)); + }); + + test('[OptimisticReportNames] missing policies and reports', async () => { + const contextWithMissingData: UpdateContext = { + betas: ['authAutoReportTitle'], + allReports: {}, + allPolicies: {}, + allReportNameValuePairs: {}, + allTransactions: {}, }; - await measurePerformance( - (scenario: () => T) => scenario(() => OptimisticReportNames.computeReportNameIfNeeded(undefined, update, mockContext)), - {runs: 100}, - ); + const updates = Array.from({length: 10}, (_, i) => ({ + key: `report_missing${i}` as OnyxKey, + onyxMethod: Onyx.METHOD.SET, + value: { + reportID: `missing${i}`, + policyID: 'nonexistent', + total: -10000, + currency: 'USD', + }, + })); + + await measureFunction(() => updateOptimisticReportNamesFromUpdates(updates, contextWithMissingData)); }); }); -}); \ No newline at end of file +}); diff --git a/tests/unit/OptimisticReportNamesTest.ts b/tests/unit/OptimisticReportNamesTest.ts index dbaf529e84ec4..824f26394b3c1 100644 --- a/tests/unit/OptimisticReportNamesTest.ts +++ b/tests/unit/OptimisticReportNamesTest.ts @@ -15,50 +15,31 @@ jest.mock('@libs/ReportUtils', () => ({ })); jest.mock('@libs/CurrencyUtils', () => ({ - getCurrencySymbol: jest.fn(() => '$'), -})); - -jest.mock('@libs/Performance', () => ({ - markStart: jest.fn(), - markEnd: jest.fn(), -})); - -jest.mock('@libs/actions/Timing', () => ({ - start: jest.fn(), - end: jest.fn(), -})); - -jest.mock('@libs/Log', () => ({ - info: jest.fn(), -})); - -jest.mock('@libs/Permissions', () => ({ - canUseCustomReportNames: jest.fn(() => true), + getCurrencySymbol: jest.fn().mockReturnValue('$'), })); const mockReportUtils = ReportUtils as jest.Mocked; describe('OptimisticReportNames', () => { - const mockReport: Report = { - reportID: '123', - type: 'expense', - policyID: 'policy123', - reportName: 'Original Report Name', - currency: 'USD', - total: 5000, - }; - - const mockPolicy: Policy = { - id: 'policy123', - name: 'Test Workspace', + const mockPolicy = { + id: 'policy1', fieldList: { - title: { - type: 'text', - key: 'title', - defaultValue: 'Report from {report:policyname}', - } as PolicyReportField, + // eslint-disable-next-line @typescript-eslint/naming-convention + text_title: { + defaultValue: '{report:type} - {report:total}', + }, }, - }; + } as unknown as Policy; + + const mockReport = { + reportID: '123', + reportName: 'Old Name', + policyID: 'policy1', + total: -10000, + currency: 'USD', + lastVisibleActionCreated: '2025-01-15T10:30:00Z', + type: 'expense', + } as Report; const mockContext: UpdateContext = { betas: ['authAutoReportTitle'], @@ -68,7 +49,7 @@ describe('OptimisticReportNames', () => { }, allPolicies: { // eslint-disable-next-line @typescript-eslint/naming-convention - policy_policy123: mockPolicy, + policy_policy1: mockPolicy, }, allReportNameValuePairs: { // eslint-disable-next-line @typescript-eslint/naming-convention @@ -81,202 +62,209 @@ describe('OptimisticReportNames', () => { beforeEach(() => { jest.clearAllMocks(); + mockReportUtils.isExpenseReport.mockReturnValue(true); + mockReportUtils.getTitleReportField.mockReturnValue(mockPolicy.fieldList?.text_title); }); - describe('shouldComputeReportName', () => { - beforeEach(() => { - mockReportUtils.getTitleReportField.mockReturnValue({ - defaultValue: 'Report from {report:policyname}', - } as PolicyReportField); - }); - - test('should return true for valid expense report with policy title field', () => { + describe('shouldComputeReportName()', () => { + test('should return true for expense report with title field formula', () => { const result = shouldComputeReportName(mockReport, mockPolicy); expect(result).toBe(true); }); - test('should return false for report without policy', () => { - const result = shouldComputeReportName(mockReport, undefined); + test('should return false for reports with unsupported type', () => { + mockReportUtils.isExpenseReport.mockReturnValue(false); + + const result = shouldComputeReportName( + { + ...mockReport, + type: 'iou', + } as Report, + mockPolicy, + ); expect(result).toBe(false); }); - test('should return false for unsupported report type', () => { - const chatReport = {...mockReport, type: 'chat'}; - const result = shouldComputeReportName(chatReport, mockPolicy); + test('should return false when no policy', () => { + const result = shouldComputeReportName(mockReport, undefined); expect(result).toBe(false); }); - test('should return false when policy has no title field', () => { + test('should return false when no title field', () => { mockReportUtils.getTitleReportField.mockReturnValue(undefined); const result = shouldComputeReportName(mockReport, mockPolicy); expect(result).toBe(false); }); - }); - - describe('computeReportNameIfNeeded', () => { - beforeEach(() => { - mockReportUtils.getTitleReportField.mockReturnValue({ - defaultValue: 'Report from {report:policyname}', - } as PolicyReportField); - }); - test('should compute new report name when formula changes', () => { - const update = { - key: 'report_123' as OnyxKey, - onyxMethod: Onyx.METHOD.MERGE, - value: { - total: 10000, + test('should return true when title field has no formula', () => { + const policyWithoutFormula = { + ...mockPolicy, + fieldList: { + // eslint-disable-next-line @typescript-eslint/naming-convention + text_title: {defaultValue: 'Static Title'}, }, - }; - - const result = computeReportNameIfNeeded(mockReport, update, mockContext); - - expect(result).toBe('Report from Test Workspace'); + } as unknown as Policy; + mockReportUtils.getTitleReportField.mockReturnValue(policyWithoutFormula.fieldList?.text_title); + const result = shouldComputeReportName(mockReport, policyWithoutFormula); + expect(result).toBe(true); }); + }); - test('should return null when no computation is needed', () => { - mockReportUtils.getTitleReportField.mockReturnValue(undefined); - + describe('computeReportNameIfNeeded()', () => { + test('should compute name when report data changes', () => { const update = { key: 'report_123' as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, - value: { - total: 10000, - }, + value: {total: -20000}, }; const result = computeReportNameIfNeeded(mockReport, update, mockContext); - - expect(result).toBeNull(); + expect(result).toEqual('Expense Report - $200.00'); }); - test('should handle new report creation', () => { - const newReport = { - ...mockReport, - reportID: 'new456', - reportName: '', - }; - + test('should return null when name would not change', () => { const update = { - key: 'report_new456' as OnyxKey, - onyxMethod: Onyx.METHOD.SET, - value: newReport, - }; - - const result = computeReportNameIfNeeded(undefined, update, mockContext); - - expect(result).toBe('Report from Test Workspace'); - }); - - test('should return null when computed name matches current name', () => { - const reportWithComputedName = { - ...mockReport, - reportName: 'Report from Test Workspace', - }; - - const update = { - key: 'report_123' as OnyxKey, + key: 'report_456' as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, - value: { - total: 8000, - }, + value: {description: 'Updated description'}, }; - const result = computeReportNameIfNeeded(reportWithComputedName, update, mockContext); - + const result = computeReportNameIfNeeded( + { + ...mockReport, + reportName: 'Expense Report - $100.00', + }, + update, + mockContext, + ); expect(result).toBeNull(); }); }); - describe('updateOptimisticReportNamesFromUpdates', () => { - beforeEach(() => { - mockReportUtils.getTitleReportField.mockReturnValue({ - defaultValue: 'Report from {report:policyname}', - } as PolicyReportField); - }); - - test('should process report updates and add name updates', () => { + describe('updateOptimisticReportNamesFromUpdates()', () => { + test('should detect new report creation and add name update', () => { const updates = [ { - key: 'report_123' as OnyxKey, - onyxMethod: Onyx.METHOD.MERGE, + key: 'report_456' as OnyxKey, + onyxMethod: Onyx.METHOD.SET, value: { - total: 7500, + reportID: '456', + policyID: 'policy1', + total: -15000, + currency: 'USD', + type: 'expense', }, }, ]; const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); - - expect(result).toHaveLength(2); - expect(result.at(0)).toEqual(updates.at(0)); - expect(result.at(1)?.key).toBe('report_123'); - expect(result.at(1)?.value).toEqual({ - reportName: 'Report from Test Workspace', + expect(result).toHaveLength(2); // Original + name update + expect(result.at(1)).toEqual({ + key: 'report_456', + onyxMethod: Onyx.METHOD.MERGE, + value: {reportName: 'Expense Report - $150.00'}, }); }); - test('should process policy updates and update affected reports', () => { + test('should handle existing report updates', () => { const updates = [ { - key: 'policy_policy123' as OnyxKey, + key: 'report_123' as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, - value: { - name: 'Updated Workspace', - }, + value: {total: -25000}, }, ]; const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); - - expect(result).toHaveLength(2); - expect(result.at(0)).toEqual(updates.at(0)); - expect(result.at(1)?.key).toBe('report_123'); + expect(result).toHaveLength(2); // Original + name update + expect(result.at(1)?.value).toEqual({reportName: 'Expense Report - $250.00'}); }); - test('should return original updates when no changes needed', () => { - mockReportUtils.getTitleReportField.mockReturnValue(undefined); + test('should handle policy updates affecting multiple reports', () => { + const contextWithMultipleReports = { + ...mockContext, + allReports: { + // eslint-disable-next-line @typescript-eslint/naming-convention + report_123: {...mockReport, reportID: '123'}, + // eslint-disable-next-line @typescript-eslint/naming-convention + report_456: {...mockReport, reportID: '456'}, + // eslint-disable-next-line @typescript-eslint/naming-convention + report_789: {...mockReport, reportID: '789'}, + }, + allReportNameValuePairs: { + // eslint-disable-next-line @typescript-eslint/naming-convention + reportNameValuePairs_123: {private_isArchived: ''}, + // eslint-disable-next-line @typescript-eslint/naming-convention + reportNameValuePairs_456: {private_isArchived: ''}, + // eslint-disable-next-line @typescript-eslint/naming-convention + reportNameValuePairs_789: {private_isArchived: ''}, + }, + }; + mockReportUtils.getTitleReportField.mockReturnValue({defaultValue: 'Policy: {report:policyname}'} as unknown as PolicyReportField); const updates = [ { - key: 'report_456' as OnyxKey, + key: 'policy_policy1' as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, - value: { - total: 3000, - }, + value: {name: 'Updated Policy Name'}, }, ]; - const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); + const result = updateOptimisticReportNamesFromUpdates(updates, contextWithMultipleReports); - expect(result).toEqual(updates); - }); + expect(result).toHaveLength(4); - test('should handle empty updates array', () => { - const result = updateOptimisticReportNamesFromUpdates([], mockContext); + // Assert the original policy update + expect(result.at(0)).toEqual({ + key: 'policy_policy1', + onyxMethod: Onyx.METHOD.MERGE, + value: {name: 'Updated Policy Name'}, + }); - expect(result).toEqual([]); - }); + // Assert individual report name updates + expect(result.at(1)).toEqual({ + key: 'report_123', + onyxMethod: Onyx.METHOD.MERGE, + value: {reportName: 'Policy: Updated Policy Name'}, + }); - test('should return original updates when feature is disabled', () => { - const contextWithDisabledFeature = { - ...mockContext, - betas: [], - }; + expect(result.at(2)).toEqual({ + key: 'report_456', + onyxMethod: Onyx.METHOD.MERGE, + value: {reportName: 'Policy: Updated Policy Name'}, + }); + expect(result.at(3)).toEqual({ + key: 'report_789', + onyxMethod: Onyx.METHOD.MERGE, + value: {reportName: 'Policy: Updated Policy Name'}, + }); + }); + + test('should handle unknown object types gracefully', () => { const updates = [ { - key: 'report_123' as OnyxKey, + key: 'unknown_123' as OnyxKey, onyxMethod: Onyx.METHOD.MERGE, - value: { - total: 7500, - }, + value: {someData: 'value'}, }, ]; - const result = updateOptimisticReportNamesFromUpdates(updates, contextWithDisabledFeature); + const result = updateOptimisticReportNamesFromUpdates(updates, mockContext); + expect(result).toEqual(updates); // Unchanged + }); + }); - expect(result).toEqual(updates); + describe('Edge Cases', () => { + test('should handle missing report gracefully', () => { + const update = { + key: 'report_999' as OnyxKey, + onyxMethod: Onyx.METHOD.MERGE, + value: {total: -10000}, + }; + + const result = computeReportNameIfNeeded(undefined, update, mockContext); + expect(result).toBeNull(); }); }); @@ -442,4 +430,4 @@ describe('OptimisticReportNames', () => { }); }); }); -}); \ No newline at end of file +}); From c85dfa84d283b418f68cc71491cb97cf3e6aaa32 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Mon, 25 Aug 2025 13:52:47 +0200 Subject: [PATCH 03/11] formulatest reintroduction --- tests/unit/FormulaTest.ts | 360 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 360 insertions(+) create mode 100644 tests/unit/FormulaTest.ts diff --git a/tests/unit/FormulaTest.ts b/tests/unit/FormulaTest.ts new file mode 100644 index 0000000000000..1298f0414b35d --- /dev/null +++ b/tests/unit/FormulaTest.ts @@ -0,0 +1,360 @@ +// eslint-disable-next-line no-restricted-syntax -- disabled because we need CurrencyUtils to mock +import * as CurrencyUtils from '@libs/CurrencyUtils'; +import type {FormulaContext} from '@libs/Formula'; +import {compute, extract, parse} from '@libs/Formula'; +// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportActionsUtils to mock +import * as ReportActionsUtils from '@libs/ReportActionsUtils'; +// eslint-disable-next-line no-restricted-syntax -- disabled because we need ReportUtils to mock +import * as ReportUtils from '@libs/ReportUtils'; +import CONST from '@src/CONST'; +import type {Policy, Report, ReportActions, Transaction} from '@src/types/onyx'; + +jest.mock('@libs/ReportActionsUtils', () => ({ + getAllReportActions: jest.fn(), +})); + +jest.mock('@libs/ReportUtils', () => ({ + ...jest.requireActual('@libs/ReportUtils'), + getReportTransactions: jest.fn(), +})); + +jest.mock('@libs/CurrencyUtils', () => ({ + getCurrencySymbol: jest.fn(), +})); + +const mockReportActionsUtils = ReportActionsUtils as jest.Mocked; +const mockReportUtils = ReportUtils as jest.Mocked; +const mockCurrencyUtils = CurrencyUtils as jest.Mocked; + +describe('CustomFormula', () => { + describe('extract()', () => { + test('should extract formula parts with default braces', () => { + expect(extract('{report:type} - {report:total}')).toEqual(['{report:type}', '{report:total}']); + }); + + test('should handle nested braces', () => { + expect(extract('{report:{report:submit:from:firstName|substr:2}}')).toEqual(['{report:{report:submit:from:firstName|substr:2}}']); + }); + + test('should handle escaped braces', () => { + expect(extract('\\{not-formula} {report:type}')).toEqual(['{report:type}']); + }); + + test('should handle empty formula', () => { + expect(extract('')).toEqual([]); + }); + + test('should handle formula without braces', () => { + expect(extract('no braces here')).toEqual([]); + }); + }); + + describe('parse()', () => { + test('should parse report formula parts', () => { + const parts = parse('{report:type} {report:startdate}'); + expect(parts).toHaveLength(3); + expect(parts.at(0)).toEqual({ + definition: '{report:type}', + type: 'report', + fieldPath: ['type'], + functions: [], + }); + expect(parts.at(2)).toEqual({ + definition: '{report:startdate}', + type: 'report', + fieldPath: ['startdate'], + functions: [], + }); + }); + + test('should parse field formula parts', () => { + const parts = parse('{field:custom_field}'); + expect(parts.at(0)).toEqual({ + definition: '{field:custom_field}', + type: 'field', + fieldPath: ['custom_field'], + functions: [], + }); + }); + + test('should parse user formula parts with functions', () => { + const parts = parse('{user:email|frontPart}'); + expect(parts.at(0)).toEqual({ + definition: '{user:email|frontPart}', + type: 'user', + fieldPath: ['email'], + functions: ['frontPart'], + }); + }); + + test('should handle empty formula', () => { + expect(parse('')).toEqual([]); + }); + + test('should treat formula without braces as free text', () => { + const parts = parse('no braces here'); + expect(parts).toHaveLength(1); + expect(parts.at(0)?.type).toBe('freetext'); + }); + }); + + describe('compute()', () => { + const mockContext: FormulaContext = { + report: { + reportID: '123', + reportName: '', + type: 'expense', + total: -10000, // -$100.00 + currency: 'USD', + lastVisibleActionCreated: '2025-01-15T10:30:00Z', + policyID: 'policy1', + } as Report, + policy: { + name: 'Test Policy', + } as Policy, + }; + + beforeEach(() => { + jest.clearAllMocks(); + + mockCurrencyUtils.getCurrencySymbol.mockImplementation((currency: string) => { + if (currency === 'USD') { + return '$'; + } + return currency; + }); + + const mockReportActions = { + // eslint-disable-next-line @typescript-eslint/naming-convention + '1': { + reportActionID: '1', + created: '2025-01-10T08:00:00Z', // Oldest action + actionName: 'CREATED', + }, + // eslint-disable-next-line @typescript-eslint/naming-convention + '2': { + reportActionID: '2', + created: '2025-01-15T10:30:00Z', // Later action + actionName: 'IOU', + }, + // eslint-disable-next-line @typescript-eslint/naming-convention + '3': { + reportActionID: '3', + created: '2025-01-12T14:20:00Z', // Middle action + actionName: 'COMMENT', + }, + } as unknown as ReportActions; + + const mockTransactions = [ + { + transactionID: 'trans1', + created: '2025-01-08T12:00:00Z', // Oldest transaction + amount: 5000, + merchant: 'ACME Ltd.', + }, + { + transactionID: 'trans2', + created: '2025-01-14T16:45:00Z', // Later transaction + amount: 3000, + merchant: 'ACME Ltd.', + }, + { + transactionID: 'trans3', + created: '2025-01-11T09:15:00Z', // Middle transaction + amount: 2000, + merchant: 'ACME Ltd.', + }, + ] as Transaction[]; + + mockReportActionsUtils.getAllReportActions.mockReturnValue(mockReportActions); + mockReportUtils.getReportTransactions.mockReturnValue(mockTransactions); + }); + + test('should compute basic report formula', () => { + const result = compute('{report:type} {report:total}', mockContext); + expect(result).toBe('Expense Report $100.00'); // No space between parts + }); + + test('should compute startdate formula using transactions', () => { + const result = compute('{report:startdate}', mockContext); + expect(result).toBe('2025-01-08'); // Should use oldest transaction date (2025-01-08) + }); + + test('should compute created formula using report actions', () => { + const result = compute('{report:created}', mockContext); + expect(result).toBe('2025-01-10'); // Should use oldest report action date (2025-01-10) + }); + + test('should compute startdate with custom format', () => { + const result = compute('{report:startdate:MM/dd/yyyy}', mockContext); + expect(result).toBe('01/08/2025'); // Should use oldest transaction date with yyyy-MM-dd format + }); + + test('should compute created with custom format', () => { + const result = compute('{report:created:MMMM dd, yyyy}', mockContext); + expect(result).toBe('January 10, 2025'); // Should use oldest report action date with MMMM dd, yyyy format + }); + + test('should compute startdate with short month format', () => { + const result = compute('{report:startdate:dd MMM yyyy}', mockContext); + expect(result).toBe('08 Jan 2025'); // Should use oldest transaction date with dd MMM yyyy format + }); + + test('should compute policy name', () => { + const result = compute('{report:policyname}', mockContext); + expect(result).toBe('Test Policy'); + }); + + test('should handle empty formula', () => { + expect(compute('', mockContext)).toBe(''); + }); + + test('should handle unknown formula parts', () => { + const result = compute('{report:unknown}', mockContext); + expect(result).toBe('{report:unknown}'); + }); + + test('should handle missing report data gracefully', () => { + const contextWithMissingData: FormulaContext = { + report: {} as unknown as Report, + policy: null as unknown as Policy, + }; + const result = compute('{report:total} {report:policyname}', contextWithMissingData); + expect(result).toBe('{report:total} {report:policyname}'); // Empty data is replaced with definition + }); + + test('should preserve free text', () => { + const result = compute('Expense Report - {report:total}', mockContext); + expect(result).toBe('Expense Report - $100.00'); + }); + + test('should preserve exact spacing around formula parts', () => { + const result = compute('Report with type after 4 spaces {report:type}-and no space after computed part', mockContext); + expect(result).toBe('Report with type after 4 spaces Expense Report-and no space after computed part'); + }); + }); + + describe('Edge Cases', () => { + test('should handle malformed braces', () => { + const parts = parse('{incomplete'); + expect(parts.at(0)?.type).toBe('freetext'); + }); + + test('should handle undefined amounts', () => { + const context: FormulaContext = { + report: {total: undefined} as Report, + policy: null as unknown as Policy, + }; + const result = compute('{report:total}', context); + expect(result).toBe('{report:total}'); + }); + + test('should handle missing report actions for created', () => { + mockReportActionsUtils.getAllReportActions.mockReturnValue({}); + const context: FormulaContext = { + report: {reportID: '123'} as Report, + policy: null as unknown as Policy, + }; + + const result = compute('{report:created}', context); + expect(result).toBe('{report:created}'); + }); + + test('should handle missing transactions for startdate', () => { + mockReportUtils.getReportTransactions.mockReturnValue([]); + const context: FormulaContext = { + report: {reportID: '123'} as Report, + policy: null as unknown as Policy, + }; + const today = new Date(); + const expected = `${today.getFullYear()}-${String(today.getMonth() + 1).padStart(2, '0')}-${String(today.getDate()).padStart(2, '0')}`; + const result = compute('{report:startdate}', context); + expect(result).toBe(expected); + }); + + test('should call getReportTransactions with correct reportID for startdate', () => { + const context: FormulaContext = { + report: {reportID: 'test-report-123'} as Report, + policy: null as unknown as Policy, + }; + + compute('{report:startdate}', context); + expect(mockReportUtils.getReportTransactions).toHaveBeenCalledWith('test-report-123'); + }); + + test('should call getAllReportActions with correct reportID for created', () => { + const context: FormulaContext = { + report: {reportID: 'test-report-456'} as Report, + policy: null as unknown as Policy, + }; + + compute('{report:created}', context); + expect(mockReportActionsUtils.getAllReportActions).toHaveBeenCalledWith('test-report-456'); + }); + + test('should skip partial transactions (empty merchant)', () => { + const mockTransactions = [ + { + transactionID: 'trans1', + created: '2025-01-15T12:00:00Z', + amount: 5000, + merchant: 'ACME Ltd.', + }, + { + transactionID: 'trans2', + created: '2025-01-08T16:45:00Z', // Older but partial + amount: 3000, + merchant: '', // Empty merchant = partial + }, + { + transactionID: 'trans3', + created: '2025-01-12T09:15:00Z', // Should be oldest valid + amount: 2000, + merchant: 'Gamma Inc.', + }, + ] as Transaction[]; + + mockReportUtils.getReportTransactions.mockReturnValue(mockTransactions); + const context: FormulaContext = { + report: {reportID: 'test-report-123'} as Report, + policy: null as unknown as Policy, + }; + + const result = compute('{report:startdate}', context); + expect(result).toBe('2025-01-12'); + }); + + test('should skip partial transactions (zero amount)', () => { + const mockTransactions = [ + { + transactionID: 'trans1', + created: '2025-01-15T12:00:00Z', + amount: 5000, + merchant: 'ACME Ltd.', + }, + { + transactionID: 'trans2', + created: '2025-01-08T16:45:00Z', // Older but partial + amount: 0, // Zero amount = partial + merchant: 'Beta Corp.', + iouRequestType: CONST.IOU.REQUEST_TYPE.SCAN, + }, + { + transactionID: 'trans3', + created: '2025-01-12T09:15:00Z', // Should be oldest valid + amount: 2000, + merchant: 'Gamma Inc.', + }, + ] as Transaction[]; + + mockReportUtils.getReportTransactions.mockReturnValue(mockTransactions); + const context: FormulaContext = { + report: {reportID: 'test-report-123'} as Report, + policy: null as unknown as Policy, + }; + + const result = compute('{report:startdate}', context); + expect(result).toBe('2025-01-12'); + }); + }); +}); From eddaf3be1b1ffcf8b4246c6ba07a3fd48c1a6fc3 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Tue, 26 Aug 2025 11:02:42 +0200 Subject: [PATCH 04/11] formulatest reintroduction --- Mobile-Expensify | 2 +- src/libs/OptimisticReportNames.ts | 15 +++------------ .../OptimisticReportNamesConnectionManager.ts | 12 ++---------- 3 files changed, 6 insertions(+), 23 deletions(-) diff --git a/Mobile-Expensify b/Mobile-Expensify index 695da4ca8e160..939b50dace6f0 160000 --- a/Mobile-Expensify +++ b/Mobile-Expensify @@ -1 +1 @@ -Subproject commit 695da4ca8e160f4b0497db27174f1fc67902a0ad +Subproject commit 939b50dace6f0db569b351aeafd0b2b00220ab94 diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index 5d39b33fdb7e7..a1a9a0918f667 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -1,16 +1,15 @@ import type {OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; import type {OnyxKey} from '@src/ONYXKEYS'; +import ONYXKEYS from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import Timing from './actions/Timing'; -import {compute, FORMULA_PART_TYPES, parse} from './Formula'; import type {FormulaContext} from './Formula'; +import {compute, FORMULA_PART_TYPES, parse} from './Formula'; import Log from './Log'; -import {getUpdateContextAsync} from './OptimisticReportNamesConnectionManager'; import type {UpdateContext} from './OptimisticReportNamesConnectionManager'; import Performance from './Performance'; import Permissions from './Permissions'; @@ -378,13 +377,5 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: return updates.concat(additionalUpdates); } -/** - * Creates update context for optimistic report name processing. - * This should be called before processing optimistic updates - */ -function createUpdateContext(): Promise { - return getUpdateContextAsync(); -} - -export {updateOptimisticReportNamesFromUpdates, computeReportNameIfNeeded, createUpdateContext, shouldComputeReportName, getReportByTransactionID}; +export {computeReportNameIfNeeded, getReportByTransactionID, shouldComputeReportName, updateOptimisticReportNamesFromUpdates}; export type {UpdateContext}; diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts index 1d2fe41311872..68aabb2719009 100644 --- a/src/libs/OptimisticReportNamesConnectionManager.ts +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -97,21 +97,13 @@ function initialize(): Promise { return initializationPromise; } -/** - * Asynchronously get the update context - */ -async function getUpdateContextAsync(): Promise { - await initialize(); - return getUpdateContext(); -} - /** * Get the current update context synchronously * Should only be called after initialization is complete */ function getUpdateContext(): UpdateContext { return { - betas: betas ?? null, + betas: betas ?? [], allReports: allReports ?? {}, allPolicies: allPolicies ?? {}, allReportNameValuePairs: allReportNameValuePairs ?? {}, @@ -120,4 +112,4 @@ function getUpdateContext(): UpdateContext { } export type {UpdateContext}; -export {initialize, getUpdateContext, getUpdateContextAsync}; +export {initialize, getUpdateContext}; From 38ea7c70ee3759c9ad3494dfa4ec4da0ddf4510f Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 28 Aug 2025 13:15:09 +0200 Subject: [PATCH 05/11] Revert Mobile-Expensify submodule update from eddaf3be1b1f --- Mobile-Expensify | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mobile-Expensify b/Mobile-Expensify index 939b50dace6f0..695da4ca8e160 160000 --- a/Mobile-Expensify +++ b/Mobile-Expensify @@ -1 +1 @@ -Subproject commit 939b50dace6f0db569b351aeafd0b2b00220ab94 +Subproject commit 695da4ca8e160f4b0497db27174f1fc67902a0ad From be49678e496e1b3df47300912f55ff4117e7d84a Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Thu, 28 Aug 2025 13:46:04 +0200 Subject: [PATCH 06/11] Changes to the implementation --- src/libs/OptimisticReportNames.ts | 6 +-- .../OptimisticReportNamesConnectionManager.ts | 41 ++++++++++++------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index a1a9a0918f667..d44e1e65a1fab 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -1,14 +1,14 @@ import type {OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import CONST from '@src/CONST'; -import type {OnyxKey} from '@src/ONYXKEYS'; import ONYXKEYS from '@src/ONYXKEYS'; +import type {OnyxKey} from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; import Timing from './actions/Timing'; -import type {FormulaContext} from './Formula'; import {compute, FORMULA_PART_TYPES, parse} from './Formula'; +import type {FormulaContext} from './Formula'; import Log from './Log'; import type {UpdateContext} from './OptimisticReportNamesConnectionManager'; import Performance from './Performance'; @@ -377,5 +377,5 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: return updates.concat(additionalUpdates); } -export {computeReportNameIfNeeded, getReportByTransactionID, shouldComputeReportName, updateOptimisticReportNamesFromUpdates}; +export {updateOptimisticReportNamesFromUpdates, computeReportNameIfNeeded, shouldComputeReportName, getReportByTransactionID}; export type {UpdateContext}; diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts index 68aabb2719009..b91ebb08d27af 100644 --- a/src/libs/OptimisticReportNamesConnectionManager.ts +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -26,25 +26,35 @@ const totalConnections = 5; let initializationPromise: Promise | null = null; /** - * Initialize the context data - * We use connectWithoutView to prevent the connection manager from affecting React rendering performance - * This is a one-time setup that happens when the module is first loaded + * Initialize persistent connections to Onyx data needed for OptimisticReportNames + * This is called lazily when OptimisticReportNames functionality is first used + * Returns a Promise that resolves when all connections have received their initial data + * + * We use Onyx.connectWithoutView because we do not use this in React components and this logic is not tied to the UI. + * This is a centralized system that needs access to all objects of several types, so that when any updates affect + * the computed report names, we can compute the new names according to the formula and add the necessary updates. + * It wouldn't be possible to do this without connecting to all the data. + * */ function initialize(): Promise { + if (isInitialized) { + return Promise.resolve(); + } + if (initializationPromise) { return initializationPromise; } - initializationPromise = new Promise((resolve) => { - function checkAndMarkInitialized() { + initializationPromise = new Promise((resolve) => { + const checkAndMarkInitialized = () => { connectionsInitializedCount++; - if (connectionsInitializedCount === totalConnections && !isInitialized) { + if (connectionsInitializedCount === totalConnections) { isInitialized = true; resolve(); } - } + }; - // Connect to user session betas + // Connect to BETAS Onyx.connectWithoutView({ key: ONYXKEYS.BETAS, callback: (val) => { @@ -53,7 +63,7 @@ function initialize(): Promise { }, }); - // Connect to all reports + // Connect to all REPORTS Onyx.connectWithoutView({ key: ONYXKEYS.COLLECTION.REPORT, waitForCollectionCallback: true, @@ -63,7 +73,7 @@ function initialize(): Promise { }, }); - // Connect to all policies + // Connect to all POLICIES Onyx.connectWithoutView({ key: ONYXKEYS.COLLECTION.POLICY, waitForCollectionCallback: true, @@ -99,17 +109,20 @@ function initialize(): Promise { /** * Get the current update context synchronously - * Should only be called after initialization is complete + * Must be called after initialize() has completed */ function getUpdateContext(): UpdateContext { + if (!isInitialized) { + throw new Error('OptimisticReportNamesConnectionManager not initialized. Call initialize() first.'); + } + return { - betas: betas ?? [], + betas, allReports: allReports ?? {}, allPolicies: allPolicies ?? {}, allReportNameValuePairs: allReportNameValuePairs ?? {}, allTransactions: allTransactions ?? {}, }; } - -export type {UpdateContext}; export {initialize, getUpdateContext}; +export type {UpdateContext}; From 809a1ae45a0b3a575d4c7d0fa95e635f1ee418d2 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Mon, 1 Sep 2025 13:05:21 +0200 Subject: [PATCH 07/11] Introduce beta configuration into optimistic report name computation. This is necessary to make use of explit betas. --- src/libs/OptimisticReportNames.ts | 4 ++-- .../OptimisticReportNamesConnectionManager.ts | 19 ++++++++++++++----- .../OptimisticReportNames.perf-test.ts | 2 ++ tests/unit/OptimisticReportNamesTest.ts | 1 + 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index d44e1e65a1fab..019bb4186b5b4 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -279,10 +279,10 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: Performance.markStart(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); Timing.start(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - const {betas, allReports} = context; + const {betas, allReports, betaConfiguration} = context; // Check if the feature is enabled - if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas)) { + if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas, betaConfiguration)) { Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); return updates; diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts index b91ebb08d27af..ac884289fe343 100644 --- a/src/libs/OptimisticReportNamesConnectionManager.ts +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -1,14 +1,12 @@ import type {OnyxEntry} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {Transaction} from '@src/types/onyx'; -import type Beta from '@src/types/onyx/Beta'; -import type Policy from '@src/types/onyx/Policy'; -import type Report from '@src/types/onyx/Report'; +import type {Beta, BetaConfiguration, Policy, Report, Transaction} from '@src/types/onyx'; import type ReportNameValuePairs from '@src/types/onyx/ReportNameValuePairs'; type UpdateContext = { betas: OnyxEntry; + betaConfiguration: OnyxEntry; allReports: Record; allPolicies: Record; allReportNameValuePairs: Record; @@ -16,13 +14,14 @@ type UpdateContext = { }; let betas: OnyxEntry; +let betaConfiguration: OnyxEntry; let allReports: Record; let allPolicies: Record; let allReportNameValuePairs: Record; let allTransactions: Record; let isInitialized = false; let connectionsInitializedCount = 0; -const totalConnections = 5; +const totalConnections = 6; let initializationPromise: Promise | null = null; /** @@ -63,6 +62,15 @@ function initialize(): Promise { }, }); + // Connect to BETA_CONFIGURATION + Onyx.connectWithoutView({ + key: ONYXKEYS.BETA_CONFIGURATION, + callback: (val) => { + betaConfiguration = val; + checkAndMarkInitialized(); + }, + }); + // Connect to all REPORTS Onyx.connectWithoutView({ key: ONYXKEYS.COLLECTION.REPORT, @@ -118,6 +126,7 @@ function getUpdateContext(): UpdateContext { return { betas, + betaConfiguration, allReports: allReports ?? {}, allPolicies: allPolicies ?? {}, allReportNameValuePairs: allReportNameValuePairs ?? {}, diff --git a/tests/perf-test/OptimisticReportNames.perf-test.ts b/tests/perf-test/OptimisticReportNames.perf-test.ts index b6dcdbf87cfb9..61a50e80e04c3 100644 --- a/tests/perf-test/OptimisticReportNames.perf-test.ts +++ b/tests/perf-test/OptimisticReportNames.perf-test.ts @@ -73,6 +73,7 @@ describe('[OptimisticReportNames] Performance Tests', () => { const mockContext: UpdateContext = { betas: ['authAutoReportTitle'], + betaConfiguration: {}, allReports: mockReports, allPolicies: mockPolicies, allReportNameValuePairs: {}, @@ -260,6 +261,7 @@ describe('[OptimisticReportNames] Performance Tests', () => { test('[OptimisticReportNames] missing policies and reports', async () => { const contextWithMissingData: UpdateContext = { betas: ['authAutoReportTitle'], + betaConfiguration: {}, allReports: {}, allPolicies: {}, allReportNameValuePairs: {}, diff --git a/tests/unit/OptimisticReportNamesTest.ts b/tests/unit/OptimisticReportNamesTest.ts index 824f26394b3c1..9bcfbc4236ece 100644 --- a/tests/unit/OptimisticReportNamesTest.ts +++ b/tests/unit/OptimisticReportNamesTest.ts @@ -43,6 +43,7 @@ describe('OptimisticReportNames', () => { const mockContext: UpdateContext = { betas: ['authAutoReportTitle'], + betaConfiguration: {}, allReports: { // eslint-disable-next-line @typescript-eslint/naming-convention report_123: mockReport, From a8c632471089766098088e3710751e34caa829eb Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Fri, 5 Sep 2025 15:45:57 +0200 Subject: [PATCH 08/11] Remove Timing / Performance from OptimisticReportNames entry point because of unexpected logoff --- src/CONST/index.ts | 1 - src/libs/OptimisticReportNames.ts | 8 -------- 2 files changed, 9 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index d93c26c06dde8..b6579caf18ec4 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -1557,7 +1557,6 @@ const CONST = { APPLY_HTTPS_UPDATES: 'apply_https_updates', COMPUTE_REPORT_NAME: 'compute_report_name', COMPUTE_REPORT_NAME_FOR_NEW_REPORT: 'compute_report_name_for_new_report', - UPDATE_OPTIMISTIC_REPORT_NAMES: 'update_optimistic_report_names', COLD: 'cold', WARM: 'warm', REPORT_ACTION_ITEM_LAYOUT_DEBOUNCE_TIME: 1500, diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index 019bb4186b5b4..9eacccf8f0f8b 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -276,15 +276,10 @@ function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: O * This is the main middleware function that processes optimistic data */ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: UpdateContext): OnyxUpdate[] { - Performance.markStart(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - Timing.start(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - const {betas, allReports, betaConfiguration} = context; // Check if the feature is enabled if (!Permissions.isBetaEnabled(CONST.BETAS.AUTH_AUTO_REPORT_TITLE, betas, betaConfiguration)) { - Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); return updates; } @@ -371,9 +366,6 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: totalUpdatesReturned: updates.length + additionalUpdates.length, }); - Performance.markEnd(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - Timing.end(CONST.TIMING.UPDATE_OPTIMISTIC_REPORT_NAMES); - return updates.concat(additionalUpdates); } From 1a97d2720d3a0ff971d824e796ad1496f23f128b Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Tue, 9 Sep 2025 12:47:45 +0200 Subject: [PATCH 09/11] Fixes according to code review --- src/libs/API/index.ts | 2 +- src/libs/OptimisticReportNames.ts | 35 ++++--------------- .../OptimisticReportNamesConnectionManager.ts | 16 ++++----- src/libs/OptionsListUtils/index.ts | 15 ++++++++ .../OptimisticReportNames.perf-test.ts | 8 ----- 5 files changed, 30 insertions(+), 46 deletions(-) diff --git a/src/libs/API/index.ts b/src/libs/API/index.ts index 4157c20e04902..de6ead0e425b6 100644 --- a/src/libs/API/index.ts +++ b/src/libs/API/index.ts @@ -92,7 +92,7 @@ function prepareRequest( const processedOptimisticData = OptimisticReportNames.updateOptimisticReportNamesFromUpdates(optimisticData, context); Onyx.update(processedOptimisticData); } catch (error) { - Log.warn('[API] Failed to process optimistic report names', {error}); + Log.hmmm('[API] Failed to process optimistic report names', {error}); // Fallback to original optimistic data if processing fails Onyx.update(optimisticData); } diff --git a/src/libs/OptimisticReportNames.ts b/src/libs/OptimisticReportNames.ts index 9eacccf8f0f8b..8db6f73d7fb3d 100644 --- a/src/libs/OptimisticReportNames.ts +++ b/src/libs/OptimisticReportNames.ts @@ -1,17 +1,15 @@ import type {OnyxUpdate} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import CONST from '@src/CONST'; -import ONYXKEYS from '@src/ONYXKEYS'; import type {OnyxKey} from '@src/ONYXKEYS'; +import ONYXKEYS from '@src/ONYXKEYS'; import type {Transaction} from '@src/types/onyx'; import type Policy from '@src/types/onyx/Policy'; import type Report from '@src/types/onyx/Report'; -import Timing from './actions/Timing'; -import {compute, FORMULA_PART_TYPES, parse} from './Formula'; import type {FormulaContext} from './Formula'; +import {compute, FORMULA_PART_TYPES, parse} from './Formula'; import Log from './Log'; import type {UpdateContext} from './OptimisticReportNamesConnectionManager'; -import Performance from './Performance'; import Permissions from './Permissions'; import {getTitleReportField, isArchivedReport} from './ReportUtils'; @@ -80,7 +78,7 @@ function getTransactionByID(transactionID: string, allTransactions: Record, context: UpdateContext): Report[] { +function getReportsForNameComputation(policyID: string, allReports: Record, context: UpdateContext): Report[] { if (policyID === CONST.POLICY.ID_FAKE) { return []; } @@ -139,16 +137,15 @@ function getReportKey(reportID: string): OnyxKey { * Check if a report should have its name automatically computed */ function shouldComputeReportName(report: Report, policy: Policy | undefined): boolean { - // Only compute names for expense reports with policies that have title fields if (!report || !policy) { return false; } - // Check if the report is an expense report if (!isValidReportType(report.type)) { return false; } + // Only compute names for expense reports with policies that have title fields // Check if the policy has a title field with a formula const titleField = getTitleReportField(policy.fieldList ?? {}); if (!titleField?.defaultValue) { @@ -174,32 +171,23 @@ function isValidReportType(reportType?: string): boolean { * Compute a new report name if needed based on an optimistic update */ function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: OnyxUpdate, context: UpdateContext): string | null { - Performance.markStart(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.start(CONST.TIMING.COMPUTE_REPORT_NAME); - const {allPolicies} = context; // If no report is provided, extract it from the update (for new reports) const targetReport = report ?? (incomingUpdate.value as Report); if (!targetReport) { - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return null; } const policy = getPolicyByID(targetReport.policyID, allPolicies); if (!shouldComputeReportName(targetReport, policy)) { - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return null; } const titleField = getTitleReportField(policy?.fieldList ?? {}); if (!titleField?.defaultValue) { - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return null; } @@ -228,8 +216,6 @@ function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: O }); if (!isAffected) { - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return null; } @@ -253,21 +239,13 @@ function computeReportNameIfNeeded(report: Report | undefined, incomingUpdate: O // Only return an update if the name actually changed if (newName && newName !== targetReport.reportName) { Log.info('[OptimisticReportNames] Report name computed', false, { - reportID: targetReport.reportID, - oldName: targetReport.reportName, - newName, - formula, updateType, isNewReport: !report, }); - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return newName; } - Performance.markEnd(CONST.TIMING.COMPUTE_REPORT_NAME); - Timing.end(CONST.TIMING.COMPUTE_REPORT_NAME); return null; } @@ -314,7 +292,7 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: case 'policy': { const policyID = getPolicyIDFromKey(update.key); - const affectedReports = getReportsByPolicyID(policyID, allReports, context); + const affectedReports = getReportsForNameComputation(policyID, allReports, context); for (const report of affectedReports) { const reportNameUpdate = computeReportNameIfNeeded(report, update, context); @@ -363,11 +341,10 @@ function updateOptimisticReportNamesFromUpdates(updates: OnyxUpdate[], context: Log.info('[OptimisticReportNames] Processing completed', false, { additionalUpdatesCount: additionalUpdates.length, - totalUpdatesReturned: updates.length + additionalUpdates.length, }); return updates.concat(additionalUpdates); } -export {updateOptimisticReportNamesFromUpdates, computeReportNameIfNeeded, shouldComputeReportName, getReportByTransactionID}; +export {computeReportNameIfNeeded, getReportByTransactionID, shouldComputeReportName, updateOptimisticReportNamesFromUpdates}; export type {UpdateContext}; diff --git a/src/libs/OptimisticReportNamesConnectionManager.ts b/src/libs/OptimisticReportNamesConnectionManager.ts index ac884289fe343..0e596b1050612 100644 --- a/src/libs/OptimisticReportNamesConnectionManager.ts +++ b/src/libs/OptimisticReportNamesConnectionManager.ts @@ -29,7 +29,7 @@ let initializationPromise: Promise | null = null; * This is called lazily when OptimisticReportNames functionality is first used * Returns a Promise that resolves when all connections have received their initial data * - * We use Onyx.connectWithoutView because we do not use this in React components and this logic is not tied to the UI. + * We use Onyx.connectWithoutView because we do not use this in React components and this logic is not tied directly to the UI. * This is a centralized system that needs access to all objects of several types, so that when any updates affect * the computed report names, we can compute the new names according to the formula and add the necessary updates. * It wouldn't be possible to do this without connecting to all the data. @@ -45,7 +45,7 @@ function initialize(): Promise { } initializationPromise = new Promise((resolve) => { - const checkAndMarkInitialized = () => { + const incrementInitialization = () => { connectionsInitializedCount++; if (connectionsInitializedCount === totalConnections) { isInitialized = true; @@ -58,7 +58,7 @@ function initialize(): Promise { key: ONYXKEYS.BETAS, callback: (val) => { betas = val; - checkAndMarkInitialized(); + incrementInitialization(); }, }); @@ -67,7 +67,7 @@ function initialize(): Promise { key: ONYXKEYS.BETA_CONFIGURATION, callback: (val) => { betaConfiguration = val; - checkAndMarkInitialized(); + incrementInitialization(); }, }); @@ -77,7 +77,7 @@ function initialize(): Promise { waitForCollectionCallback: true, callback: (val) => { allReports = (val as Record) ?? {}; - checkAndMarkInitialized(); + incrementInitialization(); }, }); @@ -87,7 +87,7 @@ function initialize(): Promise { waitForCollectionCallback: true, callback: (val) => { allPolicies = (val as Record) ?? {}; - checkAndMarkInitialized(); + incrementInitialization(); }, }); @@ -97,7 +97,7 @@ function initialize(): Promise { waitForCollectionCallback: true, callback: (val) => { allReportNameValuePairs = (val as Record) ?? {}; - checkAndMarkInitialized(); + incrementInitialization(); }, }); @@ -107,7 +107,7 @@ function initialize(): Promise { waitForCollectionCallback: true, callback: (val) => { allTransactions = (val as Record) ?? {}; - checkAndMarkInitialized(); + incrementInitialization(); }, }); }); diff --git a/src/libs/OptionsListUtils/index.ts b/src/libs/OptionsListUtils/index.ts index 3f921db582e20..e8a524fed19ab 100644 --- a/src/libs/OptionsListUtils/index.ts +++ b/src/libs/OptionsListUtils/index.ts @@ -114,6 +114,7 @@ import { isInvoiceRoom, isMoneyRequest, isPolicyAdmin, + isUnread, isAdminRoom as reportUtilsIsAdminRoom, isAnnounceRoom as reportUtilsIsAnnounceRoom, isChatReport as reportUtilsIsChatReport, @@ -761,6 +762,7 @@ function createOption( isSelected: isSelected ?? selected ?? false, // Use isSelected preferentially, fallback to selected for compatibility isDisabled, brickRoadIndicator: null, + // isUnread: report && ((report.lastReadTime ?? '') < (report.lastVisibleActionCreated ?? '') || (report.lastReadTime ?? '') < (report.lastMentionedTime ?? '')), // Type/category flags - used in SearchOption context isPolicyExpenseChat: report ? reportUtilsIsPolicyExpenseChat(report) : false, @@ -800,6 +802,10 @@ function createOption( // Set properties that are used in SearchOption context result.private_isArchived = reportNameValuePairs?.private_isArchived; result.keyForList = String(report.reportID); + const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`]; + const oneTransactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`]); + const oneTransactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneTransactionThreadReportID}`]; + result.isUnread = isUnread(report, oneTransactionThreadReport); // Set lastMessageText - use archived message if report is archived, otherwise use report's lastMessageText if (result.private_isArchived) { @@ -841,6 +847,15 @@ function createOption( result.accountID = Number(personalDetail?.accountID); } + if (report) { + console.log('### morwa name', result.text); + console.log('morwa report', report.lastVisibleActionCreated); + console.log('morwa report', report.lastReadTime); + if (result.text === 'optimus+2@suilad.pl owes $12,347.33') { + console.log('morwa report', report); + } + } + return result; } diff --git a/tests/perf-test/OptimisticReportNames.perf-test.ts b/tests/perf-test/OptimisticReportNames.perf-test.ts index 61a50e80e04c3..8add5f932bedc 100644 --- a/tests/perf-test/OptimisticReportNames.perf-test.ts +++ b/tests/perf-test/OptimisticReportNames.perf-test.ts @@ -20,14 +20,6 @@ jest.mock('@libs/ReportUtils', () => ({ isExpenseReport: jest.fn(), getTitleReportField: jest.fn(), })); -jest.mock('@libs/Performance', () => ({ - markStart: jest.fn(), - markEnd: jest.fn(), -})); -jest.mock('@libs/actions/Timing', () => ({ - start: jest.fn(), - end: jest.fn(), -})); jest.mock('@libs/Log', () => ({ info: jest.fn(), })); From efa8973c63fe5c871134474cbefd12013d212393 Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Tue, 9 Sep 2025 13:03:09 +0200 Subject: [PATCH 10/11] remove unintended changes --- src/libs/OptionsListUtils/index.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/libs/OptionsListUtils/index.ts b/src/libs/OptionsListUtils/index.ts index e8a524fed19ab..259504435122c 100644 --- a/src/libs/OptionsListUtils/index.ts +++ b/src/libs/OptionsListUtils/index.ts @@ -114,7 +114,6 @@ import { isInvoiceRoom, isMoneyRequest, isPolicyAdmin, - isUnread, isAdminRoom as reportUtilsIsAdminRoom, isAnnounceRoom as reportUtilsIsAnnounceRoom, isChatReport as reportUtilsIsChatReport, @@ -802,10 +801,6 @@ function createOption( // Set properties that are used in SearchOption context result.private_isArchived = reportNameValuePairs?.private_isArchived; result.keyForList = String(report.reportID); - const chatReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`]; - const oneTransactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`]); - const oneTransactionThreadReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${oneTransactionThreadReportID}`]; - result.isUnread = isUnread(report, oneTransactionThreadReport); // Set lastMessageText - use archived message if report is archived, otherwise use report's lastMessageText if (result.private_isArchived) { @@ -847,15 +842,6 @@ function createOption( result.accountID = Number(personalDetail?.accountID); } - if (report) { - console.log('### morwa name', result.text); - console.log('morwa report', report.lastVisibleActionCreated); - console.log('morwa report', report.lastReadTime); - if (result.text === 'optimus+2@suilad.pl owes $12,347.33') { - console.log('morwa report', report); - } - } - return result; } From 4267e54248d212440704ed97378e4acd61081faa Mon Sep 17 00:00:00 2001 From: Hubert Sosinski Date: Tue, 9 Sep 2025 13:03:52 +0200 Subject: [PATCH 11/11] remove unintended changes --- src/libs/OptionsListUtils/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libs/OptionsListUtils/index.ts b/src/libs/OptionsListUtils/index.ts index 259504435122c..3f921db582e20 100644 --- a/src/libs/OptionsListUtils/index.ts +++ b/src/libs/OptionsListUtils/index.ts @@ -761,7 +761,6 @@ function createOption( isSelected: isSelected ?? selected ?? false, // Use isSelected preferentially, fallback to selected for compatibility isDisabled, brickRoadIndicator: null, - // isUnread: report && ((report.lastReadTime ?? '') < (report.lastVisibleActionCreated ?? '') || (report.lastReadTime ?? '') < (report.lastMentionedTime ?? '')), // Type/category flags - used in SearchOption context isPolicyExpenseChat: report ? reportUtilsIsPolicyExpenseChat(report) : false,