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>
94 lines
10 KiB
Markdown
94 lines
10 KiB
Markdown
# 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).
|