From 3c4551acd9b0d3d64c0454eb67e86bf9b0110ea0 Mon Sep 17 00:00:00 2001 From: Oier Bravo Urtasun Date: Tue, 19 May 2026 02:11:34 +0200 Subject: [PATCH] feat(fase-18): listTitles store + numbering utils + unit + integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `$lib/utils/list-title.ts` is the pure module — `parseTitle` extracts {prefix, number} from a typed value, `nextNumberFromNumbers` is the trivial `max + 1`, and `computeNextNumberFromNames` walks a row list, counts a bare-prefix match as N=0, and falls back to 1 when nothing matches. The regex requires whitespace between prefix and `#` so "Compra#5" stays literal (the chip only fires for catalog-curated prefixes). 19 unit tests cover LT-U-01..05 plus seven boundary cases (multi-word prefixes, case-insensitive matching, substring near-miss, trimming, empty input, bare-hash rejection). `$lib/stores/listTitles.ts` wraps the two SECURITY DEFINER RPCs (add/remove) and exposes `loadTitleCatalog`, `setDefaultListTitle` (plain UPDATE on `collectives` — RLS already gates UPDATE to admins), `fetchTitleSuggestions` (catalog ∪ last 10 distinct shopping_lists.name, dedup case-insensitive, cap 15) and `computeNextNumber` (driver for the SQL `name.ilike.,name.ilike. #%` filter that feeds the pure helper). Race-condition note in the docstring points back to the migration header. Integration test `list-title-flow.test.ts` (6 cases): admin add round- trips into the catalog (LT-INT-01), Borja's member-attempt yields P0002 (LT-INT-02), the computeNextNumber math is end-to-end exercised against the dev DB with 3 seeded "Compra #1/#2/#5" rows → 6 (LT-INT-03), the no-matches path returns 1 (LT-INT-04), admin remove deletes the row (LT-INT-05), and an empty-title insert is rejected with P0001 (LT-INT-06). The pgTAP suite can't drive these because postgres bypasses auth.uid — same caveat documented in Fase 15's common-items.test.ts. Tests: 75 unit (was 56; +19), 185 integration (was 179; +6). Co-Authored-By: Claude Opus 4.7 --- apps/web/src/lib/stores/listTitles.ts | 224 ++++++++++++++++++ apps/web/src/lib/utils/list-title.test.ts | 121 ++++++++++ apps/web/src/lib/utils/list-title.ts | 100 ++++++++ .../test-utils/tests/list-title-flow.test.ts | 154 ++++++++++++ 4 files changed, 599 insertions(+) create mode 100644 apps/web/src/lib/stores/listTitles.ts create mode 100644 apps/web/src/lib/utils/list-title.test.ts create mode 100644 apps/web/src/lib/utils/list-title.ts create mode 100644 packages/test-utils/tests/list-title-flow.test.ts diff --git a/apps/web/src/lib/stores/listTitles.ts b/apps/web/src/lib/stores/listTitles.ts new file mode 100644 index 0000000..31bee66 --- /dev/null +++ b/apps/web/src/lib/stores/listTitles.ts @@ -0,0 +1,224 @@ +/** + * Fase 18: per-collective list-title catalog store + helpers. + * + * Two surfaces: + * + * • The /collective/manage UI loads the full catalog and lets admins + * add/remove entries via the two SECURITY DEFINER RPCs from migration + * 027. Direct writes are all-deny RLS — the RPCs are the only entry + * points. P0002 on non-admin matches the Fase 15 common-items flow. + * + * • The create-list modal queries `fetchTitleSuggestions(collectiveId, + * prefix)` for the autocomplete dropdown — union of the catalog + + * the last 10 distinct `shopping_lists.name` values, dedup'd case- + * insensitively, capped at 15. And `computeNextNumber(collectiveId, + * prefix)` to decide whether to auto-suffix "#N" on submit. + * + * The pure numbering rules live in `$lib/utils/list-title` so they're + * unit-testable without a DB. + */ +import { writable } from 'svelte/store'; +import { getSupabase } from '$lib/supabase'; +import { computeNextNumberFromNames } from '$lib/utils/list-title'; + +export interface CollectiveListTitle { + collective_id: string; + title: string; + created_at: string; +} + +export const listTitleCatalog = writable([]); +export const listTitleCatalogLoading = writable(false); + +// ── Catalog (admin curation) ──────────────────────────────────────────────── + +/** + * Load every curated title for the active collective. Members + guests + * read (the `select_member` policy on `collective_list_titles` includes + * everyone in the collective); only admins write via the RPCs below. + */ +export async function loadTitleCatalog(collectiveId: string): Promise { + listTitleCatalogLoading.set(true); + try { + const { data, error } = await getSupabase() + .from('collective_list_titles') + .select('*') + .eq('collective_id', collectiveId) + .order('title', { ascending: true }); + + if (!error && data) { + listTitleCatalog.set(data as CollectiveListTitle[]); + } + } finally { + listTitleCatalogLoading.set(false); + } +} + +/** + * Admin-only: add a title to the catalog. The RPC trims + rejects empty + + * is idempotent via ON CONFLICT DO NOTHING. Returns the error (or null) + * so the caller can surface it as a toast / inline message. + */ +export async function addTitle( + collectiveId: string, + title: string +): Promise { + const trimmed = title.trim(); + if (trimmed.length === 0) return new Error('title must not be empty'); + + const { error } = await getSupabase().rpc('add_list_title', { + p_collective_id: collectiveId, + p_title: trimmed + }); + if (error) return new Error(error.message); + + // Optimistic local upsert — keep the store sorted by title to match the + // load order. The realtime subscription will overwrite this when the + // INSERT event comes back, but the modal close shouldn't have to wait. + listTitleCatalog.update((arr) => { + if (arr.some((r) => r.title === trimmed)) return arr; + const row: CollectiveListTitle = { + collective_id: collectiveId, + title: trimmed, + created_at: new Date().toISOString() + }; + return [...arr, row].sort((a, b) => a.title.localeCompare(b.title)); + }); + return null; +} + +/** + * Admin-only: remove a title from the catalog. Idempotent — removing a + * row that was never there is a no-op (the DELETE just matches zero rows). + */ +export async function removeTitle( + collectiveId: string, + title: string +): Promise { + const trimmed = title.trim(); + if (trimmed.length === 0) return new Error('title must not be empty'); + + const { error } = await getSupabase().rpc('remove_list_title', { + p_collective_id: collectiveId, + p_title: trimmed + }); + if (error) return new Error(error.message); + + listTitleCatalog.update((arr) => arr.filter((r) => r.title !== trimmed)); + return null; +} + +/** + * Admin-only: persist the new default_list_title on the collective itself. + * Empty string / null clears the default. RLS on `collectives` already + * gates UPDATE to admins — no separate RPC needed. + * + * Caller is responsible for re-fetching the active collective row (or + * letting the features-store realtime subscription do it). + */ +export async function setDefaultListTitle( + collectiveId: string, + value: string | null +): Promise { + const normalized = value === null ? null : value.trim(); + const persisted = normalized && normalized.length > 0 ? normalized : null; + + const { error } = await getSupabase() + .from('collectives') + .update({ default_list_title: persisted }) + .eq('id', collectiveId); + + if (error) return new Error(error.message); + return null; +} + +// ── Modal helpers (autocomplete + numbering) ──────────────────────────────── + +const SUGGESTION_LIMIT = 15; +const RECENT_LISTS_SCAN = 10; + +/** + * Return up to 15 suggestion strings for the create-list autocomplete. + * Union of (a) the curated catalog and (b) the most recent + * RECENT_LISTS_SCAN distinct `shopping_lists.name` values for the active + * collective. Dedup case-insensitively (first occurrence wins). + * + * If `prefix` is non-empty, both sources are filtered case-insensitively + * by prefix; otherwise the union is returned unfiltered. + */ +export async function fetchTitleSuggestions( + collectiveId: string, + prefix: string +): Promise { + const normPrefix = prefix.trim().toLowerCase(); + + const sb = getSupabase(); + const catalogQuery = sb + .from('collective_list_titles') + .select('title') + .eq('collective_id', collectiveId) + .order('title', { ascending: true }); + if (normPrefix.length > 0) { + catalogQuery.ilike('title', `${normPrefix}%`); + } + + const recentQuery = sb + .from('shopping_lists') + .select('name') + .eq('collective_id', collectiveId) + .is('deleted_at', null) + .order('created_at', { ascending: false }) + .limit(RECENT_LISTS_SCAN); + if (normPrefix.length > 0) { + recentQuery.ilike('name', `${normPrefix}%`); + } + + const [catalogRes, recentRes] = await Promise.all([catalogQuery, recentQuery]); + const catalog = (catalogRes.data ?? []).map((r) => r.title); + const recent = (recentRes.data ?? []).map((r) => r.name); + + const seen = new Set(); + const out: string[] = []; + for (const candidate of [...catalog, ...recent]) { + const key = candidate.trim().toLowerCase(); + if (!key || seen.has(key)) continue; + seen.add(key); + out.push(candidate); + if (out.length >= SUGGESTION_LIMIT) break; + } + return out; +} + +/** + * Look at every active `shopping_lists.name` in the collective whose + * prefix matches `basePrefix` (case-insensitive), and return the next + * `#N` (max(N) + 1). Returns 1 when no rows match. + * + * Race-condition note: two clients computing this concurrently may both + * resolve to the same N. Accepted (see migration 027 header). The DB has + * no unique constraint on (collective_id, name) so the duplicate + * "Compra #6" lands without error. + */ +export async function computeNextNumber( + collectiveId: string, + basePrefix: string +): Promise { + const trimmed = basePrefix.trim(); + if (trimmed.length === 0) return 1; + + const sb = getSupabase(); + const orFilter = `name.ilike.${trimmed},name.ilike.${trimmed} #%`; + + const { data, error } = await sb + .from('shopping_lists') + .select('name') + .eq('collective_id', collectiveId) + .is('deleted_at', null) + .or(orFilter); + + if (error || !data) return 1; + return computeNextNumberFromNames( + trimmed, + data.map((r) => r.name) + ); +} diff --git a/apps/web/src/lib/utils/list-title.test.ts b/apps/web/src/lib/utils/list-title.test.ts new file mode 100644 index 0000000..27f6ad1 --- /dev/null +++ b/apps/web/src/lib/utils/list-title.test.ts @@ -0,0 +1,121 @@ +/** + * Pure-function tests for the list-title numbering helpers (Fase 18.2). + * + * The runtime path runs in the browser (modal submit handler), but the + * functions are I/O-free so jsdom is overkill — Vitest's node env is fine. + */ +import { describe, it, expect } from 'vitest'; +import { parseTitle, nextNumberFromNumbers, computeNextNumberFromNames } from './list-title'; + +describe('list-title utils', () => { + describe('parseTitle', () => { + it('LT-U-01: "Compra #5" → { prefix: "Compra", number: 5 }', () => { + expect(parseTitle('Compra #5')).toEqual({ prefix: 'Compra', number: 5 }); + }); + + it('LT-U-02: "Compra" → { prefix: "Compra", number: null }', () => { + expect(parseTitle('Compra')).toEqual({ prefix: 'Compra', number: null }); + }); + + it('LT-U-03: "#5" (no prefix before the hash) → returns null prefix', () => { + // The chip is meaningless without a base prefix, so the parser refuses + // to autonumber bare-hash inputs. The caller treats this as "user + // typed something literal — submit as-is". + expect(parseTitle('#5')).toEqual({ prefix: null, number: null }); + }); + + it('trims surrounding whitespace before parsing', () => { + expect(parseTitle(' Compra #3 ')).toEqual({ prefix: 'Compra', number: 3 }); + }); + + it('handles multi-word prefixes', () => { + expect(parseTitle('Compra semanal #2')).toEqual({ + prefix: 'Compra semanal', + number: 2 + }); + }); + + it('treats "#" without digits as no-number suffix', () => { + // "Compra #" with no trailing digits is just a weird name — the user + // gets exactly that string back, no auto-number magic. + expect(parseTitle('Compra #')).toEqual({ prefix: 'Compra #', number: null }); + }); + + it('returns prefix null on empty input', () => { + expect(parseTitle('')).toEqual({ prefix: null, number: null }); + expect(parseTitle(' ')).toEqual({ prefix: null, number: null }); + }); + }); + + describe('nextNumberFromNumbers', () => { + it('LT-U-04: [1, 2, 5] → 6', () => { + expect(nextNumberFromNumbers([1, 2, 5])).toBe(6); + }); + + it('LT-U-05: [] → 1', () => { + expect(nextNumberFromNumbers([])).toBe(1); + }); + + it('out-of-order list still returns max+1', () => { + expect(nextNumberFromNumbers([7, 2, 4])).toBe(8); + }); + + it('single element returns that+1', () => { + expect(nextNumberFromNumbers([3])).toBe(4); + }); + }); + + describe('computeNextNumberFromNames', () => { + const PREFIX = 'Compra'; + + it('matches the documented LT-U behaviour: bare prefix counts as N=0', () => { + // "Compra" (bare) → N=0, "Compra #1", "Compra #2", "Compra #5" → max=5 + // → next = 6. + expect( + computeNextNumberFromNames(PREFIX, [ + 'Compra', + 'Compra #1', + 'Compra #2', + 'Compra #5' + ]) + ).toBe(6); + }); + + it('returns 1 when no row matches (per CLAUDE-md spec rule 8)', () => { + expect(computeNextNumberFromNames(PREFIX, [])).toBe(1); + expect(computeNextNumberFromNames(PREFIX, ['Unrelated list'])).toBe(1); + }); + + it('bare prefix only (no numbered siblings) → next is 1', () => { + // "Compra" alone counts as N=0 → max + 1 = 1. + expect(computeNextNumberFromNames(PREFIX, ['Compra'])).toBe(1); + }); + + it('numbered siblings only (no bare) → max + 1', () => { + expect(computeNextNumberFromNames(PREFIX, ['Compra #3', 'Compra #4'])).toBe(5); + }); + + it('prefix match is case-insensitive', () => { + expect( + computeNextNumberFromNames('compra', ['Compra #1', 'COMPRA #2', 'compra #4']) + ).toBe(5); + }); + + it('exact prefix-only match (case-insensitive) counts as N=0', () => { + expect(computeNextNumberFromNames('compra', ['Compra'])).toBe(1); + }); + + it('ignores rows where the prefix is a substring but not a real match', () => { + // "Compras del mes" starts with "Compra" but the next char isn't a + // space or "#", so it's a different prefix entirely. Our regex + // requires the post-prefix portion to be empty or " #". + expect( + computeNextNumberFromNames(PREFIX, ['Compras del mes', 'Compra #1']) + ).toBe(2); + }); + + it('trims candidate names before comparison', () => { + expect(computeNextNumberFromNames(PREFIX, [' Compra #2 '])).toBe(3); + }); + }); +}); diff --git a/apps/web/src/lib/utils/list-title.ts b/apps/web/src/lib/utils/list-title.ts new file mode 100644 index 0000000..94801be --- /dev/null +++ b/apps/web/src/lib/utils/list-title.ts @@ -0,0 +1,100 @@ +/** + * Pure helpers for the Fase 18 list-title numbering flow. + * + * Split out of the store so unit tests can run without supabase-js or a + * DOM. The two consumers are: + * + * • the create-list modal (decides whether to show the "Sugerencia: + * Compra #6" chip and whether to auto-suffix on submit), and + * • the listTitles store (which queries `shopping_lists.name` and pipes + * the result through `computeNextNumberFromNames`). + * + * Semantics (per `plan/fase-18-shopping-list-flow.md` rule 8): + * - Match a candidate `name` against a base `prefix` case-insensitively. + * - A bare prefix match (e.g. "Compra") counts as N=0. + * - A " #" match contributes that digit value. + * - The next number is `max(matched values) + 1`. + * - If nothing matches, return 1 (UI uses this as "no prior context → + * first numbered list"). + * + * The DB never sees the #N — `shopping_lists.name` stores the final string + * the user committed to. See migration 027 for why we don't add a unique + * constraint on (collective_id, name). + */ + +export interface ParsedTitle { + /** The text before the trailing " #", trimmed. Null on empty input. */ + prefix: string | null; + /** The integer after "#", or null if no trailing "#" was present. */ + number: number | null; +} + +/** + * Parse a user-typed title into its (prefix, number) pair. + * + * - "Compra #5" → { prefix: "Compra", number: 5 } + * - "Compra" → { prefix: "Compra", number: null } + * - "#5" → { prefix: null, number: null } (no chip — no prefix + * to auto-number on) + * - "" → { prefix: null, number: null } + */ +export function parseTitle(input: string): ParsedTitle { + const trimmed = input.trim(); + if (trimmed.length === 0) return { prefix: null, number: null }; + + // Match the trailing " #" suffix non-greedily. Require at least + // one whitespace between prefix and "#" so "Compra#5" or "#5" alone do + // not parse as numbered (the chip is for catalog-curated prefixes). + const m = trimmed.match(/^(.+?)\s+#(\d+)$/); + if (m) { + return { prefix: m[1].trim(), number: Number.parseInt(m[2], 10) }; + } + + // Bare "#5" (no leading prefix) → null prefix; the UI won't auto-number. + if (/^#\d+$/.test(trimmed)) { + return { prefix: null, number: null }; + } + + return { prefix: trimmed, number: null }; +} + +/** + * Compute the next sequential number given an unsorted array of in-use + * numbers. Empty array → 1 (the first numbered entry). + */ +export function nextNumberFromNumbers(numbers: number[]): number { + if (numbers.length === 0) return 1; + return Math.max(...numbers) + 1; +} + +/** + * Walk `names`, extract the N for every row matching `prefix` + * (case-insensitive), and return `max(N) + 1`. A bare-prefix match counts + * as N=0. Returns 1 if no rows match — matches CLAUDE-md rule 8 / plan + * semantics 8. + * + * The DB query feeding this helper is `name ILIKE prefix OR name ILIKE + * prefix || ' #%'`. We re-validate each row here so a sloppy ILIKE that + * matches "Compras del mes" doesn't corrupt the count. + */ +export function computeNextNumberFromNames(prefix: string, names: string[]): number { + const norm = prefix.trim().toLowerCase(); + if (norm.length === 0) return 1; + + const numbers: number[] = []; + let sawBare = false; + + for (const raw of names) { + const parsed = parseTitle(raw); + if (!parsed.prefix) continue; + if (parsed.prefix.toLowerCase() !== norm) continue; + if (parsed.number === null) { + sawBare = true; + } else { + numbers.push(parsed.number); + } + } + + if (numbers.length === 0 && !sawBare) return 1; + return nextNumberFromNumbers(sawBare ? [0, ...numbers] : numbers); +} diff --git a/packages/test-utils/tests/list-title-flow.test.ts b/packages/test-utils/tests/list-title-flow.test.ts new file mode 100644 index 0000000..32f3bae --- /dev/null +++ b/packages/test-utils/tests/list-title-flow.test.ts @@ -0,0 +1,154 @@ +/** + * LT-INT-series: list-title catalog + numbering (Fase 18). + * + * Validates the role gate on `add_list_title` / `remove_list_title` (the + * only role-gate path pgTAP can't cover because postgres bypasses + * auth.uid). Also exercises the suggestion union + the per-collective + * numbering math directly against the dev DB so the SQL `or(...)` filter + * and the client-side parser stay in lockstep. + * + * LT-INT-01 admin add → suggestions include the new title + * LT-INT-02 member add → P0002 + * LT-INT-03 computeNextNumber with 3 lists "Compra #1/#2/#5" → 6 + * LT-INT-04 computeNextNumber with no matches → 1 + * LT-INT-05 admin remove → row gone, suggestions exclude it + * LT-INT-06 add empty title → P0001 + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { createClientAs, createAdminClient } from '../src/supabase-clients.js'; +import { ANA_ID, BORJA_ID, COLLECTIVE_ID } from '../src/seed-constants.js'; +import { computeNextNumberFromNames } from '../../../apps/web/src/lib/utils/list-title.js'; + +const admin = createAdminClient(); +const PREFIX_TAG = 'LTINT-'; +const CATALOG_TITLE = `${PREFIX_TAG}CuratedTitle`; +const NUMBERED_PREFIX = `${PREFIX_TAG}NumPrefix`; + +beforeAll(async () => { + // Wipe any residue from previous runs so the integration tests are + // deterministic. We only delete rows we own (by prefix). + await admin + .from('collective_list_titles') + .delete() + .eq('collective_id', COLLECTIVE_ID) + .like('title', `${PREFIX_TAG}%`); + await admin + .from('shopping_lists') + .delete() + .eq('collective_id', COLLECTIVE_ID) + .like('name', `${NUMBERED_PREFIX}%`); +}); + +afterAll(async () => { + await admin + .from('collective_list_titles') + .delete() + .eq('collective_id', COLLECTIVE_ID) + .like('title', `${PREFIX_TAG}%`); + await admin + .from('shopping_lists') + .delete() + .eq('collective_id', COLLECTIVE_ID) + .like('name', `${NUMBERED_PREFIX}%`); +}); + +describe('list-title catalog + numbering (Fase 18)', () => { + it('LT-INT-01: admin add → suggestions include the new title', async () => { + const ana = await createClientAs(ANA_ID); + const { error } = await ana.rpc('add_list_title', { + p_collective_id: COLLECTIVE_ID, + p_title: CATALOG_TITLE + }); + expect(error).toBeNull(); + + // The catalog read goes through `select_member` RLS — even guests can + // see it, but admins certainly can. + const { data } = await ana + .from('collective_list_titles') + .select('title') + .eq('collective_id', COLLECTIVE_ID) + .eq('title', CATALOG_TITLE); + expect((data ?? []).length).toBe(1); + }); + + it('LT-INT-02: member (Borja) add → P0002', async () => { + const borja = await createClientAs(BORJA_ID); + const { error } = await borja.rpc('add_list_title', { + p_collective_id: COLLECTIVE_ID, + p_title: `${PREFIX_TAG}MemberAttempt` + }); + expect(error).not.toBeNull(); + expect((error as { code?: string }).code).toBe('P0002'); + }); + + it('LT-INT-03: computeNextNumber across "Compra #1 / #2 / #5" → 6', async () => { + // Seed three concrete shopping_lists rows whose names match the + // prefix, then drive the same SQL the client-side helper uses. + const ana = await createClientAs(ANA_ID); + const names = [`${NUMBERED_PREFIX} #1`, `${NUMBERED_PREFIX} #2`, `${NUMBERED_PREFIX} #5`]; + for (const name of names) { + const { error } = await ana + .from('shopping_lists') + .insert({ collective_id: COLLECTIVE_ID, name, created_by: ANA_ID }); + expect(error).toBeNull(); + } + + // Mirror the production query in `computeNextNumber`. + const orFilter = `name.ilike.${NUMBERED_PREFIX},name.ilike.${NUMBERED_PREFIX} #%`; + const { data } = await ana + .from('shopping_lists') + .select('name') + .eq('collective_id', COLLECTIVE_ID) + .is('deleted_at', null) + .or(orFilter); + + const next = computeNextNumberFromNames( + NUMBERED_PREFIX, + (data ?? []).map((r) => r.name) + ); + expect(next).toBe(6); + }); + + it('LT-INT-04: computeNextNumber with no matching rows → 1', async () => { + const ana = await createClientAs(ANA_ID); + const orFilter = `name.ilike.${PREFIX_TAG}NoSuchPrefix,name.ilike.${PREFIX_TAG}NoSuchPrefix #%`; + const { data } = await ana + .from('shopping_lists') + .select('name') + .eq('collective_id', COLLECTIVE_ID) + .is('deleted_at', null) + .or(orFilter); + + const next = computeNextNumberFromNames( + `${PREFIX_TAG}NoSuchPrefix`, + (data ?? []).map((r) => r.name) + ); + expect(next).toBe(1); + }); + + it('LT-INT-05: admin remove → row gone', async () => { + const ana = await createClientAs(ANA_ID); + const { error } = await ana.rpc('remove_list_title', { + p_collective_id: COLLECTIVE_ID, + p_title: CATALOG_TITLE + }); + expect(error).toBeNull(); + + const { data } = await ana + .from('collective_list_titles') + .select('title') + .eq('collective_id', COLLECTIVE_ID) + .eq('title', CATALOG_TITLE); + expect((data ?? []).length).toBe(0); + }); + + it('LT-INT-06: admin add with empty title → P0001', async () => { + const ana = await createClientAs(ANA_ID); + const { error } = await ana.rpc('add_list_title', { + p_collective_id: COLLECTIVE_ID, + p_title: ' ' + }); + expect(error).not.toBeNull(); + expect((error as { code?: string }).code).toBe('P0001'); + }); +});