Build Admin Code management — create, copy link, delete #11

New Issue

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

marcel commented

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

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

User story (US-ADM-004 + US-ADM-005 + US-ADM-006):

As an admin, I create a code by entering a display name; the code is auto-generated.
As an admin, I copy the ready-to-send link for any code.
As an admin, I delete a code (with warning about losing all reservations).

Acceptance criteria

Table shows: display name, code (monospace, tracking-widest, text-primary), number of reservations
"Code erstellen" form: name input + submit; generates 8-char cryptographically random code (reuses generateCode() from auth.ts)
Collision retry: if generated code already exists, try up to 5 times before returning error
"Link kopieren" button: calls navigator.clipboard.writeText(origin + '/?code=' + code) — client-side JS
"Löschen" button: window.confirm() dialog warns that all reservations for this person will also be deleted
Delete action: DELETE FROM codes WHERE id = ? — CASCADE removes all reservations for this code
Success flash: "✓ Code erstellt: [CODE]" shown after creation
Empty state when no codes exist

Files to create

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

Depends on: #8 | Size: S | Spec: reservierung-design §5.3, views spec View 07, NFR-SEC-001

## Task 11 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md` **User story (US-ADM-004 + US-ADM-005 + US-ADM-006):** - As an admin, I create a code by entering a display name; the code is auto-generated. - As an admin, I copy the ready-to-send link for any code. - As an admin, I delete a code (with warning about losing all reservations). ### Acceptance criteria - [ ] Table shows: display name, code (monospace, `tracking-widest`, `text-primary`), number of reservations - [ ] "Code erstellen" form: name input + submit; generates 8-char cryptographically random code (reuses `generateCode()` from auth.ts) - [ ] Collision retry: if generated code already exists, try up to 5 times before returning error - [ ] "Link kopieren" button: calls `navigator.clipboard.writeText(origin + '/?code=' + code)` — client-side JS - [ ] "Löschen" button: `window.confirm()` dialog warns that all reservations for this person will also be deleted - [ ] Delete action: `DELETE FROM codes WHERE id = ?` — CASCADE removes all reservations for this code - [ ] Success flash: "✓ Code erstellt: [CODE]" shown after creation - [ ] Empty state when no codes exist ### Files to create - `src/routes/admin/codes/+page.svelte` - `src/routes/admin/codes/+page.server.ts` **Depends on:** #8 | **Size:** S | **Spec:** reservierung-design §5.3, views spec View 07, NFR-SEC-001

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

marcel commented

2026-05-05 11:26:38 +02:00

🏗️ Markus Keller (@mkeller) — Application Architect

Observations

The issue correctly delegates code generation to generateCode() from auth.ts — good, that function is already spec'd with crypto.randomBytes and the confusable-char-free charset. No duplication.
The collision retry loop (up to 5 attempts) is the right pattern. The codes.code column has a UNIQUE constraint per the schema in system-design.md, so the retry catches the SQLite UNIQUE constraint failed error rather than doing a pre-check SELECT — this is correct.
DELETE FROM codes WHERE id = ? with CASCADE: the schema specifies code_id INTEGER NOT NULL REFERENCES codes(id) ON DELETE CASCADE on reservierungen — this will atomically remove all reservations for the deleted code. No application-level cleanup needed. Good.
The "Link kopieren" via navigator.clipboard.writeText is client-side JS, as noted. The spec acknowledges this. This is fine — the admin interface already requires JavaScript for the camera/upload flow; clipboard is not a regression.
The +page.server.ts actions shape for this page: one load returning all codes with reservation counts (a single SQL query with a COUNT join), plus two actions: createCode and deleteCode. Both should use prepared statements defined at module load in db.ts — not inline db.prepare() calls inside the action body.

Recommendations

Prepared statements belong in db.ts, not inside action handlers. Define stmtCodesAll, stmtInsertCode, stmtDeleteCode, and stmtCodeExists at module load. The +page.server.ts imports and calls them. This matches the pattern for every other route.

The reservation count query should be a single JOIN, not N+1. Use:

SELECT c.*, COUNT(r.id) AS reservierungen_anzahl
FROM codes c
LEFT JOIN reservierungen r ON r.code_id = c.id
GROUP BY c.id
ORDER BY c.created_at ASC

Prepare this once in db.ts as stmtCodesWithCount.

Collision retry: catch only SQLITE_CONSTRAINT_UNIQUE, rethrow everything else. The catch in the retry loop should check e instanceof Error && e.message.includes('UNIQUE constraint failed: codes.code') before retrying. An unrelated error (disk full, schema error) should not be silently swallowed by the retry loop.
After 5 failed collisions, return fail(500, { error: 'code_generation_failed' }) with a German user-facing message. The issue mentions "returning error" but doesn't specify the HTTP status. 500 is appropriate here since this is an internal entropy failure.
The display_name input must be validated server-side. Trim whitespace. Reject empty string. Max length 100 chars. formData.get('display_name')?.toString().trim() — if falsy, return fail(400, { error: 'name_required' }).

