diff --git a/apps/app/app/(app)/[slug]/companies/create-company-sheet.tsx b/apps/app/app/(app)/[slug]/companies/create-company-sheet.tsx index 1040c8ea0..59405fad7 100644 --- a/apps/app/app/(app)/[slug]/companies/create-company-sheet.tsx +++ b/apps/app/app/(app)/[slug]/companies/create-company-sheet.tsx @@ -33,6 +33,7 @@ import { parseAsBoolean, useQueryState } from "nuqs"; import { type ComponentProps, Suspense, useId, useState } from "react"; import { toast } from "sonner"; import { useOpenRecord } from "@/components/crm/record-sheet/record-stack"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useCrmCache } from "@/lib/trpc/cache"; import { useTRPC } from "@/lib/trpc/client"; @@ -61,7 +62,7 @@ function CreateCompanyForm() { const cache = useCrmCache(); const [open, setOpen] = useQueryState( - "new", + SEARCH_PARAM.dialog.create, parseAsBoolean.withDefault(false), ); const [name, setName] = useState(""); diff --git a/apps/app/app/(app)/[slug]/contacts/create-contact-sheet.tsx b/apps/app/app/(app)/[slug]/contacts/create-contact-sheet.tsx index 1494143e4..adbe905d0 100644 --- a/apps/app/app/(app)/[slug]/contacts/create-contact-sheet.tsx +++ b/apps/app/app/(app)/[slug]/contacts/create-contact-sheet.tsx @@ -29,6 +29,7 @@ import { type ComponentProps, Suspense, useId, useState } from "react"; import { toast } from "sonner"; import { CompanyPicker } from "@/components/crm/company-picker"; import { useOpenRecord } from "@/components/crm/record-sheet/record-stack"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useCrmCache } from "@/lib/trpc/cache"; import { useTRPC } from "@/lib/trpc/client"; @@ -57,7 +58,7 @@ function CreateContactForm({ companyId }: { companyId?: string }) { const cache = useCrmCache(); const [open, setOpen] = useQueryState( - "new", + SEARCH_PARAM.dialog.create, parseAsBoolean.withDefault(false), ); const [firstName, setFirstName] = useState(""); diff --git a/apps/app/app/(app)/[slug]/dashboard-summary.tsx b/apps/app/app/(app)/[slug]/dashboard-summary.tsx index f1d5138ae..a14e0428c 100644 --- a/apps/app/app/(app)/[slug]/dashboard-summary.tsx +++ b/apps/app/app/(app)/[slug]/dashboard-summary.tsx @@ -37,6 +37,7 @@ import { useOpenRecord } from "@/components/crm/record-sheet/record-stack"; import { LocalRelativeTime } from "@/components/local-date-time"; import { activityLabel } from "@/lib/activity-presentation"; import { dealStageColor } from "@/lib/deal-stage"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useCrmCache } from "@/lib/trpc/cache"; import { useTRPC } from "@/lib/trpc/client"; import { useWorkspaceUrl } from "@/lib/use-workspace-url"; @@ -94,7 +95,10 @@ export function DashboardSummary() { const openRecord = useOpenRecord(); const workspaceUrl = useWorkspaceUrl(); - const [scope] = useQueryState("scope", overviewParsers.scope); + const [scope] = useQueryState( + SEARCH_PARAM.overview.scope, + overviewParsers[SEARCH_PARAM.overview.scope], + ); const summaryQuery = useQuery({ ...trpc.dashboard.summary.queryOptions({ scope }), diff --git a/apps/app/app/(app)/[slug]/deals/create-deal-sheet.tsx b/apps/app/app/(app)/[slug]/deals/create-deal-sheet.tsx index 6d3e57b85..fcdd96fe8 100644 --- a/apps/app/app/(app)/[slug]/deals/create-deal-sheet.tsx +++ b/apps/app/app/(app)/[slug]/deals/create-deal-sheet.tsx @@ -37,6 +37,7 @@ import { toast } from "sonner"; import { CompanyPicker } from "@/components/crm/company-picker"; import { useOpenRecord } from "@/components/crm/record-sheet/record-stack"; import { dealStageLabel, OPEN_STAGES } from "@/lib/deal-stage"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useCrmCache } from "@/lib/trpc/cache"; import { useTRPC } from "@/lib/trpc/client"; @@ -65,7 +66,7 @@ function CreateDealForm({ companyId }: { companyId?: string }) { const cache = useCrmCache(); const [open, setOpen] = useQueryState( - "new", + SEARCH_PARAM.dialog.create, parseAsBoolean.withDefault(false), ); const [name, setName] = useState(""); diff --git a/apps/app/app/(app)/[slug]/overview-greeting.tsx b/apps/app/app/(app)/[slug]/overview-greeting.tsx index a8a1f06af..ff92369ed 100644 --- a/apps/app/app/(app)/[slug]/overview-greeting.tsx +++ b/apps/app/app/(app)/[slug]/overview-greeting.tsx @@ -2,6 +2,7 @@ import { useQueryState } from "nuqs"; import { PageShellDescription, PageShellTitle } from "@/components/page-shell"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { overviewParsers } from "./overview-search-params"; export function OverviewGreetingFallback() { @@ -16,7 +17,10 @@ export function OverviewGreetingFallback() { } export function OverviewGreeting() { - const [scope] = useQueryState("scope", overviewParsers.scope); + const [scope] = useQueryState( + SEARCH_PARAM.overview.scope, + overviewParsers[SEARCH_PARAM.overview.scope], + ); return ( <> diff --git a/apps/app/app/(app)/[slug]/overview-scope.tsx b/apps/app/app/(app)/[slug]/overview-scope.tsx index dcc0632c2..e1993fd2c 100644 --- a/apps/app/app/(app)/[slug]/overview-scope.tsx +++ b/apps/app/app/(app)/[slug]/overview-scope.tsx @@ -2,6 +2,7 @@ import { ToggleGroup, ToggleGroupItem } from "@crm/ui/components/toggle-group"; import { useQueryState } from "nuqs"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { OVERVIEW_SCOPES, type OverviewScope, @@ -37,7 +38,10 @@ export function OverviewScopeToggleFallback() { } export function OverviewScopeToggle() { - const [scope, setScope] = useQueryState("scope", overviewParsers.scope); + const [scope, setScope] = useQueryState( + SEARCH_PARAM.overview.scope, + overviewParsers[SEARCH_PARAM.overview.scope], + ); return ( { diff --git a/apps/app/components/crm/record-sheet/record-stack.ts b/apps/app/components/crm/record-sheet/record-stack.ts index 31dcaff3d..d33244434 100644 --- a/apps/app/components/crm/record-sheet/record-stack.ts +++ b/apps/app/components/crm/record-sheet/record-stack.ts @@ -7,10 +7,8 @@ import { useQueryStates, } from "nuqs"; import { useCallback, useMemo } from "react"; -import { - TIMELINE_PARAM, - timelineTabParser, -} from "@/components/crm/timeline/timeline-search-params"; +import { timelineTabParser } from "@/components/crm/timeline/timeline-search-params"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; const RECORD_KINDS = ["company", "contact", "deal"] as const; @@ -28,13 +26,15 @@ const FORM_TAB = { } satisfies Record; const params = { - record: parseAsArrayOf(parseAsString, ",").withDefault([]), - tab: parseAsString, - add: parseAsStringLiteral(RECORD_FORMS), - thread: parseAsString, - fields: parseAsStringLiteral(RECORD_KINDS), - field: parseAsString, - [TIMELINE_PARAM]: timelineTabParser, + [SEARCH_PARAM.record.stack]: parseAsArrayOf(parseAsString, ",").withDefault( + [], + ), + [SEARCH_PARAM.record.tab]: parseAsString, + [SEARCH_PARAM.record.add]: parseAsStringLiteral(RECORD_FORMS), + [SEARCH_PARAM.record.thread]: parseAsString, + [SEARCH_PARAM.fieldsSheet.entity]: parseAsStringLiteral(RECORD_KINDS), + [SEARCH_PARAM.fieldsSheet.field]: parseAsString, + [SEARCH_PARAM.record.timeline]: timelineTabParser, }; export function recordKey(ref: RecordRef): string { @@ -51,7 +51,8 @@ function parseRef(raw: string): RecordRef | null { } export function useRecordStack() { - const [{ record }, setParams] = useQueryStates(params); + const [values, setParams] = useQueryStates(params); + const record = values[SEARCH_PARAM.record.stack]; const stack = useMemo( () => record.map(parseRef).filter((ref): ref is RecordRef => ref !== null), @@ -62,13 +63,14 @@ export function useRecordStack() { (next: RecordRef[], history: "push" | "replace") => { void setParams( { - record: next.length === 0 ? null : next.map(recordKey), - tab: null, - add: null, - thread: null, - fields: null, - field: null, - [TIMELINE_PARAM]: null, + [SEARCH_PARAM.record.stack]: + next.length === 0 ? null : next.map(recordKey), + [SEARCH_PARAM.record.tab]: null, + [SEARCH_PARAM.record.add]: null, + [SEARCH_PARAM.record.thread]: null, + [SEARCH_PARAM.fieldsSheet.entity]: null, + [SEARCH_PARAM.fieldsSheet.field]: null, + [SEARCH_PARAM.record.timeline]: null, }, { history }, ); @@ -108,50 +110,66 @@ export function useOpenRecord() { } export function useFieldsSheet() { - const [{ fields, field }, setParams] = useQueryStates(params); + const [values, setParams] = useQueryStates(params); + const entity = values[SEARCH_PARAM.fieldsSheet.entity]; + const field = values[SEARCH_PARAM.fieldsSheet.field]; const open = useCallback( - (kind: RecordKind) => void setParams({ fields: kind, field: null }), + (kind: RecordKind) => + void setParams({ + [SEARCH_PARAM.fieldsSheet.entity]: kind, + [SEARCH_PARAM.fieldsSheet.field]: null, + }), [setParams], ); const close = useCallback( - () => void setParams({ fields: null, field: null }), + () => + void setParams({ + [SEARCH_PARAM.fieldsSheet.entity]: null, + [SEARCH_PARAM.fieldsSheet.field]: null, + }), [setParams], ); const edit = useCallback( - (key: string | null) => void setParams({ field: key }), + (key: string | null) => + void setParams({ [SEARCH_PARAM.fieldsSheet.field]: key }), [setParams], ); - return { entity: fields, field, open, close, edit }; + return { entity, field, open, close, edit }; } export function useRecordSheetView(fallbackTab: string) { - const [{ tab, add, thread }, setParams] = useQueryStates(params); + const [values, setParams] = useQueryStates(params); + const tab = values[SEARCH_PARAM.record.tab]; + const add = values[SEARCH_PARAM.record.add]; + const thread = values[SEARCH_PARAM.record.thread]; const active = add ? FORM_TAB[add] : (tab ?? fallbackTab); const setTab = useCallback( (next: string) => { void setParams({ - tab: next === fallbackTab ? null : next, - add: null, - thread: null, - [TIMELINE_PARAM]: null, + [SEARCH_PARAM.record.tab]: next === fallbackTab ? null : next, + [SEARCH_PARAM.record.add]: null, + [SEARCH_PARAM.record.thread]: null, + [SEARCH_PARAM.record.timeline]: null, }); }, [setParams, fallbackTab], ); const setForm = useCallback( - (next: RecordForm | null) => void setParams({ add: next }), + (next: RecordForm | null) => + void setParams({ [SEARCH_PARAM.record.add]: next }), [setParams], ); const setThread = useCallback( - (next: string | null) => void setParams({ thread: next }), + (next: string | null) => + void setParams({ [SEARCH_PARAM.record.thread]: next }), [setParams], ); diff --git a/apps/app/components/crm/stage-change.tsx b/apps/app/components/crm/stage-change.tsx index e7a580d5a..4c4dc590e 100644 --- a/apps/app/components/crm/stage-change.tsx +++ b/apps/app/components/crm/stage-change.tsx @@ -27,13 +27,14 @@ import { parseAsString, useQueryStates } from "nuqs"; import { useId, useState } from "react"; import { toast } from "sonner"; import { DEAL_STAGE_OPTIONS, LOSING_STAGES } from "@/lib/deal-stage"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useCrmCache } from "@/lib/trpc/cache"; import { useTRPC } from "@/lib/trpc/client"; import { DealStageIndicator } from "./deal-stage"; const closeReasonParams = { - closing: parseAsString, - closingStage: parseAsString, + [SEARCH_PARAM.dialog.closeDeal]: parseAsString, + [SEARCH_PARAM.dialog.closeStage]: parseAsString, }; function useStageMutation(onDone?: () => void) { @@ -99,8 +100,8 @@ export function DealStageMenu({ if (chosen === stage) return; if (LOSING_STAGES.includes(chosen)) { void setCloseParams({ - closing: dealId, - closingStage: chosen, + [SEARCH_PARAM.dialog.closeDeal]: dealId, + [SEARCH_PARAM.dialog.closeStage]: chosen, }); return; } @@ -120,13 +121,17 @@ export function DealStageMenu({ export function CloseReasonDialog() { const reasonId = useId(); - const [{ closing, closingStage }, setCloseParams] = - useQueryStates(closeReasonParams); + const [closeValues, setCloseParams] = useQueryStates(closeReasonParams); + const closing = closeValues[SEARCH_PARAM.dialog.closeDeal]; + const closingStage = closeValues[SEARCH_PARAM.dialog.closeStage]; const [reason, setReason] = useState(""); const close = () => { setReason(""); - void setCloseParams({ closing: null, closingStage: null }); + void setCloseParams({ + [SEARCH_PARAM.dialog.closeDeal]: null, + [SEARCH_PARAM.dialog.closeStage]: null, + }); }; const setStage = useStageMutation(() => { diff --git a/apps/app/components/crm/timeline/timeline-search-params.ts b/apps/app/components/crm/timeline/timeline-search-params.ts index 0fbe4e48a..d31e56cc6 100644 --- a/apps/app/components/crm/timeline/timeline-search-params.ts +++ b/apps/app/components/crm/timeline/timeline-search-params.ts @@ -11,8 +11,6 @@ export const TIMELINE_TABS = [ export type TimelineTab = (typeof TIMELINE_TABS)[number]; -export const TIMELINE_PARAM = "timeline"; - export const timelineTabParser = parseAsStringLiteral(TIMELINE_TABS).withDefault("all"); diff --git a/apps/app/components/crm/timeline/timeline.tsx b/apps/app/components/crm/timeline/timeline.tsx index e84d12c81..a19a11ebd 100644 --- a/apps/app/components/crm/timeline/timeline.tsx +++ b/apps/app/components/crm/timeline/timeline.tsx @@ -14,13 +14,13 @@ import { cn } from "@crm/ui/lib/utils"; import { useInfiniteQuery, useQuery } from "@tanstack/react-query"; import { useQueryState } from "nuqs"; import { DetailSheetEmpty, SECTION_TITLE } from "@/components/detail-sheet"; +import { SEARCH_PARAM } from "@/lib/search-param-keys"; import { useTRPC } from "@/lib/trpc/client"; import { useHydrated } from "@/lib/use-hydrated"; import { ActivityComposer } from "./activity-composer"; import { TimelineEntry, type TimelineEntryData } from "./timeline-entry"; import { historyFilter, - TIMELINE_PARAM, TIMELINE_TABS, type TimelineTab, timelineTabParser, @@ -158,7 +158,10 @@ export function Timeline({ anchor }: { anchor: TimelineAnchor }) { const trpc = useTRPC(); const hydrated = useHydrated(); - const [tab, setTab] = useQueryState(TIMELINE_PARAM, timelineTabParser); + const [tab, setTab] = useQueryState( + SEARCH_PARAM.record.timeline, + timelineTabParser, + ); const counts = useQuery(trpc.activities.timelineCounts.queryOptions(anchor)); diff --git a/apps/app/components/data-table/list-search-params.ts b/apps/app/components/data-table/list-search-params.ts index b0fd45921..d5a899f19 100644 --- a/apps/app/components/data-table/list-search-params.ts +++ b/apps/app/components/data-table/list-search-params.ts @@ -12,6 +12,10 @@ import { parseAsStringLiteral, } from "nuqs/server"; import { z } from "zod"; +import { + assertUnreservedSearchParamKeys, + SEARCH_PARAM, +} from "@/lib/search-param-keys"; const SORT_DIRECTIONS = ["asc", "desc"] as const; @@ -23,10 +27,14 @@ const fieldFiltersSchema = z.record(z.string(), z.array(z.string())); export type FieldFilters = z.infer; export const searchParsers = { - q: parseAsString.withDefault(""), - page: parseAsInteger.withDefault(1).withOptions({ history: "push" }), - fields: parseAsJson(fieldFiltersSchema.parse).withDefault({}), - archived: parseAsBoolean.withDefault(false), + [SEARCH_PARAM.list.q]: parseAsString.withDefault(""), + [SEARCH_PARAM.list.page]: parseAsInteger + .withDefault(1) + .withOptions({ history: "push" }), + [SEARCH_PARAM.list.fields]: parseAsJson( + fieldFiltersSchema.parse, + ).withDefault({}), + [SEARCH_PARAM.list.archived]: parseAsBoolean.withDefault(false), }; type ListParsers = { @@ -91,6 +99,11 @@ export function createListSearchParams< facetDefaults, } = config; + assertUnreservedSearchParamKeys( + [...(tabId ? [tabId] : []), ...facetIds], + "createListSearchParams", + ); + const tabExtras: Record = {}; if (tabId) tabExtras[tabId] = parseAsString.withDefault("all"); @@ -103,8 +116,9 @@ export function createListSearchParams< const parsers = { ...searchParsers, - sort: parseAsString.withDefault(defaultSort), - dir: parseAsStringLiteral(SORT_DIRECTIONS).withDefault(defaultDir), + [SEARCH_PARAM.list.sort]: parseAsString.withDefault(defaultSort), + [SEARCH_PARAM.list.dir]: + parseAsStringLiteral(SORT_DIRECTIONS).withDefault(defaultDir), ...tabExtras, ...facetExtras, } as ListParsers; diff --git a/apps/app/lib/search-param-keys.test.ts b/apps/app/lib/search-param-keys.test.ts new file mode 100644 index 000000000..123a040d4 --- /dev/null +++ b/apps/app/lib/search-param-keys.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "bun:test"; +import { companiesSearchParams } from "@/app/(app)/[slug]/companies/companies-search-params"; +import { contactsSearchParams } from "@/app/(app)/[slug]/contacts/contacts-search-params"; +import { dealsSearchParams } from "@/app/(app)/[slug]/deals/deals-search-params"; +import { membersSearchParams } from "@/app/(app)/[slug]/settings/members/members-search-params"; +import { + assertUnreservedSearchParamKeys, + RESERVED_SEARCH_PARAM_KEYS, + SEARCH_PARAM, +} from "./search-param-keys"; + +const registeredKeys = Object.values(SEARCH_PARAM).flatMap((group) => + Object.values(group), +); + +describe("SEARCH_PARAM", () => { + it("gives every feature its own url key", () => { + expect(registeredKeys.length).toBe(RESERVED_SEARCH_PARAM_KEYS.size); + }); + + it("keeps the fields sheet off the table's fields filter", () => { + expect(SEARCH_PARAM.fieldsSheet.entity).not.toBe(SEARCH_PARAM.list.fields); + }); +}); + +describe("assertUnreservedSearchParamKeys", () => { + it("accepts keys no other feature owns", () => { + expect(() => + assertUnreservedSearchParamKeys(["owner", "industry"], "test"), + ).not.toThrow(); + }); + + it("rejects a facet that shadows a reserved key", () => { + expect(() => + assertUnreservedSearchParamKeys( + ["owner", SEARCH_PARAM.dialog.closeDeal], + "test", + ), + ).toThrow(/closeDeal/); + }); + + it("rejects a facet that shadows a list key", () => { + expect(() => assertUnreservedSearchParamKeys(["q"], "test")).toThrow(/q/); + }); +}); + +describe("list tables", () => { + it("builds every table without a key collision", () => { + for (const table of [ + companiesSearchParams, + contactsSearchParams, + dealsSearchParams, + membersSearchParams, + ]) { + expect(Object.keys(table.parsers)).toContain(SEARCH_PARAM.list.fields); + } + }); +}); diff --git a/apps/app/lib/search-param-keys.ts b/apps/app/lib/search-param-keys.ts new file mode 100644 index 000000000..db9d288fc --- /dev/null +++ b/apps/app/lib/search-param-keys.ts @@ -0,0 +1,45 @@ +export const SEARCH_PARAM = { + list: { + q: "q", + sort: "sort", + dir: "dir", + page: "page", + fields: "fields", + archived: "archived", + }, + record: { + stack: "record", + tab: "tab", + add: "add", + thread: "thread", + timeline: "timeline", + }, + fieldsSheet: { + entity: "manageFields", + field: "manageField", + }, + dialog: { + create: "new", + switcher: "k", + closeDeal: "closeDeal", + closeStage: "closeStage", + }, + overview: { + scope: "scope", + }, +} as const; + +export const RESERVED_SEARCH_PARAM_KEYS: ReadonlySet = new Set( + Object.values(SEARCH_PARAM).flatMap((group) => Object.values(group)), +); + +export function assertUnreservedSearchParamKeys( + keys: readonly string[], + owner: string, +): void { + const clashes = keys.filter((key) => RESERVED_SEARCH_PARAM_KEYS.has(key)); + if (clashes.length === 0) return; + throw new Error( + `[${owner}] search param keys already belong to another feature: ${clashes.join(", ")}. Two parsers on one key corrupt each other. Rename the key or add it to SEARCH_PARAM in lib/search-param-keys.ts.`, + ); +} diff --git a/docs/plan/dynamic-fields-build.md b/docs/plan/dynamic-fields-build.md index d50b11742..2d708e0ff 100644 --- a/docs/plan/dynamic-fields-build.md +++ b/docs/plan/dynamic-fields-build.md @@ -41,11 +41,11 @@ Paper file **CRM**, page **crm - lewis**: | `/companies?record=company:abcd` | Cog in the DETAILS header; custom fields inline; a pending agent suggestion | | `/contacts?record=contact:abcd` | Cog placement only | | `/deals?record=deal:abcd` | Cog placement only | -| `…&fields=company` | The fields sheet — list state | -| `…&fields=company (first run)` | Empty state | -| `…&fields=company&field=new` | Create a field | -| `…&fields=company&field=runs_on` | Edit a field, with coverage | -| `…&fields=company&field=runs_on (archive)` | Archive confirmation | +| `…&manageFields=company` | The fields sheet — list state | +| `…&manageFields=company (first run)` | Empty state | +| `…&manageFields=company&manageField=new` | Create a field | +| `…&manageFields=company&manageField=runs_on` | Edit a field, with coverage | +| `…&manageFields=company&manageField=runs_on (archive)` | Archive confirmation | **Take values from the file, not from screenshots.** `get_jsx`, `get_computed_styles`, `get_node_info`. A screenshot will not tell you whether a diff --git a/docs/plan/dynamic-fields.md b/docs/plan/dynamic-fields.md index 40b1f5849..3fbc73b19 100644 --- a/docs/plan/dynamic-fields.md +++ b/docs/plan/dynamic-fields.md @@ -4,7 +4,7 @@ Fields a workspace defines for itself, on companies, contacts and deals, edited from one sheet that opens from any record. The visual design is in Paper, file **CRM**, page **crm - lewis**, artboards -`Dynamic fields — cog placement`, `/companies?record=company:abcd&fields=company`, +`Dynamic fields — cog placement`, `/companies?record=company:abcd&manageFields=company`, `Dynamic fields — new field & empty` and `Dynamic fields — on the record`. This document is the half Paper cannot hold: the model, the API and the agent. @@ -39,7 +39,7 @@ deal can still fix a contact field without closing anything. State lives in the URL beside `record`, in `record-stack.ts`: ``` -/companies?record=company:abcd&fields=company +/companies?record=company:abcd&manageFields=company ``` so it is shareable, Escape and Back close it in the right order, and the record diff --git a/docs/setup.md b/docs/setup.md index 28c1410d3..03b8912f7 100644 --- a/docs/setup.md +++ b/docs/setup.md @@ -148,3 +148,30 @@ never reuse one from an example, a tutorial, or another environment. bun run --filter=api test bun run --filter=agent test # integration specs need DATABASE_URL + real Postgres ``` + +### The test database rebuilds itself when it drifts + +`bun run db:test` creates `crm_test` and runs `migrate deploy` on it. The database +name must end in `_test`; the suite deletes rows it expects to put back, so it +refuses anything else. + +**`migrate deploy` only applies migrations that are missing. It never removes a +table, a column or a constraint the database has and the schema does not.** A +`crm_test` built on a branch that was later abandoned therefore keeps that branch's +objects forever, and `db:test` used to report `already exists` and move on. The +extra objects are invisible until one of them rejects a write, and then the failure +names a constraint that appears in no migration and in no schema — a stray +`trackedEvent_visitorId_fkey` once failed seven tracking specs this way, on every +branch, for as long as the database survived. + +So `db:test` now checks the database it found and rebuilds it when either is true: + +- **It holds a migration this branch does not have.** The database came from + another branch. The name of the first one is printed. +- **It no longer matches `schema.prisma`**, by `prisma migrate diff`. Something + was pushed or altered by hand. + +A rebuild drops the database and re-runs every migration, and it says which of the +two reasons fired. Force one with `bun run db:test --reset`. Nothing else in the +repo may drop a database, and this may only because the `_test` suffix is checked +first. diff --git a/packages/db/scripts/test-db.ts b/packages/db/scripts/test-db.ts index e132c65da..7deb36e5f 100644 --- a/packages/db/scripts/test-db.ts +++ b/packages/db/scripts/test-db.ts @@ -1,6 +1,11 @@ import { spawnSync } from "node:child_process"; +import { existsSync, readdirSync } from "node:fs"; +import { dirname, join } from "node:path"; import pg from "pg"; +const SCHEMA = join(dirname(import.meta.dirname), "prisma", "schema.prisma"); +const MIGRATIONS = join(dirname(import.meta.dirname), "prisma", "migrations"); + const url = resolve(); if (!url) { @@ -20,7 +25,7 @@ if (!name.endsWith("_test")) { ]); } -await create(url, name); +await create(url, name, process.argv.includes("--reset")); migrate(url); if (!process.env.TEST_DATABASE_URL) { @@ -35,7 +40,11 @@ if (!process.env.TEST_DATABASE_URL) { ); } -async function create(target: string, database: string): Promise { +async function create( + target: string, + database: string, + forced: boolean, +): Promise { const maintenance = new URL(target); maintenance.pathname = "/postgres"; maintenance.search = ""; @@ -60,8 +69,17 @@ async function create(target: string, database: string): Promise { ); if (existing.rowCount) { - console.log(` ${database} already exists`); - return; + const reason = forced + ? "you asked for --reset" + : await stale(target, database); + + if (!reason) { + console.log(` ${database} already exists`); + return; + } + + console.log(` rebuilding ${database}: ${reason}`); + await drop(client, database); } await client.query(`CREATE DATABASE "${database}"`); @@ -71,12 +89,92 @@ async function create(target: string, database: string): Promise { } } +async function drop(client: pg.Client, database: string): Promise { + await client.query( + `SELECT pg_terminate_backend(pid) FROM pg_stat_activity + WHERE datname = $1 AND pid <> pg_backend_pid()`, + [database], + ); + await client.query(`DROP DATABASE IF EXISTS "${database}"`); +} + +async function stale(target: string, database: string): Promise { + const applied = await appliedMigrations(target); + + if (applied === null) return null; + + const onDisk = new Set( + existsSync(MIGRATIONS) + ? readdirSync(MIGRATIONS, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + : [], + ); + + const foreign = applied.filter((migration) => !onDisk.has(migration)); + + if (foreign.length > 0) { + return `${database} holds ${foreign.length} migration(s) this branch does not have, starting with ${foreign[0]}`; + } + + return drifted(target) ? `${database} no longer matches schema.prisma` : null; +} + +async function appliedMigrations(target: string): Promise { + const client = new pg.Client({ connectionString: target }); + + try { + await client.connect(); + } catch { + return null; + } + + try { + const rows = await client.query<{ migration_name: string }>( + `SELECT migration_name FROM _prisma_migrations WHERE finished_at IS NOT NULL`, + ); + return rows.rows.map((row) => row.migration_name); + } catch { + return null; + } finally { + await client.end(); + } +} + +function drifted(target: string): boolean { + const result = spawnSync( + "prisma", + [ + "migrate", + "diff", + "--from-config-datasource", + "--to-schema", + SCHEMA, + "--exit-code", + ], + { stdio: "ignore", env: { ...process.env, DATABASE_URL: target } }, + ); + + return result.status === 2; +} + function migrate(target: string): void { const result = spawnSync("prisma", ["migrate", "deploy"], { stdio: "inherit", env: { ...process.env, DATABASE_URL: target }, }); + if (result.error) { + fail([ + "Could not run prisma migrate deploy.", + "Run this through the package script, which puts prisma on PATH:", + "", + " bun run db:test", + "", + result.error.message, + ]); + } + if (result.status !== 0) process.exit(result.status ?? 1); }