feat(settings): implement E4 variety score settings (J9) #50
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Übersicht
Der Vielfalt-Algorithmus verwendet Standard-Einstellungen, die für omnivore Haushalte optimiert sind. Vegetarische Haushalte werden durch die Protein-Wiederholungs-Prüfung unverhältnismäßig bestraft — Tofu, Eier und Hülsenfrüchte wiederholen sich strukturell häufiger, weil die Auswahl geringer ist.
Diese Issue implementiert den neuen Screen E4 — Vielfalt-Einstellungen (Journey J9) mit dem V2 Kontext-Preset-Ansatz sowie die zugehörige Erweiterung des Settings-Hubs (E1).
Spec:
specs/frontend/e4-variety-settings-kachel.htmlMockups:
specs/frontend/e4-variety-settings.htmlJourney:
specs/userjourneys.html→ J9Screens
E1 — Settings-Hub (Update)
E4 — Vielfalt-Einstellungen (neu)
Route:
/settings/variety· Nur fürrolle === 'planer'States
Kerninteraktion: Kontext-Preset Chips
Drei große Chips wählen den Haushaltskontext:
Ein vierter Chip ✦ Individuell erscheint automatisch, sobald die Einstellungen keinem der drei Presets entsprechen (nicht manuell wählbar).
Preset-Mapping:
["protein", "cuisine"]["cuisine"]["cuisine"]Score-Preview Banner
Der Banner zeigt live, wie sich der aktuelle Wochenplan mit den neuen Einstellungen entwickeln würde:
Erweiterte Einstellungen (Accordion)
4 Gewicht-Parameter, je Niedrig / Mittel / Hoch:
Deaktivierte Tag-Typen erscheinen nicht als Zeile im Accordion.
API (neue Endpoints)
GET/v1/households/mine/variety-configPATCH/v1/households/mine/variety-configDELETE/v1/households/mine/variety-configPOST/v1/week-plans/{id}/variety-score/simulateImplementierungsregeln
Akzeptanzkriterien
🧑💻 Kai — Frontend Engineer
Questions & Observations
Auto-save state machine: Auto-save on every preset/weight change without a save button means the component manages three overlapping async states: (1) the optimistic UI update, (2) the in-flight PATCH request, (3) the rollback on error. This is non-trivial. Consider modelling it as an explicit state machine (
idle | saving | error) driven by$state()rather than ad-hoc booleans — it'll make the rollback logic and the toast trigger predictable.Debounce in
$effect: The 300ms simulation debounce should use$effectwith asetTimeoutthat returns a cleanup function (clears the timeout). Do not use a module-leveldebounce()wrapper from a utility library unless it's already in the project — adding a debounce lib just for this is unnecessary."Individuell" chip as
$derived(): The chip appears automatically when the current config doesn't match any of the three presets. This is a clean$derived()— computematchedPresetfrom the current weights, and deriveisCustom = matchedPreset === null. The chip isaria-disabledor visually muted when active — notdisabled(it's informational, not interactive).Route guard — server-side only: The redirect for
rolle !== 'planer'must happen in+page.server.ts(check session and redirect), not client-side. A client-side guard is trivially bypassed by disabling JS. Do not duplicate the guard in the component.Reset confirmation — two components or one adaptive: Bottom Sheet (mobile) and centered Modal (desktop) for the same action. Is there an existing
BottomSheetorModalcomponent in the project? If so, wrap them in a singleResetConfirmDialog.sveltethat renders the appropriate variant based on a media query or a Svelte store for viewport. If neither exists, define which one to build first to avoid building two new components for one dialog.GETreturning 404 for no config: The spec says 404 = use defaults. Confirm the frontend handles 404 gracefully on load (treat as empty config, show Omnivor selected) and doesn't surface a 404 error to the user.Suggestions
E4VarietySettings.test.tsred-first. Priority: preset chip selection triggers PATCH, "Individuell" chip appears when weight is changed, optimistic rollback on 500 response, reset confirm dialog renders on delete trigger.vi.useFakeTimers()).🎨 Atlas — UI/UX Designer
Questions & Observations
"Lila Kennzahl" — purple token missing from design system: The E1 tile shows the variety score as a purple metric. Purple is not currently in the design system token file (green scale, yellow scale, neutral scale,
--color-errorare defined). A new colour needs to be formally added to the@themelayer — its tint, base, and text variants — before implementation. Don't use a one-off hex; register it as a token.Score < 6.0 → orange: The spec says the score turns orange below 6.0. Is this
--color-error(which is red in most design systems), a dedicated--color-warningtoken, or the existing yellow scale? Clarify which token to use so "orange" is consistent with how warnings are styled elsewhere in the app (e.g., the yellow in planner today-ring vs. error red)."Individuell" chip — non-interactive visual state: This chip appears automatically but isn't clickable. It needs to communicate "status only" clearly — a muted border, no hover effect,
cursor: default. Ensure it's visually distinct from the three selectable preset chips so users don't repeatedly try to click it expecting a response.Accordion for advanced settings: Four weight rows with Niedrig/Mittel/Hoch options. What control pattern is used per row — segmented button group (3 options), radio buttons, or a slider? The spec doesn't specify the input widget. This decision affects both visual weight and touch usability.
Reset confirmation — backdrop-dismiss disabled: This is correctly noted in the spec. Confirm the modal/bottom sheet implementation enforces this — it's a common accidental omission when reusing a generic dialog component that has backdrop-dismiss enabled by default.
E1 grid update: "Vorräte fullwidth oben, Haushalt + Vielfalt 1:1 unten" — the grid changes from 2 tiles to 3 tiles. Confirm the responsive breakpoints for this grid are defined: does the 1:1 bottom row stay side-by-side on all viewports, or does it stack on the smallest mobile?
Suggestions
@themevariable family before any implementation starts: at minimum--purple-tint,--purple-base,--purple-text. This unblocks both E1 and E4 implementation without ad-hoc colour choices.--color-errorfor consistency with error states elsewhere in the app, not a custom red.🧪 QA Engineer — Test Coverage
Questions & Observations
Auto-save and simulation are two separate async flows — test them independently:
vi.useFakeTimers())Optimistic update rollback: Mock the PATCH to return 500 → verify the UI reverts to the previous config value and a toast appears. What's the rollback scope — just the changed field, or the full config snapshot? Clarify in the spec for testability.
"Individuell" chip appearance: Test that selecting a non-standard weight value causes the chip to appear (and no preset chip is active). Test that resetting to a preset weight makes it disappear again.
Reset flow:
Role redirect (AC): Test in
+page.server.tsload: authenticated member session →redirect(303, '/settings')(or equivalent). This must be a server-side test, not a component test.Empty plan state:
GET /variety-configreturns 404 (no custom config) → page shows Omnivor selected, score banner shows "–". Test this load state explicitly.Accordion open/close: Advanced settings accordion — test that all 4 weight rows are visible when open, hidden when closed. Test that changing a weight while accordion is open triggers PATCH.
Suggestions
varietyConfigFixturefor tests — a fullVarietyConfigobject at Omnivor defaults. Tests then only override the fields they care about, keeping fixtures focused.🔒 Sable — Security Engineer
Questions & Observations
PATCH /v1/households/mine/variety-config— household ownership: The "mine" route pattern is a household-scoped endpoint. Verify the backend derives the household ID entirely from the authenticated session — not from a request body or query param. A household ID in the request that differs from the session's household must return 403, not silently apply changes to another household.DELETE /v1/households/mine/variety-config— role check: The spec restricts E4 torolle === 'planer', but role enforcement must happen server-side on every API call, not only on page load. A member who bypasses the frontend redirect can still call DELETE directly. The backend must checkrole === PLANNERbefore processing the delete.POST /v1/week-plans/{id}/variety-score/simulate— cross-household access: The simulate endpoint takes a week plan ID in the path. Verify the backend checks that the week plan belongs to the requesting user's household before running the simulation. An attacker with a valid session could enumerate week plan IDs from other households.Score simulation with user-supplied config body: The simulate endpoint accepts a temporary config body (weight values). The backend should validate these values strictly — only the three discrete values per weight (0.75/1.5/2.25, etc.) should be accepted. Reject out-of-range floats or negative weights. An unbounded weight value could cause the scoring algorithm to produce unexpected output that affects other users if cached.
Auto-save on every change — PATCH rate: No debounce is specified for the PATCH (only for the simulate POST). A user rapidly toggling presets could send a burst of PATCH requests. Consider rate-limiting or debouncing the PATCH as well to prevent unintentional write amplification.
GETreturning 404 for no config: A 404 for a "not configured" state leaks information about internal resource existence. AGETthat returns 200 with default values is semantically cleaner and avoids edge-case handling in the frontend.Suggestions
shouldReturn403WhenMemberTriesToDeleteVarietyConfig(). Role checks on mutation endpoints are the most common auth bypass vector — make it explicit and regression-proof.⚙️ Backend Engineer
Questions & Observations
GET404 for no config — reconsider: Returning 404 when no custom config exists is semantically awkward — it implies "resource not found" rather than "using defaults." Consider returning 200 with the default config object instead. It simplifies frontend handling (no 404 special-casing) and is more RESTful. If 404 is kept, at minimum return a clear error body explaining it means "no custom config, defaults apply."Database schema: Where does the variety config live? Options: (a) a
household_variety_configtable with one row per household and nullable columns for each weight, (b) a JSONB column on thehouseholdtable, (c) a key-value store. JSONB on the household table is appealing for schema simplicity, but makes validation harder. A dedicated table with typed columns is easier to validate with DB constraints. Which approach is intended?Flyway migration needed: A new
household_variety_configtable (or a column addition tohousehold) requires a new migration file. Confirm this is in scope for this issue and not assumed to already exist.POST /simulate— stateless computation: The simulate endpoint takes a temporary config body and returns a simulated score without persisting anything. This is clean. Ensure the simulate endpoint uses the same scoring algorithm as the real variety score computation — a divergence here would cause the preview banner to lie to the user.repeatTagTypesin preset mapping: The Omnivor preset uses["protein", "cuisine"], Vegetarisch/Vegan use["cuisine"]. This is a list of tag types to include in the repeat-penalty check. Is this stored as an array column (text[]orvarchar[]) or as individual boolean flags? An array column is flexible but harder to validate at the DB level.Weight value validation: The four weight parameters have discrete values (Niedrig/Mittel/Hoch). Enforce this with a CHECK constraint at the DB level or a
@Validannotation with custom validator in the service — don't rely on frontend-only enforcement.Suggestions
VarietyConfigRequestandVarietyConfigResponseDTOs explicitly in the spec (field names, types, nullability) before frontend and backend work starts in parallel — a field name mismatch will cause silent failures that are annoying to debug.