## 🏗️ Markus Keller (@mkeller) — Application Architect ### Observations - The issue correctly delegates code generation to `generateCode()` from `auth.ts` — good, that function is already spec'd with `crypto.randomBytes` and the confusable-char-free charset. No duplication. - The collision retry loop (up to 5 attempts) is the right pattern. The `codes.code` column has a `UNIQUE` constraint per the schema in `system-design.md`, so the retry catches the SQLite `UNIQUE constraint failed` error rather than doing a pre-check SELECT — this is correct. - `DELETE FROM codes WHERE id = ?` with CASCADE: the schema specifies `code_id INTEGER NOT NULL REFERENCES codes(id) ON DELETE CASCADE` on `reservierungen` — this will atomically remove all reservations for the deleted code. No application-level cleanup needed. Good. - The "Link kopieren" via `navigator.clipboard.writeText` is client-side JS, as noted. The spec acknowledges this. This is fine — the admin interface already requires JavaScript for the camera/upload flow; clipboard is not a regression. - The `+page.server.ts` actions shape for this page: one `load` returning all codes with reservation counts (a single SQL query with a `COUNT` join), plus two actions: `createCode` and `deleteCode`. Both should use prepared statements defined at module load in `db.ts` — not inline `db.prepare()` calls inside the action body. ### Recommendations - **Prepared statements belong in `db.ts`, not inside action handlers.** Define `stmtCodesAll`, `stmtInsertCode`, `stmtDeleteCode`, and `stmtCodeExists` at module load. The `+page.server.ts` imports and calls them. This matches the pattern for every other route. - **The reservation count query should be a single JOIN, not N+1.** Use: ```sql SELECT c.*, COUNT(r.id) AS reservierungen_anzahl FROM codes c LEFT JOIN reservierungen r ON r.code_id = c.id GROUP BY c.id ORDER BY c.created_at ASC ``` Prepare this once in `db.ts` as `stmtCodesWithCount`. - **Collision retry: catch only `SQLITE_CONSTRAINT_UNIQUE`, rethrow everything else.** The `catch` in the retry loop should check `e instanceof Error && e.message.includes('UNIQUE constraint failed: codes.code')` before retrying. An unrelated error (disk full, schema error) should not be silently swallowed by the retry loop. - **After 5 failed collisions, return `fail(500, { error: 'code_generation_failed' })` with a German user-facing message.** The issue mentions "returning error" but doesn't specify the HTTP status. 500 is appropriate here since this is an internal entropy failure. - **The `display_name` input must be validated server-side.** Trim whitespace. Reject empty string. Max length 100 chars. `formData.get('display_name')?.toString().trim()` — if falsy, `return fail(400, { error: 'name_required' })`.

marcel commented

2026-05-05 11:26:58 +02:00

👨‍💻 Felix Brandt (@felixbrandt) — Senior Fullstack Developer

Observations

