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..dde6720 100644 --- a/app/composables/useRecords.ts +++ b/app/composables/useRecords.ts @@ -17,6 +17,11 @@ export type RecordAttributes = { content: string; sourceId: string | null; source: string | 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; @@ -181,29 +186,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..33bbaf4 100644 --- a/tests/composables/useRecords.test.ts +++ b/tests/composables/useRecords.test.ts @@ -95,34 +95,65 @@ 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"); + }); + + // 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", + ); }); }); @@ -255,8 +286,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 fb9b5d0..87a7b68 100644 --- a/tests/server/api/records/list.test.ts +++ b/tests/server/api/records/list.test.ts @@ -26,6 +26,12 @@ vi.mock("drizzle-orm", () => ({ count: () => ({ count: true }), desc: (column: unknown) => ({ desc: column }), eq: (column: unknown, value: unknown) => ({ eq: { column, value } }), + // 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 }, }), @@ -78,7 +84,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. @@ -97,6 +104,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 }; } @@ -112,7 +126,14 @@ function stubSelectResults( return { from: pageFrom }; }); - return { countWhere, pageWhere, sourceSubFrom, sourceSubWhere, cursorWhere }; + return { + countWhere, + pageWhere, + pageLeftJoin, + sourceSubFrom, + sourceSubWhere, + cursorWhere, + }; } function stubRequireUser(returnedUserId: string | undefined) { @@ -372,6 +393,65 @@ 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"); + }); + + 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/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..6d5bed2 100644 --- a/tests/server/api/records/show.test.ts +++ b/tests/server/api/records/show.test.ts @@ -1,5 +1,41 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { H3Event } from "h3"; +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(); @@ -49,9 +85,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 +123,45 @@ describe("findRecordForUser", () => { expect(result).toBeNull(); }); + + 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); + + expect(leftJoin.mock.calls[0]?.[0]).toBe(sources); + + // 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); + }); + + 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", () => { @@ -106,6 +182,7 @@ describe("GET /api/records/:uuid", () => { content: sampleRecord.content, sourceId: null, source: null, + sourceType: null, status: "pending", filePath: null, tags: null, @@ -118,6 +195,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 = {