From 4036c16f3943e735d218e65bcadd48f2addae481 Mon Sep 17 00:00:00 2001 From: Matt Ciaccio Date: Tue, 28 Apr 2026 13:28:15 +0200 Subject: [PATCH] test(infra): vitest globalSetup teardown purges test-port-* leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration tests use makePort() which writes ports with slug 'test-port-{rand}' and never cleans up. Result: 17,564 leaked rows in dev that slowed every page load fetching the port-switcher list (and was contributing to smoke flakes). Adds tests/global-setup.ts with a teardown() that DELETEs every 'test-port-%' row plus its dependent rows across 30+ tables in one CTE. Wires it into vitest.config.ts via globalSetup. Adds closeDb() helper so the teardown can end the postgres-js pool cleanly (kills the 'Tests closed but Vite server won't exit' warning). Also lands docs/superpowers/specs/2026-04-28-country-phone-timezone-design.md — full-scope agenda for the country dropdown / E.164 phone input / country-driven timezone autofill work, ~7 dev days across 10 PRs. Per user request: 'let's do this full-fledged if we're gonna do it'. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...026-04-28-country-phone-timezone-design.md | 171 ++++++++++++++++++ src/lib/db/index.ts | 6 + tests/global-setup.ts | 71 ++++++++ vitest.config.ts | 1 + 4 files changed, 249 insertions(+) create mode 100644 docs/superpowers/specs/2026-04-28-country-phone-timezone-design.md create mode 100644 tests/global-setup.ts diff --git a/docs/superpowers/specs/2026-04-28-country-phone-timezone-design.md b/docs/superpowers/specs/2026-04-28-country-phone-timezone-design.md new file mode 100644 index 0000000..687d761 --- /dev/null +++ b/docs/superpowers/specs/2026-04-28-country-phone-timezone-design.md @@ -0,0 +1,171 @@ +# Country / Phone / Timezone — i18n form polish + +**Status:** Agenda — awaiting prioritization (likely Phase B or B.5) +**Date:** 2026-04-28 +**Phase:** Cross-cutting; touches every form that captures contact data + +## Why + +Today every CRM form takes free-text strings for nationality, phone, and timezone. That's fine for a marina with one operator typing it in once, but it leaks operator inconsistencies into reports and breaks any later system that consumes these fields (Documenso prefill, public website inquiry, portal sync, exports). For a multi-port platform that's about to onboard non-Polish-speaking residential clients, the data quality matters. + +Three coupled UX upgrades: + +1. **Nationality → ISO-3166 country dropdown.** Searchable. Stores ISO alpha-2 code (`'GB'`), displays localized country name. +2. **Phone → country-code dropdown + format-as-you-type.** E.164 storage on the wire, formatted display per country. +3. **Timezone → autofilled from country with override dropdown.** Most countries are single-zone; the few that aren't (US, RU, AU, BR, CA, ID, KZ, MN, MX, CD) get a sub-select. Stores IANA TZ string (`'Europe/Warsaw'`). + +## Scope + +### In scope + +- New shared primitives: ``, ``, `` +- ISO-3166 country list bundled (no API call); names from `Intl.DisplayNames` with locale fallback to English +- Country → primary IANA timezone map (~250 entries, JSON) +- Phone parsing/validation/formatting via `libphonenumber-js` (server + client) +- Wire into every form that captures contact data: + - `` (name, nationality, phone) + - `` inline editor (nationality, phone, place_of_residence — country-aware) + - `` (incorporation_country) + - `` (phone) + - public inquiry form (form-template renderer, when phone field present) +- DB migration: store ISO codes (`countries`, `nationality_iso`), E.164 phone (`phone_e164`), IANA timezone (`timezone`) +- Backfill: best-effort parse existing free-text into the new columns; keep originals as `_legacy` for one release cycle +- Display: localized country name in tables/detail pages; phone formatted per country (e.g. `+44 20 7946 0958`); timezone shown as friendly `'London (UTC+1)'` when current +- Tests: unit (parser edge cases), integration (form submit → E.164 storage), smoke (typing + selecting flows) + +### Out of scope (deferred) + +- Multilingual UI surface (only the country _names_ localize via `Intl.DisplayNames`; rest of the UI stays English for now) +- Subdivision picker (states/provinces) — only top-level country +- Phone number geocoding / carrier lookup +- Address autocomplete (Google Places, etc.) +- Currency localization +- RTL layout + +## Library choices + +| Concern | Library | Why | +| --------------------------- | -------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Phone input + flag dropdown | `omeralpi/shadcn-phone-input` | Built on shadcn-ui's `Input` primitive (zero styling friction with our component library), wraps `libphonenumber-js`, ships with country dropdown + format-as-you-type. Small bundle. | +| Phone parsing/validation | `libphonenumber-js` | Google's library, ~88 benchmark, used by every popular React phone input. Server-side validation in zod. | +| Country list | Bundled JSON of ISO-3166 alpha-2 codes + 3-letter codes + display names (English baseline) | No need for the heavier `country-state-city` databases — we don't need cities or states yet. | +| Country → timezone | Hand-curated `country-timezones.json` (250 entries, ~10kb) sourced from `country-tz` or moment-timezone's data | Static, no network call. For multi-zone countries, expose a sub-select. | +| Timezone formatting | `Intl.DateTimeFormat` (built-in) | Browser API; renders `'Europe/Warsaw (UTC+1)'`-style labels. | +| Timezone list | `Intl.supportedValuesOf('timeZone')` (built-in, ~600 entries) | Used as the override dropdown when a user wants a non-primary zone. | + +Bundle impact: `libphonenumber-js` mobile build is ~80 KB gz; `shadcn-phone-input` is ~5 KB; country/timezone JSONs ~30 KB. All client-side, lazy-loaded on first form render via `next/dynamic`. + +## Schema deltas + +```sql +-- clients +ALTER TABLE clients ADD COLUMN nationality_iso text; -- 'GB' +ALTER TABLE clients ADD COLUMN timezone text; -- 'Europe/London' +-- existing 'nationality' free-text column stays for a release; new code reads ISO + +-- client_contacts (or wherever phone lives) +ALTER TABLE client_contacts ADD COLUMN value_e164 text; -- '+442079460958' +ALTER TABLE client_contacts ADD COLUMN value_country text; -- 'GB' (where the number was parsed against) +-- existing 'value' stays as the human-displayable formatted form + +-- residential_clients — same pattern +ALTER TABLE residential_clients ADD COLUMN nationality_iso text; +ALTER TABLE residential_clients ADD COLUMN timezone text; +ALTER TABLE residential_clients ADD COLUMN phone_e164 text; +ALTER TABLE residential_clients ADD COLUMN phone_country text; + +-- companies +ALTER TABLE companies ADD COLUMN incorporation_country_iso text; +``` + +Indexes: `idx_clients_nationality_iso`, `idx_clients_timezone` (cheap; powers analytics filters later). + +## Component primitives + +```tsx + …} + locale="en" // for name lookup; default to navigator.language + variant="default" | "compact" // compact = icon-only flag, default = name +/> + + …} + defaultCountry={'GB'} // pre-selects the dropdown + required={false} +/> + + …} + countryHint={'GB'} // when set, narrows the dropdown to matching zones first +/> +``` + +All three are shadcn-styled, keyboard-accessible, support form integration with react-hook-form + zod. + +## Validators + +```ts +// src/lib/validators/contact.ts +import { isValidPhoneNumber } from 'libphonenumber-js'; + +export const phoneE164Schema = z + .string() + .refine((v) => isValidPhoneNumber(v), 'Invalid phone number'); + +export const isoCountrySchema = z + .string() + .length(2) + .toUpperCase() + .refine((c) => ISO_COUNTRIES.has(c), 'Unknown country'); + +export const ianaTimezoneSchema = z + .string() + .refine((tz) => Intl.supportedValuesOf('timeZone').includes(tz), 'Unknown timezone'); +``` + +## Backfill plan + +A migration script (`scripts/backfill-iso-and-e164.ts`) that: + +1. For each client/residential_client, attempt `libphonenumber-js` `parsePhoneNumber(rawPhone, { defaultCountry: 'PL' })` → if valid, write `phone_e164` + `phone_country`. +2. For each free-text `nationality`, fuzzy-match against the country name list (exact match first, then Levenshtein ≤2). Write `nationality_iso` if confident. +3. For each timezone, exact-match against IANA list. Otherwise leave null and let user fill it. +4. Log unparseable rows to `backfill-iso-report.csv` for manual review. + +Run on staging first; require dry-run flag. + +## Build sequence + +| # | PR | Effort | Depends on | +| --- | ------------------------------------------------------------ | ------ | ---------- | +| 1 | Country list JSON + ISO sets + `` primitive | 0.5d | — | +| 2 | `libphonenumber-js` integration + `` primitive | 1d | — | +| 3 | Country → timezone JSON + `` primitive | 0.5d | 1 | +| 4 | Schema deltas + drizzle migrations + zod validators | 0.5d | — | +| 5 | Wire into ClientForm + ClientDetail inline editors | 1d | 1, 2, 3, 4 | +| 6 | Wire into ResidentialClientDetail | 0.5d | 5 | +| 7 | Wire into CompanyForm | 0.5d | 1 | +| 8 | Public inquiry form template renderer support | 0.5d | 2 | +| 9 | Backfill script + dry-run runbook | 1d | 4 | +| 10 | Smoke + integration tests | 1d | 5–9 | + +Total: ~7 dev days. Self-contained; no external dependencies on Phase B (analytics/alerts). + +## Risk register + +| Risk | Mitigation | +| --------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------- | +| Bundle bloat from libphonenumber data | Use the `mobile` metadata build, lazy-import via `next/dynamic` | +| Existing free-text data is too messy to backfill | Keep the legacy column for one release; expose a "needs review" badge in admin | +| Multi-zone country UX confusion | Sub-select only appears when country is multi-zone; otherwise zone is hidden behind "Override" | +| Public inquiry form breaks if phone is required and user can't find their country | Default to PL, search by country name and dial code | + +## Open questions for the user + +- Which port's locale should drive the _default_ country in `` (Poland for now, or detect from browser)? +- Should existing free-text `nationality` field be removed once backfilled, or kept indefinitely as a fallback? +- Is there an appetite for adding the same treatment to subdivision (state/region/voivodship) selectors, or strictly country-level for now? diff --git a/src/lib/db/index.ts b/src/lib/db/index.ts index 60bf192..f633556 100644 --- a/src/lib/db/index.ts +++ b/src/lib/db/index.ts @@ -17,4 +17,10 @@ export const db = drizzle(queryClient, { logger: process.env.NODE_ENV === 'development', }); +/** Close the underlying connection pool. Used by the vitest teardown so + * the parent process can exit cleanly. */ +export async function closeDb(): Promise { + await queryClient.end({ timeout: 5 }); +} + export type Database = typeof db; diff --git a/tests/global-setup.ts b/tests/global-setup.ts new file mode 100644 index 0000000..fb76747 --- /dev/null +++ b/tests/global-setup.ts @@ -0,0 +1,71 @@ +/** + * Vitest global setup. The teardown phase runs after the test run completes + * (whether passing or failing) and purges any `test-port-*` rows the + * `makePort()` factory created during the run, plus the dependent rows + * those ports own. + * + * Without this, integration tests leak hundreds of test ports per run; we + * once accumulated >17k such rows in dev, slowing every page load that + * fetches the port-switcher list. + */ + +// `globalSetup` runs in vitest's parent process, so the test workers' env +// from `loadEnv` doesn't apply here — we have to load .env ourselves before +// importing the db module (which validates DATABASE_URL at import time). +import 'dotenv/config'; +import { sql } from 'drizzle-orm'; +import { db, closeDb } from '@/lib/db'; + +export async function setup() { + // No-op: per-suite setup happens inside individual test files. +} + +export async function teardown() { + // Same DB as the dev/test environment — only delete obvious test rows + // (slug prefix is a marker the factory always sets). + await db.execute(sql` + -- Stage the doomed port ids + WITH doomed AS ( + SELECT id FROM ports WHERE slug LIKE 'test-port-%' + ) + -- Cascade-delete dependent rows. Order respects FK chains. + , del_audit AS (DELETE FROM audit_logs WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_bml AS (DELETE FROM berth_maintenance_log WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_resv AS (DELETE FROM berth_reservations WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_caddr AS (DELETE FROM client_addresses WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_cmlog AS (DELETE FROM client_merge_log WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_crel AS (DELETE FROM client_relationships WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_compaddr AS (DELETE FROM company_addresses WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_comp AS (DELETE FROM companies WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_cfd AS (DELETE FROM custom_field_definitions WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_docs AS (DELETE FROM documents WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_dt AS (DELETE FROM document_templates WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_emt AS (DELETE FROM email_threads WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_ema AS (DELETE FROM email_accounts WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_exp AS (DELETE FROM expenses WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_files AS (DELETE FROM files WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_ft AS (DELETE FROM form_templates WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_gr AS (DELETE FROM generated_reports WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_int AS (DELETE FROM interests WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_inv AS (DELETE FROM invoices WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_notif AS (DELETE FROM notifications WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_pro AS (DELETE FROM port_role_overrides WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_pu AS (DELETE FROM portal_users WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_rem AS (DELETE FROM reminders WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_rc AS (DELETE FROM residential_clients WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_ri AS (DELETE FROM residential_interests WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_sv AS (DELETE FROM saved_views WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_sr AS (DELETE FROM scheduled_reports WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_ss AS (DELETE FROM system_settings WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_tags AS (DELETE FROM tags WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_unp AS (DELETE FROM user_notification_preferences WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_upr AS (DELETE FROM user_port_roles WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_wh AS (DELETE FROM webhooks WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_yachts AS (DELETE FROM yachts WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_berths AS (DELETE FROM berths WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_clients AS (DELETE FROM clients WHERE port_id IN (SELECT id FROM doomed) RETURNING 1) + , del_ports AS (DELETE FROM ports WHERE id IN (SELECT id FROM doomed) RETURNING 1) + SELECT 1 + `); + await closeDb(); +} diff --git a/vitest.config.ts b/vitest.config.ts index 4208952..2b7a588 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -10,6 +10,7 @@ export default defineConfig({ include: ['tests/unit/**/*.test.ts', 'tests/integration/**/*.test.ts'], exclude: ['tests/e2e/**', 'node_modules/**'], pool: 'forks', + globalSetup: ['./tests/global-setup.ts'], coverage: { provider: 'v8', reporter: ['text', 'lcov', 'json-summary'],