This is a two-file task (+page.svelte + +page.server.ts) with three Form Actions: create, copy-link, delete. The copy-link is client-side only and doesn't need a Form Action — it's a <button onclick> calling navigator.clipboard.writeText(). The other two (create, delete) are standard Form Actions.
The issue says "reuses generateCode() from auth.ts" — confirmed in the plan's auth.ts implementation. The function is exported and unit-tested. No duplication needed.
The window.confirm() for delete is explicitly specified. This is the simplest correct approach for an admin-only tool. The spec's impl-ref table (View 06) also confirms: "Native confirm() oder Svelte Dialog-Komponente."
The success flash "✓ Code erstellt: [CODE]" — this is the ActionData returned from the createCode action. In SvelteKit, after a Form Action returns data (not a redirect), the page re-renders with form prop set. The Svelte component reads {#if form?.success} and shows the flash inline — no toast library needed.

Recommendations

Write the failing tests before implementing. Based on the TDD pattern in Task 3 of the plan, the test file goes at src/routes/admin/codes/+page.server.test.ts (or equivalent). Tests to write first:
1. load() returns codes with reservation counts
2. createCode action inserts a row and returns { success: true, code: '...' }
3. createCode action retries on collision and returns error after 5 failures
4. createCode action rejects empty display_name with fail(400)
5. deleteCode action removes the code and cascades reservations
6. deleteCode action returns fail(400) for non-integer id
{#each codes as code (code.id)} — the key expression is mandatory. Without it, Svelte's reconciler uses position, which can corrupt local button state (e.g., a "Kopiert ✓" flash on the wrong row after a delete).
The "Kopiert ✓" feedback needs $state. After navigator.clipboard.writeText(), set a $state variable to true for ~2 seconds, then reset. Use setTimeout inside the click handler. Do NOT use $effect for this — it's a one-shot side effect triggered by the click, not a reactive derived value.
Map form?.error keys to German user messages in the template. Never render raw error keys like 'name_required' or 'code_generation_failed' to the user. Add a lookup object or {#if form?.error === 'name_required'} branches with proper German strings.
The display_name input should use:enhance on the create form. After submission, the page re-renders. Without use:enhance, the browser does a full page reload and loses scroll position. With it, only the affected part updates.
Guard clause first in both actions:
```
export const actions = {
  createCode: async ({ request, locals }) => {
    if (!locals.admin) redirect(303, '/admin/login');
    // ... rest of action
  }
}
```
Even though +layout.server.ts guards the admin layout, per-action guards are a defense-in-depth measure consistent with how other admin actions in this project should be structured.

## 👨‍💻 Felix Brandt (@felixbrandt) — Senior Fullstack Developer ### Observations - This is a two-file task (`+page.svelte` + `+page.server.ts`) with three Form Actions: create, copy-link, delete. The copy-link is client-side only and doesn't need a Form Action — it's a `<button onclick>` calling `navigator.clipboard.writeText()`. The other two (create, delete) are standard Form Actions. - The issue says "reuses `generateCode()` from auth.ts" — confirmed in the plan's `auth.ts` implementation. The function is exported and unit-tested. No duplication needed. - The `window.confirm()` for delete is explicitly specified. This is the simplest correct approach for an admin-only tool. The spec's impl-ref table (View 06) also confirms: "Native `confirm()` oder Svelte Dialog-Komponente." - The success flash "✓ Code erstellt: [CODE]" — this is the `ActionData` returned from the `createCode` action. In SvelteKit, after a Form Action returns data (not a redirect), the page re-renders with `form` prop set. The Svelte component reads `{#if form?.success}` and shows the flash inline — no toast library needed. ### Recommendations - **Write the failing tests before implementing.** Based on the TDD pattern in Task 3 of the plan, the test file goes at `src/routes/admin/codes/+page.server.test.ts` (or equivalent). Tests to write first: 1. `load()` returns codes with reservation counts 2. `createCode` action inserts a row and returns `{ success: true, code: '...' }` 3. `createCode` action retries on collision and returns error after 5 failures 4. `createCode` action rejects empty display_name with `fail(400)` 5. `deleteCode` action removes the code and cascades reservations 6. `deleteCode` action returns `fail(400)` for non-integer id - **`{#each codes as code (code.id)}` — the key expression is mandatory.** Without it, Svelte's reconciler uses position, which can corrupt local button state (e.g., a "Kopiert ✓" flash on the wrong row after a delete). - **The "Kopiert ✓" feedback needs `$state`.** After `navigator.clipboard.writeText()`, set a `$state` variable to `true` for ~2 seconds, then reset. Use `setTimeout` inside the click handler. Do NOT use `$effect` for this — it's a one-shot side effect triggered by the click, not a reactive derived value. - **Map `form?.error` keys to German user messages in the template.** Never render raw error keys like `'name_required'` or `'code_generation_failed'` to the user. Add a lookup object or `{#if form?.error === 'name_required'}` branches with proper German strings. - **The `display_name` input should `use:enhance` on the create form.** After submission, the page re-renders. Without `use:enhance`, the browser does a full page reload and loses scroll position. With it, only the affected part updates. - **Guard clause first in both actions:** ```typescript export const actions = { createCode: async ({ request, locals }) => { if (!locals.admin) redirect(303, '/admin/login'); // ... rest of action } } ``` Even though `+layout.server.ts` guards the admin layout, per-action guards are a defense-in-depth measure consistent with how other admin actions in this project should be structured.

marcel commented

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

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

The generated link contains the family code in plaintext in the URL. origin + '/?code=' + code — this is by design (it's how the app works), but worth flagging: this link, if forwarded accidentally, grants full gallery access under that person's name. The window.confirm() delete dialog should mention this when relevant. Low risk, by-design.
window.confirm() for delete: adequate for this admin-only tool. The confirm dialog text must include the person's name and the word "Reservierungen" so the admin understands the cascade. A generic "Are you sure?" is not sufficient. The issue AC specifies "warns that all reservations for this person will also be deleted" — enforce this wording in German.
The display_name field is user-controlled text that will appear in the gallery next to "● [Name]". It must be validated (trimmed, max length, non-empty) before insertion. If an admin enters <script>alert(1)</script>, SvelteKit's SSR escaping will neutralize it in the template — but enforce the constraint at the server layer anyway: max 100 chars, strip leading/trailing whitespace, reject empty.
navigator.clipboard requires a secure context (HTTPS). Since Caddy handles TLS, production is always HTTPS. In local dev without HTTPS, navigator.clipboard.writeText() will throw. Handle this gracefully: try { await navigator.clipboard.writeText(url); } catch { /* fallback: show the URL in a prompt() */ }. This is dev-only friction but worth noting.
The id parameter for delete must be validated as a positive integer before use in SQL. Even though the delete button submits a hidden field from the server-rendered table (not user-editable), Form Action inputs can be tampered with via curl or DevTools. Always: const id = Number(formData.get('code_id')); if (!Number.isInteger(id) || id <= 0) return fail(400, ...).
Cookie security: the sameSite: 'lax' in the plan's auth.ts should be 'strict'. The Nora persona spec and the system design spec both mandate SameSite=Strict. The plan's Step 3 setFamilyCodeCookie uses 'lax' — this is a deviation from the security spec. The admin session cookie especially must be 'strict'.

Recommendations

Validate display_name on the server: trim, non-empty, max 100 chars. Return fail(400, { error: 'name_required' }) for empty after trim.
Validate code_id as a positive integer in the delete action. Don't trust hidden form fields.
The delete confirmation dialog must name the person and explicitly mention cascade deletion of reservations. Suggested German text: "Code für ${name} löschen? Alle ${anzahl} Reservierungen dieser Person werden ebenfalls gelöscht. Diese Aktion kann nicht rückgängig gemacht werden." Use window.confirm() with this full string.
Write a security test: verify that a family_code cookie holder cannot reach the codes admin page. The +layout.server.ts guard covers this, but test your configuration of it explicitly — framework defaults can change.
Fix sameSite: 'lax' → 'strict' in auth.ts before Task 11 runs (this is a pre-existing issue from Task 3 of the plan, not specific to this issue, but it affects the admin session cookie set during admin login).

## 🔒 Nora "NullX" Steiner — Application Security Engineer ### Observations - **The generated link contains the family code in plaintext in the URL.** `origin + '/?code=' + code` — this is by design (it's how the app works), but worth flagging: this link, if forwarded accidentally, grants full gallery access under that person's name. The `window.confirm()` delete dialog should mention this when relevant. Low risk, by-design. - **`window.confirm()` for delete: adequate for this admin-only tool.** The confirm dialog text must include the person's name and the word "Reservierungen" so the admin understands the cascade. A generic "Are you sure?" is not sufficient. The issue AC specifies "warns that all reservations for this person will also be deleted" — enforce this wording in German. - **The `display_name` field is user-controlled text that will appear in the gallery next to "● [Name]".** It must be validated (trimmed, max length, non-empty) before insertion. If an admin enters `<script>alert(1)</script>`, SvelteKit's SSR escaping will neutralize it in the template — but enforce the constraint at the server layer anyway: max 100 chars, strip leading/trailing whitespace, reject empty. - **`navigator.clipboard` requires a secure context (HTTPS).** Since Caddy handles TLS, production is always HTTPS. In local dev without HTTPS, `navigator.clipboard.writeText()` will throw. Handle this gracefully: `try { await navigator.clipboard.writeText(url); } catch { /* fallback: show the URL in a prompt() */ }`. This is dev-only friction but worth noting. - **The `id` parameter for delete must be validated as a positive integer before use in SQL.** Even though the delete button submits a hidden field from the server-rendered table (not user-editable), Form Action inputs can be tampered with via curl or DevTools. Always: `const id = Number(formData.get('code_id')); if (!Number.isInteger(id) || id <= 0) return fail(400, ...)`. - **Cookie security: the `sameSite: 'lax'` in the plan's `auth.ts` should be `'strict'`.** The Nora persona spec and the system design spec both mandate `SameSite=Strict`. The plan's Step 3 `setFamilyCodeCookie` uses `'lax'` — this is a deviation from the security spec. The admin session cookie especially must be `'strict'`. ### Recommendations - **Validate `display_name` on the server: trim, non-empty, max 100 chars.** Return `fail(400, { error: 'name_required' })` for empty after trim. - **Validate `code_id` as a positive integer in the delete action.** Don't trust hidden form fields. - **The delete confirmation dialog must name the person and explicitly mention cascade deletion of reservations.** Suggested German text: `"Code für ${name} löschen? Alle ${anzahl} Reservierungen dieser Person werden ebenfalls gelöscht. Diese Aktion kann nicht rückgängig gemacht werden."` Use `window.confirm()` with this full string. - **Write a security test: verify that a `family_code` cookie holder cannot reach the codes admin page.** The `+layout.server.ts` guard covers this, but test your configuration of it explicitly — framework defaults can change. - **Fix `sameSite: 'lax'` → `'strict'` in `auth.ts` before Task 11 runs** (this is a pre-existing issue from Task 3 of the plan, not specific to this issue, but it affects the admin session cookie set during admin login).

marcel commented

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

🧪 Sara Holt (@saraholt) — QA Engineer & Test Strategist

Observations

This feature has a clearly defined test pyramid. I'm mapping it explicitly so nothing falls through.
The collision retry loop (up to 5 attempts) is the most unusual logic in this issue — it must have a dedicated integration test.
The cascade delete behavior (reservations disappear when code is deleted) is a database-level contract. It must be verified with a :memory: integration test, not just assumed from the schema.
The "Link kopieren" button is client-side only. This is the one E2E-level behavior in this issue that cannot be tested at the integration layer.

Test Plan

Unit tests (src/lib/auth.test.ts — already planned in Task 3 of the plan):

generateCode() returns 8-char alphanumeric — ✅ already specified in plan
generateCode() returns different values on consecutive calls — ✅ already in plan

Integration tests (src/routes/admin/codes/+page.server.test.ts):

load() returns empty array when no codes exist (empty state)
load() returns codes with correct reservierungen_anzahl = 0 for new codes
load() returns correct reservierungen_anzahl when reservations exist (JOIN correctness)
createCode action with valid name inserts a code and returns { success: true, code: /^[A-Z0-9]{8}$/ }
createCode action with empty display_name returns fail(400, { error: 'name_required' })
createCode action with whitespace-only display_name returns fail(400) after trim
createCode action retries on UNIQUE collision and eventually succeeds (mock generateCode to collide N-1 times)
createCode action returns fail(500) after 5 consecutive collisions
deleteCode action removes the code row
deleteCode action cascades: associated reservations are also deleted (FK constraint test)
deleteCode action with non-integer code_id returns fail(400)
deleteCode action with unknown code_id returns gracefully (no row → no-op or 404)

E2E tests (Playwright — addition to the critical journey list):

Admin creates a code → sees it in the table with "0" reservations and the monospace code value → "✓ Code erstellt: [CODE]" flash is visible
Admin clicks "🔗 Link" button → browser clipboard contains /?code= URL (use page.evaluate(() => navigator.clipboard.readText()) — requires clipboard permissions in Playwright config)
Admin deletes a code with 0 reservations → confirm dialog → code disappears from table
Admin views empty state when no codes exist

Recommendations

Test the collision retry by making generateCode injectable or mockable. The cleanest approach: accept an optional codeGenerator parameter in the createCode action helper, defaulting to generateCode from auth.ts. Tests pass a generator that returns a known-colliding value N times then a unique one.
Test the cascade explicitly with a :memory: DB, not just by trusting the schema: insert a code, insert a reservation for that code, delete the code, then verify SELECT * FROM reservierungen WHERE code_id = ? returns no rows.
Include the empty state in the E2E journey — the issue specifies it explicitly. Playwright should verify that a specific "Noch keine Codes" message (or equivalent) is visible on a fresh admin session before any codes are created.
The "Kopiert ✓" clipboard feedback needs an E2E test. Grant clipboard read permission in playwright.config.ts:
```
use: { permissions: ['clipboard-read', 'clipboard-write'] }
```
Then assert page.evaluate(() => navigator.clipboard.readText()) equals the expected URL format.

## 🧪 Sara Holt (@saraholt) — QA Engineer & Test Strategist ### Observations - This feature has a clearly defined test pyramid. I'm mapping it explicitly so nothing falls through. - The collision retry loop (up to 5 attempts) is the most unusual logic in this issue — it must have a dedicated integration test. - The cascade delete behavior (reservations disappear when code is deleted) is a database-level contract. It must be verified with a `:memory:` integration test, not just assumed from the schema. - The "Link kopieren" button is client-side only. This is the one E2E-level behavior in this issue that cannot be tested at the integration layer. ### Test Plan **Unit tests (`src/lib/auth.test.ts` — already planned in Task 3 of the plan):** - `generateCode()` returns 8-char alphanumeric — ✅ already specified in plan - `generateCode()` returns different values on consecutive calls — ✅ already in plan **Integration tests (`src/routes/admin/codes/+page.server.test.ts`):** 1. `load()` returns empty array when no codes exist (empty state) 2. `load()` returns codes with correct `reservierungen_anzahl = 0` for new codes 3. `load()` returns correct `reservierungen_anzahl` when reservations exist (JOIN correctness) 4. `createCode` action with valid name inserts a code and returns `{ success: true, code: /^[A-Z0-9]{8}$/ }` 5. `createCode` action with empty `display_name` returns `fail(400, { error: 'name_required' })` 6. `createCode` action with whitespace-only `display_name` returns `fail(400)` after trim 7. `createCode` action retries on UNIQUE collision and eventually succeeds (mock `generateCode` to collide N-1 times) 8. `createCode` action returns `fail(500)` after 5 consecutive collisions 9. `deleteCode` action removes the code row 10. `deleteCode` action cascades: associated reservations are also deleted (FK constraint test) 11. `deleteCode` action with non-integer `code_id` returns `fail(400)` 12. `deleteCode` action with unknown `code_id` returns gracefully (no row → no-op or 404) **E2E tests (Playwright — addition to the critical journey list):** - Admin creates a code → sees it in the table with "0" reservations and the monospace code value → "✓ Code erstellt: [CODE]" flash is visible - Admin clicks "🔗 Link" button → browser clipboard contains `/?code=` URL (use `page.evaluate(() => navigator.clipboard.readText())` — requires clipboard permissions in Playwright config) - Admin deletes a code with 0 reservations → confirm dialog → code disappears from table - Admin views empty state when no codes exist ### Recommendations - **Test the collision retry by making `generateCode` injectable or mockable.** The cleanest approach: accept an optional `codeGenerator` parameter in the `createCode` action helper, defaulting to `generateCode` from `auth.ts`. Tests pass a generator that returns a known-colliding value N times then a unique one. - **Test the cascade explicitly with a `:memory:` DB**, not just by trusting the schema: insert a code, insert a reservation for that code, delete the code, then verify `SELECT * FROM reservierungen WHERE code_id = ?` returns no rows. - **Include the empty state in the E2E journey** — the issue specifies it explicitly. Playwright should verify that a specific "Noch keine Codes" message (or equivalent) is visible on a fresh admin session before any codes are created. - **The "Kopiert ✓" clipboard feedback needs an E2E test.** Grant clipboard read permission in `playwright.config.ts`: ```typescript use: { permissions: ['clipboard-read', 'clipboard-write'] } ``` Then assert `page.evaluate(() => navigator.clipboard.readText())` equals the expected URL format.

marcel commented

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

🎨 Leonie Voss (@leonievoss) — UX Design Lead & Accessibility Strategist

Observations

View 06 in the spec (docs/superpowers/specs/2026-05-05-erbstuecke-wannsee-views.html) is the ground truth for this screen. I'm reviewing against it exactly.
The spec shows a table with columns: Name | Code (monospace, green, letter-spacing: 2px) | Res. (reservation count) | Actions (🔗 Link + ✕ Delete buttons). The issue AC matches this.
The issue says the "Code" column uses tracking-widest and text-primary — the spec's .code-mono class confirms: font-family: monospace; font-size: 11px; font-weight: 600; color: var(--pr); letter-spacing: 2px. In Tailwind: font-mono text-[11px] font-semibold text-primary tracking-[2px].
The + Neuer Code button in the spec is styled with background: var(--ac) (amber #C4874A), not green. This matches the admin-area convention where amber = primary admin action. The issue doesn't specify this — the implementer should follow the spec, not use bg-primary (green) for this button.
The "🔗 Link" action button uses .act-link in the spec: color: var(--pr); background: #DFF0E6; border: 1px solid #A8D5B8. In Tailwind: text-primary bg-[#DFF0E6] border border-[#A8D5B8] text-[9px] font-bold rounded px-1.5 py-0.5.
The Delete button uses .act-del: color: var(--tk); background: #FBF0F0; border: 1px solid #E0C0C0. The ✕ icon-only button needs an aria-label.

Recommendations

"+ Neuer Code" button must use bg-accent (amber), not bg-primary (green). See the spec's View 06 mock — this is consistent with how the admin inventar's "+ Hinzufügen" button is styled.
The "🔗 Link" button must have aria-label="Link kopieren für [Name]". The icon emoji alone is not accessible. Screen readers need to know what link is being copied and for whom.
The "✕" delete button must have aria-label="Code löschen für [Name]". Without it, screen readers announce "button, ✕" which is meaningless.
"Kopiert ✓" visual feedback: change button text, not a toast. After clipboard write succeeds, update the button's text to "Kopiert ✓" for ~2 seconds using a per-row $state variable. This approach is visible, non-dismissable, and doesn't require a toast library. The text change should also announce to aria-live="polite" so screen reader users are informed.
The success flash "✓ Code erstellt: [CODE]" should render the code in the same monospace style as the table. Use <code class="font-mono tracking-[2px] text-primary"> inline within the flash message.
Empty state: include a descriptive message, not just blank space. Suggested: a centered paragraph with text-ink-muted text-sm reading "Noch keine Zugangscodes erstellt." Below it, nudge toward the "+ Neuer Code" button.
The create form: the name input needs a visible <label> (not just placeholder). Even if the label is visually small (using the admin form label style: text-[9px] font-extrabold uppercase tracking-wide text-ink-muted), it must be a real <label for="..."> element — not a placeholder.
Touch targets on action buttons: The spec's .act-btn has padding: 3px 7px which is below 44px on touch devices. Since this is an admin-only desktop-first view, this is acceptable per the design spec. However, add min-h-[32px] (as shown in the spec's "+ Neuer Code" button) to ensure basic tap affordance on mobile admin use.

Open Decisions

window.confirm() vs. native <dialog> for the delete confirmation. The spec's impl-ref table says "Native confirm() oder Svelte Dialog-Komponente." window.confirm() is simpler but blocks the main thread, cannot be styled, and has no focus trap. A native <dialog> element would be accessible (built-in focus trap, escapeable) and consistently styled. For an admin-only tool with 3 users this is low stakes — but if the project uses <dialog> elsewhere (e.g., article modal), consistency favors <dialog> here too.

## 🎨 Leonie Voss (@leonievoss) — UX Design Lead & Accessibility Strategist ### Observations - **View 06 in the spec (`docs/superpowers/specs/2026-05-05-erbstuecke-wannsee-views.html`) is the ground truth for this screen.** I'm reviewing against it exactly. - The spec shows a table with columns: **Name** | **Code** (monospace, green, `letter-spacing: 2px`) | **Res.** (reservation count) | **Actions** (🔗 Link + ✕ Delete buttons). The issue AC matches this. - The issue says the "Code" column uses `tracking-widest` and `text-primary` — the spec's `.code-mono` class confirms: `font-family: monospace; font-size: 11px; font-weight: 600; color: var(--pr); letter-spacing: 2px`. In Tailwind: `font-mono text-[11px] font-semibold text-primary tracking-[2px]`. - The `+ Neuer Code` button in the spec is styled with `background: var(--ac)` (amber `#C4874A`), not green. This matches the admin-area convention where amber = primary admin action. The issue doesn't specify this — the implementer should follow the spec, not use `bg-primary` (green) for this button. - The "🔗 Link" action button uses `.act-link` in the spec: `color: var(--pr); background: #DFF0E6; border: 1px solid #A8D5B8`. In Tailwind: `text-primary bg-[#DFF0E6] border border-[#A8D5B8] text-[9px] font-bold rounded px-1.5 py-0.5`. - The Delete button uses `.act-del`: `color: var(--tk); background: #FBF0F0; border: 1px solid #E0C0C0`. The ✕ icon-only button needs an `aria-label`. ### Recommendations - **"+ Neuer Code" button must use `bg-accent` (amber), not `bg-primary` (green).** See the spec's View 06 mock — this is consistent with how the admin inventar's "+ Hinzufügen" button is styled. - **The "🔗 Link" button must have `aria-label="Link kopieren für [Name]"`.** The icon emoji alone is not accessible. Screen readers need to know what link is being copied and for whom. - **The "✕" delete button must have `aria-label="Code löschen für [Name]"`.** Without it, screen readers announce "button, ✕" which is meaningless. - **"Kopiert ✓" visual feedback: change button text, not a toast.** After clipboard write succeeds, update the button's text to "Kopiert ✓" for ~2 seconds using a per-row `$state` variable. This approach is visible, non-dismissable, and doesn't require a toast library. The text change should also announce to `aria-live="polite"` so screen reader users are informed. - **The success flash "✓ Code erstellt: [CODE]" should render the code in the same monospace style as the table.** Use `<code class="font-mono tracking-[2px] text-primary">` inline within the flash message. - **Empty state: include a descriptive message, not just blank space.** Suggested: a centered paragraph with `text-ink-muted text-sm` reading "Noch keine Zugangscodes erstellt." Below it, nudge toward the "+ Neuer Code" button. - **The create form: the name input needs a visible `<label>` (not just placeholder).** Even if the label is visually small (using the admin form label style: `text-[9px] font-extrabold uppercase tracking-wide text-ink-muted`), it must be a real `<label for="...">` element — not a placeholder. - **Touch targets on action buttons:** The spec's `.act-btn` has `padding: 3px 7px` which is below 44px on touch devices. Since this is an admin-only desktop-first view, this is acceptable per the design spec. However, add `min-h-[32px]` (as shown in the spec's "+ Neuer Code" button) to ensure basic tap affordance on mobile admin use. ### Open Decisions - **`window.confirm()` vs. native `<dialog>` for the delete confirmation.** The spec's impl-ref table says "Native `confirm()` oder Svelte Dialog-Komponente." `window.confirm()` is simpler but blocks the main thread, cannot be styled, and has no focus trap. A native `<dialog>` element would be accessible (built-in focus trap, escapeable) and consistently styled. For an admin-only tool with 3 users this is low stakes — but if the project uses `<dialog>` elsewhere (e.g., article modal), consistency favors `<dialog>` here too.

marcel commented

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

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

Observations

This issue creates two files entirely within src/routes/admin/codes/. No new environment variables, no new volumes, no infrastructure changes. From a DevOps standpoint, this task is clean — no deployment impact.
The navigator.clipboard.writeText() call constructs the link as origin + '/?code=' + code. In production, window.location.origin will be https://erbstuecke.raddatz.cloud. This is correct as long as the Caddyfile reverse-proxies correctly. No action needed here.
The delete CASCADE happens at the SQLite layer — no file cleanup required for codes (unlike article deletion which requires removing photos from the uploads volume). No ops concern here.
The health check endpoint (/health) referenced in the devops persona spec is not part of this issue — but it should be in place before the app goes live. If it isn't implemented by the time this feature is done, flag it.

Recommendations

No deployment changes required for this issue. The existing docker-compose.yml (with named volumes db: and uploads:) and Dockerfile cover this feature completely.
Verify the clipboard origin in the post-deploy smoke test. After deploying, manually click the "🔗 Link" button on the codes page and confirm the copied URL starts with https://erbstuecke.raddatz.cloud/ (not http://localhost:3000/). This is a one-time sanity check — window.location.origin will be correct if Caddy is properly forwarding the Host header (it does by default).
Confirm X-Forwarded-Proto is not needed. SvelteKit's url.origin in server-side code uses the actual request headers. Since the clipboard URL is built client-side with window.location.origin, this is unambiguously correct regardless of proxy config. No action needed.
No concern from my angle on the collision retry. Up to 5 retries against a SQLite WAL-mode database is microseconds of work. No timeout risk, no concurrency concern at this scale.

## 🚀 Tobias Wendt (@tobiwendt) — DevOps & Platform Engineer ### Observations - This issue creates two files entirely within `src/routes/admin/codes/`. No new environment variables, no new volumes, no infrastructure changes. From a DevOps standpoint, this task is clean — no deployment impact. - The `navigator.clipboard.writeText()` call constructs the link as `origin + '/?code=' + code`. In production, `window.location.origin` will be `https://erbstuecke.raddatz.cloud`. This is correct as long as the Caddyfile reverse-proxies correctly. No action needed here. - The delete CASCADE happens at the SQLite layer — no file cleanup required for codes (unlike article deletion which requires removing photos from the uploads volume). No ops concern here. - The health check endpoint (`/health`) referenced in the devops persona spec is not part of this issue — but it should be in place before the app goes live. If it isn't implemented by the time this feature is done, flag it. ### Recommendations - **No deployment changes required for this issue.** The existing `docker-compose.yml` (with named volumes `db:` and `uploads:`) and `Dockerfile` cover this feature completely. - **Verify the clipboard origin in the post-deploy smoke test.** After deploying, manually click the "🔗 Link" button on the codes page and confirm the copied URL starts with `https://erbstuecke.raddatz.cloud/` (not `http://localhost:3000/`). This is a one-time sanity check — `window.location.origin` will be correct if Caddy is properly forwarding the `Host` header (it does by default). - **Confirm `X-Forwarded-Proto` is not needed.** SvelteKit's `url.origin` in server-side code uses the actual request headers. Since the clipboard URL is built client-side with `window.location.origin`, this is unambiguously correct regardless of proxy config. No action needed. - **No concern from my angle on the collision retry.** Up to 5 retries against a SQLite WAL-mode database is microseconds of work. No timeout risk, no concurrency concern at this scale.

marcel commented

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

📋 Elicit — Requirements Engineer

Observations

The acceptance criteria are well-formed and implementation-ready. All three user stories (US-ADM-004, US-ADM-005, US-ADM-006) are represented in the AC. Each has a clear observable outcome. The issue passes the Definition of Ready check.
US-ADM-005 (copy link) is the only AC without a server-side failure mode. navigator.clipboard.writeText() can fail silently if the browser denies clipboard permission (e.g., focus lost, permissions API blocked). The issue doesn't specify what happens in this case. This is a gap.
The empty state AC is present ("Empty state when no codes exist") but the exact text/content of the empty state is not specified. The implementer will need to decide the German copy. This is minor but worth noting to avoid a back-and-forth.
The delete confirmation copy is underspecified. The AC says "warns that all reservations for this person will also be deleted" — but the exact German wording isn't given. The implementer will need to write it. Given the project's German-only policy, this should be explicit.
The success flash is specified: "✓ Code erstellt: [CODE]" — clear. ✅
The collision retry edge case (>5 failures) outcome is specified as "returning error" — but what the user sees in this case isn't defined. What German message? What does the form state look like?
No NFR for the code list page performance — with the expected number of codes (likely <20 for this family-scale app), this is a non-issue. Correctly omitted.

Recommendations

Add the clipboard failure fallback to the AC: "If navigator.clipboard.writeText() fails, show the URL in a prompt() dialog or display it as selectable text so the admin can copy manually." This is a completeness gap in US-ADM-005.
Specify the empty state German copy in the AC. Suggested: "Noch keine Zugangscodes erstellt." — prevents ambiguity during implementation.
Specify the delete confirmation dialog German wording. Suggested AC addition: window.confirm() text: "Code für [Name] löschen? Alle [N] Reservierungen dieser Person werden ebenfalls unwiderruflich gelöscht."
Specify the collision error German message. Suggested: "Code konnte nicht erstellt werden. Bitte versuche es erneut." — this covers the 5-retry-exhaustion case.
The issue correctly links dependency on #8 — good. Verify #8 includes the generateCode() export from auth.ts as a completed deliverable before this task starts.

Open Decisions

Empty state copy: The exact German text for when no codes exist is not specified. Options: (a) decide now and add to AC, (b) leave to implementer's discretion. Given German-only policy and the admin audience of 3 people, either is fine — but (a) avoids a review round-trip.

## 📋 Elicit — Requirements Engineer ### Observations - **The acceptance criteria are well-formed and implementation-ready.** All three user stories (US-ADM-004, US-ADM-005, US-ADM-006) are represented in the AC. Each has a clear observable outcome. The issue passes the Definition of Ready check. - **US-ADM-005 (copy link) is the only AC without a server-side failure mode.** `navigator.clipboard.writeText()` can fail silently if the browser denies clipboard permission (e.g., focus lost, permissions API blocked). The issue doesn't specify what happens in this case. This is a gap. - **The empty state AC is present** ("Empty state when no codes exist") but the exact text/content of the empty state is not specified. The implementer will need to decide the German copy. This is minor but worth noting to avoid a back-and-forth. - **The delete confirmation copy is underspecified.** The AC says "warns that all reservations for this person will also be deleted" — but the exact German wording isn't given. The implementer will need to write it. Given the project's German-only policy, this should be explicit. - **The success flash is specified**: "✓ Code erstellt: [CODE]" — clear. ✅ - **The collision retry edge case (>5 failures) outcome is specified as "returning error"** — but what the user sees in this case isn't defined. What German message? What does the form state look like? - **No NFR for the code list page performance** — with the expected number of codes (likely <20 for this family-scale app), this is a non-issue. Correctly omitted. ### Recommendations - **Add the clipboard failure fallback to the AC:** "If `navigator.clipboard.writeText()` fails, show the URL in a `prompt()` dialog or display it as selectable text so the admin can copy manually." This is a completeness gap in US-ADM-005. - **Specify the empty state German copy in the AC.** Suggested: "Noch keine Zugangscodes erstellt." — prevents ambiguity during implementation. - **Specify the delete confirmation dialog German wording.** Suggested AC addition: `window.confirm()` text: "Code für [Name] löschen? Alle [N] Reservierungen dieser Person werden ebenfalls unwiderruflich gelöscht." - **Specify the collision error German message.** Suggested: "Code konnte nicht erstellt werden. Bitte versuche es erneut." — this covers the 5-retry-exhaustion case. - **The issue correctly links dependency on #8** — good. Verify #8 includes the `generateCode()` export from `auth.ts` as a completed deliverable before this task starts. ### Open Decisions - **Empty state copy:** The exact German text for when no codes exist is not specified. Options: (a) decide now and add to AC, (b) leave to implementer's discretion. Given German-only policy and the admin audience of 3 people, either is fine — but (a) avoids a review round-trip.

marcel commented

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

🗳️ Decision Queue — Action Required

2 decisions need your input before implementation starts.

UX / Interaction Pattern

Delete confirmation: window.confirm() vs. native <dialog> — window.confirm() is simpler (3 lines of code), unblockable by popup blockers in most browsers, and explicitly permitted by the spec ("Native confirm() oder Svelte Dialog-Komponente"). A native <dialog> is properly styled, has a built-in focus trap, and is consistent if the article modal uses <dialog> too. Cost of window.confirm(): cannot be styled, blocks the main thread, looks out of place on modern browsers. Cost of <dialog>: ~20 extra lines of Svelte, a per-row $state binding. (Raised by: Leonie)

Requirements / Copy

Empty state German copy — The AC specifies an empty state must exist but doesn't give the text. Options: (a) Add to AC now: "Noch keine Zugangscodes erstellt." with a nudge toward the create form. (b) Leave to implementer's discretion. Option (a) is a 30-second decision that prevents a review round-trip. (Raised by: Elicit)

## 🗳️ Decision Queue — Action Required _2 decisions need your input before implementation starts._ ### UX / Interaction Pattern - **Delete confirmation: `window.confirm()` vs. native `<dialog>`** — `window.confirm()` is simpler (3 lines of code), unblockable by popup blockers in most browsers, and explicitly permitted by the spec ("Native `confirm()` oder Svelte Dialog-Komponente"). A native `<dialog>` is properly styled, has a built-in focus trap, and is consistent if the article modal uses `<dialog>` too. Cost of `window.confirm()`: cannot be styled, blocks the main thread, looks out of place on modern browsers. Cost of `<dialog>`: ~20 extra lines of Svelte, a per-row `$state` binding. _(Raised by: Leonie)_ ### Requirements / Copy - **Empty state German copy** — The AC specifies an empty state must exist but doesn't give the text. Options: (a) Add to AC now: "Noch keine Zugangscodes erstellt." with a nudge toward the create form. (b) Leave to implementer's discretion. Option (a) is a 30-second decision that prevents a review round-trip. _(Raised by: Elicit)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/wannsee-kram#11

Build Admin Code management — create, copy link, delete #11

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

Acceptance criteria

Files to create

🏗️ Markus Keller (@mkeller) — Application Architect

Observations

Recommendations

👨‍💻 Felix Brandt (@felixbrandt) — Senior Fullstack Developer

Observations

Recommendations

🔒 Nora "NullX" Steiner — Application Security Engineer

Observations

Recommendations

🧪 Sara Holt (@saraholt) — QA Engineer & Test Strategist

Observations

Test Plan

Recommendations

🎨 Leonie Voss (@leonievoss) — UX Design Lead & Accessibility Strategist

Observations

Recommendations

Open Decisions

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

Observations

Recommendations

📋 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🗳️ Decision Queue — Action Required

UX / Interaction Pattern

Requirements / Copy

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