From 8af658c90b15133db64b566a6569a4a8a551fbe6 Mon Sep 17 00:00:00 2001 From: grimicorn-agent Date: Sun, 23 Aug 2026 16:23:42 -0500 Subject: [PATCH 1/3] Key record source icon/label off the real source type Join sources into the record list/detail read queries and expose sourceType on the record resource, then drive sourceTypeIcon/ formatSourceLabel off it instead of a type prefix that the free-text records.source column never carried. Closes #153 --- app/components/RecordDetailModal.vue | 9 ++- app/composables/useRecords.ts | 51 +++++++++++---- app/pages/inbox.vue | 9 ++- public/openapi.json | 7 ++ server/api/records/[uuid].get.ts | 13 +++- server/api/records/index.get.ts | 33 ++++++++-- server/utils/response.ts | 13 +++- tests/components/RecordDetailModal.test.ts | 21 +++++- .../RecordDetailModal.test.ts.snap | 2 +- tests/composables/useRecords.test.ts | 52 ++++++++++----- tests/pages/__snapshots__/inbox.test.ts.snap | 4 +- tests/pages/inbox.test.ts | 12 ++-- tests/server/api/records/create.test.ts | 2 + tests/server/api/records/list.test.ts | 65 ++++++++++++++++++- tests/server/api/records/patch.test.ts | 1 + tests/server/api/records/show.test.ts | 36 +++++++++- tests/server/utils/response.test.ts | 27 ++++++++ 17 files changed, 298 insertions(+), 59 deletions(-) diff --git a/app/components/RecordDetailModal.vue b/app/components/RecordDetailModal.vue index 5fcb388..c2f3041 100644 --- a/app/components/RecordDetailModal.vue +++ b/app/components/RecordDetailModal.vue @@ -78,12 +78,17 @@ > - {{ formatSourceLabel(record.attributes.source) }} + {{ + formatSourceLabel( + record.attributes.source, + record.attributes.sourceType, + ) + }} diff --git a/app/composables/useRecords.ts b/app/composables/useRecords.ts index 99ef1d5..9696815 100644 --- a/app/composables/useRecords.ts +++ b/app/composables/useRecords.ts @@ -17,6 +17,7 @@ export type RecordAttributes = { content: string; sourceId: string | null; source: string | null; + sourceType: SourceType | null; status: RecordStatus; filePath: string | null; tags: unknown; @@ -181,29 +182,51 @@ export const STATUS_TONE_MAP: Record = { error: "err", }; -export function sourceTypeIcon(source: string | null): string { - if (!source) { - return "zap"; - } +const DEFAULT_SOURCE_ICON = "zap"; +const UNKNOWN_SOURCE_LABEL = "unknown"; + +// Icon per canonical source type (`sources.type`). Record rows resolve their +// icon from the real type, not the free-text `source` display name (which never +// carried a reliable type prefix). Keyed by SourceType so the compiler flags a +// missing icon whenever a new source type is added to the shared contract. +const SOURCE_TYPE_ICONS: Record = { + webhook: "zap", + email: "mail", + stripe: "card", + github: "github", + zapier: "zap", + shortcuts: "plug", +}; - if (source.startsWith("email/")) { - return "mail"; +export function sourceTypeIcon(sourceType: string | null): string { + if (!sourceType || !isSourceType(sourceType)) { + return DEFAULT_SOURCE_ICON; } - return "zap"; + return SOURCE_TYPE_ICONS[sourceType]; } -export function formatSourceLabel(source: string | null): string { - if (!source) { - return "unknown"; +export function formatSourceLabel( + source: string | null, + sourceType: string | null, +): string { + // Prefer the source's display name so two sources of the same type (e.g. two + // webhooks, "Prod deploys" and "Staging deploys") stay distinguishable — the + // real type is already conveyed by the icon. Fall back to the type name, then + // to "unknown", when no name is stored. + if (source) { + return source; } - const slashIndex = source.indexOf("/"); - if (slashIndex === -1) { - return source; + // Show whatever type the server resolved, even a legacy value outside the + // current SOURCE_TYPES set (sources.type is free text) — a real type name is + // a better label than "unknown". The output is escaped by Vue, so it needs + // no isSourceType guard here (the icon lookup still does). + if (sourceType) { + return sourceType; } - return source.slice(slashIndex + 1).replaceAll("/", " · "); + return UNKNOWN_SOURCE_LABEL; } export function formatRelativeTime(isoString: string): string { diff --git a/app/pages/inbox.vue b/app/pages/inbox.vue index 42df92f..51461c3 100644 --- a/app/pages/inbox.vue +++ b/app/pages/inbox.vue @@ -197,7 +197,7 @@ > @@ -211,7 +211,12 @@ text-overflow: ellipsis; " > - {{ formatSourceLabel(record.attributes.source) }} + {{ + formatSourceLabel( + record.attributes.source, + record.attributes.sourceType, + ) + }} & { + sourceType?: string | null; +}; type RecordResource = ApiResourceObject & { type: "records"; @@ -121,6 +131,7 @@ export function recordSerializer( content: record.content, sourceId: record.sourceId, source: record.source, + sourceType: record.sourceType ?? null, status: record.status, filePath: record.filePath, tags: record.tags, diff --git a/tests/components/RecordDetailModal.test.ts b/tests/components/RecordDetailModal.test.ts index a2bc190..54e46aa 100644 --- a/tests/components/RecordDetailModal.test.ts +++ b/tests/components/RecordDetailModal.test.ts @@ -18,8 +18,9 @@ function makeRecord(overrides: Record = {}) { userId: "user-1", title: "Test Record", content: "# Heading\n\nBody", - sourceId: null, - source: "webhook/github", + sourceId: "source-1", + source: "My GitHub hook", + sourceType: "github", status: "synced", filePath: "99-incoming/test.md", tags: null, @@ -39,7 +40,7 @@ const stubs = { props: ["variant", "size", "icon"], emits: ["click"], }, - AppIcon: { template: "", props: ["name", "size"] }, + AppIcon: { template: '', props: ["name", "size"] }, AppBadge: { template: '', props: ["tone", "dot"], @@ -83,6 +84,20 @@ describe("RecordDetailModal", () => { expect(wrapper.find(".app-code-block").text()).toContain("Heading"); }); + it("resolves the source icon from the real type and labels with the source name", () => { + const wrapper = mountModal(); + expect(wrapper.find("[data-icon]").attributes("data-icon")).toBe("github"); + expect(wrapper.text()).toContain("My GitHub hook"); + }); + + it("falls back to the zap icon and stored name when the source type is unresolved", () => { + const wrapper = mountModal({ + record: makeRecord({ sourceType: null, source: "Legacy hook" }), + }); + expect(wrapper.find("[data-icon]").attributes("data-icon")).toBe("zap"); + expect(wrapper.text()).toContain("Legacy hook"); + }); + it("shows a loading indicator while loading", () => { const wrapper = mountModal({ record: null, isLoading: true }); expect(wrapper.text()).toContain("loading record"); diff --git a/tests/components/__snapshots__/RecordDetailModal.test.ts.snap b/tests/components/__snapshots__/RecordDetailModal.test.ts.snap index a1b4af8..23a6faa 100644 --- a/tests/components/__snapshots__/RecordDetailModal.test.ts.snap +++ b/tests/components/__snapshots__/RecordDetailModal.test.ts.snap @@ -5,7 +5,7 @@ exports[`RecordDetailModal > matches the snapshot for a loaded record 1`] = `
record detail

