diff --git a/apps/server/src/core/page/services/page.service.ts b/apps/server/src/core/page/services/page.service.ts index 8e5c28b6..dc17e188 100644 --- a/apps/server/src/core/page/services/page.service.ts +++ b/apps/server/src/core/page/services/page.service.ts @@ -62,6 +62,7 @@ import { markdownToHtml } from '@docmost/editor-ext'; import { WatcherService } from '../../watcher/watcher.service'; import { sql } from 'kysely'; import { TransclusionService } from '../transclusion/transclusion.service'; +import { remapPageEmbedSourceId } from '../transclusion/utils/transclusion-prosemirror.util'; import { AuthProvenanceData } from '../../../common/decorators/auth-provenance.decorator'; @Injectable() @@ -713,12 +714,11 @@ export class PageService { // source page is also part of the copied set, point at its new copy; // otherwise leave it pointing at the original (live embed of original). if (node.type.name === 'pageEmbed') { - const sourcePageId = node.attrs.sourcePageId; - if (sourcePageId && pageMap.has(sourcePageId)) { - const mappedPage = pageMap.get(sourcePageId); - // @ts-expect-error ProseMirror Attrs is read-only typed; reassigning sourcePageId to the duplicated page copy is intentional here - node.attrs.sourcePageId = mappedPage.newPageId; - } + // @ts-expect-error ProseMirror Attrs is read-only typed; intentional remap to the duplicated copy + node.attrs.sourcePageId = remapPageEmbedSourceId( + node.attrs.sourcePageId, + (id) => pageMap.get(id)?.newPageId, + ); } // Update internal page links in link marks diff --git a/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts b/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts new file mode 100644 index 00000000..47fa46c4 --- /dev/null +++ b/apps/server/src/core/page/transclusion/spec/page-embed-remap.util.spec.ts @@ -0,0 +1,200 @@ +import { + remapPageEmbedSourceId, + remapPageEmbedSourceIds, +} from '../utils/transclusion-prosemirror.util'; + +/** + * Unit tests for the `pageEmbed` remap used by `PageService.duplicatePage`: + * + * - source page within the copied set -> rewrite to the COPY's new id + * - source page NOT in the copied set -> keep the ORIGINAL id (live embed) + * + * `remapPageEmbedSourceId` is the per-node decision the production + * `duplicatePage` callback now calls directly, so these tests guard the real + * path rather than a parallel copy. `remapPageEmbedSourceIds` is the JSON + * walker that delegates to the same helper; its tests exercise the shared + * decision transitively across nested ProseMirror containers. + */ +describe('remapPageEmbedSourceId (shared per-node decision used by duplicatePage)', () => { + it('returns the new copy id when the source IS in the copied set', () => { + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceId('old-src', (id) => idMap.get(id)); + + expect(out).toBe('new-copy'); + }); + + it('returns the original id when the source is NOT in the copied set', () => { + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceId('external', (id) => idMap.get(id)); + + expect(out).toBe('external'); + }); + + it('returns the original id when resolveNewId yields undefined', () => { + const out = remapPageEmbedSourceId('some-id', () => undefined); + + expect(out).toBe('some-id'); + }); + + it('leaves a null source unchanged without consulting the resolver', () => { + const resolve = jest.fn(() => 'should-not-be-used'); + + const out = remapPageEmbedSourceId(null, resolve); + + expect(out).toBeNull(); + expect(resolve).not.toHaveBeenCalled(); + }); + + it('leaves an undefined source unchanged without consulting the resolver', () => { + const resolve = jest.fn(() => 'should-not-be-used'); + + const out = remapPageEmbedSourceId(undefined, resolve); + + expect(out).toBeUndefined(); + expect(resolve).not.toHaveBeenCalled(); + }); +}); + +describe('remapPageEmbedSourceIds (duplicatePage pageEmbed remap)', () => { + const docWithEmbeds = (ids: string[]) => ({ + type: 'doc', + content: ids.map((id) => ({ + type: 'pageEmbed', + attrs: { sourcePageId: id }, + })), + }); + + it('remaps a source that IS within the copied set to its new copy id', () => { + const doc = docWithEmbeds(['old-src']); + const idMap = new Map([['old-src', 'new-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content[0].attrs.sourcePageId).toBe('new-copy'); + }); + + it('keeps the original id for a source NOT in the copied set', () => { + const doc = docWithEmbeds(['external']); + const idMap = new Map([['old-src', 'new-copy']]); // does not contain "external" + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content[0].attrs.sourcePageId).toBe('external'); + }); + + it('handles a mixed doc: in-set remapped, out-of-set preserved', () => { + const doc = docWithEmbeds(['in-set', 'external']); + const idMap = new Map([['in-set', 'in-set-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap); + + expect(out.content.map((n: any) => n.attrs.sourcePageId)).toEqual([ + 'in-set-copy', + 'external', + ]); + }); + + it('remaps pageEmbeds nested inside columns', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'columnList', + content: [ + { + type: 'column', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'nested-in' } }, + ], + }, + { + type: 'column', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'nested-out' } }, + ], + }, + ], + }, + ], + }; + const idMap = new Map([['nested-in', 'nested-in-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + const col0 = out.content[0].content[0].content[0]; + const col1 = out.content[0].content[1].content[0]; + expect(col0.attrs.sourcePageId).toBe('nested-in-copy'); + expect(col1.attrs.sourcePageId).toBe('nested-out'); + }); + + it('remaps pageEmbeds nested inside a callout', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'callout', + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'in-callout' } }, + ], + }, + ], + }; + const idMap = new Map([['in-callout', 'in-callout-copy']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + expect(out.content[0].content[0].attrs.sourcePageId).toBe( + 'in-callout-copy', + ); + }); + + it('does not descend into a transclusionSource (schema-isolated)', () => { + const doc = { + type: 'doc', + content: [ + { + type: 'transclusionSource', + attrs: { id: 'src' }, + content: [ + { type: 'pageEmbed', attrs: { sourcePageId: 'hidden' } }, + ], + }, + ], + }; + const idMap = new Map([['hidden', 'should-not-apply']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + // The embed inside a source must be left untouched. + expect(out.content[0].content[0].attrs.sourcePageId).toBe('hidden'); + }); + + it('leaves embeds missing a sourcePageId untouched', () => { + const doc = { + type: 'doc', + content: [ + { type: 'pageEmbed', attrs: {} }, + { type: 'pageEmbed', attrs: { sourcePageId: '' } }, + ], + }; + const idMap = new Map([['', 'x']]); + + const out = remapPageEmbedSourceIds(doc, idMap) as any; + + expect(out.content[0].attrs.sourcePageId).toBeUndefined(); + expect(out.content[1].attrs.sourcePageId).toBe(''); + }); + + it('returns the doc unchanged when idMap is empty', () => { + const doc = docWithEmbeds(['a', 'b']); + + const out = remapPageEmbedSourceIds(doc, new Map()); + + expect(out.content.map((n: any) => n.attrs.sourcePageId)).toEqual([ + 'a', + 'b', + ]); + }); +}); diff --git a/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts b/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts new file mode 100644 index 00000000..4afad554 --- /dev/null +++ b/apps/server/src/core/page/transclusion/spec/page-template-references-sync.spec.ts @@ -0,0 +1,310 @@ +import { TransclusionService } from '../transclusion.service'; + +/** + * Covers two untested, high-risk write paths around `page_template_references`: + * + * 1. `syncPageTemplateReferences` — the `toDelete` branch: stale references are + * removed when the host page no longer embeds a source, while genuinely new + * embeds are inserted. We assert `deleteByReferenceAndSources` / `insertMany` + * receive the correct rows and the returned `{ inserted, deleted }` counts. + * + * 2. `insertTemplateReferencesForPages` — the multi-workspace grouping/filtering + * branch: candidate source ids are grouped per workspace, each workspace is + * validated independently, and cross-workspace sources are dropped. + * + * Setup/mocking mirrors the existing transclusion specs (page-template-access / + * page-template-lookup): `new TransclusionService(...)` is built with the same + * 11 positional mock args; only the deps each test touches are real stubs. + */ + +/** + * Chainable kysely `db` stub used by `filterInWorkspaceSourceIds`. Every + * `selectFrom(...).select(...).where(...)` returns the same builder; `.execute()` + * resolves whatever rows the per-call resolver returns. The resolver receives + * the captured `where('id','in', )` and `where('workspaceId','=', ws)` + * arguments so a test can decide, per workspace, which ids "exist". + */ +function makeWorkspaceScopedDb( + resolve: (ids: string[], workspaceId: string) => string[], +) { + const state = { ids: [] as string[], workspaceId: '' }; + const builder: any = {}; + builder.selectFrom = jest.fn(() => builder); + builder.select = jest.fn(() => builder); + builder.where = jest.fn((col: string, _op: string, val: any) => { + if (col === 'id') state.ids = val as string[]; + if (col === 'workspaceId') state.workspaceId = val as string; + return builder; + }); + builder.execute = jest.fn(async () => + resolve(state.ids, state.workspaceId).map((id) => ({ id })), + ); + return builder; +} + +function buildService(opts: { + db: any; + pageTemplateReferencesRepo: any; +}) { + return new TransclusionService( + opts.db, + {} as any, // pageTransclusionsRepo + {} as any, // pageTransclusionReferencesRepo + opts.pageTemplateReferencesRepo, + {} as any, // pageRepo + {} as any, // pagePermissionRepo + {} as any, // spaceMemberRepo + {} as any, // attachmentRepo + {} as any, // storageService + {} as any, // pageAccessService + {} as any, // workspaceRepo + ); +} + +const pageEmbedDoc = (sourceIds: string[]) => ({ + type: 'doc', + content: sourceIds.map((id) => ({ + type: 'pageEmbed', + attrs: { sourcePageId: id }, + })), +}); + +describe('TransclusionService.syncPageTemplateReferences — toDelete branch', () => { + it('deletes stale references and inserts new ones with correct args/counts', async () => { + // Every candidate id is treated as in-workspace by the existence query. + const db = makeWorkspaceScopedDb((ids) => ids); + + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + // existing refs: "keep" stays embedded, "stale-a"/"stale-b" no longer are + findByReferencePageId: jest.fn().mockResolvedValue([ + { sourcePageId: 'keep' }, + { sourcePageId: 'stale-a' }, + { sourcePageId: 'stale-b' }, + ]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // host now embeds: keep (unchanged) + fresh (new). stale-a/stale-b gone. + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['keep', 'fresh']), + ); + + expect(result).toEqual({ inserted: 1, deleted: 2 }); + + // only the genuinely new embed is inserted (keep already existed) + expect(insertMany).toHaveBeenCalledTimes(1); + expect(insertMany.mock.calls[0][0]).toEqual([ + { workspaceId: 'w1', referencePageId: 'host', sourcePageId: 'fresh' }, + ]); + + // stale references removed, scoped to host + workspace + expect(deleteByReferenceAndSources).toHaveBeenCalledTimes(1); + const [refPageId, workspaceId, staleSources] = + deleteByReferenceAndSources.mock.calls[0]; + expect(refPageId).toBe('host'); + expect(workspaceId).toBe('w1'); + expect([...staleSources].sort()).toEqual(['stale-a', 'stale-b']); + }); + + it('deletes ALL existing references when the host embeds nothing anymore', async () => { + const db = makeWorkspaceScopedDb((ids) => ids); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'a' }, { sourcePageId: 'b' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc([]), // no embeds left + ); + + expect(result).toEqual({ inserted: 0, deleted: 2 }); + expect(insertMany).not.toHaveBeenCalled(); + const [, , staleSources] = deleteByReferenceAndSources.mock.calls[0]; + expect([...staleSources].sort()).toEqual(['a', 'b']); + }); + + it('treats a cross-workspace embed as stale: it never survives to be kept', async () => { + // existence query drops "cross-ws"; so an existing ref to it must be deleted + const db = makeWorkspaceScopedDb((ids) => ids.filter((id) => id !== 'cross-ws')); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'cross-ws' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // host still "embeds" cross-ws in its doc, but it is not in-workspace + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['cross-ws']), + ); + + expect(result).toEqual({ inserted: 0, deleted: 1 }); + expect(insertMany).not.toHaveBeenCalled(); + const [, , staleSources] = deleteByReferenceAndSources.mock.calls[0]; + expect([...staleSources]).toEqual(['cross-ws']); + }); + + it('no-ops both repos when desired and existing already match', async () => { + const db = makeWorkspaceScopedDb((ids) => ids); + const insertMany = jest.fn().mockResolvedValue(undefined); + const deleteByReferenceAndSources = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { + findByReferencePageId: jest + .fn() + .mockResolvedValue([{ sourcePageId: 'same' }]), + insertMany, + deleteByReferenceAndSources, + }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + const result = await service.syncPageTemplateReferences( + 'host', + 'w1', + pageEmbedDoc(['same']), + ); + + expect(result).toEqual({ inserted: 0, deleted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + expect(deleteByReferenceAndSources).not.toHaveBeenCalled(); + }); +}); + +describe('TransclusionService.insertTemplateReferencesForPages — multi-workspace grouping', () => { + it('groups candidates per workspace and validates each workspace independently', async () => { + // Each workspace "owns" only its own source ids. The existence query is + // workspace-scoped, so an id from another workspace is dropped. + const owned: Record = { + w1: ['s1'], + w2: ['s2'], + }; + const executeArgs: Array<{ ids: string[]; workspaceId: string }> = []; + const db = makeWorkspaceScopedDb((ids, workspaceId) => { + executeArgs.push({ ids: [...ids], workspaceId }); + const ownedSet = new Set(owned[workspaceId] ?? []); + return ids.filter((id) => ownedSet.has(id)); + }); + + const insertMany = jest.fn().mockResolvedValue(undefined); + const pageTemplateReferencesRepo = { insertMany }; + + const service = buildService({ db, pageTemplateReferencesRepo }); + + // page-a in w1 embeds s1 (valid) + s2 (belongs to w2 -> dropped) + // page-b in w2 embeds s2 (valid) + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['s1', 's2']) }, + { id: 'page-b', workspaceId: 'w2', content: pageEmbedDoc(['s2']) }, + ]); + + expect(result).toEqual({ inserted: 2 }); + + expect(insertMany).toHaveBeenCalledTimes(1); + const rows = insertMany.mock.calls[0][0]; + expect(rows).toEqual([ + { workspaceId: 'w1', referencePageId: 'page-a', sourcePageId: 's1' }, + { workspaceId: 'w2', referencePageId: 'page-b', sourcePageId: 's2' }, + ]); + + // one existence query per workspace, each scoped to that workspace's candidates + expect(executeArgs).toHaveLength(2); + const w1Call = executeArgs.find((c) => c.workspaceId === 'w1'); + const w2Call = executeArgs.find((c) => c.workspaceId === 'w2'); + expect(w1Call?.ids.sort()).toEqual(['s1', 's2']); + expect(w2Call?.ids).toEqual(['s2']); + }); + + it('drops every cross-workspace source and inserts nothing when none are in-workspace', async () => { + // No id is owned by its page's workspace -> all filtered out. + const db = makeWorkspaceScopedDb(() => []); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['x']) }, + { id: 'page-b', workspaceId: 'w2', content: pageEmbedDoc(['y']) }, + ]); + + expect(result).toEqual({ inserted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + }); + + it('dedupes a sourceId shared by two pages in the same workspace into one validation', async () => { + const executeArgs: Array<{ ids: string[]; workspaceId: string }> = []; + const db = makeWorkspaceScopedDb((ids, workspaceId) => { + executeArgs.push({ ids: [...ids], workspaceId }); + return ids; // all in-workspace + }); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + // both pages embed the same source "shared" in w1 + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc(['shared']) }, + { id: 'page-b', workspaceId: 'w1', content: pageEmbedDoc(['shared']) }, + ]); + + // a row per (page, source) pair, but only one existence query for w1 + expect(result).toEqual({ inserted: 2 }); + expect(executeArgs).toHaveLength(1); + expect(executeArgs[0]).toEqual({ ids: ['shared'], workspaceId: 'w1' }); + + const rows = insertMany.mock.calls[0][0]; + expect(rows).toEqual([ + { workspaceId: 'w1', referencePageId: 'page-a', sourcePageId: 'shared' }, + { workspaceId: 'w1', referencePageId: 'page-b', sourcePageId: 'shared' }, + ]); + }); + + it('returns inserted:0 without querying when no page has embeds', async () => { + const execute = jest.fn(); + const db = makeWorkspaceScopedDb(() => { + execute(); + return []; + }); + const insertMany = jest.fn().mockResolvedValue(undefined); + const service = buildService({ + db, + pageTemplateReferencesRepo: { insertMany }, + }); + + const result = await service.insertTemplateReferencesForPages([ + { id: 'page-a', workspaceId: 'w1', content: pageEmbedDoc([]) }, + ]); + + expect(result).toEqual({ inserted: 0 }); + expect(insertMany).not.toHaveBeenCalled(); + // filterInWorkspaceSourceIds short-circuits on empty candidates, so the + // existence query never runs. + expect(execute).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts b/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts index eeeea0e2..d8cdf3bf 100644 --- a/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts +++ b/apps/server/src/core/page/transclusion/utils/transclusion-prosemirror.util.ts @@ -99,6 +99,64 @@ export function collectReferencesFromPmJson( return out; } +/** + * Decide the sourcePageId a duplicated pageEmbed should point to: the copy's new + * id when the embedded source is part of the copied set, otherwise the original + * (a live embed of the original page). Pure — shared by PageService.duplicatePage + * (the real path) and the JSON walker below, so both stay in lockstep. + */ +export function remapPageEmbedSourceId( + sourcePageId: string | null | undefined, + resolveNewId: (id: string) => string | undefined, +): string | null | undefined { + if (sourcePageId) { + const mapped = resolveNewId(sourcePageId); + if (mapped) return mapped; + } + return sourcePageId; +} + +/** + * Remap the `sourcePageId` of every `pageEmbed` node in a ProseMirror JSON doc + * according to `idMap` (old page id -> new page id). Delegates the per-node + * decision to the shared `remapPageEmbedSourceId` helper that + * `PageService.duplicatePage` also uses, so the production path and this walker + * stay in lockstep: when the embedded source page is part of the copied set + * (present in `idMap`) the embed is pointed at its new copy; otherwise the + * original `sourcePageId` is preserved so it stays a live embed of the original + * page. Mutates `doc` in place (and returns it) to match the service's in-place + * ProseMirror mutation. Recurses through arbitrary block containers (columns, + * callouts, etc.) the same way the collectors do, but does NOT descend into a + * `transclusionSource` (schema-isolated). + */ +export function remapPageEmbedSourceIds( + doc: T, + idMap: Map, +): T { + const visit = (node: any): void => { + if (!node || typeof node !== 'object') return; + + if (node.type === PAGE_EMBED_TYPE) { + if (node.attrs) { + node.attrs.sourcePageId = remapPageEmbedSourceId( + node.attrs.sourcePageId, + (id) => idMap.get(id), + ); + } + return; // atom node - no children + } + + if (node.type === TRANSCLUSION_TYPE) return; + + if (Array.isArray(node.content)) { + for (const child of node.content) visit(child); + } + }; + + visit(doc); + return doc; +} + /** * Walks a ProseMirror JSON document and returns one snapshot per unique * `sourcePageId` found on `pageEmbed` nodes (whole-page live embeds). Order