diff --git a/apps/server/src/core/share/share-alias.service.spec.ts b/apps/server/src/core/share/share-alias.service.spec.ts index 349142e4..d2c5a269 100644 --- a/apps/server/src/core/share/share-alias.service.spec.ts +++ b/apps/server/src/core/share/share-alias.service.spec.ts @@ -7,13 +7,18 @@ import { ShareAliasService } from './share-alias.service'; * request-time readable-target resolution (which re-runs the share boundary). */ describe('ShareAliasService', () => { + // Sentinel handed to repo calls so tests can assert they ran inside the tx. + const trx = { __trx: true }; + function makeService() { const shareAliasRepo = { findByAliasAndWorkspace: jest.fn(), findByPageId: jest.fn(), findById: jest.fn(), insert: jest.fn(), + updateAlias: jest.fn(), updatePageId: jest.fn(), + deleteOthersForPage: jest.fn(), delete: jest.fn(), }; const pageRepo = { findById: jest.fn() }; @@ -21,12 +26,19 @@ describe('ShareAliasService', () => { resolveReadableSharePage: jest.fn(), isSharingAllowed: jest.fn(), }; + // Fake kysely db: only .transaction().execute(cb) is used by setAlias. + const db = { + transaction: jest.fn(() => ({ + execute: jest.fn(async (cb: any) => cb(trx)), + })), + }; const service = new ShareAliasService( shareAliasRepo as any, pageRepo as any, shareService as any, + db as any, ); - return { service, shareAliasRepo, pageRepo, shareService }; + return { service, shareAliasRepo, pageRepo, shareService, db }; } describe('setAlias', () => { @@ -43,9 +55,10 @@ describe('ShareAliasService', () => { expect(shareAliasRepo.findByAliasAndWorkspace).not.toHaveBeenCalled(); }); - it('normalizes then inserts a brand-new alias', async () => { + it('normalizes then inserts a brand-new alias (page has none yet)', async () => { const { service, shareAliasRepo } = makeService(); shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined); + shareAliasRepo.findByPageId.mockResolvedValue(undefined); shareAliasRepo.insert.mockResolvedValue({ id: 'a-1', alias: 'my-page' }); const res = await service.setAlias({ @@ -58,17 +71,70 @@ describe('ShareAliasService', () => { expect(shareAliasRepo.findByAliasAndWorkspace).toHaveBeenCalledWith( 'my-page', 'ws-1', + trx, + ); + expect(shareAliasRepo.insert).toHaveBeenCalledWith( + { + workspaceId: 'ws-1', + alias: 'my-page', + pageId: 'p-1', + creatorId: 'u-1', + }, + trx, + ); + expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled(); + // self-heal still runs, keeping just the inserted row + expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith( + 'p-1', + 'a-1', + 'ws-1', + trx, ); - expect(shareAliasRepo.insert).toHaveBeenCalledWith({ - workspaceId: 'ws-1', - alias: 'my-page', - pageId: 'p-1', - creatorId: 'u-1', - }); expect(res).toMatchObject({ id: 'a-1' }); }); - it('is a no-op when the alias already points at the same page', async () => { + it('renames the existing row in place when editing to a free name (te -> ted)', async () => { + const { service, shareAliasRepo } = makeService(); + // The new slug is free... + shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined); + // ...but the page already owns an alias named `te`. + shareAliasRepo.findByPageId.mockResolvedValue({ + id: 'a-1', + alias: 'te', + pageId: 'p-1', + }); + shareAliasRepo.updateAlias.mockResolvedValue({ + id: 'a-1', + alias: 'ted', + pageId: 'p-1', + }); + + const res = await service.setAlias({ + workspaceId: 'ws-1', + pageId: 'p-1', + creatorId: 'u-1', + alias: 'ted', + }); + + // RENAME, not INSERT a second row. + expect(shareAliasRepo.insert).not.toHaveBeenCalled(); + expect(shareAliasRepo.updateAlias).toHaveBeenCalledWith( + 'a-1', + 'ted', + 'ws-1', + trx, + ); + // ...and any other row for the page is reaped, so `te` cannot survive. + expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith( + 'p-1', + 'a-1', + 'ws-1', + trx, + ); + expect(res).toMatchObject({ id: 'a-1', alias: 'ted' }); + }); + + it('is a no-op when the alias already points at the same page (and self-heals)', async () => { const { service, shareAliasRepo } = makeService(); const existing = { id: 'a-1', alias: 'foo', pageId: 'p-1' }; shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(existing); @@ -82,7 +148,45 @@ describe('ShareAliasService', () => { expect(res).toBe(existing); expect(shareAliasRepo.insert).not.toHaveBeenCalled(); + expect(shareAliasRepo.updateAlias).not.toHaveBeenCalled(); expect(shareAliasRepo.updatePageId).not.toHaveBeenCalled(); + // self-heal reaps any legacy duplicate rows for the page + expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith( + 'p-1', + 'a-1', + 'ws-1', + trx, + ); + }); + + it('self-heals a page with pre-existing duplicate rows down to one', async () => { + const { service, shareAliasRepo } = makeService(); + // Name free; the page already has a (legacy) alias row we rename. + shareAliasRepo.findByAliasAndWorkspace.mockResolvedValue(undefined); + shareAliasRepo.findByPageId.mockResolvedValue({ + id: 'a-keep', + alias: 'old', + pageId: 'p-1', + }); + shareAliasRepo.updateAlias.mockResolvedValue({ + id: 'a-keep', + alias: 'new', + pageId: 'p-1', + }); + + await service.setAlias({ + workspaceId: 'ws-1', + pageId: 'p-1', + creatorId: 'u-1', + alias: 'new', + }); + + expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith( + 'p-1', + 'a-keep', + 'ws-1', + trx, + ); }); it('throws 409 with current target when name is taken and not confirmed', async () => { @@ -134,6 +238,14 @@ describe('ShareAliasService', () => { 'a-1', 'p-1', 'ws-1', + trx, + ); + // the page's previous alias row(s) are reaped after the swap + expect(shareAliasRepo.deleteOthersForPage).toHaveBeenCalledWith( + 'p-1', + 'a-1', + 'ws-1', + trx, ); expect(res).toMatchObject({ pageId: 'p-1' }); }); diff --git a/apps/server/src/core/share/share-alias.service.ts b/apps/server/src/core/share/share-alias.service.ts index b70d6d3e..e31a35cd 100644 --- a/apps/server/src/core/share/share-alias.service.ts +++ b/apps/server/src/core/share/share-alias.service.ts @@ -9,6 +9,9 @@ import { PageRepo } from '@docmost/db/repos/page/page.repo'; import { ShareService } from './share.service'; import { Page, ShareAlias } from '@docmost/db/types/entity.types'; import { isValidShareAlias, normalizeShareAlias } from './share-alias.util'; +import { InjectKysely } from 'nestjs-kysely'; +import { KyselyDB } from '@docmost/db/types/kysely.types'; +import { executeTx } from '@docmost/db/utils'; /** Postgres unique_violation; the (workspace_id, alias) constraint races here. */ const PG_UNIQUE_VIOLATION = '23505'; @@ -28,16 +31,26 @@ export class ShareAliasService { private readonly shareAliasRepo: ShareAliasRepo, private readonly pageRepo: PageRepo, private readonly shareService: ShareService, + @InjectKysely() private readonly db: KyselyDB, ) {} /** - * Create or retarget a vanity alias. The alias is workspace-scoped: - * - no row for this name -> INSERT a new pointer - * - row already points at pageId -> no-op (idempotent) - * - row points elsewhere -> the "swap". Without confirmReassign we - * throw 409 carrying the current target so the client can confirm; with - * it we UPDATE the single row's page_id (every /l/ link follows the - * 302 to the new page instantly — no stale 301 cache). + * Create, RENAME or retarget a page's vanity alias. INVARIANT: a page has + * EXACTLY ONE custom address. The alias name is workspace-scoped: + * - name free, page has no alias yet -> INSERT a new pointer + * - name free, page already has one -> RENAME that row in place (the slug + * edit, e.g. `te` -> `ted`); we never spawn a second row, so no orphan + * `/l/` link survives + * - name already points at pageId -> no-op (idempotent) + * - name points at ANOTHER page -> the "swap". Without confirmReassign + * we throw 409 carrying the current target so the client can confirm; + * with it we UPDATE the single row's page_id (every /l/ link + * follows the 302 to the new page instantly — no stale cache). + * + * After ANY successful write we DELETE every other alias row still pointing + * at this page (the previous name after a rename/retarget, plus any legacy + * duplicates) so the invariant self-heals. The whole thing runs in one + * transaction so the page never transiently has zero or duplicate rows. * * Caller is responsible for authorizing the page (edit rights + public * readability); this method owns only the alias-name semantics. @@ -57,48 +70,91 @@ export class ShareAliasService { ); } - const existing = await this.shareAliasRepo.findByAliasAndWorkspace( - alias, - workspaceId, - ); - - if (!existing) { - try { - return await this.shareAliasRepo.insert({ - workspaceId, + try { + return await executeTx(this.db, async (trx) => { + const byName = await this.shareAliasRepo.findByAliasAndWorkspace( alias, - pageId, - creatorId, - }); - } catch (err: any) { - // Lost a uniqueness race: another request claimed the name first. - if (err?.code === PG_UNIQUE_VIOLATION) { - throw new ConflictException({ message: 'Alias already taken' }); + workspaceId, + trx, + ); + + // The name is occupied by a DIFFERENT (or dangling) target page. + if (byName && byName.pageId !== pageId) { + if (!confirmReassign) { + const currentPage = byName.pageId + ? await this.pageRepo.findById(byName.pageId) + : null; + throw new ConflictException({ + message: 'Alias already in use', + code: 'ALIAS_REASSIGN_REQUIRED', + currentPageId: byName.pageId, + currentPageTitle: currentPage?.title ?? null, + }); + } + // Confirmed: claim the existing name row for this page, then drop the + // page's previous alias row(s) so it ends with exactly this one. + const retargeted = await this.shareAliasRepo.updatePageId( + byName.id, + pageId, + workspaceId, + trx, + ); + await this.shareAliasRepo.deleteOthersForPage( + pageId, + retargeted.id, + workspaceId, + trx, + ); + return retargeted; } - this.logger.error(err); - throw new BadRequestException('Failed to set alias'); - } - } - // Already points at this page -> nothing to do. - if (existing.pageId === pageId) { - return existing; - } + // The name is FREE, or already points at THIS page. Ensure the page has + // a single row carrying this name: rename its current one, or insert. + const current = + byName ?? + (await this.shareAliasRepo.findByPageId(pageId, workspaceId, trx)); - // Name occupied by a different (or dangling) target: require confirmation. - if (!confirmReassign) { - const currentPage = existing.pageId - ? await this.pageRepo.findById(existing.pageId) - : null; - throw new ConflictException({ - message: 'Alias already in use', - code: 'ALIAS_REASSIGN_REQUIRED', - currentPageId: existing.pageId, - currentPageTitle: currentPage?.title ?? null, + let row: ShareAlias; + if (current) { + row = + current.alias === alias + ? current // same-name no-op + : await this.shareAliasRepo.updateAlias( + current.id, + alias, + workspaceId, + trx, + ); + } else { + row = await this.shareAliasRepo.insert( + { workspaceId, alias, pageId, creatorId }, + trx, + ); + } + + // Self-heal: a page keeps EXACTLY ONE custom address. + await this.shareAliasRepo.deleteOthersForPage( + pageId, + row.id, + workspaceId, + trx, + ); + return row; }); + } catch (err: any) { + if ( + err instanceof ConflictException || + err instanceof BadRequestException + ) { + throw err; + } + // Lost a uniqueness race: another request claimed the name first. + if (err?.code === PG_UNIQUE_VIOLATION) { + throw new ConflictException({ message: 'Alias already taken' }); + } + this.logger.error(err); + throw new BadRequestException('Failed to set alias'); } - - return this.shareAliasRepo.updatePageId(existing.id, pageId, workspaceId); } /** Free a vanity name (no history kept). */ diff --git a/apps/server/src/database/migrations/20260627T120000-share-aliases-one-per-page.ts b/apps/server/src/database/migrations/20260627T120000-share-aliases-one-per-page.ts new file mode 100644 index 00000000..58042e4f --- /dev/null +++ b/apps/server/src/database/migrations/20260627T120000-share-aliases-one-per-page.ts @@ -0,0 +1,41 @@ +import { type Kysely, sql } from 'kysely'; + +/** + * Enforce "a page has EXACTLY ONE custom address" at the DB level. The original + * `share_aliases` table only had a unique index on `(workspace_id, alias)`, so a + * page could accumulate several alias rows (every slug edit used to INSERT a new + * one), leaving orphan `/l/` links live forever and making the share + * modal's `findByPageId` lookup nondeterministic. + * + * We first dedup any pre-existing rows (keeping the NEWEST per page — the same + * "current" choice the read path now makes), then add a PARTIAL unique index on + * `(workspace_id, page_id)`. It is partial (`WHERE page_id IS NOT NULL`) so that + * multiple DANGLING aliases (target page deleted -> `page_id` SET NULL) can + * still coexist without colliding. + */ +export async function up(db: Kysely): Promise { + // Reap legacy duplicates: for each (workspace_id, page_id) keep only the row + // with the greatest (created_at, id) — matches ShareAliasRepo.findByPageId. + await sql` + DELETE FROM share_aliases sa + USING share_aliases keep + WHERE sa.page_id IS NOT NULL + AND sa.workspace_id = keep.workspace_id + AND sa.page_id = keep.page_id + AND (keep.created_at, keep.id) > (sa.created_at, sa.id) + `.execute(db); + + await db.schema + .createIndex('share_aliases_workspace_id_page_id_unique') + .on('share_aliases') + .columns(['workspace_id', 'page_id']) + .unique() + .where('page_id', 'is not', null) + .execute(); +} + +export async function down(db: Kysely): Promise { + await db.schema + .dropIndex('share_aliases_workspace_id_page_id_unique') + .execute(); +} diff --git a/apps/server/src/database/repos/share-alias/share-alias.repo.spec.ts b/apps/server/src/database/repos/share-alias/share-alias.repo.spec.ts index 89ac1e52..505ba9b5 100644 --- a/apps/server/src/database/repos/share-alias/share-alias.repo.spec.ts +++ b/apps/server/src/database/repos/share-alias/share-alias.repo.spec.ts @@ -10,16 +10,21 @@ import type { KyselyDB } from '../../types/kysely.types'; describe('ShareAliasRepo', () => { function makeSelectRepo(result: unknown) { const where = jest.fn(); + const orderBy = jest.fn(); const builder: any = { select: jest.fn(() => builder), where: jest.fn((...args: unknown[]) => { where(...args); return builder; }), + orderBy: jest.fn((...args: unknown[]) => { + orderBy(...args); + return builder; + }), executeTakeFirst: jest.fn().mockResolvedValue(result), }; const db = { selectFrom: jest.fn(() => builder) } as unknown as KyselyDB; - return { repo: new ShareAliasRepo(db), db, where, builder }; + return { repo: new ShareAliasRepo(db), db, where, orderBy, builder }; } it('findByAliasAndWorkspace scopes by alias AND workspace', async () => { @@ -34,11 +39,15 @@ describe('ShareAliasRepo', () => { expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1'); }); - it('findByPageId scopes by page AND workspace', async () => { - const { repo, where } = makeSelectRepo(undefined); + it('findByPageId scopes by page AND workspace, deterministically ordered', async () => { + const { repo, where, orderBy } = makeSelectRepo(undefined); await repo.findByPageId('p-1', 'ws-1'); expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1'); expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1'); + // Explicit ORDER BY removes the nondeterministic heap order for any legacy + // duplicate rows (newest createdAt wins, id as a stable tiebreak). + expect(orderBy).toHaveBeenCalledWith('createdAt', 'desc'); + expect(orderBy).toHaveBeenCalledWith('id', 'desc'); }); it('insert writes the provided columns and returns the row', async () => { @@ -99,6 +108,56 @@ describe('ShareAliasRepo', () => { expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1'); }); + it('updateAlias renames a single row scoped by id + workspace', async () => { + const set = jest.fn(); + const where = jest.fn(); + const builder: any = { + set: jest.fn((s: unknown) => { + set(s); + return builder; + }), + where: jest.fn((...args: unknown[]) => { + where(...args); + return builder; + }), + returning: jest.fn(() => builder), + executeTakeFirst: jest.fn().mockResolvedValue({ id: 'a-1', alias: 'ted' }), + }; + const db = { updateTable: jest.fn(() => builder) } as unknown as KyselyDB; + const repo = new ShareAliasRepo(db); + + const res = await repo.updateAlias('a-1', 'ted', 'ws-1'); + + expect(db.updateTable).toHaveBeenCalledWith('shareAliases'); + expect(set.mock.calls[0][0].alias).toBe('ted'); + expect(set.mock.calls[0][0].updatedAt).toBeInstanceOf(Date); + // a rename must NOT touch page_id (the page's pointer is preserved) + expect(set.mock.calls[0][0]).not.toHaveProperty('pageId'); + expect(where).toHaveBeenCalledWith('id', '=', 'a-1'); + expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1'); + expect(res).toMatchObject({ alias: 'ted' }); + }); + + it('deleteOthersForPage reaps every row for the page except keepId', async () => { + const where = jest.fn(); + const builder: any = { + where: jest.fn((...args: unknown[]) => { + where(...args); + return builder; + }), + execute: jest.fn().mockResolvedValue(undefined), + }; + const db = { deleteFrom: jest.fn(() => builder) } as unknown as KyselyDB; + const repo = new ShareAliasRepo(db); + + await repo.deleteOthersForPage('p-1', 'a-keep', 'ws-1'); + + expect(db.deleteFrom).toHaveBeenCalledWith('shareAliases'); + expect(where).toHaveBeenCalledWith('pageId', '=', 'p-1'); + expect(where).toHaveBeenCalledWith('workspaceId', '=', 'ws-1'); + expect(where).toHaveBeenCalledWith('id', '!=', 'a-keep'); + }); + it('delete scopes by id + workspace', async () => { const where = jest.fn(); const builder: any = { diff --git a/apps/server/src/database/repos/share-alias/share-alias.repo.ts b/apps/server/src/database/repos/share-alias/share-alias.repo.ts index 4c24f33c..39c94a81 100644 --- a/apps/server/src/database/repos/share-alias/share-alias.repo.ts +++ b/apps/server/src/database/repos/share-alias/share-alias.repo.ts @@ -41,7 +41,14 @@ export class ShareAliasRepo { .executeTakeFirst(); } - /** The alias currently pointing at a page (for the share modal). */ + /** + * The alias currently pointing at a page (for the share modal). The service + * enforces a single alias row per page, but legacy rows (pre-invariant) may + * still exist until self-healed; the explicit ORDER BY makes the "current" + * choice DETERMINISTIC (newest wins — i.e. the most recently created address, + * which is the one the user last asked for) instead of an arbitrary Postgres + * heap order. + */ async findByPageId( pageId: string, workspaceId: string, @@ -52,6 +59,8 @@ export class ShareAliasRepo { .select(this.baseFields) .where('pageId', '=', pageId) .where('workspaceId', '=', workspaceId) + .orderBy('createdAt', 'desc') + .orderBy('id', 'desc') .executeTakeFirst(); } @@ -79,6 +88,45 @@ export class ShareAliasRepo { .executeTakeFirst(); } + /** + * Rename an existing alias row in place (the vanity-slug edit, e.g. + * `te` -> `ted`). Keeps the row's id/page_id/creator so the page's single + * alias pointer is preserved — only the human-readable name changes. + */ + async updateAlias( + id: string, + alias: string, + workspaceId: string, + trx?: KyselyTransaction, + ): Promise { + return dbOrTx(this.db, trx) + .updateTable('shareAliases') + .set({ alias, updatedAt: new Date() }) + .where('id', '=', id) + .where('workspaceId', '=', workspaceId) + .returning(this.baseFields) + .executeTakeFirst(); + } + + /** + * Self-heal helper: drop every OTHER alias row still pointing at a page, + * keeping only `keepId`. Enforces the "exactly one custom address per page" + * invariant after a rename/retarget and reaps any legacy duplicates. + */ + async deleteOthersForPage( + pageId: string, + keepId: string, + workspaceId: string, + trx?: KyselyTransaction, + ): Promise { + await dbOrTx(this.db, trx) + .deleteFrom('shareAliases') + .where('pageId', '=', pageId) + .where('workspaceId', '=', workspaceId) + .where('id', '!=', keepId) + .execute(); + } + /** Retarget an existing alias to a new page (the "swap" operation). */ async updatePageId( id: string, diff --git a/apps/server/test/integration/share-alias-one-per-page.int-spec.ts b/apps/server/test/integration/share-alias-one-per-page.int-spec.ts new file mode 100644 index 00000000..e0829583 --- /dev/null +++ b/apps/server/test/integration/share-alias-one-per-page.int-spec.ts @@ -0,0 +1,215 @@ +import { Kysely, sql } from 'kysely'; +import { randomUUID } from 'node:crypto'; +import { ConflictException } from '@nestjs/common'; +import { ShareAliasRepo } from '@docmost/db/repos/share-alias/share-alias.repo'; +import { ShareAliasService } from 'src/core/share/share-alias.service'; +import { + getTestDb, + destroyTestDb, + createWorkspace, + createSpace, + createPage, +} from './db'; + +/** + * Issue #226 (regression of #205): "a page has EXACTLY ONE custom address". + * Exercises against real Postgres: + * - the partial unique index `(workspace_id, page_id) WHERE page_id IS NOT NULL` + * (migration 20260627T120000) — one alias per page, but dangling aliases + * (page_id NULL) may coexist; + * - the migration's dedup DELETE keeps the NEWEST row per page; + * - ShareAliasService.setAlias renames in place (te -> ted) instead of + * spawning a second row, and self-heals the page down to one alias. + */ +describe('share_aliases one-per-page invariant [integration]', () => { + let db: Kysely; + let repo: ShareAliasRepo; + let service: ShareAliasService; + let wsId: string; + let spaceId: string; + + // setAlias only consults pageRepo on the unconfirmed-reassign (409) path. + const pageRepo = { + findById: async (id: string) => ({ id, title: `title-${id}` }), + }; + + beforeAll(async () => { + db = getTestDb(); + repo = new ShareAliasRepo(db as any); + service = new ShareAliasService( + repo as any, + pageRepo as any, + {} as any, // shareService — unused by setAlias + db as any, + ); + wsId = (await createWorkspace(db)).id; + spaceId = (await createSpace(db, wsId)).id; + }); + + afterAll(async () => { + await destroyTestDb(); + }); + + const newPage = async (): Promise => + (await createPage(db, { workspaceId: wsId, spaceId })).id; + + const aliasRowsFor = (pageId: string) => + db + .selectFrom('shareAliases') + .select(['id', 'alias']) + .where('pageId', '=', pageId) + .where('workspaceId', '=', wsId) + .orderBy('alias') + .execute(); + + it('partial unique index rejects a second alias for the same page (23505)', async () => { + const pageId = await newPage(); + await repo.insert({ workspaceId: wsId, alias: 'first', pageId }); + + let code: string | undefined; + try { + await repo.insert({ workspaceId: wsId, alias: 'second', pageId }); + } catch (err: any) { + code = err?.code ?? err?.cause?.code; + } + expect(code).toBe('23505'); + }); + + it('allows multiple DANGLING aliases (page_id NULL) — partial index excludes them', async () => { + const a = await repo.insert({ + workspaceId: wsId, + alias: `dangling-${randomUUID().slice(0, 8)}`, + pageId: null as any, + }); + const b = await repo.insert({ + workspaceId: wsId, + alias: `dangling-${randomUUID().slice(0, 8)}`, + pageId: null as any, + }); + expect(a.id).toBeDefined(); + expect(b.id).toBeDefined(); + expect(a.id).not.toBe(b.id); + }); + + it("migration dedup DELETE keeps the page's NEWEST alias row", async () => { + const pageId = await newPage(); + // Temporarily drop the guard so we can seed the legacy duplicate shape. + await sql`DROP INDEX share_aliases_workspace_id_page_id_unique`.execute(db); + try { + const mk = async (alias: string, createdAt: string): Promise => { + const id = randomUUID(); + await db + .insertInto('shareAliases') + .values({ id, workspaceId: wsId, alias, pageId, createdAt }) + .execute(); + return id; + }; + await mk('oldest', '2026-01-01T00:00:00Z'); + await mk('middle', '2026-02-01T00:00:00Z'); + const newest = await mk('newest', '2026-03-01T00:00:00Z'); + + // Exact dedup statement from the migration. + await sql` + DELETE FROM share_aliases sa + USING share_aliases keep + WHERE sa.page_id IS NOT NULL + AND sa.workspace_id = keep.workspace_id + AND sa.page_id = keep.page_id + AND (keep.created_at, keep.id) > (sa.created_at, sa.id) + `.execute(db); + + const rows = await aliasRowsFor(pageId); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ id: newest, alias: 'newest' }); + } finally { + await sql` + CREATE UNIQUE INDEX share_aliases_workspace_id_page_id_unique + ON share_aliases (workspace_id, page_id) + WHERE page_id IS NOT NULL + `.execute(db); + } + }); + + it('setAlias renames te -> ted in place: page ends with ONE row named ted', async () => { + const pageId = await newPage(); + const creatorId = null as any; + const first = await service.setAlias({ + workspaceId: wsId, + pageId, + creatorId, + alias: 'te', + }); + expect(first.alias).toBe('te'); + + const renamed = await service.setAlias({ + workspaceId: wsId, + pageId, + creatorId, + alias: 'ted', + }); + // Same row id — a RENAME, not a new insert. + expect(renamed.id).toBe(first.id); + expect(renamed.alias).toBe('ted'); + + const rows = await aliasRowsFor(pageId); + expect(rows).toHaveLength(1); + expect(rows[0].alias).toBe('ted'); // the stale `te` row is gone + + // The modal read resolves the current (only) row deterministically. + const shown = await service.getAliasForPage(pageId, wsId); + expect(shown?.alias).toBe('ted'); + }); + + it('setAlias inserts the first alias, then is a no-op for the same name', async () => { + const pageId = await newPage(); + const inserted = await service.setAlias({ + workspaceId: wsId, + pageId, + creatorId: null as any, + alias: 'hello', + }); + const again = await service.setAlias({ + workspaceId: wsId, + pageId, + creatorId: null as any, + alias: 'hello', + }); + expect(again.id).toBe(inserted.id); + expect(await aliasRowsFor(pageId)).toHaveLength(1); + }); + + it('cross-page collision throws 409, and confirmReassign moves the single row', async () => { + const pageA = await newPage(); + const pageB = await newPage(); + await service.setAlias({ + workspaceId: wsId, + pageId: pageA, + creatorId: null as any, + alias: 'shared', + }); + + await expect( + service.setAlias({ + workspaceId: wsId, + pageId: pageB, + creatorId: null as any, + alias: 'shared', + }), + ).rejects.toBeInstanceOf(ConflictException); + + const moved = await service.setAlias({ + workspaceId: wsId, + pageId: pageB, + creatorId: null as any, + alias: 'shared', + confirmReassign: true, + }); + expect(moved.alias).toBe('shared'); + + // The name now belongs to pageB only; pageA has no alias. + expect(await aliasRowsFor(pageA)).toHaveLength(0); + const bRows = await aliasRowsFor(pageB); + expect(bRows).toHaveLength(1); + expect(bRows[0].alias).toBe('shared'); + }); +});