Build Galerie — responsive article grid with category filter #6
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?
Task 6 — Plan reference:
docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.mdUser story (US-GAL-001 + US-GAL-002 + US-RES-003):
Acceptance criteria
/if no validfamily_codecookiebg-primary) shows "Erbstücke Wannsee" and logged-in display namebg-primary text-white; inactive:border-line text-ink-mutedtext-status-free"✓ Frei"text-status-taken"● [DisplayName]"selectedIdstate → opens modal (placeholder for Task 7)SQL query
Single JOIN query loads articles + reservation status + thumbnail filename in one round-trip.
Files to create
src/routes/galerie/+page.sveltesrc/routes/galerie/+page.server.tsDepends on: #3 | Size: M | Spec: reservierung-design §4.2, views spec View 02, NFR-PERF-001, NFR-RESP-001
👤 Markus Keller — Application Architect (SvelteKit · SQLite · Node.js)
Observations
+page.server.tsis written inline as an ad-hoc.prepare(...)call inside theloadfunction, not as a named prepared statement defined at module-load time. This contradicts the architecture rule: prepare once at module load, reuse per request.+page.server.tsplan code does not export the prepared statement fromlib/db.ts. Instead the query is defined and executed insideload— meaning it is re-parsed on every request. For 500 articles this is a measurable (if small) overhead, and more importantly it breaks the single-source-of-truth principle for all statements.locals.familyCode— auth enforcement is properly delegated tohooks.server.ts. No raw cookie reads inside the route. Good.galerie/+page.sveltemodal placeholder inline rather than in a separate component. That is explicitly scoped for Task 7, so it is acceptable as-is — but should not be merged long-term without theArtikelModal.sveltecomponent.selectedIdstate andselectedArtikelderived are stored in+page.sveltedirectly, which is the correct location for view-level state that does not need to survive navigation.artikel/[id]/+page.server.tshandles bothreservierenandaufheben— the action boundary for mutations is correctly placed outside the galerie route. Galerie page itself stays read-only. This is good architecture.reservierensilently swallows the UNIQUE constraint error and redirects as if success. The user receives no feedback that someone else beat them to the reservation. The issue acceptance criteria do not require a conflict message on the galerie itself (that is Task 7 scope), but the silent swallow in the action should at minimum pass an error signal.awaitonbetter-sqlite3calls anywhere in the plan. Correct — the library is synchronous.Recommendations
lib/db.tsas a named exported prepared statement:+page.server.tswith.all(). This is consistent with howstmtArtikelByIdis defined in the architect persona's examples.reservierenaction catch block should returnfail(409, { error: 'bereits_reserviert' })rather than silently redirecting. Even if the galerie page doesn't display it yet, the structured error is the correct contract. Task 7 can surface it in the modal.target.startsWith(base)without appendingpath.sep. Addpath.septo prevent/uploads-evilmatching/uploadsas a prefix:sameSite: 'lax'insetFamilyCodeCookieshould be'strict'per the security architecture.laxallows cross-site GET requests to send the cookie — not needed here and broader than required.Open Decisions
lib/db.ts. One of these must win consistently. If inline is chosen for simplicity, that decision should be explicit and applied everywhere, not mixed.👤 Felix Brandt — Fullstack Developer (SvelteKit · TypeScript · SQLite · Node.js)
Observations
{#each}: The plan's+page.svelteiterates{#each gefilterteArtikel as artikel}without a key expression. This causes position-based DOM reconciliation when the filter changes, which can corrupt local hover/focus state. Always:{#each gefilterteArtikel as artikel (artikel.id)}.$derived:GalerieKarte.sveltecomponent, not inline in the page.GalerieKarte.svelteandKategorieFilter.svelteas separate components. This issue creates those files as part of Task 6 — the plan skips that decomposition until Task 7, which is scope creep deferral rather than design.{#if}for thumbnail presence: The placeholder<div class="bg-[#C4B8A8]">for articles without a photo uses a hardcoded hex value (#C4B8A8) instead of a design token. All color values must come from Tailwind token names.bg-[#E8F5EC] text-[#2E6645] border-[#A8D5B8]in the "Meine Reservierung" badge. These are not defined in the Tailwind config. Either define areservationtoken set or usestatus-freevariants already configured.+page.server.ts. Theloadfunction is testable — it can be imported directly in Vitest with a mockedlocals. This is the exact pattern described in the developer persona: "Test SvelteKit load functions by importing them directly."sameSite: 'lax'on the family code cookie (from Task 3): The auth helper useslax, the architecture and security specs requirestrict. This will affect galerie auth behavior on cross-site navigations.reservierenaction swallows the UNIQUE constraint without returning afail(). The developer persona's pattern is explicit: catch the constraint, returnfail(409, { error: 'already_reserved' }). A silent redirect on conflict is incorrect.Recommendations
{#each}loops immediately:{#each gefilterteArtikel as artikel (artikel.id)}.GalerieKarte.sveltenow, not in a later task. The component boundary is clear: one card = one file. Pass exact props:KategorieFilter.svelteas well — the pill bar is its own visual region.tailwind.config.js, add them.reservierencatch block: returnfail(409, { error: 'bereits_reserviert' }), do not redirect silently.👤 Leonie Voss — UI/UX Design Lead & Accessibility Advocate
Observations
text-white/75(75% opacity white on primary green). On#5B7A66background, white at 75% opacity is approximately#C6D4CB— which drops below the 4.5:1 WCAG AA threshold for normal-size text at10px font-bold. Either use fulltext-whiteor increase font size to 14px minimum where contrast can be confirmed.text-[10px]. The design system spec states "minimum 12px for any visible text" with an exception for uppercase tracking-wide labels. The pills are uppercase + bold, so this is borderline acceptable — butmin-h-[30px]touch target is below the required 44×44px minimum. Category filter pills must meet 44px height minimum since they are interactive elements on touch devices. Increase tomin-h-[44px].text-[9px] font-extrabold uppercase tracking-[.6px]is below the absolute minimum. 9px is unreadable on any mobile device. Usetext-[10px]at minimum, preferablytext-xs(12px).font-serif text-[12px]is the spec minimum — acceptable, but only just. This is the primary content label; considertext-sm(14px) for readability on phones.text-[10px] font-boldfor "✓ Frei" and "● Name" — at the absolute minimum. Confirm these remain readable at 375px viewport with device-level text scaling.<main>landmark: The gallery content renders inside a<div class="p-2.5">with no semantic<main>element. Screen readers use<main>to skip to content. Add<main>wrapping the article grid.<nav aria-label>on filter bar: The category filter<div>should be<nav aria-label="Kategorie-Filter">to provide a navigable landmark for assistive technology.role="list"on the article grid: When CSS removes list semantics (which Tailwind'sflex/gridon a<div>implies), the list must be reinstated via<ul>withrole="list"and<li>wrappers per card. Screen readers announce item count on<ul role="list">.<div class="bg-[#C4B8A8]">with no text alternative. Addaria-label="Kein Foto"or use a meaningful placeholder icon with sr-only text.alt=""on the card thumbnail image: Empty alt is correct for decorative images, but these card thumbnails serve as the primary visual identifier for each article — they are not decorative. Usealt="{artikel.titel ?? artikel.kategorie}"to give screen reader users equivalent information.<div role="dialog">does not trap focus. Until ArtikelModal.svelte (Task 7) is in place, the modal should either use a native<dialog>element (which provides focus trapping built-in) or not be interactive. A<div role="dialog">without focus management fails WCAG 2.1 SC 2.1.2.<button>has no visible focus indicator: Tailwind resets focus styles by default. The card buttons needfocus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2.Recommendations
text-white(remove opacity) and verify contrast with WebAIM checker on#5B7A66.min-h-[44px]andpx-3 py-2.5for adequate touch target.text-[9px]category labels in cards withtext-[11px]minimum (or Tailwindtext-xs).<main>and the filter bar in<nav aria-label="Kategorie-Filter">.<div class="flex flex-col gap-2 ...">wrapping<button>elements to a<ul role="list" class="flex flex-col gap-2 ...">with each card inside<li>.alt="{artikel.titel ?? artikel.kategorie}"to card thumbnail images.focus-visible:ring-2 focus-visible:ring-primary focus-visible:ring-offset-2to the card button.<div role="dialog">placeholder with a native<dialog>element — browsers provide focus trapping, Escape-to-close, and ARIA semantics automatically.font-serif— correct) and confirm Inter is applied to all badge/label/button text.Open Decisions
alt="") treats thumbnails as decorative, but they are the primary visual content differentiator. The spec does not mandate alt text per article. Decide: require admins to enter a title (which becomes alt text), or generate generic alt like "Foto von Artikel in Kategorie [X]". This affects the data model (title currently optional).👤 Sara Holt — QA Engineer & Test Strategist
Observations
- [ ] Step N: Write the failing teststep, making it the only task in the plan that completely skips TDD. Tasks 2, 3, and 4 all start with a failing test — Task 6 breaks that pattern without explanation.loadfunction: Thegalerie/+page.server.tsload function is the most complex data-shaping code in this task (join query, status mapping, redirect guard). It is importable TypeScript and trivially testable with an in-memory SQLite db. There is no reason to skip it.frei,meine,belegt). These are precisely the statesvitest-browser-sveltecomponent tests are designed for. The tester persona's examples literally showGalerieKarteas the specimen.reservierenaction catch block swallows errors silently: There is no test that verifies the UNIQUE constraint actually prevents double reservation and surfaces a user-facing response. The architect persona's db tests cover the constraint at the SQLite level, but the Form Action's response contract (thefail(409)) has no test./if no validfamily_codecookie." This redirect must have an integration test. Without it, a future change tohooks.server.tscould silently break the guard.Recommendations
Add the following test steps to Task 6 before Step 2 (implement the svelte file):
Step 1a — Integration tests for
+page.server.ts(write failing first):Step 2a — Component tests for GalerieKarte (requires extracting the component):
Add to E2E critical journeys list:
Open Decisions
loadfunction importsgetDb()at module level. Testing it directly requires the_resetForTestingexport and careful module cache handling. Decide whether to injectdbas a parameter (more testable) or use the reset pattern (simpler). The reset pattern is already established in Tasks 2–4, so use it — but this must be explicit in the task steps.👤 Nora Steiner ("NullX") — Application Security Engineer
Observations
1. Cookie SameSite downgraded from Strict to Lax (CWE-1004 / Security Smell)
The plan's
setFamilyCodeCookieinlib/auth.tsusessameSite: 'lax'. The security architecture spec and the architect persona both specifysameSite: 'strict'.laxallows the cookie to be sent on cross-site top-level GET navigations — meaning a link from any external site to/galeriewould include thefamily_codecookie. For a reservation app,strictis correct: it forces re-authentication on cross-site navigation, which is the intended behavior.Fix:
2. Upload path traversal check missing
path.sep(CWE-22 — Path Traversal)The uploads route in the plan:
target.startsWith(base)is insufficient. IfUPLOAD_DIRresolves to/app/uploads, then a request for/uploads/../../app/uploads-evil/secret.webpresolvesbase=/app/uploadsandtarget=/app/uploads-evil/secret.webp, which does NOT start with/app/uploads— so this specific example is caught. However the correct defensive pattern is always:Using
path.sepeliminates the class of prefix-confusion attacks entirely, and returning400rather than404does not leak whether the file exists outside the upload root.3.
reservierenaction silently swallows all errors (CWE-390)The bare
catch {}swallows ALL exceptions, not onlyUNIQUE constraintviolations. A database connection error, a permissions error, or any unexpected exception is silently consumed and the user is redirected as if success. This masks real failures.Fix — distinguish the specific constraint error:
4. Admin session cookie not verified against database (Security Architecture Violation)
The
hooks.server.tsplan code:This trusts the cookie value directly. The security architecture specification requires whitelisting against known admin names:
Without the whitelist check, an attacker who can forge or modify cookies could set
admin_sessionto any value and gainlocals.admintrust. The whitelist is thin protection but it is the specified pattern and must be followed.5. No
secure: !devflag on cookiesBoth
setFamilyCodeCookieandsetAdminCookiein the plan omit thesecureflag. Withoutsecure: truein production, cookies are transmitted over plain HTTP, which is accessible to network-level attackers. The Caddy reverse proxy provides TLS but the cookie flag must still be set at the application layer:Recommendations
sameSite: 'lax'→'strict'in both cookie setters inlib/auth.ts.secure: !devto both cookie setters.path.septo the uploads path traversal check and change the error code to400.catch {}in thereservierenaction to distinguish constraint errors from unexpected errors, and re-throw the latter.ADMIN_NAMES.includes()whitelist check inhooks.server.tsbefore assigninglocals.admin.👤 Tobias Wendt — DevOps & Platform Engineer
Observations
tailwind.config.jsandpostcss.config.jsfrom Task 1 must be present and correct before the gallery Tailwind classes render. This is a build-time dependency, not a runtime one, and it is already covered in Task 1. No blockers here.Tailwind CSS 3in the tech stack line. Tailwind 4 is available and changes the config format significantly. Task 1'stailwind.config.jsuses the Tailwind 3 format (theme.extend.colors,plugins: []). Confirm the installed version is v3 before implementing to avoid silent class breakage.SESSION_SECRETstartup guard inhooks.server.ts: The plan code does not fail to start whenSESSION_SECRETis absent. The security architecture (and the architect persona) both specify that the app mustprocess.exit(1)ifSESSION_SECRETis missing. This is a deployment safety requirement — without it, a misconfigured container starts and serves traffic with no secret. Add tohooks.server.tsorlib/db.tsstartup block:/healthendpoint yet: Thedocker-compose.ymlplan healthcheck callswget -qO- http://localhost:3000/health. No route exists yet that returns 200 OK on/health. This means the container will be reported asunhealthyuntil that route is added. It should be a single-file route:src/routes/health/+server.tsreturningnew Response('OK').Dockerfilein the file map but no task creates it. Before the project can be deployed, someone needs to verify the multi-stage build actually works with@sveltejs/adapter-node. In particular:better-sqlite3requires native compilation — confirm thatnpm ci --omit=devin the runner stage either bundles the prebuilt binary or triggers a rebuild correctly.Recommendations
SESSION_SECRETpresence check to the app startup. Do it insrc/hooks.server.tsat the top of the file before any other logic — it runs on every request and will catch the misconfiguration immediately on first load./healthroute before deploying:src/routes/health/+server.ts:tailwindcssto^3.xexplicitly inpackage.jsonif the project has not done so. Runningnpm install tailwindcsstoday might install v4 which has an incompatible config format.better-sqlite3builds correctly in the multi-stage Docker image before the first full deploy. The native addon requires Python and a C compiler in the builder stage —node:22-alpinehas these viaapk add python3 make g++. Add to the Dockerfile builder stage if not already present.DATABASE_PATHandUPLOAD_DIRenvironment variable requirements in.env.examplebefore the first deployment run.Open Decisions (omit if none)
npm installfresh, they may get v4. Decide whether to lock totailwindcss@3inpackage.jsondependencies or migrate the config to v4 format. Either is fine — but it must be decided before Task 1 runs, not discovered when the gallery classes fail to render.👤 Elicit — Requirements Engineer & Business Analyst
Observations
text-status-taken● [DisplayName]" — but does not specify the fallback whenreserviertVonisnull. In the data model,display_nameisNOT NULL, so this cannot be null in practice. However the TypeScript type in the plan hasreserviert_name: string | null. The implementation must handle the null case even if the schema prevents it, to avoid a runtime● nullrendering.selectedIdstate → opens modal (placeholder for Task 7): This acceptance criterion creates an implicit dependency: Task 6 merges with a non-functional modal stub. If Task 7 is blocked or delayed, Task 6 ships with a broken user experience (clicking any card opens a useless overlay). This dependency is noted in the issue (Depends on: #3) but the task boundary implies Task 6 is independently shippable — which it is not from a user perspective.{data.displayName}in the nav. This satisfies the story. But the story also says family members can "see who has reserved an article." The display name is shown on the card — this is correct. No gap.Recommendations
+page.sveltetemplate forreserviertVonbeing null even though the schema prevents it:● nullfrom ever reaching the DOM.selectedIdstate and modal placeholder until Task 7 is ready to land simultaneously.Open Decisions
🗳️ Decision Queue — Action Required
6 decisions need your input before implementation starts.
Architecture
loadfunctions in route files. The architecture spec and architect persona require all prepared statements to be defined at module-load time inlib/db.tsand imported. Inline is simpler to read locally; centralized indb.tsis consistent with the stated architecture and avoids re-parsing on every request. Choose one pattern and apply it everywhere. (Raised by: Markus Keller)Deployment & Build
tailwind.config.js. Runningnpm install tailwindcsstoday installs v4, which has an incompatible configuration format and would silently break all gallery classes. Options: (A) pintailwindcss@^3inpackage.json, or (B) rewrite the config in Tailwind v4 format (CSS-native, notailwind.config.js). Must be decided before Task 1 runs. (Raised by: Tobias Wendt)Accessibility & Content
alt=""(decorative) on card thumbnail images. These thumbnails are the primary visual differentiator for each article, so empty alt leaves screen reader users without meaningful content. Options: (A) make article title required in the data model soalt="{artikel.titel}"always resolves, (B) use a generated fallback likealt="Foto von Artikel in Kategorie {artikel.kategorie}", or (C) keepalt=""and accept the accessibility tradeoff for this short-lived app. Affects theartikeltable schema and admin form UX. (Raised by: Leonie Voss)Requirements & UX Copy
Empty state messaging — Two distinct empty states exist but only one is specified: (1) zero articles in the database at all, and (2) zero articles matching the active category filter. The current AC covers only case 2 ("Keine Artikel in dieser Kategorie"). Case 1 needs a different message ("Noch keine Artikel vorhanden") or the same message must serve both. Decide the copy for each state before implementation to avoid inconsistent German text across tasks. (Raised by: Elicit)
NFR-PERF-001 measurable threshold — "Flüssig scrollen" (smooth scroll) is not a testable requirement. There is no defined pass/fail criterion for the 500-article gallery. Options: (A) add a concrete threshold (e.g., HTML response under 500ms on the production server for 500 articles), or (B) accept SSR + no client fetch as a sufficient implementation proxy and skip measurement. If B is chosen, the NFR cannot be verified at launch. (Raised by: Elicit)
Testing
loadfunction callsgetDb()at module level. Testing it directly requires either (A) using the existing_resetForTesting()export +process.env.DATABASE_PATH = ':memory:'pattern (already established in Tasks 2–4, consistent but requires careful import ordering), or (B) injectingdbas a parameter toload(more explicit, but changes the function signature away from SvelteKit's expected shape). Option A is recommended for consistency — but this must be spelled out explicitly in the Task 6 test steps. (Raised by: Sara Holt)