diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 52d0210840c1..2e5335686225 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -2211,6 +2211,7 @@ const CONST = { APP_OPEN: 'NavigationAppOpen', }, // Attribute names + ATTRIBUTE_DERIVED_TRIGGER: 'derived_trigger', ATTRIBUTE_IOU_TYPE: 'iou_type', ATTRIBUTE_IS_ONE_TRANSACTION_REPORT: 'is_one_transaction_report', ATTRIBUTE_IS_TRANSACTION_THREAD: 'is_transaction_thread', diff --git a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts index c1c8f9d28268..4332e80762d4 100644 --- a/src/libs/actions/OnyxDerived/configs/reportAttributes.ts +++ b/src/libs/actions/OnyxDerived/configs/reportAttributes.ts @@ -1,6 +1,7 @@ import type {LocalizedTranslate} from '@components/LocaleContextProvider'; import {getReportPreviewAction} from '@libs/actions/IOU/MoneyRequestBuilder'; +import hashCode from '@libs/hashCode'; import {translate as translateForLocale} from '@libs/Localize'; import {getIsOffline} from '@libs/NetworkState'; import {getLinkedTransactionID} from '@libs/ReportActionsUtils'; @@ -33,7 +34,6 @@ import {isTrackIntentUserSelector} from '@selectors/Onboarding'; // The name-related fields we saw for each account last time, so we can spot which accounts changed. let previousDisplayNames: Record = {}; let previousPersonalDetails: OnyxEntry | undefined; -let previousPolicies: OnyxCollection; const RECOMPUTE_ALL = 'all' as const; @@ -50,21 +50,55 @@ const prepareReportKeys = (keys: string[]) => { ]; }; -const hasPolicyRelevantFieldChanged = (prev: Policy | null | undefined, next: Policy | null | undefined): boolean => { - if (!prev && !next) { - return false; +// Keys excluded from policy signatures because they change without affecting the computed attributes: +// Onyx write-bookkeeping keys and loading flags flip during every optimistic write / API call, and +// `connections` (accounting integration configs, incl. per-sync timestamps) is both volatile and large +// while the attribute computation never reads it. Excluding them prevents recompute churn. +const POLICY_SIGNATURE_EXCLUDED_KEYS = new Set([ + 'pendingAction', + 'pendingFields', + 'errors', + 'errorFields', + 'connections', + 'isLoading', + 'isLoadingWorkspaceReimbursement', + 'isLoadingReceiptPartners', + 'isChangeOwnerSuccessful', + 'isChangeOwnerFailed', + 'lastModified', +]); + +// Bump when the signature format or exclusion set changes: stored signatures from an older format then +// mismatch, which safely degrades to a scoped recompute of the delivered policies' reports once. +const POLICY_SIGNATURE_VERSION = '2'; + +// Deterministic stringify: object keys are sorted at every level so the same policy content always +// produces the same string regardless of key insertion order after Onyx merges. +const stableStringify = (value: unknown): string => { + if (value === null || typeof value !== 'object') { + return JSON.stringify(value) ?? 'undefined'; } - if (!prev || !next) { - return true; + if (Array.isArray(value)) { + return `[${value.map(stableStringify).join(',')}]`; + } + const entries = Object.entries(value) + .filter(([key]) => !POLICY_SIGNATURE_EXCLUDED_KEYS.has(key)) + .sort(([a], [b]) => (a < b ? -1 : 1)); + return `{${entries.map(([key, entryValue]) => `${JSON.stringify(key)}:${stableStringify(entryValue)}`).join(',')}}`; +}; + +// A signature of the whole policy content (minus write-bookkeeping keys), stored in the derived value +// (like `locale`) so the change-detection baseline survives app restarts. Hashing the full content +// instead of an enumerated field list means a policy field the compute reads can never silently fall +// outside change detection; over-triggering is safe because the recompute is scoped to that policy's +// reports. The serialized length rides along as a second dimension so a 32-bit hash collision alone +// cannot mask a change; a full collision would only skip one scoped recompute for one policy change. +const policyRelevantSignature = (policy: Policy | null | undefined): string | null => { + if (!policy) { + return null; } - return ( - prev.type !== next.type || - prev.approvalMode !== next.approvalMode || - prev.reimbursementChoice !== next.reimbursementChoice || - prev.autoReimbursementLimit !== next.autoReimbursementLimit || - prev.role !== next.role || - prev.autoReimbursement?.limit !== next.autoReimbursement?.limit - ); + const serialized = stableStringify(policy); + return `${POLICY_SIGNATURE_VERSION}|${hashCode(serialized)}.${serialized.length}`; }; // A short string built from the fields a report name can come from: displayName and firstName. @@ -250,56 +284,151 @@ export default createOnyxDerivedValueConfig({ seedDisplayNamesBaseline(personalDetails); } + const nextIsTrackIntentUser = isTrackIntentUserSelector(introSelected); + // conciergeReportID and introSelected are re-delivered on every OpenApp/reconnect merge, so a full + // recompute happens only when the value differs from the one the attributes were computed with. + // A missing stored value (attributes written by an older app version) seeds the baseline instead. + // "No concierge report yet" is stored as an empty string, NOT null — Onyx.set strips nested null + // values on persist, so a null baseline would read back as missing after a restart and seed + // instead of detecting the change. + // eslint-disable-next-line rulesdir/no-default-id-values -- '' is a persistable baseline sentinel for "computed without a concierge report ID", never used as a lookup ID + const nextConciergeReportID = conciergeReportID ?? ''; + // eslint-disable-next-line rulesdir/no-default-id-values -- same sentinel when reading values persisted before the field became non-nullable + const storedConciergeReportID = currentValue && 'conciergeReportID' in currentValue ? (currentValue.conciergeReportID ?? '') : undefined; + const storedIsTrackIntentUser = currentValue && 'isTrackIntentUser' in currentValue ? currentValue.isTrackIntentUser : undefined; + const hasConciergeReportIDChanged = + hasKeyTriggeredCompute(ONYXKEYS.CONCIERGE_REPORT_ID, sourceValues) && storedConciergeReportID !== undefined && storedConciergeReportID !== nextConciergeReportID; + const hasIsTrackIntentUserChanged = + hasKeyTriggeredCompute(ONYXKEYS.NVP_INTRO_SELECTED, sourceValues) && storedIsTrackIntentUser !== undefined && storedIsTrackIntentUser !== nextIsTrackIntentUser; + // A full recompute is needed when locale changes (report names are locale-dependent) or display names change. // We compare preferredLocale against currentValue?.locale so that the first locale load on startup // (where both equal the same persisted value) does not trigger an unnecessary full recompute. const needsFullRecompute = (hasKeyTriggeredCompute(ONYXKEYS.NVP_PREFERRED_LOCALE, sourceValues) && preferredLocale !== currentValue?.locale) || displayNameChanges === RECOMPUTE_ALL || - hasKeyTriggeredCompute(ONYXKEYS.CONCIERGE_REPORT_ID, sourceValues) || - hasKeyTriggeredCompute(ONYXKEYS.NVP_INTRO_SELECTED, sourceValues); - + hasConciergeReportIDChanged || + hasIsTrackIntentUserChanged; + + // Policy change detection compares signatures stored in the derived value, so the baseline survives + // app restarts. Signatures advance only when the recompute they imply actually runs (or vacuously + // has no reports to touch); otherwise the next delivery re-diffs against the old baseline. + const storedPolicySignatures = currentValue?.policySignatures; + const hasComputedReports = !!currentValue?.reports && Object.keys(currentValue.reports).length > 0; + let nextPolicySignatures = storedPolicySignatures; + // True when the signatures may persist through an early return: a pure baseline seed, or a diff + // whose changed policies have no reports referencing them (nothing to recompute). + let canPersistSignaturesWithoutRecompute = false; const policyChangedReportKeys: string[] = []; + + // Lazily built and cached — a full-recompute pass with a policy trigger would otherwise + // serialize every policy twice (once here, once in the pre-return snapshot). + let allPolicySignaturesCache: Record | undefined; + const buildAllPolicySignatures = () => { + if (allPolicySignaturesCache) { + return allPolicySignaturesCache; + } + const signatures: Record = {}; + for (const [key, policy] of Object.entries(policies ?? {})) { + const signature = policyRelevantSignature(policy); + if (signature !== null) { + signatures[key] = signature; + } + } + allPolicySignaturesCache = signatures; + return signatures; + }; + const collectReportKeysForPolicies = (changedPolicyIDs: Set) => { + for (const [reportKey, report] of Object.entries(reports ?? {})) { + if (!report) { + continue; + } + // The report's own policy — the sender workspace for an invoice. + if (report.policyID && changedPolicyIDs.has(report.policyID)) { + policyChangedReportKeys.push(reportKey); + continue; + } + // An invoice follows its receiver workspace. The invoice room carries the receiver + // on itself; a child invoice report doesn't, so we read it from its parent room + // (chatReportID) — the same place computeReportName looks for the invoice name. + const ownReceiverPolicyID = report.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined; + const room = report.chatReportID ? reports?.[`${ONYXKEYS.COLLECTION.REPORT}${report.chatReportID}`] : undefined; + const roomReceiverPolicyID = room?.invoiceReceiver && 'policyID' in room.invoiceReceiver ? room.invoiceReceiver.policyID : undefined; + if ((ownReceiverPolicyID && changedPolicyIDs.has(ownReceiverPolicyID)) || (roomReceiverPolicyID && changedPolicyIDs.has(roomReceiverPolicyID))) { + policyChangedReportKeys.push(reportKey); + } + } + }; + if (hasKeyTriggeredCompute(ONYXKEYS.COLLECTION.POLICY, sourceValues)) { - if (!needsFullRecompute) { + if (needsFullRecompute) { + // Every report recomputes with the current policies anyway — snapshot the full baseline. + nextPolicySignatures = buildAllPolicySignatures(); + } else if (storedPolicySignatures) { const changedPolicyIDs = new Set(); + const updatedSignatures = {...storedPolicySignatures}; for (const key of Object.keys(sourceValues?.[ONYXKEYS.COLLECTION.POLICY] ?? {})) { - if (hasPolicyRelevantFieldChanged(previousPolicies?.[key], policies?.[key])) { - changedPolicyIDs.add(key.replace(ONYXKEYS.COLLECTION.POLICY, '')); + const signature = policyRelevantSignature(policies?.[key]); + if ((storedPolicySignatures[key] ?? null) === signature) { + continue; + } + changedPolicyIDs.add(key.replace(ONYXKEYS.COLLECTION.POLICY, '')); + if (signature === null) { + delete updatedSignatures[key]; + } else { + updatedSignatures[key] = signature; } } if (changedPolicyIDs.size > 0) { - for (const [reportKey, report] of Object.entries(reports ?? {})) { - if (!report) { - continue; - } - // The report's own policy — the sender workspace for an invoice. - if (report.policyID && changedPolicyIDs.has(report.policyID)) { - policyChangedReportKeys.push(reportKey); - continue; - } - // An invoice follows its receiver workspace. The invoice room carries the receiver - // on itself; a child invoice report doesn't, so we read it from its parent room - // (chatReportID) — the same place computeReportName looks for the invoice name. - const ownReceiverPolicyID = report.invoiceReceiver && 'policyID' in report.invoiceReceiver ? report.invoiceReceiver.policyID : undefined; - const room = report.chatReportID ? reports?.[`${ONYXKEYS.COLLECTION.REPORT}${report.chatReportID}`] : undefined; - const roomReceiverPolicyID = room?.invoiceReceiver && 'policyID' in room.invoiceReceiver ? room.invoiceReceiver.policyID : undefined; - if ((ownReceiverPolicyID && changedPolicyIDs.has(ownReceiverPolicyID)) || (roomReceiverPolicyID && changedPolicyIDs.has(roomReceiverPolicyID))) { - policyChangedReportKeys.push(reportKey); - } - } + nextPolicySignatures = updatedSignatures; + collectReportKeysForPolicies(changedPolicyIDs); + // Vacuously satisfied only when the reports collection is actually available — + // with it missing, the recompute is skipped rather than unnecessary. + canPersistSignaturesWithoutRecompute = !!reports && policyChangedReportKeys.length === 0; } + } else if (hasComputedReports) { + // No stored baseline but attributes exist (value written by an older app version, or computed + // in-session before policies loaded) — we cannot tell whether these policies match the ones the + // attributes were computed with, so recompute the delivered policies' reports like the + // pre-signature code did, then snapshot the full baseline. + const deliveredPolicyIDs = new Set(Object.keys(sourceValues?.[ONYXKEYS.COLLECTION.POLICY] ?? {}).map((key) => key.replace(ONYXKEYS.COLLECTION.POLICY, ''))); + collectReportKeysForPolicies(deliveredPolicyIDs); + canPersistSignaturesWithoutRecompute = !!reports && policyChangedReportKeys.length === 0; + if (reports) { + nextPolicySignatures = buildAllPolicySignatures(); + } + } else { + // No attributes computed yet — the pass below runs a full scan, so seeding is safe. + nextPolicySignatures = buildAllPolicySignatures(); } - previousPolicies = policies; + } else if (!storedPolicySignatures && policies && hasComputedReports) { + // Baseline seeding on a pass that did not deliver policies: both the attributes and the policies + // come from the same disk-hydrated state (an in-session value computed without policies is + // impossible here — policies would have arrived as a policy trigger), so they are consistent. + // Known one-time limitation: on the FIRST session after this feature ships, a policy change + // merged while startup computes are still deferred is seeded here without a recompute; from the + // next value write onward the persisted baseline closes that window. + nextPolicySignatures = buildAllPolicySignatures(); + canPersistSignaturesWithoutRecompute = true; + } + + // Baseline writes that may ride an early return: only signature snapshots that require no recompute. + // conciergeReportID/isTrackIntentUser baselines advance exclusively in the final return, and only on + // passes that recomputed everything with the current values — advancing them on other passes would + // silently absorb a change and suppress the full recompute its next delivery should cause. + const metaPatch: Partial = {}; + if (canPersistSignaturesWithoutRecompute && nextPolicySignatures !== storedPolicySignatures) { + metaPatch.policySignatures = nextPolicySignatures; } + const withMetaPatch = (value: ReportAttributesDerivedValue): ReportAttributesDerivedValue => (Object.keys(metaPatch).length > 0 ? {...value, ...metaPatch} : value); // Use incremental updates when currentValue is already populated and no full recompute is required. // If currentValue has no reports (fresh install or cleared storage), fall back to a full scan. - const useIncrementalUpdates = !!currentValue?.reports && Object.keys(currentValue.reports).length > 0 && !needsFullRecompute; + const useIncrementalUpdates = hasComputedReports && !needsFullRecompute; // if we already computed the report attributes and there is no new reports data, return the current value if ((useIncrementalUpdates && !sourceValues) || !reports) { - return currentValue ?? {reports: {}, locale: null}; + return withMetaPatch(currentValue ?? {reports: {}, locale: null}); } const reportUpdates = sourceValues?.[ONYXKEYS.COLLECTION.REPORT] ?? {}; @@ -424,7 +553,7 @@ export default createOnyxDerivedValueConfig({ } } else { // No updates to process, return current value to prevent unnecessary computation - return currentValue ?? {reports: {}, locale: null}; + return withMetaPatch(currentValue ?? {reports: {}, locale: null}); } } @@ -529,7 +658,7 @@ export default createOnyxDerivedValueConfig({ allPolicyTags: policyTags, conciergeReportID: conciergeReportID ?? undefined, reportAttributes: currentValue?.reports, - isTrackIntentUser: isTrackIntentUserSelector(introSelected), + isTrackIntentUser: nextIsTrackIntentUser, }) : '', isEmpty: generateIsEmptyReport(report, isReportArchived), @@ -601,11 +730,26 @@ export default createOnyxDerivedValueConfig({ }; } + // A full scan just recomputed every report with the current policies, so the baseline can be + // snapshotted even when no policy trigger fired this pass (e.g. policies merged while computes + // were deferred during startup, or a locale-driven full recompute). + if (!useIncrementalUpdates && policies) { + nextPolicySignatures = buildAllPolicySignatures(); + } + + // The stored conciergeReportID/isTrackIntentUser must always equal the values the full attribute + // set was computed with, so they advance only when everything recomputed with the current values + // (or on the very first write). Incremental passes keep the stored baseline — if the current value + // drifted (e.g. a change landed while computes were deferred), the next delivery of that key still + // compares unequal and triggers the healing full recompute. return { reports: reportAttributes, locale: preferredLocale ?? null, + policySignatures: nextPolicySignatures, + conciergeReportID: storedConciergeReportID === undefined || needsFullRecompute ? nextConciergeReportID : storedConciergeReportID, + isTrackIntentUser: storedIsTrackIntentUser === undefined || needsFullRecompute ? nextIsTrackIntentUser : storedIsTrackIntentUser, }; }, }); -export {hasPolicyRelevantFieldChanged}; +export {policyRelevantSignature}; diff --git a/src/libs/actions/OnyxDerived/index.ts b/src/libs/actions/OnyxDerived/index.ts index ecd0764882e4..73aafa4ad17c 100644 --- a/src/libs/actions/OnyxDerived/index.ts +++ b/src/libs/actions/OnyxDerived/index.ts @@ -88,7 +88,7 @@ function init() { name: CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE, op: CONST.TELEMETRY.SPAN_ONYX_DERIVED_COMPUTE, parentSpan: getSpan(CONST.TELEMETRY.SPAN_APP_STARTUP), - attributes: {derivedKey: key}, + attributes: {derivedKey: key, [CONST.TELEMETRY.ATTRIBUTE_DERIVED_TRIGGER]: sourceKey ?? 'unknown'}, }); try { diff --git a/src/types/onyx/DerivedValues.ts b/src/types/onyx/DerivedValues.ts index 4223a0ed8183..9c56ea5083f5 100644 --- a/src/types/onyx/DerivedValues.ts +++ b/src/types/onyx/DerivedValues.ts @@ -69,6 +69,20 @@ type ReportAttributesDerivedValue = { * The locale used to compute the report attributes. */ locale: string | null; + /** + * Signatures of the policy fields the attributes depend on, keyed by policy Onyx key. + * Persisted so a fresh app session can tell real policy changes from the first post-startup merge. + */ + policySignatures?: Record; + /** + * The conciergeReportID used to compute the report attributes. An empty string means the attributes + * were computed without one (null is not stored — Onyx.set strips nested null values on persist). + */ + conciergeReportID?: string; + /** + * Whether the user was a track-intent user when the attributes were computed. + */ + isTrackIntentUser?: boolean; }; /** diff --git a/tests/unit/reportAttributesTest.ts b/tests/unit/reportAttributesTest.ts index 7c76a9ac4116..0ff12c3e4a0f 100644 --- a/tests/unit/reportAttributesTest.ts +++ b/tests/unit/reportAttributesTest.ts @@ -1,5 +1,5 @@ import type reportAttributesModuleDefault from '@userActions/OnyxDerived/configs/reportAttributes'; -import {hasPolicyRelevantFieldChanged} from '@userActions/OnyxDerived/configs/reportAttributes'; +import {policyRelevantSignature} from '@userActions/OnyxDerived/configs/reportAttributes'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; @@ -51,81 +51,45 @@ const basePolicy: Policy = { autoReimbursement: {limit: 500}, } as unknown as Policy; -describe('hasPolicyRelevantFieldChanged', () => { +describe('policyRelevantSignature', () => { describe('null / undefined edge cases', () => { - it('returns false when both are null', () => { - expect(hasPolicyRelevantFieldChanged(null, null)).toBe(false); - }); - - it('returns false when both are undefined', () => { - expect(hasPolicyRelevantFieldChanged(undefined, undefined)).toBe(false); - }); - - it('returns false when both are null/undefined mix', () => { - expect(hasPolicyRelevantFieldChanged(null, undefined)).toBe(false); - expect(hasPolicyRelevantFieldChanged(undefined, null)).toBe(false); - }); - - it('returns true when prev is null and next has a policy', () => { - expect(hasPolicyRelevantFieldChanged(null, basePolicy)).toBe(true); - }); - - it('returns true when next is null and prev had a policy', () => { - expect(hasPolicyRelevantFieldChanged(basePolicy, null)).toBe(true); - }); - }); - - describe('identical policies', () => { - it('returns false when all tracked fields are the same', () => { - const copy = {...basePolicy}; - expect(hasPolicyRelevantFieldChanged(basePolicy, copy)).toBe(false); - }); - - it('returns false when only a non-tracked field changes', () => { - const updated = {...basePolicy, name: 'Updated Name'} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(false); + it('returns null for missing policies', () => { + expect(policyRelevantSignature(null)).toBeNull(); + expect(policyRelevantSignature(undefined)).toBeNull(); }); }); - describe('tracked field changes', () => { - it('returns true when type changes', () => { - const updated = {...basePolicy, type: CONST.POLICY.TYPE.TEAM} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); - }); - - it('returns true when approvalMode changes', () => { - const updated = {...basePolicy, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); - }); - - it('returns true when reimbursementChoice changes', () => { - const updated = {...basePolicy, reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_NO} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); + describe('identical content', () => { + it('produces the same signature for a shallow copy', () => { + expect(policyRelevantSignature({...basePolicy})).toBe(policyRelevantSignature(basePolicy)); }); - it('returns true when autoReimbursementLimit changes', () => { - const updated = {...basePolicy, autoReimbursementLimit: 2000} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); + it('produces the same signature regardless of key insertion order', () => { + const reordered = Object.fromEntries(Object.entries(basePolicy).reverse()) as unknown as Policy; + expect(policyRelevantSignature(reordered)).toBe(policyRelevantSignature(basePolicy)); }); - it('returns true when role changes', () => { - const updated = {...basePolicy, role: CONST.POLICY.ROLE.USER} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); - }); - - it('returns true when autoReimbursement.limit changes', () => { - const updated = {...basePolicy, autoReimbursement: {limit: 999}} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); - }); - - it('returns true when autoReimbursement goes from defined to undefined', () => { - const updated = {...basePolicy, autoReimbursement: undefined} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(basePolicy, updated)).toBe(true); + it('ignores Onyx write-bookkeeping keys', () => { + const withWriteNoise = {...basePolicy, pendingAction: 'update', pendingFields: {name: 'update'}, errors: {a: 'err'}, errorFields: {name: {a: 'err'}}} as unknown as Policy; + expect(policyRelevantSignature(withWriteNoise)).toBe(policyRelevantSignature(basePolicy)); }); + }); - it('returns true when autoReimbursement goes from undefined to defined', () => { - const withoutAutoReimburse = {...basePolicy, autoReimbursement: undefined} as unknown as Policy; - expect(hasPolicyRelevantFieldChanged(withoutAutoReimburse, basePolicy)).toBe(true); + describe('content changes', () => { + const changes: Array<[string, Partial]> = [ + ['type', {type: CONST.POLICY.TYPE.TEAM}], + ['approvalMode', {approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL}], + ['reimbursementChoice', {reimbursementChoice: CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_NO}], + ['autoReimbursementLimit', {autoReimbursementLimit: 2000}], + ['role', {role: CONST.POLICY.ROLE.USER}], + ['autoReimbursement.limit', {autoReimbursement: {limit: 999}}], + ['autoReimbursement removed', {autoReimbursement: undefined}], + ['name', {name: 'Renamed Workspace'}], + ['employeeList', {employeeList: {['a@b.com' as string]: {submitsTo: 'c@d.com'}}}], + ]; + it.each(changes)('changes the signature when %s changes', (_label, change) => { + const updated = {...basePolicy, ...change} as unknown as Policy; + expect(policyRelevantSignature(updated)).not.toBe(policyRelevantSignature(basePolicy)); }); }); }); @@ -160,13 +124,21 @@ describe('reportAttributes compute — policy change code flow', () => { [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: policy2, }; + // policyRelevantSignature returns null only for a missing policy, so tests can rely on a string. + const signatureOf = (policy: Policy): string => policyRelevantSignature(policy) ?? ''; + beforeEach(() => { jest.resetModules(); config = (require('@userActions/OnyxDerived/configs/reportAttributes') as {default: ReportAttributesConfig}).default; }); - const buildArgs = (overridePolicies?: OnyxCollection, overrideReports?: OnyxCollection, transactionsUpdate?: OnyxCollection | null) => + const buildArgs = ( + overridePolicies?: OnyxCollection, + overrideReports?: OnyxCollection, + transactionsUpdate?: OnyxCollection | null, + conciergeReportID?: string, + ) => [ overrideReports ?? reports, // reports null, // preferredLocale @@ -178,8 +150,8 @@ describe('reportAttributes compute — policy change code flow', () => { null, // session overridePolicies ?? policies, // policies null, // policyTags - null, // reportViolations - null, // reportMetadata + conciergeReportID ?? null, // conciergeReportID + null, // introSelected ] as unknown as Parameters[0]; it('computes every report on a cold start (no currentValue) when policies load', () => { @@ -192,9 +164,11 @@ describe('reportAttributes compute — policy change code flow', () => { expect(result?.reports).toHaveProperty('r2'); }); - it('scopes the first policy load to reports referencing the loaded policies when currentValue is already populated', () => { - // Reproduces the ReconnectApp-after-open case: attributes were already computed, then ~1k policies - // land. Only reports whose policy actually arrived should recompute — not every report. + it('recomputes reports of delivered policies when the stored value has no signature baseline', () => { + // Reproduces the first policy delivery on a value with no baseline (written by an older app + // version, or computed in-session before policies loaded): we cannot tell whether the stored + // attributes match these policies, so the delivered policies' reports recompute — exactly like + // the pre-signature code did on its first in-session delivery — and the full baseline is recorded. const report3: Report = {...createRandomReport(12, undefined), reportID: 'r3', policyID: 'policyOther', chatReportID: undefined}; const reportsWithUnrelated: OnyxCollection = { ...reports, @@ -215,11 +189,78 @@ describe('reportAttributes compute — policy change code flow', () => { sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies}, }); - // r1/r2 reference the loaded policies → recomputed (default mock name). + // r1/r2 reference the delivered policies → recomputed (default mock name). expect(result?.reports.r1?.reportName).toBe('Test Report'); expect(result?.reports.r2?.reportName).toBe('Test Report'); + // r3 references a policy that was not delivered → keeps its existing value. + expect(result?.reports.r3?.reportName).toBe('Old Name 3'); + expect(result?.policySignatures).toEqual({ + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policyRelevantSignature(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: policyRelevantSignature(policy2), + }); + }); + + it('seeds policy signatures without recomputing on a pass that did not deliver policies', () => { + // Disk-hydrated policies and a disk-restored value are consistent, so a non-policy pass just + // records the baseline; only the report from this pass's own update recomputes. + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + }; + + const result = config.compute(buildArgs(), { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.REPORT]: {[`${ONYXKEYS.COLLECTION.REPORT}r1`]: report1}}, + }); + + expect(result?.reports.r1?.reportName).toBe('Test Report'); + expect(result?.reports.r2?.reportName).toBe('Old Name 2'); + expect(result?.policySignatures).toEqual({ + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policyRelevantSignature(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: policyRelevantSignature(policy2), + }); + }); + + it('scopes a policy delivery to reports whose stored signature differs when currentValue is already populated', () => { + // Reproduces the ReconnectApp-after-restart case: attributes and signatures were persisted, then + // ~1k policies land and some actually changed. Only reports whose policy signature differs should + // recompute — not every report. + const report3: Report = {...createRandomReport(12, undefined), reportID: 'r3', policyID: 'policyOther', chatReportID: undefined}; + const reportsWithUnrelated: OnyxCollection = { + ...reports, + [`${ONYXKEYS.COLLECTION.REPORT}r3`]: report3, + }; + + const stalePolicy1: Policy = {...policy1, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL}; + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r3: {reportName: 'Old Name 3', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(stalePolicy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, + }; + + const result = config.compute(buildArgs(policies, reportsWithUnrelated), { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies}, + }); + + // r1's policy signature differs from the stored one → recomputed (default mock name). + expect(result?.reports.r1?.reportName).toBe('Test Report'); + // r2's policy matches its stored signature → keeps its existing value. + expect(result?.reports.r2?.reportName).toBe('Old Name 2'); // r3 references a policy that did not load → keeps its existing value (not recomputed). expect(result?.reports.r3?.reportName).toBe('Old Name 3'); + // The changed policy's signature is refreshed in the stored baseline. + expect(result?.policySignatures?.[`${ONYXKEYS.COLLECTION.POLICY}policy1`]).toBe(policyRelevantSignature(policy1)); }); it('recomputes a child invoice report when only its receiver workspace policy loads', () => { @@ -243,18 +284,16 @@ describe('reportAttributes compute — policy change code flow', () => { [`${ONYXKEYS.COLLECTION.REPORT}invoiceChild`]: invoiceChild, }; - // Seed previousPolicies with just the sender policy, as if it arrived in an earlier batch. - config.compute(buildArgs({[`${ONYXKEYS.COLLECTION.POLICY}senderPolicy`]: senderPolicy}, invoiceReports), { - currentValue: undefined, - sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}senderPolicy`]: senderPolicy}}, - }); - + // The stored baseline knows only the sender policy, as if it arrived in an earlier batch. const existingValue: ReportAttributesDerivedValue = { reports: { invoiceRoom: {reportName: 'Old Room', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, invoiceChild: {reportName: 'Old Child', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, }, locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}senderPolicy`]: signatureOf(senderPolicy), + }, }; // The receiver policy now arrives in its own batch. @@ -273,12 +312,6 @@ describe('reportAttributes compute — policy change code flow', () => { }); it('only recomputes reports for the changed policy when a tracked field changes', () => { - // Seed previousPolicies by doing an initial compute - config.compute(buildArgs(), { - currentValue: undefined, - sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies as never}, - }); - const policy1Changed = {...policy1, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL} as unknown as Policy; const updatedPolicies: OnyxCollection = { ...policies, @@ -291,6 +324,10 @@ describe('reportAttributes compute — policy change code flow', () => { r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, }, locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, }; const computeReportNameMock = (jest.requireMock('@libs/ReportNameUtils') as unknown as {computeReportName: jest.Mock}).computeReportName; @@ -307,17 +344,11 @@ describe('reportAttributes compute — policy change code flow', () => { expect(result?.reports.r2?.reportName).toBe('Old Name 2'); }); - it('skips recompute when a non-tracked policy field changes', () => { - // Seed previousPolicies - config.compute(buildArgs(), { - currentValue: undefined, - sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: policies as never}, - }); - - const policy1WithNameChange = {...policy1, name: 'New Policy Name'} as unknown as Policy; + it('skips recompute when only Onyx write-bookkeeping keys change on a policy', () => { + const policy1WithWriteNoise = {...policy1, pendingFields: {generalSettings: 'update'}, errorFields: {generalSettings: {a: 'err'}}} as unknown as Policy; const updatedPolicies: OnyxCollection = { ...policies, - [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1WithNameChange, + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1WithWriteNoise, }; const existingValue: ReportAttributesDerivedValue = { @@ -326,17 +357,173 @@ describe('reportAttributes compute — policy change code flow', () => { r2: {reportName: 'Existing r2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, }, locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, + conciergeReportID: '', + isTrackIntentUser: false, }; const result = config.compute(buildArgs(updatedPolicies), { currentValue: existingValue, - sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1WithNameChange} as never}, + sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1WithWriteNoise} as never}, }); - // No tracked fields changed → return currentValue unchanged + // No content change → return currentValue unchanged expect(result).toEqual(existingValue); }); + it('recomputes reports of a policy whose name changes', () => { + const policy1Renamed = {...policy1, name: 'Renamed Workspace'} as unknown as Policy; + const updatedPolicies: OnyxCollection = { + ...policies, + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1Renamed, + }; + + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, + }; + + const result = config.compute(buildArgs(updatedPolicies), { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1Renamed} as never}, + }); + + // Report names embed the workspace name, so a rename recomputes that policy's reports. + expect(result?.reports.r1?.reportName).toBe('Test Report'); + expect(result?.reports.r2?.reportName).toBe('Old Name 2'); + expect(result?.policySignatures?.[`${ONYXKEYS.COLLECTION.POLICY}policy1`]).toBe(signatureOf(policy1Renamed)); + }); + + it('keeps the concierge baseline on passes that did not recompute everything', () => { + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, + conciergeReportID: 'conciergeOld', + isTrackIntentUser: false, + }; + + // An incremental pass while the current conciergeReportID already drifted (it changed while + // computes were deferred): the baseline must NOT advance, or the change would be absorbed. + const incrementalResult = config.compute(buildArgs(policies, undefined, null, 'conciergeNew'), { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.REPORT]: {[`${ONYXKEYS.COLLECTION.REPORT}r1`]: report1}}, + }); + expect(incrementalResult?.conciergeReportID).toBe('conciergeOld'); + expect(incrementalResult?.reports.r2?.reportName).toBe('Old Name 2'); + + // The next delivery of the key compares against the preserved baseline → full recompute + advance. + const deliveryResult = config.compute(buildArgs(policies, undefined, null, 'conciergeNew'), { + currentValue: incrementalResult, + sourceValues: {[ONYXKEYS.CONCIERGE_REPORT_ID]: 'conciergeNew' as never}, + }); + expect(deliveryResult?.conciergeReportID).toBe('conciergeNew'); + expect(deliveryResult?.reports.r2?.reportName).toBe('Test Report'); + }); + + it('does not persist advanced policy signatures when the reports collection is unavailable', () => { + const policy1Changed = {...policy1, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL} as unknown as Policy; + const updatedPolicies: OnyxCollection = { + ...policies, + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1Changed, + }; + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }, + }; + + const argsWithoutReports = [ + undefined, // reports + null, // preferredLocale + null, // transactionViolations + null, // reportActions + null, // reportNameValuePairs + null, // transactions + null, // personalDetails + null, // session + updatedPolicies, // policies + null, // policyTags + null, // conciergeReportID + null, // introSelected + ] as unknown as Parameters[0]; + + const result = config.compute(argsWithoutReports, { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policy1`]: policy1Changed} as never}, + }); + + // The scoped recompute could not run, so the old baseline must survive — the next delivery + // re-diffs against it and retries the recompute. + expect(result?.policySignatures?.[`${ONYXKEYS.COLLECTION.POLICY}policy1`]).toBe(signatureOf(policy1)); + }); + + it('persists advanced signatures without recompute when the changed policy has no reports', () => { + const policyNoReports: Policy = {...basePolicy, id: 'policyNoReports'}; + const policyNoReportsChanged: Policy = {...policyNoReports, approvalMode: CONST.POLICY.APPROVAL_MODE.OPTIONAL}; + const policiesWithExtra: OnyxCollection = { + ...policies, + [`${ONYXKEYS.COLLECTION.POLICY}policyNoReports`]: policyNoReportsChanged, + }; + const existingValue: ReportAttributesDerivedValue = { + reports: { + r1: {reportName: 'Old Name 1', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + r2: {reportName: 'Old Name 2', isEmpty: false, brickRoadStatus: undefined, requiresAttention: false, reportErrors: {}}, + }, + locale: null, + policySignatures: { + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + [`${ONYXKEYS.COLLECTION.POLICY}policyNoReports`]: signatureOf(policyNoReports), + }, + }; + + const result = config.compute(buildArgs(policiesWithExtra), { + currentValue: existingValue, + sourceValues: {[ONYXKEYS.COLLECTION.POLICY]: {[`${ONYXKEYS.COLLECTION.POLICY}policyNoReports`]: policyNoReportsChanged} as never}, + }); + + // No report references the changed policy, so the recompute is vacuous — nothing recomputes, + // but the advanced signature persists so the same delivery is not re-diffed forever. + expect(result?.reports.r1?.reportName).toBe('Old Name 1'); + expect(result?.reports.r2?.reportName).toBe('Old Name 2'); + expect(result?.policySignatures?.[`${ONYXKEYS.COLLECTION.POLICY}policyNoReports`]).toBe(signatureOf(policyNoReportsChanged)); + }); + + it('snapshots the policy baseline after a full scan even without a policy trigger', () => { + const result = config.compute(buildArgs(), { + currentValue: undefined, + sourceValues: {[ONYXKEYS.COLLECTION.REPORT]: {[`${ONYXKEYS.COLLECTION.REPORT}r1`]: report1}}, + }); + + expect(result?.reports.r1?.reportName).toBe('Test Report'); + expect(result?.policySignatures).toEqual({ + [`${ONYXKEYS.COLLECTION.POLICY}policy1`]: signatureOf(policy1), + [`${ONYXKEYS.COLLECTION.POLICY}policy2`]: signatureOf(policy2), + }); + }); + it('recomputes the parent workspace chat when a transaction on its expense report changes', () => { const expenseReport: Report = {...createRandomReport(10, undefined), reportID: 'expense1', policyID: 'policy3', chatReportID: 'chat1'}; const chatReport: Report = {...createRandomReport(11, CONST.REPORT.CHAT_TYPE.POLICY_EXPENSE_CHAT), reportID: 'chat1', policyID: 'policy3', chatReportID: undefined};