Build Admin Reservierungen — overview grouped by person #12

New Issue

marcel · 2026-05-05T10:58:12+02:00

marcel commented

2026-05-05 10:58:12 +02:00

Task 12 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`

User story (US-ADM-007):
As an admin, I see all reservations grouped by person, with article thumbnails, so I can quickly see who has reserved what.

Acceptance criteria

All codes are listed, including those with zero reservations
Each person has a dark header row (bg-admin-bg text-white) with their display name and reservation count
Under each header: list of reserved articles (thumbnail 26×26px, title or "—", category badge)
Persons with zero reservations show grayed italic "Keine Reservierungen" message (spec: "ausgegraut aber angezeigt")
Thumbnails loaded via /uploads/ route; fallback placeholder shown if no photo exists
Data loaded in one page load — no client-side fetching

SQL approach

Query all codes, then JOIN reservierungen → artikel → artikel_fotos for thumbnails. Group in server-side JS.

Files to create

src/routes/admin/reservierungen/+page.svelte
src/routes/admin/reservierungen/+page.server.ts

Depends on: #8 | Size: M | Spec: reservierung-design §5.4, views spec View 08

## Task 12 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md` **User story (US-ADM-007):** As an admin, I see all reservations grouped by person, with article thumbnails, so I can quickly see who has reserved what. ### Acceptance criteria - [ ] All codes are listed, including those with zero reservations - [ ] Each person has a dark header row (`bg-admin-bg text-white`) with their display name and reservation count - [ ] Under each header: list of reserved articles (thumbnail 26×26px, title or "—", category badge) - [ ] Persons with zero reservations show grayed italic "Keine Reservierungen" message (spec: "ausgegraut aber angezeigt") - [ ] Thumbnails loaded via `/uploads/` route; fallback placeholder shown if no photo exists - [ ] Data loaded in one page load — no client-side fetching ### SQL approach Query all codes, then JOIN reservierungen → artikel → artikel_fotos for thumbnails. Group in server-side JS. ### Files to create - `src/routes/admin/reservierungen/+page.svelte` - `src/routes/admin/reservierungen/+page.server.ts` **Depends on:** #8 | **Size:** M | **Spec:** reservierung-design §5.4, views spec View 08

marcel added this to the v1.0 — MVP milestone 2026-05-05 10:58:13 +02:00

marcel commented

2026-05-05 11:27:32 +02:00

👤 Markus Keller — Application Architect

Observations

