From b7d26dfae757e348775d0dfeced594184d7dc0f4 Mon Sep 17 00:00:00 2001 From: sid597 Date: Wed, 4 Mar 2026 01:45:27 +0530 Subject: [PATCH 1/3] ENG-1469: Refactor getDiscourseRelations and getDiscourseNodes to read from block props --- .../components/settings/utils/accessors.ts | 87 ++++++++--- .../components/settings/utils/zodSchema.ts | 2 +- apps/roam/src/utils/getDiscourseNodes.ts | 143 ++++++++++-------- apps/roam/src/utils/getDiscourseRelations.ts | 8 + 4 files changed, 157 insertions(+), 83 deletions(-) diff --git a/apps/roam/src/components/settings/utils/accessors.ts b/apps/roam/src/components/settings/utils/accessors.ts index dd8b4a840..4ef2bf77d 100644 --- a/apps/roam/src/components/settings/utils/accessors.ts +++ b/apps/roam/src/components/settings/utils/accessors.ts @@ -12,6 +12,9 @@ import internalError from "~/utils/internalError"; import { getSetting } from "~/utils/extensionSettings"; import { getFormattedConfigTree } from "~/utils/discourseConfigRef"; import { roamNodeToCondition } from "~/utils/parseQuery"; +import type { DiscourseRelation } from "~/utils/getDiscourseRelations"; +import type { DiscourseNode } from "~/utils/getDiscourseNodes"; +import type { Condition } from "~/utils/types"; import { z } from "zod"; import { @@ -27,7 +30,7 @@ import { type GlobalSettings, type PersonalSettings, type DiscourseNodeSettings, - type DiscourseRelationSettings, + type Condition as SchemaCondition, } from "./zodSchema"; const isRecord = (value: unknown): value is Record => @@ -250,11 +253,6 @@ const getLegacyPersonalSetting = (keys: string[]): unknown => { return undefined; }; -// NOTE(ENG-1469): This returns the block props schema shape (Record). Runtime consumers use getDiscourseRelations() -// which returns a flat DiscourseRelation[] with a different structure (one entry per -// if-block, triples at top level, no nodePositions). When migrating getDiscourseRelations() -// to read from block props, it will need a conversion from this shape to the flat array. const getLegacyRelationsSetting = (): Record => { const settingsUid = getPageUidByPageTitle(DG_BLOCK_PROP_SETTINGS_PAGE_TITLE); if (!settingsUid) return DEFAULT_GLOBAL_SETTINGS.Relations; @@ -766,13 +764,19 @@ export const setGlobalSetting = (keys: string[], value: json): void => { }); }; -export const getAllRelations = (): DiscourseRelationSettings[] => { +export const getAllRelations = (): DiscourseRelation[] => { const settings = getGlobalSettings(); - return Object.entries(settings.Relations).map(([id, relation]) => ({ - ...relation, - id, - })); + return Object.entries(settings.Relations).flatMap(([id, relation]) => + relation.ifConditions.map((ifCondition) => ({ + id, + label: relation.label, + source: relation.source, + destination: relation.destination, + complement: relation.complement, + triples: ifCondition.triples, + })), + ); }; export const getPersonalSettings = (): PersonalSettings => { @@ -936,7 +940,52 @@ export const setDiscourseNodeSetting = ( setBlockPropAtPath(pageUid, keys, value); }; -export const getAllDiscourseNodes = (): DiscourseNodeSettings[] => { +const addConditionUids = (conditions: SchemaCondition[]): Condition[] => + conditions.map((c) => { + const uid = window.roamAlphaAPI.util.generateUID(); + if (c.type === "or" || c.type === "not or") { + return { + uid, + type: c.type, + conditions: c.conditions.map(addConditionUids), + }; + } + return { + uid, + type: c.type, + source: c.source, + relation: c.relation, + target: c.target, + not: c.not, + }; + }) as Condition[]; + +const toDiscourseNode = (settings: DiscourseNodeSettings): DiscourseNode => ({ + text: settings.text, + type: settings.type, + shortcut: settings.shortcut, + tag: settings.tag || undefined, + format: settings.format, + description: settings.description || undefined, + graphOverview: settings.graphOverview || undefined, + backedBy: settings.backedBy, + specification: addConditionUids( + settings.specification.query.conditions as SchemaCondition[], + ), + canvasSettings: Object.fromEntries( + Object.entries(settings.canvasSettings).map(([k, v]) => [ + k, + typeof v === "boolean" ? (v ? "true" : "") : String(v), + ]), + ), + template: settings.template.length > 0 ? settings.template : undefined, + embeddingRef: settings.suggestiveRules.embeddingRef || undefined, + isFirstChild: settings.suggestiveRules.isFirstChild + ? { uid: "", value: true } + : undefined, +}); + +export const getAllDiscourseNodes = (): DiscourseNode[] => { const results = window.roamAlphaAPI.data.fast.q(` [:find ?uid ?title (pull ?page [:block/props]) :where @@ -945,7 +994,7 @@ export const getAllDiscourseNodes = (): DiscourseNodeSettings[] => { [(clojure.string/starts-with? ?title "${DISCOURSE_NODE_PAGE_PREFIX}")]] `) as [string, string, Record | null][]; - const nodes: DiscourseNodeSettings[] = []; + const nodes: DiscourseNode[] = []; for (const [pageUid, title, rawProps] of results) { if (typeof pageUid !== "string" || typeof title !== "string") continue; @@ -962,11 +1011,13 @@ export const getAllDiscourseNodes = (): DiscourseNodeSettings[] => { const result = DiscourseNodeSchema.safeParse(blockProps); if (result.success) { - nodes.push({ - ...result.data, - type: pageUid, - text: title.replace(DISCOURSE_NODE_PAGE_PREFIX, ""), - }); + nodes.push( + toDiscourseNode({ + ...result.data, + type: pageUid, + text: title.replace(DISCOURSE_NODE_PAGE_PREFIX, ""), + }), + ); } else { internalError({ error: result.error, diff --git a/apps/roam/src/components/settings/utils/zodSchema.ts b/apps/roam/src/components/settings/utils/zodSchema.ts index 212c09e21..4df25e6c7 100644 --- a/apps/roam/src/components/settings/utils/zodSchema.ts +++ b/apps/roam/src/components/settings/utils/zodSchema.ts @@ -22,7 +22,7 @@ const QBClauseDataSchema = z.object({ not: z.boolean().optional(), }); -type Condition = +export type Condition = | (z.infer & { type: "clause" }) | (z.infer & { type: "not" }) | { type: "or"; conditions: Condition[][] } diff --git a/apps/roam/src/utils/getDiscourseNodes.ts b/apps/roam/src/utils/getDiscourseNodes.ts index 3ee6befb5..cd83f940f 100644 --- a/apps/roam/src/utils/getDiscourseNodes.ts +++ b/apps/roam/src/utils/getDiscourseNodes.ts @@ -1,5 +1,9 @@ import getSettingValueFromTree from "roamjs-components/util/getSettingValueFromTree"; import getSubTree from "roamjs-components/util/getSubTree"; +import { + isNewSettingsStoreEnabled, + getAllDiscourseNodes, +} from "~/components/settings/utils/accessors"; import discourseConfigRef from "./discourseConfigRef"; import getDiscourseRelations from "./getDiscourseRelations"; import { roamNodeToCondition } from "./parseQuery"; @@ -103,72 +107,83 @@ const getUidAndBooleanSetting = ({ }; const getDiscourseNodes = (relations = getDiscourseRelations()) => { - const configuredNodes = Object.entries(discourseConfigRef.nodes) - .map(([type, { text, children }]): DiscourseNode => { - const suggestiveRules = getSubTree({ - tree: children, - key: "Suggestive Rules", - }); - const embeddingBlockRef = getSubTree({ - tree: suggestiveRules.children, - key: "Embedding Block Ref", - }); + const configuredNodes = ( + isNewSettingsStoreEnabled() + ? getAllDiscourseNodes() + : Object.entries(discourseConfigRef.nodes).map( + ([type, { text, children }]): DiscourseNode => { + const suggestiveRules = getSubTree({ + tree: children, + key: "Suggestive Rules", + }); + const embeddingBlockRef = getSubTree({ + tree: suggestiveRules.children, + key: "Embedding Block Ref", + }); - return { - format: getSettingValueFromTree({ tree: children, key: "format" }), - text, - shortcut: getSettingValueFromTree({ tree: children, key: "shortcut" }), - tag: getSettingValueFromTree({ tree: children, key: "tag" }), - type, - specification: getSpecification(children), - backedBy: "user", - canvasSettings: Object.fromEntries( - getSubTree({ tree: children, key: "canvas" }).children.map( - (c) => [c.text, c.children[0]?.text || ""] as const, - ), - ), - graphOverview: - children.filter((c) => c.text === "Graph Overview").length > 0, - description: getSettingValueFromTree({ - tree: children, - key: "description", - }), - template: getSubTree({ tree: children, key: "template" }).children, - embeddingRef: embeddingBlockRef?.children?.[0]?.text, - embeddingRefUid: embeddingBlockRef?.uid, - isFirstChild: getUidAndBooleanSetting({ - tree: suggestiveRules.children, - text: "First Child", - }), - }; - }) - .concat( - relations - .filter((r) => r.triples.some((t) => t.some((n) => /anchor/i.test(n)))) - .map((r) => ({ - format: "", - text: r.label, - type: r.id, - shortcut: r.label.slice(0, 1), - tag: "", - specification: r.triples.map(([source, relation, target]) => ({ - type: "clause", - source: /anchor/i.test(source) ? r.label : source, - relation, - target: - target === "source" - ? r.source - : target === "destination" - ? r.destination - : /anchor/i.test(target) - ? r.label - : target, - uid: window.roamAlphaAPI.util.generateUID(), - })), - backedBy: "relation", - canvasSettings: {}, + return { + format: getSettingValueFromTree({ + tree: children, + key: "format", + }), + text, + shortcut: getSettingValueFromTree({ + tree: children, + key: "shortcut", + }), + tag: getSettingValueFromTree({ tree: children, key: "tag" }), + type, + specification: getSpecification(children), + backedBy: "user", + canvasSettings: Object.fromEntries( + getSubTree({ tree: children, key: "canvas" }).children.map( + (c) => [c.text, c.children[0]?.text || ""] as const, + ), + ), + graphOverview: + children.filter((c) => c.text === "Graph Overview").length > 0, + description: getSettingValueFromTree({ + tree: children, + key: "description", + }), + template: getSubTree({ tree: children, key: "template" }) + .children, + embeddingRef: embeddingBlockRef?.children?.[0]?.text, + embeddingRefUid: embeddingBlockRef?.uid, + isFirstChild: getUidAndBooleanSetting({ + tree: suggestiveRules.children, + text: "First Child", + }), + }; + }, + ) + ).concat( + relations + .filter((r) => r.triples.some((t) => t.some((n) => /anchor/i.test(n)))) + .map((r) => ({ + format: "", + text: r.label, + type: r.id, + shortcut: r.label.slice(0, 1), + tag: "", + specification: r.triples.map(([source, relation, target]) => ({ + type: "clause", + source: /anchor/i.test(source) ? r.label : source, + relation, + target: + target === "source" + ? r.source + : target === "destination" + ? r.destination + : /anchor/i.test(target) + ? r.label + : target, + uid: window.roamAlphaAPI.util.generateUID(), })), - ); + backedBy: "relation", + canvasSettings: {}, + })), + ); const configuredNodeTexts = new Set(configuredNodes.map((n) => n.text)); const defaultNodes = DEFAULT_NODES.filter( (n) => !configuredNodeTexts.has(n.text), diff --git a/apps/roam/src/utils/getDiscourseRelations.ts b/apps/roam/src/utils/getDiscourseRelations.ts index b99b986bb..c9f24a911 100644 --- a/apps/roam/src/utils/getDiscourseRelations.ts +++ b/apps/roam/src/utils/getDiscourseRelations.ts @@ -6,6 +6,10 @@ import type { import getSettingValueFromTree from "roamjs-components/util/getSettingValueFromTree"; import toFlexRegex from "roamjs-components/util/toFlexRegex"; import DEFAULT_RELATION_VALUES from "~/data/defaultDiscourseRelations"; +import { + isNewSettingsStoreEnabled, + getAllRelations, +} from "~/components/settings/utils/accessors"; import discourseConfigRef from "./discourseConfigRef"; export type Triple = readonly [string, string, string]; @@ -32,6 +36,10 @@ export const getRelationsNode = (grammarNode = getGrammarNode()) => { }; const getDiscourseRelations = () => { + if (isNewSettingsStoreEnabled()) { + return getAllRelations(); + } + const grammarNode = getGrammarNode(); const relationsNode = getRelationsNode(grammarNode); const relationNodes = relationsNode?.children || DEFAULT_RELATION_VALUES; From a1ce92205332d5a94d05e064d330dc0d56d1a052 Mon Sep 17 00:00:00 2001 From: sid597 Date: Wed, 4 Mar 2026 23:46:06 +0530 Subject: [PATCH 2/3] Fix: use backedBy 'user' for initial discourse nodes in block props initSingleDiscourseNode was writing backedBy: "default" which caused nodes to be filtered out of the settings panel when the flag is ON. Also fixes up existing graphs that already stored the wrong value. --- apps/roam/src/components/settings/utils/init.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/roam/src/components/settings/utils/init.ts b/apps/roam/src/components/settings/utils/init.ts index 94c8b4180..5289fce3e 100644 --- a/apps/roam/src/components/settings/utils/init.ts +++ b/apps/roam/src/components/settings/utils/init.ts @@ -147,10 +147,12 @@ const initSingleDiscourseNode = async ( tag: node.tag || "", graphOverview: node.graphOverview ?? false, canvasSettings: node.canvasSettings || {}, - backedBy: "default", + backedBy: "user", }); setBlockProps(pageUid, nodeData, false); + } else if (existingProps && existingProps["backedBy"] === "default") { + setBlockProps(pageUid, { ...existingProps, backedBy: "user" }, false); } return { label: node.text, pageUid }; From 915c0eb6e97dae4312bcee1d03b683e8b8fee7b9 Mon Sep 17 00:00:00 2001 From: sid597 Date: Thu, 5 Mar 2026 00:30:42 +0530 Subject: [PATCH 3/3] Hardcode backedBy 'user' in toDiscourseNode to match legacy behavior --- apps/roam/src/components/settings/utils/accessors.ts | 2 +- apps/roam/src/components/settings/utils/init.ts | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/roam/src/components/settings/utils/accessors.ts b/apps/roam/src/components/settings/utils/accessors.ts index 4ef2bf77d..a6462c787 100644 --- a/apps/roam/src/components/settings/utils/accessors.ts +++ b/apps/roam/src/components/settings/utils/accessors.ts @@ -968,7 +968,7 @@ const toDiscourseNode = (settings: DiscourseNodeSettings): DiscourseNode => ({ format: settings.format, description: settings.description || undefined, graphOverview: settings.graphOverview || undefined, - backedBy: settings.backedBy, + backedBy: "user", specification: addConditionUids( settings.specification.query.conditions as SchemaCondition[], ), diff --git a/apps/roam/src/components/settings/utils/init.ts b/apps/roam/src/components/settings/utils/init.ts index 5289fce3e..90536774f 100644 --- a/apps/roam/src/components/settings/utils/init.ts +++ b/apps/roam/src/components/settings/utils/init.ts @@ -151,8 +151,6 @@ const initSingleDiscourseNode = async ( }); setBlockProps(pageUid, nodeData, false); - } else if (existingProps && existingProps["backedBy"] === "default") { - setBlockProps(pageUid, { ...existingProps, backedBy: "user" }, false); } return { label: node.text, pageUid };