Test Record

-
githubsynced35d ago
+
My GitHub hooksynced35d ago
file
99-incoming/test.md
diff --git a/tests/composables/useRecords.test.ts b/tests/composables/useRecords.test.ts index 02641e8..8223eeb 100644 --- a/tests/composables/useRecords.test.ts +++ b/tests/composables/useRecords.test.ts @@ -95,34 +95,55 @@ describe("buildFetchUrl", () => { }); describe("sourceTypeIcon", () => { - it("returns the mail icon for email sources", () => { - expect(sourceTypeIcon("email/inbound/gmail")).toBe("mail"); + it("returns the mail icon for the email type", () => { + expect(sourceTypeIcon("email")).toBe("mail"); }); - it("returns the zap icon for non-email sources", () => { - expect(sourceTypeIcon("webhook/github")).toBe("zap"); + it("returns the card icon for the stripe type", () => { + expect(sourceTypeIcon("stripe")).toBe("card"); }); - it("returns the zap icon for a null source", () => { + it("returns the github icon for the github type", () => { + expect(sourceTypeIcon("github")).toBe("github"); + }); + + it("returns the plug icon for the shortcuts type", () => { + expect(sourceTypeIcon("shortcuts")).toBe("plug"); + }); + + it("returns the zap icon for the webhook and zapier types", () => { + expect(sourceTypeIcon("webhook")).toBe("zap"); + expect(sourceTypeIcon("zapier")).toBe("zap"); + }); + + it("returns the zap icon for a null type", () => { expect(sourceTypeIcon(null)).toBe("zap"); }); + + it("returns the zap icon for an unrecognized type", () => { + expect(sourceTypeIcon("mystery")).toBe("zap"); + }); }); describe("formatSourceLabel", () => { - it("returns 'unknown' for a null source", () => { - expect(formatSourceLabel(null)).toBe("unknown"); + it("prefers the source display name so same-type sources stay distinct", () => { + expect(formatSourceLabel("Prod deploys", "github")).toBe("Prod deploys"); + expect(formatSourceLabel("Staging deploys", "github")).toBe( + "Staging deploys", + ); }); - it("returns the source unchanged when it has no slash", () => { - expect(formatSourceLabel("webhook")).toBe("webhook"); + it("falls back to the resolved type name when no source name is stored", () => { + expect(formatSourceLabel(null, "webhook")).toBe("webhook"); + expect(formatSourceLabel(null, "email")).toBe("email"); }); - it("drops the type prefix for a single-slash source", () => { - expect(formatSourceLabel("webhook/github")).toBe("github"); + it("shows a legacy type name outside the current set rather than 'unknown'", () => { + expect(formatSourceLabel(null, "rss")).toBe("rss"); }); - it("joins remaining segments with a middot for a multi-slash source", () => { - expect(formatSourceLabel("email/inbound/gmail")).toBe("inbound · gmail"); + it("returns 'unknown' only when neither a source name nor a type is present", () => { + expect(formatSourceLabel(null, null)).toBe("unknown"); }); }); @@ -255,8 +276,9 @@ function makeRecordResource(uuid: string): RecordResource { userId: "user-1", title: `Record ${uuid}`, content: "content", - sourceId: null, - source: "webhook/github", + sourceId: "source-1", + source: "My GitHub hook", + sourceType: "github", status: "synced", filePath: null, tags: null, diff --git a/tests/pages/__snapshots__/inbox.test.ts.snap b/tests/pages/__snapshots__/inbox.test.ts.snap index 0d73869..9840d71 100644 --- a/tests/pages/__snapshots__/inbox.test.ts.snap +++ b/tests/pages/__snapshots__/inbox.test.ts.snap @@ -162,8 +162,8 @@ exports[`inbox page > matches snapshot with records 1`] = `
sourcerecordfilestatustime
-
label:webhook/githubTest Record99-incoming/test.mdsynced2m ago
-
label:webhook/githubAnother99-incoming/test.mdsynced2m ago
+
label:My GitHub hookTest Record99-incoming/test.mdsynced2m ago
+
label:My GitHub hookAnother99-incoming/test.mdsynced2m ago
diff --git a/tests/pages/inbox.test.ts b/tests/pages/inbox.test.ts index f93abab..ad1e2ce 100644 --- a/tests/pages/inbox.test.ts +++ b/tests/pages/inbox.test.ts @@ -38,9 +38,10 @@ vi.mock("../../app/composables/useRecords", async (importOriginal) => { void isoString; return "2m ago"; }, - formatSourceLabel: (source: string | null) => - `label:${source ?? "unknown"}`, - sourceTypeIcon: () => "zap", + formatSourceLabel: (source: string | null, sourceType: string | null) => + `label:${source ?? sourceType ?? "unknown"}`, + sourceTypeIcon: (sourceType: string | null) => + `icon:${sourceType ?? "none"}`, get triggerRecordExportDownload() { return mockTriggerRecordExport; }, @@ -129,8 +130,9 @@ function makeRecord(overrides: Record = {}) { userId: "user-1", title: "Test Record", content: "Content here", - sourceId: null, - source: "webhook/github", + sourceId: "source-1", + source: "My GitHub hook", + sourceType: "github", status: "synced", filePath: "99-incoming/test.md", tags: null, diff --git a/tests/server/api/records/create.test.ts b/tests/server/api/records/create.test.ts index 79da3d0..c81fffd 100644 --- a/tests/server/api/records/create.test.ts +++ b/tests/server/api/records/create.test.ts @@ -146,6 +146,7 @@ describe("POST /api/records", () => { content: sampleRecord.content, sourceId: null, source: null, + sourceType: null, status: "pending", filePath: null, tags: null, @@ -359,6 +360,7 @@ describe("POST /api/records", () => { content: sampleRecordWithExtras.content, sourceId: validSourceId, source: "webhook/github", + sourceType: null, status: "synced", filePath: "99-incoming/2026-06-14-deploy.md", tags: ["deploy"], diff --git a/tests/server/api/records/list.test.ts b/tests/server/api/records/list.test.ts index 0a670d3..ab3ba40 100644 --- a/tests/server/api/records/list.test.ts +++ b/tests/server/api/records/list.test.ts @@ -14,6 +14,11 @@ vi.mock("drizzle-orm", () => ({ count: () => ({ count: true }), desc: (column: unknown) => ({ desc: column }), eq: (column: unknown, value: unknown) => ({ eq: { column, value } }), + // The real handler spreads the record columns into the page select; the + // select router below only needs to tell the page query apart from the + // count/cursor/subquery selects, which it does via the sourceType key the + // page query adds, so this mock can return an empty column set. + getTableColumns: () => ({}), ilike: (column: unknown, pattern: unknown) => ({ ilike: { column, pattern }, }), @@ -60,7 +65,8 @@ function stubSelectResults( const pageLimit = vi.fn(() => Promise.resolve(pageRows)); const pageOrderBy = vi.fn(() => ({ limit: pageLimit })); const pageWhere = vi.fn(() => ({ orderBy: pageOrderBy })); - const pageFrom = vi.fn(() => ({ where: pageWhere })); + const pageLeftJoin = vi.fn(() => ({ where: pageWhere })); + const pageFrom = vi.fn(() => ({ leftJoin: pageLeftJoin })); // Cursor lookup selects { createdAt, uuid }; keep it distinct from the // source subquery (which selects { uuid } only) by checking createdAt first. @@ -79,6 +85,13 @@ function stubSelectResults( // subquery and cursor lookup add extra db.select() calls, so a call-count // heuristic would misroute the count/page queries. selectMock.mockImplementation((columns?: Record) => { + // The page query is the only select that adds a sourceType column (from the + // sources join), so match it on that key. Checked first so it can never be + // misrouted by the cursor/subquery column checks below. + if (columns && "sourceType" in columns) { + return { from: pageFrom }; + } + if (columns && "createdAt" in columns) { return { from: cursorFrom }; } @@ -94,7 +107,14 @@ function stubSelectResults( return { from: pageFrom }; }); - return { countWhere, pageWhere, sourceSubFrom, sourceSubWhere, cursorWhere }; + return { + countWhere, + pageWhere, + pageLeftJoin, + sourceSubFrom, + sourceSubWhere, + cursorWhere, + }; } function stubRequireUser(returnedUserId: string | undefined) { @@ -311,6 +331,47 @@ describe("GET /api/records", () => { expect(findSourceIdInArray(pageWhereArg.and)).toBeDefined(); }); + it("left-joins sources on records.sourceId so the real source type is available", async () => { + const { pageLeftJoin } = stubSelectResults({ value: 0 }, []); + + await handler(buildEvent(userId)); + + const [joinedTable, joinPredicate] = pageLeftJoin.mock.calls[0] ?? []; + expect(joinedTable).toBe(sources); + const joinConditions = (joinPredicate as { and: unknown[] }).and; + expect(hasEqCondition(joinConditions, records.sourceId, sources.uuid)).toBe( + true, + ); + // Tenant-scoped: the join must also pin sources.userId so it can never + // surface another user's source type. + expect(hasEqCondition(joinConditions, sources.userId, userId)).toBe(true); + }); + + it("exposes the joined source type on each serialized record", async () => { + stubSelectResults({ value: 1 }, [ + { + uuid: "550e8400-e29b-41d4-a716-446655440000", + createdAt: new Date("2024-01-15T10:00:00Z"), + userId, + title: "Test", + content: "Body", + sourceId: "550e8400-e29b-41d4-a716-446655440099", + source: "My GitHub hook", + sourceType: "github", + status: "synced", + filePath: null, + tags: null, + frontmatter: null, + syncedAt: null, + errorMessage: null, + }, + ]); + + const response = await handler(buildEvent(userId)); + + expect(response.data[0]?.attributes.sourceType).toBe("github"); + }); + // Regression guard for the original bug: type filtering matched // `like(records.source, "webhook/%")`, but ingest stores the source's // display name in records.source (no type prefix), so it matched nothing. diff --git a/tests/server/api/records/patch.test.ts b/tests/server/api/records/patch.test.ts index a7425b7..a4734af 100644 --- a/tests/server/api/records/patch.test.ts +++ b/tests/server/api/records/patch.test.ts @@ -105,6 +105,7 @@ describe("PATCH /api/records/:uuid", () => { content: updatedRecord.content, sourceId: null, source: null, + sourceType: null, status: "synced", filePath: "05-stripe/note.md", tags: null, diff --git a/tests/server/api/records/show.test.ts b/tests/server/api/records/show.test.ts index dbe856e..f89f944 100644 --- a/tests/server/api/records/show.test.ts +++ b/tests/server/api/records/show.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { H3Event } from "h3"; +import { sources } from "../../../../server/db/schema"; const selectMock = vi.fn(); @@ -49,9 +50,10 @@ function buildEvent(contextUserId: string | undefined): H3Event { function stubSelectResult(rows: unknown[]) { const limit = vi.fn(() => Promise.resolve(rows)); const where = vi.fn(() => ({ limit })); - const from = vi.fn(() => ({ where })); + const leftJoin = vi.fn(() => ({ where })); + const from = vi.fn(() => ({ leftJoin })); selectMock.mockReturnValue({ from }); - return { from, where, limit }; + return { from, leftJoin, where, limit }; } beforeEach(() => { @@ -86,6 +88,20 @@ describe("findRecordForUser", () => { expect(result).toBeNull(); }); + + it("left-joins the sources table so the real source type is resolved", async () => { + const { leftJoin } = stubSelectResult([sampleRecord]); + + const db = (await import("../../../../server/db")).getDb(); + await findRecordForUser(db, validUuid, userId); + + // The join predicate is tenant-scoped (records.sourceId = sources.uuid AND + // sources.userId = userId); the exact predicate shape is asserted with the + // mocked drizzle in list.test.ts. Here (real drizzle) we pin the joined + // table so removing the join fails loudly. + expect(leftJoin.mock.calls[0]?.[0]).toBe(sources); + expect(leftJoin.mock.calls[0]?.[1]).toBeDefined(); + }); }); describe("GET /api/records/:uuid", () => { @@ -106,6 +122,7 @@ describe("GET /api/records/:uuid", () => { content: sampleRecord.content, sourceId: null, source: null, + sourceType: null, status: "pending", filePath: null, tags: null, @@ -118,6 +135,21 @@ describe("GET /api/records/:uuid", () => { }); }); + it("surfaces the joined source type on the serialized record", async () => { + stubSelectResult([ + { + ...sampleRecord, + sourceId: "550e8400-e29b-41d4-a716-446655440099", + source: "My Zapier hook", + sourceType: "zapier", + }, + ]); + + const response = await handler(buildEvent(userId)); + + expect(response.data?.attributes.sourceType).toBe("zapier"); + }); + it("throws a 404 when no record exists for the authenticated user", async () => { stubSelectResult([]); diff --git a/tests/server/utils/response.test.ts b/tests/server/utils/response.test.ts index 8d36f7b..34dfecc 100644 --- a/tests/server/utils/response.test.ts +++ b/tests/server/utils/response.test.ts @@ -15,6 +15,7 @@ const baseRecord = { content: "Some content here", sourceId: null, source: null, + sourceType: null, status: "pending", filePath: null, tags: null, @@ -38,6 +39,7 @@ describe("recordSerializer", () => { content: baseRecord.content, sourceId: null, source: null, + sourceType: null, status: "pending", filePath: null, tags: null, @@ -51,6 +53,31 @@ describe("recordSerializer", () => { }); }); + it("exposes the joined source type when present", () => { + const recordWithType = { + ...baseRecord, + sourceId: "550e8400-e29b-41d4-a716-446655440099", + source: "My Zapier hook", + sourceType: "zapier", + }; + + const result = recordSerializer(recordWithType); + + expect(result?.attributes.sourceType).toBe("zapier"); + expect(result?.attributes.source).toBe("My Zapier hook"); + }); + + it("defaults sourceType to null when the row was not joined to sources", () => { + const recordWithoutJoin = { + ...baseRecord, + sourceType: undefined, + }; + + const result = recordSerializer(recordWithoutJoin); + + expect(result?.attributes.sourceType).toBeNull(); + }); + it("includes optional fields when present", () => { const syncedAt = new Date("2026-06-14T12:00:00Z"); const recordWithExtras = { From 82c3d2d9012ea31f36f0dadc3c2082b32ec39268 Mon Sep 17 00:00:00 2001 From: Danny Holloran Date: Thu, 27 Aug 2026 21:28:18 -0500 Subject: [PATCH 2/3] Address review: honest sourceType contract type, assert tenant-scoped detail join and page column set --- app/composables/useRecords.ts | 6 ++- server/utils/openapi.template.json | 7 +--- tests/composables/useRecords.test.ts | 10 +++++ tests/server/api/records/list.test.ts | 29 +++++++++++--- tests/server/api/records/show.test.ts | 56 +++++++++++++++++++++++---- 5 files changed, 90 insertions(+), 18 deletions(-) diff --git a/app/composables/useRecords.ts b/app/composables/useRecords.ts index 9696815..dde6720 100644 --- a/app/composables/useRecords.ts +++ b/app/composables/useRecords.ts @@ -17,7 +17,11 @@ export type RecordAttributes = { content: string; sourceId: string | null; source: string | null; - sourceType: SourceType | null; + // Free text on the server (sources.type has no CHECK constraint), so the + // contract is `string | null`, not `SourceType | null` — a legacy value + // outside SOURCE_TYPES can arrive. Consumers (sourceTypeIcon) narrow with + // isSourceType at the point of use. + sourceType: string | null; status: RecordStatus; filePath: string | null; tags: unknown; diff --git a/server/utils/openapi.template.json b/server/utils/openapi.template.json index 2a9210b..4b90ad5 100644 --- a/server/utils/openapi.template.json +++ b/server/utils/openapi.template.json @@ -1194,11 +1194,8 @@ "sourceId": { "type": ["string", "null"], "format": "uuid" }, "source": { "type": ["string", "null"] }, "sourceType": { - "description": "Canonical source type, resolved by joining the record's source. Authoritative on the list and detail GET endpoints. On create/patch responses (which do not join sources) this is always null and should be treated as a write receipt, not the resolved type — re-read the record via GET to obtain it.", - "oneOf": [ - { "$ref": "#/components/schemas/SourceType" }, - { "type": "null" } - ] + "description": "Source type resolved by joining the record's source; canonical values are those of SourceType, but the underlying column is free text so a legacy value outside that set may be returned. Authoritative on the list and detail GET endpoints. On create/patch responses (which do not join sources) this is always null and should be treated as a write receipt, not the resolved type — re-read the record via GET to obtain it.", + "type": ["string", "null"] }, "status": { "$ref": "#/components/schemas/RecordStatus" }, "filePath": { "type": ["string", "null"] }, diff --git a/tests/composables/useRecords.test.ts b/tests/composables/useRecords.test.ts index 8223eeb..33bbaf4 100644 --- a/tests/composables/useRecords.test.ts +++ b/tests/composables/useRecords.test.ts @@ -145,6 +145,16 @@ describe("formatSourceLabel", () => { it("returns 'unknown' only when neither a source name nor a type is present", () => { expect(formatSourceLabel(null, null)).toBe("unknown"); }); + + // Deliberate: a legacy `type/`-prefixed source name renders verbatim rather + // than being split on `/`. Re-adding slash-stripping would reintroduce the + // fragile prefix-parsing this change removed; the icon now conveys the type, + // and the stored name is shown honestly. Pinned so the behavior stays chosen. + it("renders a legacy prefixed source name verbatim (no slash-stripping)", () => { + expect(formatSourceLabel("webhook/github", "webhook")).toBe( + "webhook/github", + ); + }); }); describe("formatRelativeTime", () => { diff --git a/tests/server/api/records/list.test.ts b/tests/server/api/records/list.test.ts index 07b5912..87a7b68 100644 --- a/tests/server/api/records/list.test.ts +++ b/tests/server/api/records/list.test.ts @@ -26,11 +26,12 @@ vi.mock("drizzle-orm", () => ({ count: () => ({ count: true }), desc: (column: unknown) => ({ desc: column }), eq: (column: unknown, value: unknown) => ({ eq: { column, value } }), - // The real handler spreads the record columns into the page select; the - // select router below only needs to tell the page query apart from the - // count/cursor/subquery selects, which it does via the sourceType key the - // page query adds, so this mock can return an empty column set. - getTableColumns: () => ({}), + // Echo the table so the page select carries the real record columns; the + // router below still tells the page query apart via the sourceType key it + // adds (checked first), and a test can assert the record columns are actually + // selected (a dropped `...getTableColumns(records)` spread would otherwise + // stay green while returning sourceType-only rows). + getTableColumns: (table: Record) => table, ilike: (column: unknown, pattern: unknown) => ({ ilike: { column, pattern }, }), @@ -433,6 +434,24 @@ describe("GET /api/records", () => { expect(response.data[0]?.attributes.sourceType).toBe("github"); }); + it("selects the record columns alongside the joined sourceType", async () => { + stubSelectResults({ value: 0 }, []); + + await handler(buildEvent(userId)); + + // Pin that the page select spreads the record columns; without this a + // dropped `...getTableColumns(records)` spread would return sourceType-only + // rows (undefined title/content/status) yet keep the suite green. + expect(selectMock).toHaveBeenCalledWith( + expect.objectContaining({ + uuid: records.uuid, + createdAt: records.createdAt, + status: records.status, + sourceType: sources.type, + }), + ); + }); + // Regression guard for the original bug: type filtering matched // `like(records.source, "webhook/%")`, but ingest stores the source's // display name in records.source (no type prefix), so it matched nothing. diff --git a/tests/server/api/records/show.test.ts b/tests/server/api/records/show.test.ts index f89f944..43409fd 100644 --- a/tests/server/api/records/show.test.ts +++ b/tests/server/api/records/show.test.ts @@ -1,6 +1,41 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { H3Event } from "h3"; -import { sources } from "../../../../server/db/schema"; +import { records, sources } from "../../../../server/db/schema"; + +// Walks a real drizzle SQL predicate's queryChunks, collecting the Column +// objects it references (by identity) and any bound Param values, so a test +// can assert the predicate's structure without depending on generated SQL text. +function collectPredicate( + node: unknown, + columns: Set = new Set(), + paramValues: unknown[] = [], +): { columns: Set; paramValues: unknown[] } { + if (!node || typeof node !== "object") { + return { columns, paramValues }; + } + + const candidate = node as { + queryChunks?: unknown[]; + value?: unknown; + constructor?: { name?: string }; + }; + + if (candidate.constructor?.name === "Param") { + paramValues.push(candidate.value); + } + + if ("table" in node && "name" in node) { + columns.add(node); + } + + if (Array.isArray(candidate.queryChunks)) { + for (const chunk of candidate.queryChunks) { + collectPredicate(chunk, columns, paramValues); + } + } + + return { columns, paramValues }; +} const selectMock = vi.fn(); @@ -89,18 +124,25 @@ describe("findRecordForUser", () => { expect(result).toBeNull(); }); - it("left-joins the sources table so the real source type is resolved", async () => { + it("left-joins the sources table with a tenant-scoped predicate", async () => { const { leftJoin } = stubSelectResult([sampleRecord]); const db = (await import("../../../../server/db")).getDb(); await findRecordForUser(db, validUuid, userId); - // The join predicate is tenant-scoped (records.sourceId = sources.uuid AND - // sources.userId = userId); the exact predicate shape is asserted with the - // mocked drizzle in list.test.ts. Here (real drizzle) we pin the joined - // table so removing the join fails loudly. expect(leftJoin.mock.calls[0]?.[0]).toBe(sources); - expect(leftJoin.mock.calls[0]?.[1]).toBeDefined(); + + // Tenant-scoping is the security-relevant half of the join: without + // `sources.userId = userId` a record could surface another user's source + // type. Walk the real drizzle predicate's chunks and assert it references + // the sources.userId column and binds this userId, so deleting that clause + // fails loudly (list.test.ts pins the same shape via mocked drizzle). + const joinPredicate = leftJoin.mock.calls[0]?.[1]; + const { columns, paramValues } = collectPredicate(joinPredicate); + expect(columns.has(records.sourceId)).toBe(true); + expect(columns.has(sources.uuid)).toBe(true); + expect(columns.has(sources.userId)).toBe(true); + expect(paramValues).toContain(userId); }); }); From a6b0772d29b142d4dfd985d811f0e7e5e4a8f234 Mon Sep 17 00:00:00 2001 From: Danny Holloran Date: Thu, 27 Aug 2026 21:33:48 -0500 Subject: [PATCH 3/3] Pin detail-endpoint record column set in show test --- tests/server/api/records/show.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/server/api/records/show.test.ts b/tests/server/api/records/show.test.ts index 43409fd..6d5bed2 100644 --- a/tests/server/api/records/show.test.ts +++ b/tests/server/api/records/show.test.ts @@ -144,6 +144,24 @@ describe("findRecordForUser", () => { expect(columns.has(sources.userId)).toBe(true); expect(paramValues).toContain(userId); }); + + it("selects the record columns alongside the joined sourceType", async () => { + stubSelectResult([sampleRecord]); + + const db = (await import("../../../../server/db")).getDb(); + await findRecordForUser(db, validUuid, userId); + + // Pin that the detail select spreads the record columns; dropping the + // `...getTableColumns(records)` spread would 200 with undefined title/status + // and a `/api/records/undefined` self link, yet keep the suite green. + expect(selectMock).toHaveBeenCalledWith( + expect.objectContaining({ + uuid: records.uuid, + status: records.status, + sourceType: sources.type, + }), + ); + }); }); describe("GET /api/records/:uuid", () => {