diff --git a/apps/roam/src/utils/__tests__/conceptConversion.test.ts b/apps/roam/src/utils/__tests__/conceptConversion.test.ts index 88b439313..bb3a57d91 100644 --- a/apps/roam/src/utils/__tests__/conceptConversion.test.ts +++ b/apps/roam/src/utils/__tests__/conceptConversion.test.ts @@ -1,16 +1,25 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { DiscourseNode } from "~/utils/getDiscourseNodes"; - -const { mockedGetPageUidByPageTitle, mockedGetDiscourseNodes } = vi.hoisted( - () => ({ - // eslint-disable-next-line @typescript-eslint/no-unused-vars - mockedGetPageUidByPageTitle: vi.fn((_title: string) => ""), - mockedGetDiscourseNodes: vi.fn((): DiscourseNode[] => []), - }), -); +import type { ImportedSourceIdentity } from "~/utils/importedSourceIdentity"; + +const { + mockedGetPageUidByPageTitle, + mockedGetDiscourseNodes, + mockedReadImportedSourceIdentity, +} = vi.hoisted(() => ({ + // eslint-disable-next-line @typescript-eslint/no-unused-vars + mockedGetPageUidByPageTitle: vi.fn((_title: string) => ""), + mockedGetDiscourseNodes: vi.fn((): DiscourseNode[] => []), + mockedReadImportedSourceIdentity: vi.fn( + (): ImportedSourceIdentity | undefined => undefined, + ), +})); vi.mock("roamjs-components/queries/getPageUidByPageTitle", () => ({ default: mockedGetPageUidByPageTitle, })); +vi.mock("~/utils/importedSourceIdentity", () => ({ + readImportedSourceIdentity: mockedReadImportedSourceIdentity, +})); vi.mock("~/utils/getDiscourseNodes", () => ({ default: mockedGetDiscourseNodes, })); @@ -65,6 +74,7 @@ beforeEach(() => { (title: string) => PAGE_UIDS[title] ?? "", ); mockedGetDiscourseNodes.mockReturnValue([SOURCE_TYPE]); + mockedReadImportedSourceIdentity.mockReset(); }); describe("discourseNodeSchemaToLocalConcept source slot", () => { @@ -159,6 +169,20 @@ describe("discourseNodeBlockToLocalConcept source slot", () => { }); }); + it("writes the origin RID when the source page was imported from another app", () => { + mockedReadImportedSourceIdentity.mockReturnValue({ + sourceModifiedAt: "2026-06-14T15:00:00.000Z", + sourceNodeRid: "orn:obsidian.note:vault-a/node-1", + }); + const concept = convert( + "[[EVD]] - REM sleep aids recall - [[@sun2019direct]]", + ); + expect(concept.local_reference_content).toEqual({ + sourceDocument: "orn:obsidian.note:vault-a/node-1", + }); + expect(mockedReadImportedSourceIdentity).toHaveBeenCalledWith("source-1"); + }); + // Leniency on the target type: see sourceSlot.ts it("accepts a source that is a node of another type", () => { mockedGetDiscourseNodes.mockReturnValue([ diff --git a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts index 61658b584..8af509bda 100644 --- a/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts +++ b/apps/roam/src/utils/__tests__/publishNodesToGroups.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { CrossAppNode } from "@repo/database/crossAppContracts"; import type { DGSupabaseClient } from "@repo/database/lib/client"; import type { DiscourseNode } from "~/utils/getDiscourseNodes"; @@ -31,6 +31,10 @@ vi.mock("~/utils/importedSourceIdentity", () => ({ readImportedSourceIdentity: () => undefined, })); +vi.mock("roamjs-components/queries/getPageTitleByPageUid", () => ({ + default: (uid: string) => (uid === SOURCE_UID ? SOURCE_TITLE : ""), +})); + vi.mock("~/utils/roamToCrossAppConverters", () => ({ nodeUidsWithTypeToCrossApp: vi.fn(), nodeSchemaToCrossApp: (s: DiscourseNode) => ({ @@ -59,6 +63,9 @@ import { publishNodesToGroups } from "~/utils/publishNodesToGroups"; const SPACE_ID = 42; const GROUP_ID = "group-1"; const SCHEMA_UID = "schema-1"; +const SOURCE_UID = "source-1"; +const SOURCE_TITLE = "@sun2019direct"; +const SOURCE_RID = "orn:obsidian.note:vault-a/node-1"; const claimSchema: DiscourseNode = { type: SCHEMA_UID, @@ -74,10 +81,12 @@ const makeCrossAppNode = ({ uid, title, coreTitle = title, + slots, }: { uid: string; title: string; coreTitle?: string; + slots?: CrossAppNode["slots"]; }): CrossAppNode => ({ localId: uid, nodeType: SCHEMA_UID, @@ -94,6 +103,7 @@ const makeCrossAppNode = ({ scale: "document", }, }, + ...(slots ? { slots } : {}), }); type RpcArgs = { v_space_id: number; data: Record[] }; @@ -106,7 +116,7 @@ type SelectResponse = { type FakeSelectBuilder = PromiseLike & { url: { search: string }; eq: () => FakeSelectBuilder; - in: () => FakeSelectBuilder; + in: (column: string, values: string[]) => FakeSelectBuilder; order: (column: string) => FakeSelectBuilder; range: () => Promise; }; @@ -119,6 +129,7 @@ const makeFakeClient = ({ rpcResponse?: { data: number[] | null; error: { message: string } | null }; }) => { const rpcCalls: { fn: string; args: RpcArgs }[] = []; + const conceptLookups: string[][] = []; const upsertCalls: { table: string; rows: Record[]; @@ -136,7 +147,10 @@ const makeFakeClient = ({ const builder: FakeSelectBuilder = { url: { search: "" }, eq: () => builder, - in: () => builder, + in: (_column, values) => { + if (table === "my_concepts") conceptLookups.push(values); + return builder; + }, order: (column) => { builder.url.search += `&order=${column}`; return builder; @@ -165,7 +179,7 @@ const makeFakeClient = ({ ); }, } as unknown as DGSupabaseClient; - return { client, rpcCalls, upsertCalls }; + return { client, rpcCalls, conceptLookups, upsertCalls }; }; describe("publishNodesToGroups", () => { @@ -350,4 +364,141 @@ describe("publishNodesToGroups", () => { expect(result.publishedNodeSchemaUids).toEqual([]); expect(upsertCalls[0].rows).toEqual([]); }); + + describe("source slot", () => { + const evidenceNode = (sourceId: string) => + makeCrossAppNode({ + uid: "node-1", + title: `[[EVD]] - finding - [[${SOURCE_TITLE}]]`, + slots: { sourceDocument: sourceId }, + }); + const publish = ( + client: DGSupabaseClient, + nodes: CrossAppNode[] = [evidenceNode(SOURCE_UID)], + ) => + publishNodesToGroups({ + client, + spaceId: SPACE_ID, + groupIds: [GROUP_ID], + nodes, + }); + + beforeEach(() => { + vi.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("keeps the source slot when the source is already a concept in the space", async () => { + const { client, rpcCalls, conceptLookups } = makeFakeClient({ + syncedUids: [SCHEMA_UID, SOURCE_UID], + }); + + await publish(client); + + expect(conceptLookups[0]).toContain(SOURCE_UID); + expect(rpcCalls[0].args.data).toHaveLength(1); + expect(rpcCalls[0].args.data[0]).toMatchObject({ + source_local_id: "node-1", + local_reference_content: { sourceDocument: SOURCE_UID }, + }); + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("looks a source up once however many nodes reference it", async () => { + const { client, conceptLookups } = makeFakeClient({ + syncedUids: [SCHEMA_UID, SOURCE_UID], + }); + + await publish(client, [ + evidenceNode(SOURCE_UID), + makeCrossAppNode({ + uid: "node-2", + title: `[[EVD]] - another finding - [[${SOURCE_TITLE}]]`, + slots: { sourceDocument: SOURCE_UID }, + }), + ]); + + expect(conceptLookups[0].filter((id) => id === SOURCE_UID)).toHaveLength( + 1, + ); + }); + + it("omits the source slot and warns when the source is not a concept in the space", async () => { + const { client, rpcCalls, upsertCalls } = makeFakeClient({ + syncedUids: [SCHEMA_UID], + }); + + const result = await publish(client); + + expect(rpcCalls[0].args.data).toHaveLength(1); + expect(rpcCalls[0].args.data[0]).toMatchObject({ + source_local_id: "node-1", + }); + expect(rpcCalls[0].args.data[0].local_reference_content).toBeUndefined(); + expect(console.warn).toHaveBeenCalledTimes(1); + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining(`"${SOURCE_TITLE}" (${SOURCE_UID})`), + ); + expect(result.publishedNodeUids).toEqual(["node-1"]); + expect(result.failedUpsertUids).toEqual([]); + expect(upsertCalls[0].rows.map((r) => r.source_local_id)).toContain( + "node-1", + ); + }); + + it("upserts a source published in the same batch before the node referencing it", async () => { + const { client, rpcCalls } = makeFakeClient({ + syncedUids: [SCHEMA_UID], + }); + + await publish(client, [ + evidenceNode(SOURCE_UID), + makeCrossAppNode({ uid: SOURCE_UID, title: SOURCE_TITLE }), + ]); + + const { data } = rpcCalls[0].args; + expect(data.map((row) => row.source_local_id)).toEqual([ + SOURCE_UID, + "node-1", + ]); + expect(data[1].local_reference_content).toEqual({ + sourceDocument: SOURCE_UID, + }); + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("passes an imported source's RID through without looking it up in the space", async () => { + const { client, rpcCalls, conceptLookups } = makeFakeClient({ + syncedUids: [SCHEMA_UID], + }); + + await publish(client, [evidenceNode(SOURCE_RID)]); + + expect(conceptLookups[0]).not.toContain(SOURCE_RID); + expect(rpcCalls[0].args.data[0].local_reference_content).toEqual({ + sourceDocument: SOURCE_RID, + }); + expect(console.warn).not.toHaveBeenCalled(); + }); + + it("writes the same sourceDocument value on repeated publishes", async () => { + const { client, rpcCalls } = makeFakeClient({ + syncedUids: [SCHEMA_UID, SOURCE_UID], + }); + + await publish(client); + await publish(client); + + expect(rpcCalls).toHaveLength(2); + expect(rpcCalls[1].args.data[0].local_reference_content).toEqual({ + sourceDocument: SOURCE_UID, + }); + expect(rpcCalls[1].args.data[0].local_reference_content).toEqual( + rpcCalls[0].args.data[0].local_reference_content, + ); + }); + }); }); diff --git a/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts index c44a55a04..a050e04cf 100644 --- a/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts +++ b/apps/roam/src/utils/__tests__/roamToCrossAppConverters.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Json } from "@repo/database/dbTypes"; import defaultDiscourseNodes from "~/data/defaultDiscourseNodes"; +import type { ImportedSourceIdentity } from "~/utils/importedSourceIdentity"; vi.mock("roamjs-components/queries/getFullTreeByParentUid", () => ({ default: () => ({ children: [] }), @@ -13,13 +14,20 @@ vi.mock("~/utils/getDiscourseNodes", () => ({ default: vi.fn(() => defaultDiscourseNodes), })); -const { mockedGetPageUidByPageTitle } = vi.hoisted(() => ({ - // eslint-disable-next-line @typescript-eslint/no-unused-vars - mockedGetPageUidByPageTitle: vi.fn((_title: string) => ""), -})); +const { mockedGetPageUidByPageTitle, mockedReadImportedSourceIdentity } = + vi.hoisted(() => ({ + // eslint-disable-next-line @typescript-eslint/no-unused-vars + mockedGetPageUidByPageTitle: vi.fn((_title: string) => ""), + mockedReadImportedSourceIdentity: vi.fn( + (): ImportedSourceIdentity | undefined => undefined, + ), + })); vi.mock("roamjs-components/queries/getPageUidByPageTitle", () => ({ default: mockedGetPageUidByPageTitle, })); +vi.mock("~/utils/importedSourceIdentity", () => ({ + readImportedSourceIdentity: mockedReadImportedSourceIdentity, +})); // Runs before the imports below: getDiscourseNodes calls generateUID at module load. vi.hoisted(() => { @@ -251,6 +259,7 @@ describe("nodeUidsWithTypeToCrossApp source slot", () => { mockedGetPageUidByPageTitle.mockImplementation( (title: string) => PAGE_UIDS[title] ?? "", ); + mockedReadImportedSourceIdentity.mockReset(); }); it("resolves the source page from the title into a sourceDocument slot", async () => { @@ -262,6 +271,49 @@ describe("nodeUidsWithTypeToCrossApp source slot", () => { expect(node.slots).toEqual({ sourceDocument: "source-1" }); }); + it.each([ + "orn:obsidian.note:vault-a/node-1", + "orn:obsidian:vault-a/node-1", + "https://roamresearch.com/#/app/graph-b/node-1", + ])( + "writes the origin RID %j when the source page was imported from another app", + async (sourceNodeRid) => { + mockedGetDiscourseNodes.mockReturnValue([EVIDENCE_SCHEMA, SOURCE_SCHEMA]); + mockedReadImportedSourceIdentity.mockReturnValue({ + sourceModifiedAt: "2026-06-14T15:00:00.000Z", + sourceNodeRid, + }); + const node = await convertRow({ + ...baseRow, + ":node/title": "[[EVD]] - REM sleep aids recall - [[@sun2019direct]]", + }); + expect(node.slots).toEqual({ sourceDocument: sourceNodeRid }); + expect(mockedReadImportedSourceIdentity).toHaveBeenCalledWith("source-1"); + }, + ); + + it.each([ + "not a rid", + "orn:bad", + "orn:obsidian.note:vault-a/", + "orn:broken/node-1", + "https:///node-1", + ])( + "keeps the page uid when the imported identity %j is not a well-formed RID", + async (sourceNodeRid) => { + mockedGetDiscourseNodes.mockReturnValue([EVIDENCE_SCHEMA, SOURCE_SCHEMA]); + mockedReadImportedSourceIdentity.mockReturnValue({ + sourceModifiedAt: "2026-06-14T15:00:00.000Z", + sourceNodeRid, + }); + const node = await convertRow({ + ...baseRow, + ":node/title": "[[EVD]] - REM sleep aids recall - [[@sun2019direct]]", + }); + expect(node.slots).toEqual({ sourceDocument: "source-1" }); + }, + ); + // Leniency on the target type: see sourceSlot.ts it("accepts a source that is a node of another type", async () => { mockedGetDiscourseNodes.mockReturnValue([ diff --git a/apps/roam/src/utils/conceptConversion.ts b/apps/roam/src/utils/conceptConversion.ts index daabf6db7..3792ce0d7 100644 --- a/apps/roam/src/utils/conceptConversion.ts +++ b/apps/roam/src/utils/conceptConversion.ts @@ -5,7 +5,7 @@ import { SOURCE_SLOT, schemaHasSourceSlot, sourceSlotSchemaId, - sourceUidOfNode, + sourceIdOfNode, } from "./sourceSlot"; import extractContentFromTitle from "./extractContentFromTitle"; import getDiscourseRelations from "./getDiscourseRelations"; @@ -122,7 +122,7 @@ export const discourseNodeBlockToLocalConcept = ( schema?: DiscourseNode; }, ): LocalConceptDataInput => { - const sourceUid = title ? sourceUidOfNode(title, schema) : undefined; + const sourceId = title ? sourceIdOfNode(title, schema) : undefined; return { space_id: context.spaceId, name: title, @@ -134,8 +134,8 @@ export const discourseNodeBlockToLocalConcept = ( format: schema?.format ?? "", }), }, - ...(sourceUid - ? { local_reference_content: { [SOURCE_SLOT]: sourceUid } } + ...(sourceId + ? { local_reference_content: { [SOURCE_SLOT]: sourceId } } : {}), /* eslint-enable @typescript-eslint/naming-convention */ ...getNodeExtraData(nodeUid), diff --git a/apps/roam/src/utils/publishNodesToGroups.ts b/apps/roam/src/utils/publishNodesToGroups.ts index e357cd777..097c8ba65 100644 --- a/apps/roam/src/utils/publishNodesToGroups.ts +++ b/apps/roam/src/utils/publishNodesToGroups.ts @@ -22,11 +22,14 @@ import { import { ensurePartialSpaceAccess } from "@repo/database/lib/groups"; import { isIgnorableUpsertError } from "@repo/database/lib/contextFunctions"; import { getAllPages } from "@repo/database/lib/pagination"; -import { ridToSpaceUriAndLocalId } from "@repo/database/lib/rid"; +import { isRid, ridToSpaceUriAndLocalId } from "@repo/database/lib/rid"; import getDiscourseNodes from "./getDiscourseNodes"; import { difference, intersection } from "@repo/utils/setOperations"; import internalError from "./internalError"; import { readImportedSourceIdentity } from "./importedSourceIdentity"; +import { orderConceptsByDependency } from "./conceptConversion"; +import { SOURCE_SLOT } from "./sourceSlot"; +import getPageTitleByPageUid from "roamjs-components/queries/getPageTitleByPageUid"; export type NodeUidWithType = { uid: string; @@ -297,10 +300,17 @@ export const publishNodesToGroups = async ({ const relationUids = relations.map((r) => r.localId); const relationTripleSchemaUids = relationTripleSchemas.map((r) => r.localId); + const localSourceUids = new Set( + nodes + .map((node) => node.slots?.[SOURCE_SLOT]) + .filter((id): id is string => id !== undefined && !isRid(id)), + ); + const neededUids = [ ...nodeSchemaUids, ...relationTripleSchemaUids, ...relationUids, + ...localSourceUids, ]; const syncedRes = await client @@ -323,14 +333,33 @@ export const publishNodesToGroups = async ({ ); const missingRelations = relations.filter((r) => !syncedUids.has(r.localId)); - const upsertConcepts = [ - ...missingNodeSchemas.map((s) => crossAppNodeSchemaToDbConcept(s)), - ...[...nodesByUid.values()].map((node) => crossAppNodeToDbConcept(node)), - ...missingRelationTripleSchemas.map((rs3) => - crossAppRelationTripleSchemaToDbConcept(rs3), - ), - ...missingRelations.map((r) => crossAppRelationToDbConcept(r)), - ].filter((r) => r !== undefined); + const omitMissingSource = (node: CrossAppNode): CrossAppNode => { + const sourceId = node.slots?.[SOURCE_SLOT]; + if ( + sourceId === undefined || + isRid(sourceId) || + nodesByUid.has(sourceId) || + syncedUids.has(sourceId) + ) + return node; + console.warn( + `Source "${getPageTitleByPageUid(sourceId)}" (${sourceId}) is not in this space yet; publishing "${node.content.direct.value}" without it.`, + ); + return { ...node, slots: undefined }; + }; + + const { ordered: upsertConcepts } = orderConceptsByDependency( + [ + ...missingNodeSchemas.map((s) => crossAppNodeSchemaToDbConcept(s)), + ...[...nodesByUid.values()].map((node) => + crossAppNodeToDbConcept(omitMissingSource(node)), + ), + ...missingRelationTripleSchemas.map((rs3) => + crossAppRelationTripleSchemaToDbConcept(rs3), + ), + ...missingRelations.map((r) => crossAppRelationToDbConcept(r)), + ].filter((r) => r !== undefined), + ); const upsertedNodeUids = new Set(nodeUids); const syncedRelationUids = new Set(missingRelations.map((s) => s.localId)); diff --git a/apps/roam/src/utils/roamToCrossAppConverters.ts b/apps/roam/src/utils/roamToCrossAppConverters.ts index bc4b17549..bbaeb61b8 100644 --- a/apps/roam/src/utils/roamToCrossAppConverters.ts +++ b/apps/roam/src/utils/roamToCrossAppConverters.ts @@ -21,7 +21,7 @@ import { SOURCE_SLOT, schemaHasSourceSlot, sourceSlotSchemaId, - sourceUidOfNode, + sourceIdOfNode, } from "./sourceSlot"; const FULL_MARKDOWN_OPTS = { @@ -128,7 +128,7 @@ export const nodeUidsWithTypeToCrossApp = async ( const pageEditTime = (row[":page/edit-time"] as number | undefined) ?? editTime; const nodeType = typesByUid[uid]; - const sourceUid = sourceUidOfNode(title, schemasById[nodeType]); + const sourceId = sourceIdOfNode(title, schemasById[nodeType]); return { localId: uid, @@ -146,7 +146,7 @@ export const nodeUidsWithTypeToCrossApp = async ( }, full: buildFullInlineContent({ uid, title }), }, - ...(sourceUid ? { slots: { [SOURCE_SLOT]: sourceUid } } : {}), + ...(sourceId ? { slots: { [SOURCE_SLOT]: sourceId } } : {}), }; }); return results; diff --git a/apps/roam/src/utils/sourceSlot.ts b/apps/roam/src/utils/sourceSlot.ts index 9a6c88561..da5a4a122 100644 --- a/apps/roam/src/utils/sourceSlot.ts +++ b/apps/roam/src/utils/sourceSlot.ts @@ -2,6 +2,7 @@ import getDiscourseNodes, { type DiscourseNode } from "./getDiscourseNodes"; import getPageUidByPageTitle from "roamjs-components/queries/getPageUidByPageTitle"; import getDiscourseNodeFormatExpression from "./getDiscourseNodeFormatExpression"; import { extractFieldFromTitle } from "./extractContentFromTitle"; +import { readImportedSourceIdentity } from "./importedSourceIdentity"; // Temporary hack, until slots are a first-class node type setting: a node type whose // format has a {source} placeholder (Evidence, among the default node types) is taken @@ -51,10 +52,34 @@ const isDiscourseNodeTitle = ( .filter((n) => n.format !== "{content}") // exclude page and block .some((node) => matcherFor(node.format).test(title)); -// The page a node's {source} placeholder resolves to, when there is one. The +// The two RID shapes the database resolves (see rid_to_space_id_and_local_id). The +// shared parser is not used here: its fallback splits any string at its last slash, so +// it accepts values the database will fail to resolve. +const ORN_RID = /^orn:\w+(\.\w+)?:.+\/[^/]+$/; + +const isHttpsRid = (value: string): boolean => { + try { + const { protocol, host } = new URL(value); + const lastSlash = value.lastIndexOf("/"); + return ( + protocol === "https:" && + host !== "" && + lastSlash > "https://".length && + lastSlash < value.length - 1 + ); + } catch { + return false; + } +}; + +const isWellFormedRid = (value: string): boolean => + ORN_RID.test(value) || isHttpsRid(value); + +// The page a node's {source} placeholder resolves to, when there is one: its uid, or +// the RID it is known by elsewhere when it was imported from another app. The // placeholder is usually filled with a page reference, and a title holding a slash is // a namespaced page rather than a source, so it is left alone. -export const sourceUidOfNode = ( +export const sourceIdOfNode = ( title: string, schema: NodeFormat | undefined, allNodes?: DiscourseNode[], @@ -67,5 +92,10 @@ export const sourceUidOfNode = ( if (!sourceTitle || sourceTitle.includes("/")) return undefined; if (!isDiscourseNodeTitle(sourceTitle, allNodes ?? getDiscourseNodes())) return undefined; - return getPageUidByPageTitle(sourceTitle) || undefined; + const sourceUid = getPageUidByPageTitle(sourceTitle); + if (!sourceUid) return undefined; + const sourceRid = readImportedSourceIdentity(sourceUid)?.sourceNodeRid; + return sourceRid !== undefined && isWellFormedRid(sourceRid) + ? sourceRid + : sourceUid; };