From e814f3b5b38950124085a9d3691a10464bff4dc0 Mon Sep 17 00:00:00 2001
From: Oier Bravo Urtasun <oierbravo@gmail.com>
Date: Mon, 18 May 2026 12:38:52 +0200
Subject: [PATCH] =?UTF-8?q?docs(fase-15):=20history=20record=20=E2=80=94?=
 =?UTF-8?q?=20common=20items=20management=20+=20exclude-on-list?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the Fase-15 cycle. Final counts: 177 pgTAP + 179 Vitest
integration + 47 Vitest unit + 93 Playwright + 1 gated = 497 + 3
skipped. The plan estimated +14; actual is +21 (the extra came from the
schema-invariant pgTAP assertions on the new index + RPC posture).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---
 docs/history/fase-15-common-items.md | 93 ++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)
 create mode 100644 docs/history/fase-15-common-items.md

diff --git a/docs/history/fase-15-common-items.md b/docs/history/fase-15-common-items.md
new file mode 100644
index 0000000..4a3dc82
--- /dev/null
+++ b/docs/history/fase-15-common-items.md
@@ -0,0 +1,93 @@
+# Fase 15 — Common items management (weighted suggestions + exclude-on-list)
+
+**MVP2 · 7/7. Re-opened the cycle after Fase 14 with two concrete asks on the "add item" flow.**
+
+**Status: ✅ Shipped 2026-05-18.**
+
+Two user-driven additions on top of the existing `item_frequency` table from migration 006:
+
+1. **Promote items** — admins can boost specific names so they always lead the suggestion strip, regardless of historical `use_count`.
+2. **Hide items already on the list** — the dropdown no longer nudges the user to re-add names that are already present in the active list.
+
+Plan: `plan/fase-15-common-items.md`. Migration: `supabase/migrations/026_item_frequency_weight.sql`. pgTAP: `supabase/tests/018_item_frequency_weight.sql`.
+
+## What changed
+
+### 15.1 — `item_frequency.weight` + admin RPCs
+
+- `weight integer NOT NULL DEFAULT 0` added to `item_frequency`. No CHECK constraint — the column stays open so future tiers ("strong boost", "soft hide") don't force another schema diff. The UI is the contract for the 3-way mapping, not the DB.
+- `item_frequency_collective_count_idx` dropped, replaced by `item_frequency_collective_order_idx (collective_id, weight DESC, use_count DESC, last_used_at DESC)` so the suggestion query scans in order without a sort step.
+- Two SECURITY DEFINER RPCs gate writes:
+  - `public.set_item_frequency_weight(p_collective_id uuid, p_name text, p_weight integer)` — upsert: if the row doesn't exist, inserts with `use_count = 0`; if it does, sets `weight`. Raises `P0002` for non-admins, `P0001` for unauthenticated / empty name.
+  - `public.purge_item_frequency(p_collective_id uuid, p_name text)` — deletes the row from `item_frequency` only. Does NOT touch `shopping_items` (the modal copy explicitly says this; existing items on lists are unaffected). The next time someone adds that name to a list, the existing trigger recreates the row with `weight = 0`.
+- The deny-all INSERT/UPDATE/DELETE policies from migration 006 are preserved; the RPCs are the only write path. Both `GRANT EXECUTE ... TO authenticated` so PostgREST will call them.
+
+### 15.2 — Store + query
+
+- `fetchSuggestions(collectiveId, prefix, options?)` in `apps/web/src/lib/stores/lists.ts` extended with `{ excludeNames?: string[]; limit?: number }`. Ordering switches from `.order('use_count')` to `weight DESC, use_count DESC, last_used_at DESC`. `excludeNames` is lowercased + trimmed + deduped before being sent; when empty, the `.not()` filter is skipped entirely (PostgREST renders an empty `(...)` list as a SQL error, and zero entries would mean zero effect anyway). When the list exceeds `EXCLUDE_NAMES_HARD_CAP = 200`, the filter degrades to a no-op rather than fail the whole query — past that point we'd risk hitting Kong's ~16 KiB request URI cap (see "Gotchas" below).
+- New `apps/web/src/lib/stores/commonItems.ts` is the unbounded counterpart used by the manage view: `loadCommonItems` (full catalogue, same ordering, no `LIMIT`), `setWeight` (RPC + optimistic in-store update), `purge` (RPC + optimistic remove).
+
+### 15.3 — Manage UI
+
+`apps/web/src/routes/(app)/collective/manage/+page.svelte` gains a new "Common items" subsection visible to admins + members (guests see nothing):
+
+- Table per row: name, last_used + use_count blurb, a 3-way **Hide / Normal / Boost** segmented control that writes `weight = -50 / 0 / 50`, a small "Remove" button that opens a confirmation modal explaining "items already in lists are not touched".
+- Members see the same table but every action button is disabled with a tooltip "Only admins can manage common items".
+- "Add common item" button (admin only) — opens a modal with a name input + the same 3-way control, defaulting to **Boost**. Useful for seeding the catalogue before lists actually mention an item.
+- Client-side substring search filters the table; empty / filtered-empty states have their own copy.
+- `data-weight-state` attribute on each `<li>` exposes the current visual state to the e2e suite so the segmented control is testable without scraping classnames.
+- The segmented control is a row of three `<button>` elements — visually distinct from the existing section-visibility toggles (which are `<input type="checkbox">`), so there's no ambiguity at a glance.
+
+### 15.4 — Exclude-on-list wiring
+
+`apps/web/src/routes/(app)/lists/[id]/+page.svelte`:
+
+- New `excludedItemNames = $derived(items.map((i) => i.name))` recomputes the moment an optimistic insert lands.
+- Both the cold-load `fetchSuggestions` call and the keystroke-debounced `updateSuggestions` now pass `{ excludeNames: ... }`. The `$effect` reads the `$derived` directly so adding an item to the list drops it from the strip on the next keystroke.
+
+`apps/web/src/lib/components/ItemSuggestions.svelte`:
+
+- Added `data-testid="item-suggestions-strip"` on the container + `data-testid="item-suggestion-{name}"` on each chip so the e2e suite can address them without fragile text matching.
+- The component still hides itself entirely when `suggestions.length === 0` — the dropdown was already gated on the array being non-empty, so 15.4.3 ("don't show the dropdown when the post-filter result is empty") needed no new code.
+
+### 15.5 — Tests
+
+- pgTAP `supabase/tests/018_item_frequency_weight.sql` — 11 schema-level assertions on the new column, the recreated index, both RPCs, the SECURITY DEFINER posture, and the trigger-default-weight-zero behaviour.
+- Vitest integration `packages/test-utils/tests/common-items.test.ts` — 6 specs covering admin-boost-promotes-low-use-count ordering, the `excludeNames` filter, the role gate (admin OK / member + guest get `P0002`), purge happy + denied path, and `set_item_frequency_weight` on a brand-new name inserting `use_count = 0`.
+- Playwright `apps/web/tests/e2e/common-items.test.ts` — 4 specs: CI-01 (Ana boosts yogurt → it leads the dropdown ahead of higher-use-count items on a fresh empty list), CI-02 (added item disappears from the suggestion strip on the next keystroke, using fresh seeded names on a fresh list to dodge the dev-DB pollution), CI-03 (Borja sees the table, every action disabled, "Add common item" hidden), CI-04 (Ana purges → row gone from the table). Cleanup uses the admin purge RPC because the deny-all RLS from migration 006 blocks direct `DELETE` from authenticated browser sessions.
+
+## Test deltas (Fase 15 only)
+
+| Suite | Before | After | Δ |
+|---|---|---|---|
+| pgTAP (`just test-db`) | 166 | 177 | +11 (IFW-T01..T11) |
+| Vitest integration (`just test-integration`) | 173 passed + 3 skipped | 179 + 3 skipped | +6 (CI-INT-01..06) |
+| Vitest unit (`just test-unit`) | 47 | 47 | 0 (the plan flagged "probably no" for a client-side ordering unit test; the server `ORDER BY` is the only ordering path) |
+| Playwright e2e (`just test-e2e`) | 89 | 93 | +4 (CI-01..04) |
+| Gated rate-limit | 1 | 1 | 0 |
+
+**Cumulative MVP2 + Fase 15: 497 tests + 3 skipped (177 pgTAP + 179 Vitest integration + 47 Vitest unit + 93 Playwright + 1 gated).**
+
+All green modulo the pre-existing realtime flakes (R-E-01/02, RO-02, SV-02, TG-01) which are sensitive to dev-DB pollution but unrelated to this fase.
+
+## Gotchas surfaced
+
+- **Direct `DELETE FROM item_frequency` from the browser is denied by RLS.** The all-deny INSERT/UPDATE/DELETE policies from migration 006 stayed in place — the admin purge RPC is the only delete path. Tests that try to clean up via `supabase.from('item_frequency').delete().eq(...)` silently no-op (PostgREST returns 200 with empty data). The e2e suite goes through `supabase.rpc('purge_item_frequency', ...)` for all cleanup; an early version of the suite leaked rows because of this.
+- **`fetchSuggestions` URL can exceed Kong's ~16 KiB request-URI cap.** A polluted seed list with 300+ items + the literal `not.in.("a","b",…)` PostgREST encoding pushed the URL over the limit, which froze the query and broke the realtime + reorder + section-visibility tests (all of which load the seed list in two browser contexts). Hard-cap at 200 names; past that the filter degrades to a no-op (the dropdown may surface a few names already on the list — acceptable UX, never a 500). For a future RPC-backed `LEFT JOIN ... WHERE shopping_items.id IS NULL` form, see the plan's "Riesgos / notas" section.
+- **`set_item_frequency_weight` on a brand-new name inserts with `use_count = 0`.** The trigger fn from migration 006 only fires on `shopping_items` INSERT; it never sees an admin's curation-only seed. The upsert in the RPC has `use_count = 0` in its INSERT branch and only updates `weight` in the ON CONFLICT branch — that way a real `shopping_items` add later will increment the existing row's `use_count` from 0 to 1 without clobbering the admin-set weight.
+- **The seeded `auth.users.role` migration trigger from 013 / 014 has nothing to do with this fase**, but the integration tests still need fresh JWTs whenever they're re-run after a schema-changing migration — covered by the existing per-test `await createClientAs(...)` flow.
+
+## Deviations from plan
+
+- The plan flagged a possible Vitest unit test for client-side ordering (15.5.1). Server `ORDER BY` is the only ordering path; no client-side ordering exists; the unit test was skipped per the plan's own "probably no".
+- `EXCLUDE_NAMES_HARD_CAP = 200` was not in the original plan — added as a hot-fix after the full `just test-all` surfaced URL-length flakes in the realtime / reorder tests on the polluted dev DB. The plan's "Riesgos" section did acknowledge "para >>100 reconsiderar a un LEFT JOIN ... WHERE shopping_items.id IS NULL vía RPC"; the hard cap is the lighter-weight intermediate step.
+- D-06 (existing items.test.ts) had to be re-pointed at a freshly-created empty list because the seed list pollution + new exclude-on-list filter together suppressed every assertion target. Same applies to CI-01 and CI-02 in the new e2e file.
+
+## Out of scope (still)
+
+- Import/export of common items between collectives.
+- Tag suggestions per category.
+- Server-wide common items (cross-collective).
+- ML / cross-collective recommendations.
+- Pre-assigned tags on promoted items.
+- The audit table for "who promoted what" (`item_frequency` has no `created_by`; the trigger that owns inserts is SECURITY DEFINER).