The planned SQL query in +page.server.ts does two separate round-trips: one SELECT for all codes, then one SELECT for all reservierungen. This is correct and idiomatic for better-sqlite3 — prepared statements run synchronously, so no async overhead. The in-process JS grouping via filter() is fine for the expected data volume (dozens of codes, hundreds of reservierungen at most).
The subquery (SELECT filename FROM artikel_fotos WHERE artikel_id = a.id ORDER BY position LIMIT 1) AS thumbnail is correct. It selects the primary photo without a second round-trip or an N+1 problem. This is the right pattern.
Auth guard is inherited from +layout.server.ts under /admin/ — no per-route check needed, and the issue correctly does not add one. This matches the single-enforcement-point principle.
The load function returns { byPerson } shaped data — a flat, typed array. No client-side fetch, no API route. Correct.
The planned query orders reservierungen by r.created_at DESC. This means within each person's list the most recently reserved article appears first. The spec (§5.4 and View 07 mockup) does not specify an ordering within person groups — the current choice is reasonable but worth noting as a decision.
The fallback {#if data.byPerson.length === 0} handles the empty-codes case cleanly. The zero-reservation per-person case is handled with the {:else} branch. Both edge cases are covered.
The thumbnail column is derived from artikel_fotos.filename (relative path). The template renders it as /uploads/{r.thumbnail}. This is consistent with how other views in the plan handle photo URLs — the uploads route serves from UPLOAD_DIR.

Recommendations

Prepare the two statements at module load time in db.ts, not inline inside load(). The pattern across all other tasks in the plan is: prepare once at module load, import and call in the route. Keeping inline db.prepare(...) calls inside load functions creates statements that are re-parsed on every request. Extract:

// lib/db.ts (additions)
export const stmtAllCodes = db.prepare('SELECT id, display_name FROM codes ORDER BY display_name')
export const stmtReservierungenMitArtikeln = db.prepare(`
  SELECT r.code_id, a.id AS artikel_id, a.titel, a.kategorie,
    (SELECT filename FROM artikel_fotos WHERE artikel_id = a.id ORDER BY position LIMIT 1) AS thumbnail,
    r.created_at
  FROM reservierungen r
  JOIN artikel a ON a.id = r.artikel_id
  ORDER BY r.created_at DESC
`)

The getDb() call inside the load function is correct only if getDb() returns the singleton. Verify it does — the plan shows getDb() as a factory wrapper, but the singleton is the expected return value after first initialization.
The byPerson grouping is done in JS after fetching all rows. For the expected scale this is fine. Do not prematurely optimize with a GROUP BY — the JS grouping is more readable and the data set is tiny.
No mutations are needed on this page (read-only overview). No Form Actions required. This is correct — do not add any.

Open Decisions (omit if none)

Intra-person sort order — reservierungen within each person's list are currently ordered by r.created_at DESC (most recent first). The spec does not prescribe an order. Alternative: alphabetical by article title. Low cost either way, but the choice should be explicit before implementation starts.

## 👤 Markus Keller — Application Architect ### Observations - The planned SQL query in `+page.server.ts` does two separate round-trips: one `SELECT` for all codes, then one `SELECT` for all reservierungen. This is correct and idiomatic for better-sqlite3 — prepared statements run synchronously, so no async overhead. The in-process JS grouping via `filter()` is fine for the expected data volume (dozens of codes, hundreds of reservierungen at most). - The subquery `(SELECT filename FROM artikel_fotos WHERE artikel_id = a.id ORDER BY position LIMIT 1) AS thumbnail` is correct. It selects the primary photo without a second round-trip or an N+1 problem. This is the right pattern. - Auth guard is inherited from `+layout.server.ts` under `/admin/` — no per-route check needed, and the issue correctly does not add one. This matches the single-enforcement-point principle. - The load function returns `{ byPerson }` shaped data — a flat, typed array. No client-side fetch, no API route. Correct. - The planned query orders reservierungen by `r.created_at DESC`. This means within each person's list the most recently reserved article appears first. The spec (§5.4 and View 07 mockup) does not specify an ordering within person groups — the current choice is reasonable but worth noting as a decision. - The fallback `{#if data.byPerson.length === 0}` handles the empty-codes case cleanly. The zero-reservation per-person case is handled with the `{:else}` branch. Both edge cases are covered. - The `thumbnail` column is derived from `artikel_fotos.filename` (relative path). The template renders it as `/uploads/{r.thumbnail}`. This is consistent with how other views in the plan handle photo URLs — the uploads route serves from `UPLOAD_DIR`. ### Recommendations - Prepare the two statements at module load time in `db.ts`, not inline inside `load()`. The pattern across all other tasks in the plan is: prepare once at module load, import and call in the route. Keeping inline `db.prepare(...)` calls inside load functions creates statements that are re-parsed on every request. Extract: ```typescript // lib/db.ts (additions) export const stmtAllCodes = db.prepare('SELECT id, display_name FROM codes ORDER BY display_name') export const stmtReservierungenMitArtikeln = db.prepare(` SELECT r.code_id, a.id AS artikel_id, a.titel, a.kategorie, (SELECT filename FROM artikel_fotos WHERE artikel_id = a.id ORDER BY position LIMIT 1) AS thumbnail, r.created_at FROM reservierungen r JOIN artikel a ON a.id = r.artikel_id ORDER BY r.created_at DESC `) ``` - The `getDb()` call inside the load function is correct only if `getDb()` returns the singleton. Verify it does — the plan shows `getDb()` as a factory wrapper, but the singleton is the expected return value after first initialization. - The `byPerson` grouping is done in JS after fetching all rows. For the expected scale this is fine. Do not prematurely optimize with a GROUP BY — the JS grouping is more readable and the data set is tiny. - No mutations are needed on this page (read-only overview). No Form Actions required. This is correct — do not add any. ### Open Decisions _(omit if none)_ - **Intra-person sort order** — reservierungen within each person's list are currently ordered by `r.created_at DESC` (most recent first). The spec does not prescribe an order. Alternative: alphabetical by article title. Low cost either way, but the choice should be explicit before implementation starts.

marcel commented

2026-05-05 11:27:48 +02:00

👤 Felix Brandt — Fullstack Developer

Observations

The +page.svelte plan uses $props() with the modern Svelte 5 runes syntax (let { data }: { data: PageData } = $props()). Correct — no legacy export let data.
The {#each data.byPerson as person} loop lacks a key expression. Each person has an id field — it must be keyed: {#each data.byPerson as person (person.id)}. Same for the inner loop: {#each person.reservierungen as r (r.artikel_id)}. Without keys, Svelte falls back to position-based reconciliation.
The template mixes display logic cleanly: the thumbnail fallback <div> vs <img> is a simple {#if r.thumbnail} branch — no complex derived state needed here since this is a static read-only view.
r.titel ?? '—' correctly handles null article titles. Matches the acceptance criteria ("title or '—'").
The planned +page.server.ts does not import prepared statements from db.ts — instead it calls db.prepare(...) inline. Per project convention, prepared statements belong in lib/db.ts, prepared once at module load, exported, and imported into the route. Fix this before implementation.
The load function does not need locals — this page is admin-only but auth is enforced by the layout guard, so no redundant check is needed. The plan correctly omits it.
The category badge uses bg-canvas px-1.5 py-0.5 rounded — it renders as a subtle canvas-colored chip. The spec mockup (View 07) shows the category as a small all-caps label below the article name, not a badge. The plan's implementation diverges slightly from the mockup here.
No use:enhance, no onMount, no client-side fetch — this is a pure SSR load-function page. Correct.

Recommendations

Add key expressions to both {#each} loops immediately:

{#each data.byPerson as person (person.id)}
  ...
  {#each person.reservierungen as r (r.artikel_id)}

Move the two db.prepare(...) calls out of the load function into lib/db.ts as module-level exported prepared statements. This is both a performance improvement and a consistency requirement across the codebase.
The category display should match the spec mockup: small, all-caps, text-ink-muted, text-[8.5px] font-extrabold uppercase tracking-[.4px] — not a colored badge chip. Revise the category element in the template to:
```
<span class="text-[8.5px] font-extrabold uppercase tracking-[.4px] text-ink-muted">{r.kategorie}</span>
```
The data: PageData type annotation on $props() is correct — keep it. Do not use any.

Open Decisions (omit if none)

Category display style — the plan uses a canvas-colored badge chip (bg-canvas px-1.5 py-0.5 rounded) but the View 07 mockup shows plain all-caps text. The badge approach is defensible on its own but it is not spec-compliant. (Raised by: Felix Brandt)

## 👤 Felix Brandt — Fullstack Developer ### Observations - The `+page.svelte` plan uses `$props()` with the modern Svelte 5 runes syntax (`let { data }: { data: PageData } = $props()`). Correct — no legacy `export let data`. - The `{#each data.byPerson as person}` loop lacks a key expression. Each person has an `id` field — it must be keyed: `{#each data.byPerson as person (person.id)}`. Same for the inner loop: `{#each person.reservierungen as r (r.artikel_id)}`. Without keys, Svelte falls back to position-based reconciliation. - The template mixes display logic cleanly: the thumbnail fallback `<div>` vs `<img>` is a simple `{#if r.thumbnail}` branch — no complex derived state needed here since this is a static read-only view. - `r.titel ?? '—'` correctly handles null article titles. Matches the acceptance criteria ("title or '—'"). - The planned `+page.server.ts` does not import prepared statements from `db.ts` — instead it calls `db.prepare(...)` inline. Per project convention, prepared statements belong in `lib/db.ts`, prepared once at module load, exported, and imported into the route. Fix this before implementation. - The load function does not need `locals` — this page is admin-only but auth is enforced by the layout guard, so no redundant check is needed. The plan correctly omits it. - The category badge uses `bg-canvas px-1.5 py-0.5 rounded` — it renders as a subtle canvas-colored chip. The spec mockup (View 07) shows the category as a small all-caps label below the article name, not a badge. The plan's implementation diverges slightly from the mockup here. - No `use:enhance`, no `onMount`, no client-side fetch — this is a pure SSR load-function page. Correct. ### Recommendations - Add key expressions to both `{#each}` loops immediately: ```svelte {#each data.byPerson as person (person.id)} ... {#each person.reservierungen as r (r.artikel_id)} ``` - Move the two `db.prepare(...)` calls out of the load function into `lib/db.ts` as module-level exported prepared statements. This is both a performance improvement and a consistency requirement across the codebase. - The category display should match the spec mockup: small, all-caps, `text-ink-muted`, `text-[8.5px] font-extrabold uppercase tracking-[.4px]` — not a colored badge chip. Revise the category element in the template to: ```svelte <span class="text-[8.5px] font-extrabold uppercase tracking-[.4px] text-ink-muted">{r.kategorie}</span> ``` - The `data: PageData` type annotation on `$props()` is correct — keep it. Do not use `any`. ### Open Decisions _(omit if none)_ - **Category display style** — the plan uses a canvas-colored badge chip (`bg-canvas px-1.5 py-0.5 rounded`) but the View 07 mockup shows plain all-caps text. The badge approach is defensible on its own but it is not spec-compliant. _(Raised by: Felix Brandt)_

marcel commented

2026-05-05 11:28:03 +02:00

👤 Nora "NullX" Steiner — Application Security Engineer

Observations

This page is read-only — no Form Actions, no mutations. The attack surface is narrow: the only risk vector is the photo URL construction and the admin auth gate.
The template renders photo URLs as /uploads/{r.thumbnail}. The thumbnail column value comes directly from artikel_fotos.filename, which is stored server-side as a UUID-based relative path (e.g., 1/uuid.webp) — not user-supplied. This is safe: the value originates from the server's own randomUUID() call at upload time, never from user input.
The uploads serve route (/uploads/[...path]/+server.ts) has a path traversal guard (path.resolve() bounds check). Even if the database were compromised and contained a traversal payload, the serve route would reject it with 400. Defense in depth is present.
Admin auth is enforced by +layout.server.ts — this page has no direct auth check, which is correct. The layout guard redirects to /admin/login for unauthenticated requests. This matches the "redirect, don't 403" pattern.
The thumbnail value is used in an <img src="..."> attribute. Since the value comes from the server database (not reflected user input), there is no XSS risk here. SvelteKit's template engine escapes attribute values by default.
No form inputs exist on this page — no CSRF surface.
The alt="" on thumbnail images is appropriate: these are decorative thumbnails. Screen readers skip them. The article title adjacent to the image provides the accessible label.

Recommendations

The alt attribute on thumbnails is correctly empty (alt=""). Keep it — do not add article titles as alt text for these decorative thumbnails, as the title text is already rendered adjacent in the DOM. Redundant alt text degrades screen reader UX.
Confirm that artikel_fotos.filename is always stored as a relative path (e.g., 1/uuid.webp) and never as an absolute path or URL. If the upload logic ever writes an absolute path to the database, the /uploads/{r.thumbnail} URL construction would silently produce a broken URL rather than a traversal risk — but it should be validated at write time regardless.
No rate-limiting concern: this is a GET page behind admin auth. No additional controls needed.

(No open decisions — security posture on this read-only page is sound as specified.)

## 👤 Nora "NullX" Steiner — Application Security Engineer ### Observations - This page is read-only — no Form Actions, no mutations. The attack surface is narrow: the only risk vector is the photo URL construction and the admin auth gate. - The template renders photo URLs as `/uploads/{r.thumbnail}`. The `thumbnail` column value comes directly from `artikel_fotos.filename`, which is stored server-side as a UUID-based relative path (e.g., `1/uuid.webp`) — not user-supplied. This is safe: the value originates from the server's own `randomUUID()` call at upload time, never from user input. - The uploads serve route (`/uploads/[...path]/+server.ts`) has a path traversal guard (`path.resolve()` bounds check). Even if the database were compromised and contained a traversal payload, the serve route would reject it with 400. Defense in depth is present. - Admin auth is enforced by `+layout.server.ts` — this page has no direct auth check, which is correct. The layout guard redirects to `/admin/login` for unauthenticated requests. This matches the "redirect, don't 403" pattern. - The `thumbnail` value is used in an `<img src="...">` attribute. Since the value comes from the server database (not reflected user input), there is no XSS risk here. SvelteKit's template engine escapes attribute values by default. - No form inputs exist on this page — no CSRF surface. - The `alt=""` on thumbnail images is appropriate: these are decorative thumbnails. Screen readers skip them. The article title adjacent to the image provides the accessible label. ### Recommendations - The `alt` attribute on thumbnails is correctly empty (`alt=""`). Keep it — do not add article titles as alt text for these decorative thumbnails, as the title text is already rendered adjacent in the DOM. Redundant alt text degrades screen reader UX. - Confirm that `artikel_fotos.filename` is always stored as a relative path (e.g., `1/uuid.webp`) and never as an absolute path or URL. If the upload logic ever writes an absolute path to the database, the `/uploads/{r.thumbnail}` URL construction would silently produce a broken URL rather than a traversal risk — but it should be validated at write time regardless. - No rate-limiting concern: this is a GET page behind admin auth. No additional controls needed. _(No open decisions — security posture on this read-only page is sound as specified.)_

marcel commented

2026-05-05 11:28:21 +02:00

👤 Sara Holt — QA Engineer & Test Strategist

Observations

The issue acceptance criteria are testable: "all codes listed including zero-reservation codes", "dark header row", "thumbnail 26×26px", "grayed italic empty state", "thumbnail fallback placeholder", "no client-side fetching". Each criterion maps to a verifiable behavior.
The plan does not include a test step for this task. Every other task in the plan has a Step 1: Write the failing test. Task 12 skips directly to implementation. This breaks the TDD discipline the project established.
The critical integration behavior to test: when a code exists with no reservierungen, it still appears in byPerson with an empty array. This depends on the codes.map(...) JS grouping logic — it is not protected by a database constraint and must be explicitly tested.
The thumbnail fallback (placeholder <div> when r.thumbnail is null) is a rendering branch that needs a component test, not just a load function test.
The acceptance criterion "Data loaded in one page load — no client-side fetching" is verifiable: the load function must return byPerson and the Svelte component must not contain onMount or fetch. This is a static code check but should be part of the code review checklist.
E2E coverage: no new E2E journey is strictly required here — this is an admin-only read-only view. A unit/integration test of the load function is sufficient.

Recommendations

Add an integration test step before the implementation step:

// src/routes/admin/reservierungen/+page.server.test.ts
import { describe, it, expect, beforeEach } from 'vitest'
import { getDb, _resetForTesting } from '$lib/db'

beforeEach(() => {
  process.env.DATABASE_PATH = ':memory:'
  _resetForTesting()
})

describe('admin reservierungen load', () => {
  it('includes codes with zero reservations in byPerson', () => {
    const db = getDb()
    db.prepare("INSERT INTO codes (code, display_name) VALUES ('AB3K7MN2', 'Markus')").run()
    // no reservierungen inserted
    // call load() and assert byPerson[0].reservierungen.length === 0
  })

  it('includes article thumbnail path when foto exists', () => {
    // insert code, artikel, artikel_foto, reservierung
    // assert thumbnail is the filename of the foto with position 0
  })

  it('uses null thumbnail when artikel has no fotos', () => {
    // insert code, artikel (no fotos), reservierung
    // assert thumbnail is null
  })
})

Add a component test using vitest-browser-svelte to verify the empty-state renders the italic "Keine Reservierungen" text and that the person header is visible even for zero-reservation codes.
Verify the {#each} key expressions are present before testing — missing keys can cause incorrect DOM reconciliation in tests.
The integration test should import the actual load function (not mock it) and call it with a real :memory: database. Do not mock getDb().

Open Decisions (omit if none)

Test file location — the plan places lib tests in src/lib/ but route load function tests have no established location yet. Propose: src/routes/admin/reservierungen/+page.server.test.ts alongside the implementation. Confirm this is acceptable before implementation starts. (Raised by: Sara Holt)

## 👤 Sara Holt — QA Engineer & Test Strategist ### Observations - The issue acceptance criteria are testable: "all codes listed including zero-reservation codes", "dark header row", "thumbnail 26×26px", "grayed italic empty state", "thumbnail fallback placeholder", "no client-side fetching". Each criterion maps to a verifiable behavior. - The plan does **not include a test step** for this task. Every other task in the plan has a Step 1: Write the failing test. Task 12 skips directly to implementation. This breaks the TDD discipline the project established. - The critical integration behavior to test: when a code exists with no reservierungen, it still appears in `byPerson` with an empty array. This depends on the `codes.map(...)` JS grouping logic — it is not protected by a database constraint and must be explicitly tested. - The thumbnail fallback (placeholder `<div>` when `r.thumbnail` is null) is a rendering branch that needs a component test, not just a load function test. - The acceptance criterion "Data loaded in one page load — no client-side fetching" is verifiable: the load function must return `byPerson` and the Svelte component must not contain `onMount` or `fetch`. This is a static code check but should be part of the code review checklist. - E2E coverage: no new E2E journey is strictly required here — this is an admin-only read-only view. A unit/integration test of the load function is sufficient. ### Recommendations - Add an integration test step before the implementation step: ```typescript // src/routes/admin/reservierungen/+page.server.test.ts import { describe, it, expect, beforeEach } from 'vitest' import { getDb, _resetForTesting } from '$lib/db' beforeEach(() => { process.env.DATABASE_PATH = ':memory:' _resetForTesting() }) describe('admin reservierungen load', () => { it('includes codes with zero reservations in byPerson', () => { const db = getDb() db.prepare("INSERT INTO codes (code, display_name) VALUES ('AB3K7MN2', 'Markus')").run() // no reservierungen inserted // call load() and assert byPerson[0].reservierungen.length === 0 }) it('includes article thumbnail path when foto exists', () => { // insert code, artikel, artikel_foto, reservierung // assert thumbnail is the filename of the foto with position 0 }) it('uses null thumbnail when artikel has no fotos', () => { // insert code, artikel (no fotos), reservierung // assert thumbnail is null }) }) ``` - Add a component test using `vitest-browser-svelte` to verify the empty-state renders the italic "Keine Reservierungen" text and that the person header is visible even for zero-reservation codes. - Verify the `{#each}` key expressions are present before testing — missing keys can cause incorrect DOM reconciliation in tests. - The integration test should import the actual load function (not mock it) and call it with a real `:memory:` database. Do not mock `getDb()`. ### Open Decisions _(omit if none)_ - **Test file location** — the plan places lib tests in `src/lib/` but route load function tests have no established location yet. Propose: `src/routes/admin/reservierungen/+page.server.test.ts` alongside the implementation. Confirm this is acceptable before implementation starts. _(Raised by: Sara Holt)_

marcel commented

2026-05-05 11:28:44 +02:00

👤 Leonie Voss — UI/UX Design Lead

Observations

Thumbnail size discrepancy: The issue says 26×26px. The functional spec (§5.4) says 48px. The View 07 spec mockup CSS uses .res-thumb { width:26px; height:26px }. The plan's Svelte template uses w-[26px] h-[26px]. So 26px is the implemented spec value — but the acceptance criterion referencing "thumbnail 26×26px" is inconsistent with the functional spec paragraph which says 48px. One of these is wrong and needs to be authoritative.
Zero-reservation graying: The View 07 mockup uses opacity:.4 on both the dark header row and the res-body for zero-reservation people. The plan's Svelte template does not apply any opacity or visual graying to the header row for zero-reservation codes — only the body's empty-state text uses text-ink-muted italic. This is a significant spec deviation. The header should also be visually dimmed.
"Keine Reservierungen" text color: The plan uses text-ink-muted italic (color: #6B6050). The spec mockup uses color: rgba(255,255,255,.35) on a white background body — which renders as near-invisible light text. The plan's approach (text-ink-muted) is readable on the white surface, but may not match the intended "ausgegraut" feel. The spec mockup's white text on white background at 35% opacity seems intentional for the subdued style.
Header row styling: The plan uses bg-admin-bg text-white px-2.5 py-1.5 text-[9.5px] font-bold rounded-t. The spec mockup uses .res-head { background:var(--ab); color:#fff; padding:6px 9px; font-size:9.5px; font-weight:700; border-radius:4px 4px 0 0 }. These match well — padding and border-radius are equivalent.
Count in header: The plan renders ({person.reservierungen.length}) only when count > 0. The spec mockup shows "Markus — 3 Artikel" format with an em dash separator, not parentheses. Spec: {person.displayName} — {count} Artikel (with " — 0 Artikel" even for empty, to be consistent).
Touch targets: This is an admin-only read-only view. No interactive elements beyond navigation. No touch target concerns.
Font compliance: The header uses font-bold (Inter 700). The page title font-serif text-[14px] font-bold correctly uses Lora. Design tokens used correctly — no hardcoded hex values in the template.
Brand tokens: bg-admin-bg, bg-surface, bg-canvas, text-ink, text-ink-muted — all correct token names from the Tailwind config.

Recommendations

Fix the zero-reservation header opacity to match the spec mockup. Apply opacity-40 to the entire person block (both header and body) when person.reservierungen.length === 0:
```
<div class="mb-2 {person.reservierungen.length === 0 ? 'opacity-40' : ''}">
```
This replicates the spec's opacity:.4 on the combined block.
Fix the header count format to use an em dash separator consistent with the spec mockup: {person.displayName} — {person.reservierungen.length} Artikel. Show "0 Artikel" for empty persons rather than hiding the count.
The thumbnail fallback placeholder <div class="... bg-[#C4B8A8]"> uses a hardcoded hex value. Replace with the established canvas placeholder color. The spec uses #C4B8A8 as a placeholder tone — add it as a CSS token placeholder or use bg-canvas as a reasonable fallback that stays within the token system.

Open Decisions (omit if none)

Thumbnail size: 26px vs 48px — §5.4 of the functional spec says 48px; the View 07 CSS mockup and the plan both use 26px. The mockup is scaled at ~55% (stated in the spec header), so 26px at display scale could correspond to ~47px at actual scale — consistent with 48px in the functional spec. The implementation should use 26px (which is the literal spec mockup value), but this needs explicit confirmation from the product owner. (Raised by: Leonie Voss)

## 👤 Leonie Voss — UI/UX Design Lead ### Observations - **Thumbnail size discrepancy:** The issue says 26×26px. The functional spec (§5.4) says 48px. The View 07 spec mockup CSS uses `.res-thumb { width:26px; height:26px }`. The plan's Svelte template uses `w-[26px] h-[26px]`. So 26px is the implemented spec value — but the acceptance criterion referencing "thumbnail 26×26px" is inconsistent with the functional spec paragraph which says 48px. One of these is wrong and needs to be authoritative. - **Zero-reservation graying:** The View 07 mockup uses `opacity:.4` on both the dark header row and the res-body for zero-reservation people. The plan's Svelte template does **not** apply any opacity or visual graying to the header row for zero-reservation codes — only the body's empty-state text uses `text-ink-muted italic`. This is a significant spec deviation. The header should also be visually dimmed. - **"Keine Reservierungen" text color:** The plan uses `text-ink-muted italic` (color: `#6B6050`). The spec mockup uses `color: rgba(255,255,255,.35)` on a white background body — which renders as near-invisible light text. The plan's approach (`text-ink-muted`) is readable on the white surface, but may not match the intended "ausgegraut" feel. The spec mockup's white text on white background at 35% opacity seems intentional for the subdued style. - **Header row styling:** The plan uses `bg-admin-bg text-white px-2.5 py-1.5 text-[9.5px] font-bold rounded-t`. The spec mockup uses `.res-head { background:var(--ab); color:#fff; padding:6px 9px; font-size:9.5px; font-weight:700; border-radius:4px 4px 0 0 }`. These match well — padding and border-radius are equivalent. - **Count in header:** The plan renders `({person.reservierungen.length})` only when count > 0. The spec mockup shows "Markus — 3 Artikel" format with an em dash separator, not parentheses. Spec: `{person.displayName} — {count} Artikel` (with " — 0 Artikel" even for empty, to be consistent). - **Touch targets:** This is an admin-only read-only view. No interactive elements beyond navigation. No touch target concerns. - **Font compliance:** The header uses `font-bold` (Inter 700). The page title `font-serif text-[14px] font-bold` correctly uses Lora. Design tokens used correctly — no hardcoded hex values in the template. - **Brand tokens:** `bg-admin-bg`, `bg-surface`, `bg-canvas`, `text-ink`, `text-ink-muted` — all correct token names from the Tailwind config. ### Recommendations - Fix the zero-reservation header opacity to match the spec mockup. Apply `opacity-40` to the entire person block (both header and body) when `person.reservierungen.length === 0`: ```svelte <div class="mb-2 {person.reservierungen.length === 0 ? 'opacity-40' : ''}"> ``` This replicates the spec's `opacity:.4` on the combined block. - Fix the header count format to use an em dash separator consistent with the spec mockup: `{person.displayName} — {person.reservierungen.length} Artikel`. Show "0 Artikel" for empty persons rather than hiding the count. - The thumbnail fallback placeholder `<div class="... bg-[#C4B8A8]">` uses a hardcoded hex value. Replace with the established canvas placeholder color. The spec uses `#C4B8A8` as a placeholder tone — add it as a CSS token `placeholder` or use `bg-canvas` as a reasonable fallback that stays within the token system. ### Open Decisions _(omit if none)_ - **Thumbnail size: 26px vs 48px** — §5.4 of the functional spec says 48px; the View 07 CSS mockup and the plan both use 26px. The mockup is scaled at ~55% (stated in the spec header), so 26px at display scale could correspond to ~47px at actual scale — consistent with 48px in the functional spec. **The implementation should use 26px** (which is the literal spec mockup value), but this needs explicit confirmation from the product owner. _(Raised by: Leonie Voss)_

marcel commented

2026-05-05 11:28:57 +02:00

👤 Tobias Wendt (@tobiwendt) — DevOps & Platform Engineer

Observations

This task creates two new files inside src/routes/admin/reservierungen/. No new infrastructure, environment variables, volumes, or Docker changes required. The deployment footprint is zero — the feature rides entirely on the existing container.
The /uploads/{r.thumbnail} URL in the template is served by the existing uploads route (src/routes/uploads/[...path]/+server.ts). No new file-serving configuration needed.
The page performs two synchronous SQLite reads on every load. better-sqlite3 is synchronous and single-process — no concurrency concern for a family-scale app. No connection pooling, no async queue, no caching layer needed.
The getDb() call returns the module-level singleton. The singleton is initialized once when the Node process starts. This is stable across requests — no per-request database connection overhead.
No new environment variables are introduced. The existing DATABASE_PATH and UPLOAD_DIR cover all requirements for this page.
The health check endpoint (/health) is not affected by this change.

Recommendations

Nothing to change on the infrastructure side for this feature. The existing Dockerfile, docker-compose.yml, Caddyfile, and environment variable set are all sufficient.
After deploying a build that includes this feature, the post-deploy smoke test should include a manual check that /admin/reservierungen loads without a 500 error. Add it to the deploy checklist:
```
curl -fsS -b "admin_session=Marcel" https://erbstuecke.raddatz.cloud/admin/reservierungen > /dev/null && echo "Reservierungen: OK"
```
(Use a valid session cookie from a real login — do not hardcode credentials in a script.)
The prepared statements in the load function (if left inline rather than moved to db.ts) will be re-prepared on every module import during SSR. This is not a reliability risk but it is unnecessary overhead. Support the move to module-level prepared statements that the architect and developer are recommending.

(No open decisions on the infrastructure side.)

## 👤 Tobias Wendt (@tobiwendt) — DevOps & Platform Engineer ### Observations - This task creates two new files inside `src/routes/admin/reservierungen/`. No new infrastructure, environment variables, volumes, or Docker changes required. The deployment footprint is zero — the feature rides entirely on the existing container. - The `/uploads/{r.thumbnail}` URL in the template is served by the existing uploads route (`src/routes/uploads/[...path]/+server.ts`). No new file-serving configuration needed. - The page performs two synchronous SQLite reads on every load. better-sqlite3 is synchronous and single-process — no concurrency concern for a family-scale app. No connection pooling, no async queue, no caching layer needed. - The `getDb()` call returns the module-level singleton. The singleton is initialized once when the Node process starts. This is stable across requests — no per-request database connection overhead. - No new environment variables are introduced. The existing `DATABASE_PATH` and `UPLOAD_DIR` cover all requirements for this page. - The health check endpoint (`/health`) is not affected by this change. ### Recommendations - Nothing to change on the infrastructure side for this feature. The existing Dockerfile, docker-compose.yml, Caddyfile, and environment variable set are all sufficient. - After deploying a build that includes this feature, the post-deploy smoke test should include a manual check that `/admin/reservierungen` loads without a 500 error. Add it to the deploy checklist: ```bash curl -fsS -b "admin_session=Marcel" https://erbstuecke.raddatz.cloud/admin/reservierungen > /dev/null && echo "Reservierungen: OK" ``` (Use a valid session cookie from a real login — do not hardcode credentials in a script.) - The prepared statements in the load function (if left inline rather than moved to `db.ts`) will be re-prepared on every module import during SSR. This is not a reliability risk but it is unnecessary overhead. Support the move to module-level prepared statements that the architect and developer are recommending. _(No open decisions on the infrastructure side.)_

marcel commented

2026-05-05 11:29:19 +02:00

👤 Elicit — Requirements Engineer

Observations

Issue quality: The issue is well-formed. It has a clear user story (US-ADM-007), explicit acceptance criteria, SQL approach, file list, dependency reference, and size estimate. It passes 7 of 8 Definition of Ready criteria.
Spec reference mismatch: The issue states "Spec: reservierung-design §5.4, views spec View 08." The Admin Reservierungen view is View 07 in the views spec (/admin/reservierungen). View 08 in the spec is the Übersicht dashboard (/admin/uebersicht). This is a documentation error in the issue — the correct reference is View 07. Low practical risk since the implementation target is unambiguous, but the mismatch should be corrected to avoid confusion.
Thumbnail size ambiguity: The acceptance criterion states "thumbnail 26×26px". Section §5.4 of reservierung-design states "Thumbnail (48 px)". These are contradictory. The views spec CSS mockup uses 26px (but states all mockups are at ~55% scale, which would put 48px at full scale as ~26px displayed). The issue criterion "26×26px" propagates the unresolved ambiguity from the spec into the acceptance criteria.
"Grayed italic" empty state: The acceptance criterion says "grayed italic 'Keine Reservierungen' message (spec: 'ausgegraut aber angezeigt')". The plan implements only the body text as italic+muted — the header row is not grayed. The spec mockup grays the entire person block (header + body) at opacity 0.4. The acceptance criterion is ambiguous about whether "ausgegraut" applies to the text alone or the entire person block.
Acceptance criterion completeness: The AC does not specify the count format in the header. The spec mockup shows "Markus — 3 Artikel". The plan implements "(3)" in parentheses. "Title or '—'" is mentioned for article titles. These are missing from the written ACs.
Dependency #8: The issue depends on #8 (likely the admin codes management). The issue does not specify what exactly from #8 is required — the codes table must be seeded with data. This is a prerequisite for the page to display anything meaningful.
No mutation path: The issue correctly identifies this as a read-only view. There are no actions (reserve, cancel, delete) on this page. This is correct per §5.4 — no AC is missing on the mutation side.

Recommendations

Correct the spec reference in the issue body: "views spec View 07" (not View 08).
Resolve the thumbnail size ambiguity explicitly: add a single authoritative value to the acceptance criteria. Based on the mockup CSS values (which are the implementation-ready artifact), use 26×26px and note that the §5.4 "48px" was written for the full-resolution design and the mockup scale applies.
Sharpen AC bullet 4: "Persons with zero reservations show their header row and body both visually dimmed (opacity-40) with grayed italic 'Keine Reservierungen' in the body." The current wording "ausgegraut aber angezeigt" is not testable in its present form.
Add a missing AC: "Person header shows display name and reservation count in the format '[Name] — [N] Artikel'."
The issue is otherwise implementation-ready. The plan's code is detailed enough that the developer can proceed, with the above clarifications resolved first.

Open Decisions (omit if none)

View 07 vs View 08 reference — the issue body says "View 08" but the Reservierungen page is View 07 in the spec. Confirm and correct. (Raised by: Elicit)
Thumbnail size canonical value — §5.4 says 48px, the issue/plan/mockup say 26px. One of these must be declared authoritative before implementation. (Raised by: Elicit)
"Ausgegraut" scope — does the opacity-40 dimming apply only to the body empty-state text, or to the entire person block (header + body)? The mockup grays both; the plan grays neither the header nor uses opacity. (Raised by: Elicit)

## 👤 Elicit — Requirements Engineer ### Observations - **Issue quality:** The issue is well-formed. It has a clear user story (US-ADM-007), explicit acceptance criteria, SQL approach, file list, dependency reference, and size estimate. It passes 7 of 8 Definition of Ready criteria. - **Spec reference mismatch:** The issue states "Spec: reservierung-design §5.4, views spec View 08." The Admin Reservierungen view is **View 07** in the views spec (`/admin/reservierungen`). View 08 in the spec is the Übersicht dashboard (`/admin/uebersicht`). This is a documentation error in the issue — the correct reference is View 07. Low practical risk since the implementation target is unambiguous, but the mismatch should be corrected to avoid confusion. - **Thumbnail size ambiguity:** The acceptance criterion states "thumbnail 26×26px". Section §5.4 of `reservierung-design` states "Thumbnail (48 px)". These are contradictory. The views spec CSS mockup uses 26px (but states all mockups are at ~55% scale, which would put 48px at full scale as ~26px displayed). The issue criterion "26×26px" propagates the unresolved ambiguity from the spec into the acceptance criteria. - **"Grayed italic" empty state:** The acceptance criterion says "grayed italic 'Keine Reservierungen' message (spec: 'ausgegraut aber angezeigt')". The plan implements only the body text as italic+muted — the header row is not grayed. The spec mockup grays the entire person block (header + body) at opacity 0.4. The acceptance criterion is ambiguous about whether "ausgegraut" applies to the text alone or the entire person block. - **Acceptance criterion completeness:** The AC does not specify the count format in the header. The spec mockup shows "Markus — 3 Artikel". The plan implements "(3)" in parentheses. "Title or '—'" is mentioned for article titles. These are missing from the written ACs. - **Dependency #8:** The issue depends on #8 (likely the admin codes management). The issue does not specify what exactly from #8 is required — the `codes` table must be seeded with data. This is a prerequisite for the page to display anything meaningful. - **No mutation path:** The issue correctly identifies this as a read-only view. There are no actions (reserve, cancel, delete) on this page. This is correct per §5.4 — no AC is missing on the mutation side. ### Recommendations - Correct the spec reference in the issue body: "views spec View 07" (not View 08). - Resolve the thumbnail size ambiguity explicitly: add a single authoritative value to the acceptance criteria. Based on the mockup CSS values (which are the implementation-ready artifact), use 26×26px and note that the §5.4 "48px" was written for the full-resolution design and the mockup scale applies. - Sharpen AC bullet 4: "Persons with zero reservations show their header row and body both visually dimmed (opacity-40) with grayed italic 'Keine Reservierungen' in the body." The current wording "ausgegraut aber angezeigt" is not testable in its present form. - Add a missing AC: "Person header shows display name and reservation count in the format '[Name] — [N] Artikel'." - The issue is otherwise implementation-ready. The plan's code is detailed enough that the developer can proceed, with the above clarifications resolved first. ### Open Decisions _(omit if none)_ - **View 07 vs View 08 reference** — the issue body says "View 08" but the Reservierungen page is View 07 in the spec. Confirm and correct. _(Raised by: Elicit)_ - **Thumbnail size canonical value** — §5.4 says 48px, the issue/plan/mockup say 26px. One of these must be declared authoritative before implementation. _(Raised by: Elicit)_ - **"Ausgegraut" scope** — does the opacity-40 dimming apply only to the body empty-state text, or to the entire person block (header + body)? The mockup grays both; the plan grays neither the header nor uses opacity. _(Raised by: Elicit)_

marcel commented

2026-05-05 11:29:43 +02:00

🗳️ Decision Queue — Action Required

6 decisions need your input before implementation starts.

Spec Accuracy

View reference correction — The issue body references "views spec View 08" but the Admin Reservierungen page is View 07 in the spec. View 08 is the Übersicht dashboard. No implementation impact, but the issue body should be corrected. (Raised by: Elicit)

Visual Design

Thumbnail size: 26px vs 48px — The functional spec §5.4 says "Thumbnail (48 px)". The views spec CSS mockup, the plan, and the issue acceptance criteria all say 26×26px. The mockup is stated to be at ~55% scale, so 26px displayed ≈ 47px full-size, consistent with 48px. Recommended decision: use 26×26px as specified in the implementation plan. Needs explicit product owner confirmation. (Raised by: Leonie Voss, Elicit)
"Ausgegraut" scope for zero-reservation persons — The spec mockup applies opacity: 0.4 to the entire person block (dark header row + white body). The implementation plan only shows the body's empty-state text as italic+muted — neither the header nor the body block is visually dimmed. Options: (A) dim the entire person block at opacity-40 (matches spec mockup), (B) dim only the body text (plan's current approach, easier to implement). (Raised by: Leonie Voss, Elicit)
Header count format — The spec mockup shows "Markus — 3 Artikel" with an em dash. The plan shows "(3)" in parentheses, hidden when count is 0. Options: (A) use em dash format "Markus — 3 Artikel" including "0 Artikel" for empty (spec-compliant), (B) keep parenthetical with count hidden for zero (plan's current approach, less visual noise). (Raised by: Leonie Voss)
Category display style — The plan renders the category as a canvas-colored rounded chip (bg-canvas px-1.5 py-0.5 rounded). The spec mockup shows plain all-caps text in text-ink-muted styling. Options: (A) plain all-caps label matching spec mockup, (B) keep badge chip (not spec-compliant but visually distinguishable). (Raised by: Felix Brandt)

Architecture / Code Convention

Intra-person sort order for reservierungen — The plan orders articles within each person's list by r.created_at DESC (newest first). The spec does not prescribe an order. Alternative: alphabetical by article title (more predictable for admin scanning). Low effort to change before implementation but high effort to change after. (Raised by: Markus Keller)

Testing

Route-level test file location — The plan has no test step for Task 12, breaking the established TDD pattern. A test file should be added. Proposed location: src/routes/admin/reservierungen/+page.server.test.ts (co-located with implementation). Confirm this location is acceptable, then add tests before writing the implementation. (Raised by: Sara Holt)

## 🗳️ Decision Queue — Action Required _6 decisions need your input before implementation starts._ --- ### Spec Accuracy - **View reference correction** — The issue body references "views spec View 08" but the Admin Reservierungen page is View 07 in the spec. View 08 is the Übersicht dashboard. No implementation impact, but the issue body should be corrected. _(Raised by: Elicit)_ --- ### Visual Design - **Thumbnail size: 26px vs 48px** — The functional spec §5.4 says "Thumbnail (48 px)". The views spec CSS mockup, the plan, and the issue acceptance criteria all say 26×26px. The mockup is stated to be at ~55% scale, so 26px displayed ≈ 47px full-size, consistent with 48px. **Recommended decision: use 26×26px as specified in the implementation plan.** Needs explicit product owner confirmation. _(Raised by: Leonie Voss, Elicit)_ - **"Ausgegraut" scope for zero-reservation persons** — The spec mockup applies `opacity: 0.4` to the entire person block (dark header row + white body). The implementation plan only shows the body's empty-state text as italic+muted — neither the header nor the body block is visually dimmed. Options: (A) dim the entire person block at opacity-40 (matches spec mockup), (B) dim only the body text (plan's current approach, easier to implement). _(Raised by: Leonie Voss, Elicit)_ - **Header count format** — The spec mockup shows "Markus — 3 Artikel" with an em dash. The plan shows "(3)" in parentheses, hidden when count is 0. Options: (A) use em dash format "Markus — 3 Artikel" including "0 Artikel" for empty (spec-compliant), (B) keep parenthetical with count hidden for zero (plan's current approach, less visual noise). _(Raised by: Leonie Voss)_ - **Category display style** — The plan renders the category as a canvas-colored rounded chip (`bg-canvas px-1.5 py-0.5 rounded`). The spec mockup shows plain all-caps text in `text-ink-muted` styling. Options: (A) plain all-caps label matching spec mockup, (B) keep badge chip (not spec-compliant but visually distinguishable). _(Raised by: Felix Brandt)_ --- ### Architecture / Code Convention - **Intra-person sort order for reservierungen** — The plan orders articles within each person's list by `r.created_at DESC` (newest first). The spec does not prescribe an order. Alternative: alphabetical by article title (more predictable for admin scanning). Low effort to change before implementation but high effort to change after. _(Raised by: Markus Keller)_ --- ### Testing - **Route-level test file location** — The plan has no test step for Task 12, breaking the established TDD pattern. A test file should be added. Proposed location: `src/routes/admin/reservierungen/+page.server.test.ts` (co-located with implementation). Confirm this location is acceptable, then add tests before writing the implementation. _(Raised by: Sara Holt)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/wannsee-kram#12

Build Admin Reservierungen — overview grouped by person #12

Task 12 — Plan reference: docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md

Acceptance criteria

SQL approach

Files to create

👤 Markus Keller — Application Architect

Observations

Recommendations

Open Decisions (omit if none)

👤 Felix Brandt — Fullstack Developer

Observations

Recommendations

Open Decisions (omit if none)

👤 Nora "NullX" Steiner — Application Security Engineer

Observations

Recommendations

👤 Sara Holt — QA Engineer & Test Strategist

Observations

Recommendations

Open Decisions (omit if none)

👤 Leonie Voss — UI/UX Design Lead

Observations

Recommendations

Open Decisions (omit if none)

👤 Tobias Wendt (@tobiwendt) — DevOps & Platform Engineer

Observations

Recommendations

👤 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions (omit if none)

🗳️ Decision Queue — Action Required

Spec Accuracy

Visual Design

Architecture / Code Convention

Testing

Task 12 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md`