diff --git a/apps/roam/src/components/settings/AdminPanel.tsx b/apps/roam/src/components/settings/AdminPanel.tsx index da19cd120..641ad060c 100644 --- a/apps/roam/src/components/settings/AdminPanel.tsx +++ b/apps/roam/src/components/settings/AdminPanel.tsx @@ -422,7 +422,7 @@ const FeatureFlagsTab = (): React.ReactElement => { diff --git a/apps/roam/src/components/settings/utils/init.ts b/apps/roam/src/components/settings/utils/init.ts index 9d6116766..42f03d3d5 100644 --- a/apps/roam/src/components/settings/utils/init.ts +++ b/apps/roam/src/components/settings/utils/init.ts @@ -23,6 +23,7 @@ import { migrateGraphLevel, migratePersonalSettings, } from "./migrateLegacyToBlockProps"; +import { migratePropsStoreDefault } from "./migratePropsStoreDefault"; import { getTopLevelBlockPropsConfig } from "~/components/settings/utils/zodSchema"; import { DG_BLOCK_PROP_SETTINGS_PAGE_TITLE } from "./zodSchema"; import toFlexRegex from "roamjs-components/util/toFlexRegex"; @@ -376,8 +377,11 @@ export const initSchema = async (): Promise => { refreshConfigTree(); } - await migrateGraphLevel(blockUids); - await migratePersonalSettings(blockUids); + const graphSettingsMigrated = await migrateGraphLevel(blockUids); + const personalSettingsMigrated = await migratePersonalSettings(blockUids); + if (graphSettingsMigrated && personalSettingsMigrated) { + await migratePropsStoreDefault(blockUids); + } (window as unknown as Record).dgDualReadLog = logDualReadComparison; return { blockUids, nodePageUids: {} }; diff --git a/apps/roam/src/components/settings/utils/migrateLegacyToBlockProps.ts b/apps/roam/src/components/settings/utils/migrateLegacyToBlockProps.ts index da29fa11b..d8c9bd888 100644 --- a/apps/roam/src/components/settings/utils/migrateLegacyToBlockProps.ts +++ b/apps/roam/src/components/settings/utils/migrateLegacyToBlockProps.ts @@ -1,6 +1,6 @@ import getBlockProps from "~/utils/getBlockProps"; import type { json } from "~/utils/getBlockProps"; -import setBlockProps from "~/utils/setBlockProps"; +import { setBlockPropsAsync } from "~/utils/setBlockProps"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import { createBlock } from "roamjs-components/writes"; import { getSetting, setSetting } from "~/utils/extensionSettings"; @@ -26,8 +26,8 @@ import type { z } from "zod"; import { invalidateDiscourseNodeTypeCaches } from "~/utils/discourseNodeTypeCache"; const LOG_PREFIX = "[DG BlockProps Migration]"; -const GRAPH_MIGRATION_MARKER = "Block props migrated"; -const PERSONAL_MIGRATION_MARKER = "dg-personal-settings-migrated"; +const GRAPH_MIGRATION_MARKER = "Block props migrated v2"; +const PERSONAL_MIGRATION_MARKER = "dg-personal-settings-migrated-v2"; const MAX_ERROR_CONTEXT_LENGTH = 5000; const hasGraphMigrationMarker = (blockMap: Record): boolean => @@ -59,7 +59,7 @@ const shouldWrite = ( return JSON.stringify(parsedLegacy) !== JSON.stringify(currentProps); }; -const migrateSection = ({ +const migrateSection = async ({ label, blockUid, schema, @@ -71,17 +71,11 @@ const migrateSection = ({ schema: z.ZodTypeAny; legacyData: Record; onWrite?: () => void; -}): boolean => { +}): Promise => { const currentProps = getBlockProps(blockUid); const parseResult = schema.safeParse(legacyData); if (!parseResult.success) { - if (isPropsValid(schema, currentProps)) { - console.log( - `${LOG_PREFIX} ${label}: legacy malformed but props already valid, skipping`, - ); - return true; - } console.warn(`${LOG_PREFIX} ${label}: Zod validation failed, skipping`, { error: parseResult.error.message, }); @@ -105,10 +99,26 @@ const migrateSection = ({ return true; } - setBlockProps(blockUid, parsedLegacy, false); - onWrite?.(); - console.log(`${LOG_PREFIX} ${label}: migrated`); - return true; + try { + await setBlockPropsAsync(blockUid, parsedLegacy, false); + onWrite?.(); + console.log(`${LOG_PREFIX} ${label}: migrated`); + return true; + } catch (error) { + console.warn(`${LOG_PREFIX} ${label}: write failed, skipping`, error); + internalError({ + error, + type: "DG Block Props Migration", + context: { + label, + blockUid, + legacyData: serializeErrorContext(legacyData), + currentProps: serializeErrorContext(currentProps), + }, + sendEmail: false, + }); + return false; + } }; const migrateDiscourseNodes = async (): Promise => { @@ -155,13 +165,13 @@ const migrateDiscourseNodes = async (): Promise => { } if ( - !migrateSection({ + !(await migrateSection({ label: `Discourse Node (${nodeText})`, blockUid: nodePageUid, schema: DiscourseNodeSchema, legacyData, onWrite: invalidateDiscourseNodeTypeCaches, - }) + })) ) { allOk = false; } @@ -172,7 +182,7 @@ const migrateDiscourseNodes = async (): Promise => { export const migrateGraphLevel = async ( blockUids: Record, -): Promise => { +): Promise => { const pageUid = getPageUidByPageTitle(DG_BLOCK_PROP_SETTINGS_PAGE_TITLE); if (!pageUid) { internalError({ @@ -181,12 +191,12 @@ export const migrateGraphLevel = async ( context: { scope: "graph" }, sendEmail: false, }); - return; + return false; } if (hasGraphMigrationMarker(blockUids)) { console.log(`${LOG_PREFIX} graph-level: skipped (already migrated)`); - return; + return true; } let failures = 0; @@ -215,12 +225,12 @@ export const migrateGraphLevel = async ( } } if ( - !migrateSection({ + !(await migrateSection({ label: "Feature Flags", blockUid: featureFlagUid, schema: FeatureFlagsSchema, legacyData: mergedFlags, - }) + })) ) { failures++; } @@ -241,12 +251,12 @@ export const migrateGraphLevel = async ( } else { const legacyGlobal = readAllLegacyGlobalSettings(); if ( - !migrateSection({ + !(await migrateSection({ label: "Global", blockUid: globalUid, schema: GlobalSettingsSchema, legacyData: legacyGlobal, - }) + })) ) { failures++; } @@ -263,25 +273,28 @@ export const migrateGraphLevel = async ( node: { text: GRAPH_MIGRATION_MARKER }, }); console.log(`${LOG_PREFIX} graph-level: completed`); + return true; } catch (e) { console.warn( `${LOG_PREFIX} graph-level: data migrated but marker write failed (will retry next load)`, e, ); + return false; } - } else { - console.warn( - `${LOG_PREFIX} graph-level: ${failures} section(s) failed, marker not created (will retry next load)`, - ); } + + console.warn( + `${LOG_PREFIX} graph-level: ${failures} section(s) failed, marker not created (will retry next load)`, + ); + return false; }; export const migratePersonalSettings = async ( blockUids: Record, -): Promise => { +): Promise => { if (getSetting(PERSONAL_MIGRATION_MARKER, false)) { console.log(`${LOG_PREFIX} personal: skipped (already migrated)`); - return; + return true; } const personalKey = getPersonalSettingsKey(); @@ -300,11 +313,11 @@ export const migratePersonalSettings = async ( }, sendEmail: false, }); - return; + return false; } const legacyPersonal = readAllLegacyPersonalSettings(); - const ok = migrateSection({ + const ok = await migrateSection({ label: "Personal", blockUid: personalUid, schema: PersonalSettingsSchema, @@ -315,15 +328,18 @@ export const migratePersonalSettings = async ( try { await setSetting(PERSONAL_MIGRATION_MARKER, true); console.log(`${LOG_PREFIX} personal: completed`); + return true; } catch (e) { console.warn( `${LOG_PREFIX} personal: data migrated but marker write failed (will retry next load)`, e, ); + return false; } - } else { - console.warn( - `${LOG_PREFIX} personal: failed, marker not created (will retry next load)`, - ); } + + console.warn( + `${LOG_PREFIX} personal: failed, marker not created (will retry next load)`, + ); + return false; }; diff --git a/apps/roam/src/components/settings/utils/migratePropsStoreDefault.ts b/apps/roam/src/components/settings/utils/migratePropsStoreDefault.ts new file mode 100644 index 000000000..db5026299 --- /dev/null +++ b/apps/roam/src/components/settings/utils/migratePropsStoreDefault.ts @@ -0,0 +1,59 @@ +import getBlockProps from "~/utils/getBlockProps"; +import { setBlockPropsAsync } from "~/utils/setBlockProps"; +import internalError from "~/utils/internalError"; +import { invalidateDiscourseNodeTypeCaches } from "~/utils/discourseNodeTypeCache"; +import { FEATURE_FLAG_KEYS } from "./settingKeys"; +import { STATIC_TOP_LEVEL_ENTRIES } from "./zodSchema"; + +const LOG_PREFIX = "[DG Props Default Migration]"; +export const PROPS_STORE_DEFAULT_MIGRATION_KEY = + "Props settings default migrated"; + +export const migratePropsStoreDefault = async ( + blockUids: Record, +): Promise => { + const featureFlagsUid = blockUids[STATIC_TOP_LEVEL_ENTRIES.featureFlags.key]; + + if (!featureFlagsUid) { + internalError({ + error: "Cannot enable props-based settings by default", + type: "DG Props Default Migration", + context: { featureFlagsUid }, + sendEmail: false, + }); + return; + } + + try { + const featureFlags = getBlockProps(featureFlagsUid); + if (featureFlags[PROPS_STORE_DEFAULT_MIGRATION_KEY] === true) { + console.log(`${LOG_PREFIX} skipped (already migrated)`); + return; + } + + const propsStoreAlreadyEnabled = + featureFlags[FEATURE_FLAG_KEYS.useNewSettingsStore] === true; + await setBlockPropsAsync( + featureFlagsUid, + { + [PROPS_STORE_DEFAULT_MIGRATION_KEY]: true, + ...(propsStoreAlreadyEnabled + ? {} + : { [FEATURE_FLAG_KEYS.useNewSettingsStore]: true }), + }, + false, + ); + + if (!propsStoreAlreadyEnabled) { + invalidateDiscourseNodeTypeCaches(); + } + console.log(`${LOG_PREFIX} completed`); + } catch (error) { + internalError({ + error, + type: "DG Props Default Migration", + context: { featureFlagsUid }, + sendEmail: false, + }); + } +}; diff --git a/apps/roam/src/utils/__tests__/migrateLegacyToBlockProps.test.ts b/apps/roam/src/utils/__tests__/migrateLegacyToBlockProps.test.ts new file mode 100644 index 000000000..55f9282fa --- /dev/null +++ b/apps/roam/src/utils/__tests__/migrateLegacyToBlockProps.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + createBlock: vi.fn(), + getBlockProps: vi.fn(), + getPageUidByPageTitle: vi.fn(), + internalError: vi.fn(), + invalidateDiscourseNodeTypeCaches: vi.fn(), + readAllLegacyDiscourseNodeSettings: vi.fn(), + readAllLegacyFeatureFlags: vi.fn(), + readAllLegacyGlobalSettings: vi.fn(), + readAllLegacyPersonalSettings: vi.fn(), + getSetting: vi.fn(), + setSetting: vi.fn(), + setBlockPropsAsync: vi.fn(), +})); + +vi.mock("roamjs-components/queries/getPageUidByPageTitle", () => ({ + default: mocks.getPageUidByPageTitle, +})); +vi.mock("roamjs-components/writes", () => ({ + createBlock: mocks.createBlock, +})); +vi.mock("~/utils/getBlockProps", () => ({ + default: mocks.getBlockProps, +})); +vi.mock("~/utils/setBlockProps", () => ({ + setBlockPropsAsync: mocks.setBlockPropsAsync, +})); +vi.mock("~/utils/extensionSettings", () => ({ + getSetting: mocks.getSetting, + setSetting: mocks.setSetting, +})); +vi.mock("~/utils/internalError", () => ({ + default: mocks.internalError, +})); +vi.mock("~/utils/discourseNodeTypeCache", () => ({ + invalidateDiscourseNodeTypeCaches: mocks.invalidateDiscourseNodeTypeCaches, +})); +vi.mock("~/components/settings/utils/accessors", () => ({ + LEGACY_SOURCED_FEATURE_FLAG_KEYS: ["Enable left sidebar"], + readAllLegacyDiscourseNodeSettings: mocks.readAllLegacyDiscourseNodeSettings, + readAllLegacyFeatureFlags: mocks.readAllLegacyFeatureFlags, + readAllLegacyGlobalSettings: mocks.readAllLegacyGlobalSettings, + readAllLegacyPersonalSettings: mocks.readAllLegacyPersonalSettings, +})); + +import { + migrateGraphLevel, + migratePersonalSettings, +} from "~/components/settings/utils/migrateLegacyToBlockProps"; +import { + FeatureFlagsSchema, + GlobalSettingsSchema, +} from "~/components/settings/utils/zodSchema"; + +describe("legacy settings migration", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.getPageUidByPageTitle.mockReturnValue("settings-page-uid"); + mocks.readAllLegacyFeatureFlags.mockReturnValue({}); + mocks.readAllLegacyGlobalSettings.mockReturnValue({}); + mocks.readAllLegacyPersonalSettings.mockReturnValue({}); + mocks.getBlockProps.mockImplementation((uid: string) => + uid === "feature-flags-uid" ? FeatureFlagsSchema.parse({}) : {}, + ); + mocks.setBlockPropsAsync.mockResolvedValue({}); + mocks.setSetting.mockResolvedValue(undefined); + (globalThis as { window: unknown }).window = { + roamAlphaAPI: { + data: { async: { fast: { q: vi.fn().mockResolvedValue([]) } } }, + user: { uid: () => "user-uid" }, + }, + }; + }); + + it("revalidates an old graph marker and does not complete when a write fails", async () => { + mocks.setBlockPropsAsync.mockRejectedValue(new Error("write failed")); + + const migrated = await migrateGraphLevel({ + "Block props migrated": "old-marker-uid", + "Feature Flags": "feature-flags-uid", + Global: "global-uid", + }); + + expect(migrated).toBe(false); + expect(mocks.createBlock).not.toHaveBeenCalled(); + expect(mocks.internalError).toHaveBeenCalledWith( + expect.objectContaining({ type: "DG Block Props Migration" }), + ); + }); + + it("records a versioned graph marker after revalidating an old marker", async () => { + const migrated = await migrateGraphLevel({ + "Block props migrated": "old-marker-uid", + "Feature Flags": "feature-flags-uid", + Global: "global-uid", + }); + + expect(migrated).toBe(true); + expect(mocks.createBlock).toHaveBeenCalledWith({ + parentUid: "settings-page-uid", + node: { text: "Block props migrated v2" }, + }); + }); + + it("rejects malformed legacy data even when current props are valid", async () => { + mocks.readAllLegacyGlobalSettings.mockReturnValue({ Trigger: 123 }); + mocks.getBlockProps.mockImplementation((uid: string) => { + if (uid === "feature-flags-uid") return FeatureFlagsSchema.parse({}); + if (uid === "global-uid") return GlobalSettingsSchema.parse({}); + return {}; + }); + + const migrated = await migrateGraphLevel({ + "Feature Flags": "feature-flags-uid", + Global: "global-uid", + }); + + expect(migrated).toBe(false); + expect(mocks.createBlock).not.toHaveBeenCalled(); + expect(mocks.internalError).toHaveBeenCalledWith( + expect.objectContaining({ type: "DG Block Props Migration" }), + ); + }); + + it("uses a versioned personal marker before enabling props", async () => { + mocks.getSetting.mockReturnValue(false); + + const migrated = await migratePersonalSettings({ + "user-uid": "personal-settings-uid", + }); + + expect(migrated).toBe(true); + expect(mocks.getSetting).toHaveBeenCalledWith( + "dg-personal-settings-migrated-v2", + false, + ); + expect(mocks.setSetting).toHaveBeenCalledWith( + "dg-personal-settings-migrated-v2", + true, + ); + }); +}); diff --git a/apps/roam/src/utils/__tests__/migratePropsStoreDefault.test.ts b/apps/roam/src/utils/__tests__/migratePropsStoreDefault.test.ts new file mode 100644 index 000000000..f268bc9c5 --- /dev/null +++ b/apps/roam/src/utils/__tests__/migratePropsStoreDefault.test.ts @@ -0,0 +1,96 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + getBlockProps: vi.fn(), + internalError: vi.fn(), + invalidateDiscourseNodeTypeCaches: vi.fn(), + setBlockPropsAsync: vi.fn(), +})); + +vi.mock("~/utils/getBlockProps", () => ({ + default: mocks.getBlockProps, +})); +vi.mock("~/utils/setBlockProps", () => ({ + setBlockPropsAsync: mocks.setBlockPropsAsync, +})); +vi.mock("~/utils/internalError", () => ({ + default: mocks.internalError, +})); +vi.mock("~/utils/discourseNodeTypeCache", () => ({ + invalidateDiscourseNodeTypeCaches: mocks.invalidateDiscourseNodeTypeCaches, +})); + +import { + migratePropsStoreDefault, + PROPS_STORE_DEFAULT_MIGRATION_KEY, +} from "~/components/settings/utils/migratePropsStoreDefault"; +import { FeatureFlagsSchema } from "~/components/settings/utils/zodSchema"; + +const BLOCK_UIDS = { "Feature Flags": "feature-flags-uid" }; + +describe("props settings default migration", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.setBlockPropsAsync.mockResolvedValue({}); + }); + + it("keeps legacy as the safe fallback before the rollout completes", () => { + expect(FeatureFlagsSchema.parse({})["Use new settings store"]).toBe(false); + }); + + it("enables props when an existing graph has no stored flag", async () => { + mocks.getBlockProps.mockReturnValue({}); + + await migratePropsStoreDefault(BLOCK_UIDS); + + expect(mocks.setBlockPropsAsync).toHaveBeenCalledWith( + "feature-flags-uid", + { + [PROPS_STORE_DEFAULT_MIGRATION_KEY]: true, + "Use new settings store": true, + }, + false, + ); + }); + + it("enables props once when an existing graph has the previous false default", async () => { + mocks.getBlockProps.mockReturnValue({ "Use new settings store": false }); + + await migratePropsStoreDefault(BLOCK_UIDS); + + expect(mocks.setBlockPropsAsync).toHaveBeenCalledWith( + "feature-flags-uid", + { + [PROPS_STORE_DEFAULT_MIGRATION_KEY]: true, + "Use new settings store": true, + }, + false, + ); + expect(mocks.invalidateDiscourseNodeTypeCaches).toHaveBeenCalledOnce(); + }); + + it("keeps an existing true value and records the migration", async () => { + mocks.getBlockProps.mockReturnValue({ "Use new settings store": true }); + + await migratePropsStoreDefault(BLOCK_UIDS); + + expect(mocks.setBlockPropsAsync).toHaveBeenCalledWith( + "feature-flags-uid", + { [PROPS_STORE_DEFAULT_MIGRATION_KEY]: true }, + false, + ); + expect(mocks.invalidateDiscourseNodeTypeCaches).not.toHaveBeenCalled(); + }); + + it("preserves an admin false value after the migration marker exists", async () => { + mocks.getBlockProps.mockReturnValue({ + [PROPS_STORE_DEFAULT_MIGRATION_KEY]: true, + "Use new settings store": false, + }); + + await migratePropsStoreDefault(BLOCK_UIDS); + + expect(mocks.setBlockPropsAsync).not.toHaveBeenCalled(); + expect(mocks.invalidateDiscourseNodeTypeCaches).not.toHaveBeenCalled(); + }); +});