Build Admin Inventar — article list and delete #9

New Issue

marcel · 2026-05-05T10:57:50+02:00

marcel commented

2026-05-05 10:57:50 +02:00

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

User story (US-ADM-003):
As an admin, I see all articles in a table with thumbnails, category, and reservation status, and can delete unreserved articles.

Acceptance criteria

Table shows: thumbnail (30×30px), title (or "—"), category, status (Frei / Reserviert)
"Reserviert" status in text-status-taken, "Frei" in text-status-free
"Bearbeiten" button links to /admin/inventar/{id}/bearbeiten
"Löschen" button only visible for unreserved articles
Delete requires window.confirm() confirmation dialog
Attempting to delete a reserved article server-side returns error: "Artikel ist reserviert. Reservierung zuerst aufheben."
After delete: deleteArtikelPhotos(id) removes files from disk, DB row deleted (CASCADE removes artikel_fotos)
"+ Artikel hinzufügen" button links to /admin/inventar/neu
Empty state message when no articles exist

Files to create

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

Depends on: #8, #4 | Size: S | Spec: reservierung-design §5.2 (Listenansicht)

## Task 9 — Plan reference: `docs/superpowers/plans/2026-05-05-erbstuecke-wannsee.md` **User story (US-ADM-003):** As an admin, I see all articles in a table with thumbnails, category, and reservation status, and can delete unreserved articles. ### Acceptance criteria - [ ] Table shows: thumbnail (30×30px), title (or "—"), category, status (Frei / Reserviert) - [ ] "Reserviert" status in `text-status-taken`, "Frei" in `text-status-free` - [ ] "Bearbeiten" button links to `/admin/inventar/{id}/bearbeiten` - [ ] "Löschen" button only visible for unreserved articles - [ ] Delete requires `window.confirm()` confirmation dialog - [ ] Attempting to delete a reserved article server-side returns error: "Artikel ist reserviert. Reservierung zuerst aufheben." - [ ] After delete: `deleteArtikelPhotos(id)` removes files from disk, DB row deleted (CASCADE removes `artikel_fotos`) - [ ] "+ Artikel hinzufügen" button links to `/admin/inventar/neu` - [ ] Empty state message when no articles exist ### Files to create - `src/routes/admin/inventar/+page.svelte` - `src/routes/admin/inventar/+page.server.ts` **Depends on:** #8, #4 | **Size:** S | **Spec:** reservierung-design §5.2 (Listenansicht)

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

marcel referenced this issue

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

Build Admin Artikel form — camera-first photo upload and edit #10

marcel commented

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

👤 Markus Keller — Application Architect

Observations

Route structure is correct. The two files to create (src/routes/admin/inventar/+page.svelte and +page.server.ts) sit exactly where the system design specifies. No deviation.
Delete flow depends on deleteArtikelPhotos(id) — this function belongs in lib/photos.ts and must remove the entire uploads/{artikel_id}/ directory. The issue correctly states files must be removed from disk before (or in the same transactional cleanup block as) the DB row deletion. The DB CASCADE handles artikel_fotos rows atomically; the filesystem cleanup is the non-atomic side that needs explicit handling.
Server-side reservation guard is critical. The "Löschen-Button nur für nicht reservierte Artikel sichtbar" acceptance criterion is a UI-only guard. The actual protection must live in the Form Action: query reservierungen for the artikel_id before deleting, and return the specified error if a row exists. Never trust the UI-only visibility rule alone.
Prepared statements must be defined at module load in lib/db.ts, not inside the route handler. The load function for this page needs: all articles with their reservation status (LEFT JOIN reservierungen + codes), plus a delete action that checks the constraint.
window.confirm() is synchronous and blocks the main thread — acceptable for this project scope. The Form Action must still enforce the server-side check regardless of whether the client confirms.
Dependency on #8 and #4 — confirm lib/db.ts already exports the stmtArtikelAll prepared statement with the LEFT JOIN. If #8 and #4 are done, this statement should already exist and can be reused here directly.

Recommendations

Export a dedicated prepared statement from lib/db.ts for the admin article list. It should be identical to or extend the family gallery query, adding nothing extra — the same LEFT JOIN shape works for both contexts.
In the delete Form Action, query reservierungen WHERE artikel_id = ? before calling deleteArtikelPhotos. If a row exists, return fail(409, { error: 'artikel_reserviert' }) with the German message from the acceptance criteria. Do not rely on a SQLite error for this — a FOREIGN KEY constraint won't prevent deletion here since the reservation references the article, not the other way around.
Delete order must be: (1) deleteArtikelPhotos(id) removes files from disk, (2) db.prepare('DELETE FROM artikel WHERE id = ?').run(id) — CASCADE removes artikel_fotos rows. If the file deletion fails, still proceed with DB deletion (best-effort filesystem cleanup, same pattern as photo upload rollback).
The empty-state message in the .svelte file should be an {#if artikel.length === 0} block, not hidden via CSS.
Do not create a new prepared statement inside the Form Action handler. Prepare it at module load in lib/db.ts and import it.

Open Decisions (omit if none)

Orphaned uploads on failed delete: If deleteArtikelPhotos(id) partially fails (some files deleted, then an I/O error), the DB row is then deleted successfully, leaving the article directory in an inconsistent state. Accept this as tolerable for project scope, or add a best-effort directory cleanup with rm -rf? The system design implies best-effort is acceptable, but this should be confirmed.

## 👤 Markus Keller — Application Architect ### Observations - **Route structure is correct.** The two files to create (`src/routes/admin/inventar/+page.svelte` and `+page.server.ts`) sit exactly where the system design specifies. No deviation. - **Delete flow depends on `deleteArtikelPhotos(id)`** — this function belongs in `lib/photos.ts` and must remove the entire `uploads/{artikel_id}/` directory. The issue correctly states files must be removed from disk before (or in the same transactional cleanup block as) the DB row deletion. The DB CASCADE handles `artikel_fotos` rows atomically; the filesystem cleanup is the non-atomic side that needs explicit handling. - **Server-side reservation guard is critical.** The "Löschen-Button nur für nicht reservierte Artikel sichtbar" acceptance criterion is a UI-only guard. The actual protection must live in the Form Action: query `reservierungen` for the `artikel_id` before deleting, and return the specified error if a row exists. Never trust the UI-only visibility rule alone. - **Prepared statements must be defined at module load in `lib/db.ts`**, not inside the route handler. The load function for this page needs: all articles with their reservation status (LEFT JOIN `reservierungen` + `codes`), plus a delete action that checks the constraint. - **`window.confirm()` is synchronous and blocks the main thread** — acceptable for this project scope. The Form Action must still enforce the server-side check regardless of whether the client confirms. - **Dependency on #8 and #4** — confirm `lib/db.ts` already exports the `stmtArtikelAll` prepared statement with the LEFT JOIN. If #8 and #4 are done, this statement should already exist and can be reused here directly. ### Recommendations - Export a dedicated prepared statement from `lib/db.ts` for the admin article list. It should be identical to or extend the family gallery query, adding nothing extra — the same LEFT JOIN shape works for both contexts. - In the delete Form Action, query `reservierungen WHERE artikel_id = ?` before calling `deleteArtikelPhotos`. If a row exists, return `fail(409, { error: 'artikel_reserviert' })` with the German message from the acceptance criteria. Do not rely on a SQLite error for this — a FOREIGN KEY constraint won't prevent deletion here since the reservation references the article, not the other way around. - Delete order must be: (1) `deleteArtikelPhotos(id)` removes files from disk, (2) `db.prepare('DELETE FROM artikel WHERE id = ?').run(id)` — CASCADE removes `artikel_fotos` rows. If the file deletion fails, still proceed with DB deletion (best-effort filesystem cleanup, same pattern as photo upload rollback). - The empty-state message in the `.svelte` file should be an `{#if artikel.length === 0}` block, not hidden via CSS. - **Do not create a new prepared statement inside the Form Action handler.** Prepare it at module load in `lib/db.ts` and import it. ### Open Decisions _(omit if none)_ - **Orphaned uploads on failed delete:** If `deleteArtikelPhotos(id)` partially fails (some files deleted, then an I/O error), the DB row is then deleted successfully, leaving the article directory in an inconsistent state. Accept this as tolerable for project scope, or add a best-effort directory cleanup with `rm -rf`? The system design implies best-effort is acceptable, but this should be confirmed.

marcel commented

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

👤 Felix Brandt — Fullstack Developer

Observations

Two files, clear scope. +page.svelte is an orchestrator component: it composes the table rows and holds no business logic. +page.server.ts owns the load query and the delete action. This is clean SvelteKit form.
$derived for computed delete-visibility per row. In the template, the "Löschen"-button visibility (artikel.reservierung === null) is a derived condition per row — but since this is inside {#each} with no reactive row state, a simple inline expression is fine. No need for a module-level $derived here.
{#each artikel as a (a.id)} — key by a.id is required. No exception.

Form Actions shape for delete:

export const actions = {
  deleteArtikel: async ({ request, locals }) => {
    if (!locals.admin) redirect(303, '/admin/login');
    const formData = await request.formData();
    const id = Number(formData.get('artikel_id'));
    if (!Number.isInteger(id) || id <= 0) return fail(400, { error: 'invalid_id' });
    // Check reservation
    const reservierung = stmtReservierungByArtikel.get(id);
    if (reservierung) return fail(409, { error: 'artikel_reserviert' });
    await deleteArtikelPhotos(id);
    stmtDeleteArtikel.run(id);
  }
};

window.confirm() in Svelte 5: Use an event handler with onclick rune syntax on the delete button, not on:click. The confirm must gate the form submission, not a fetch call.
Thumbnail rendering: The <img> src for the 30×30px thumbnail comes from artikel_fotos position 0. The load query needs to LEFT JOIN artikel_fotos on position = 0 as well. This adds one more join to the query — confirm the plan for the SQL shape.
"—" for missing title: {artikel.titel ?? '—'} in the template. Simple nullish coalescing, no need for a helper.
No onMount + fetch anywhere. All data via load function. No API endpoints needed.

Recommendations

Write the failing test before the Form Action. Priority tests:
1. it('returns 409 when deleting a reserved article') — integration test with :memory: DB
2. it('deletes article and photos when unreserved') — verify DB row gone + deleteArtikelPhotos called
3. it('returns 400 for invalid artikel_id') — boundary guard

The load function SQL should be one prepared statement in lib/db.ts:

SELECT a.id, a.titel, a.kategorie,
       f.filename AS thumbnail,
       r.code_id,
       c.display_name AS reserviert_von
FROM artikel a
LEFT JOIN artikel_fotos f ON f.artikel_id = a.id AND f.position = 0
LEFT JOIN reservierungen r ON r.artikel_id = a.id
LEFT JOIN codes c ON c.id = r.code_id
ORDER BY a.created_at DESC

Map the form?.error key in the template to the German message from the acceptance criteria: "Artikel ist reserviert. Reservierung zuerst aufheben." — do not render the raw key.
The delete <form> must use method="POST" action="?/deleteArtikel" use:enhance. The use:enhance callback should show a loading state (disable the button) to prevent double-submit.
Bearbeiten-Link: use SvelteKit's <a href="/admin/inventar/{a.id}/bearbeiten"> — not a button, not a form. An anchor is semantically correct for navigation.

Open Decisions (omit if none)

Thumbnail join vs. separate query: The LEFT JOIN on artikel_fotos WHERE position = 0 is clean but adds complexity to the main list query. Alternative: load article list, then load all position-0 photos in a second query and merge in the load function. The single-query approach is preferable for this scale, but confirm with Markus whether the prepared statement in lib/db.ts for the family gallery is to be reused (with the thumbnail join already present) or if a separate admin statement is warranted.

## 👤 Felix Brandt — Fullstack Developer ### Observations - **Two files, clear scope.** `+page.svelte` is an orchestrator component: it composes the table rows and holds no business logic. `+page.server.ts` owns the load query and the delete action. This is clean SvelteKit form. - **`$derived` for computed delete-visibility per row.** In the template, the "Löschen"-button visibility (`artikel.reservierung === null`) is a derived condition per row — but since this is inside `{#each}` with no reactive row state, a simple inline expression is fine. No need for a module-level `$derived` here. - **`{#each artikel as a (a.id)}`** — key by `a.id` is required. No exception. - **Form Actions shape for delete:** ```typescript export const actions = { deleteArtikel: async ({ request, locals }) => { if (!locals.admin) redirect(303, '/admin/login'); const formData = await request.formData(); const id = Number(formData.get('artikel_id')); if (!Number.isInteger(id) || id <= 0) return fail(400, { error: 'invalid_id' }); // Check reservation const reservierung = stmtReservierungByArtikel.get(id); if (reservierung) return fail(409, { error: 'artikel_reserviert' }); await deleteArtikelPhotos(id); stmtDeleteArtikel.run(id); } }; ``` - **`window.confirm()` in Svelte 5:** Use an event handler with `onclick` rune syntax on the delete button, not `on:click`. The confirm must gate the form submission, not a fetch call. - **Thumbnail rendering:** The `<img>` src for the 30×30px thumbnail comes from `artikel_fotos` position 0. The load query needs to LEFT JOIN `artikel_fotos` on `position = 0` as well. This adds one more join to the query — confirm the plan for the SQL shape. - **"—" for missing title:** `{artikel.titel ?? '—'}` in the template. Simple nullish coalescing, no need for a helper. - **No `onMount` + fetch anywhere.** All data via load function. No API endpoints needed. ### Recommendations - Write the failing test before the Form Action. Priority tests: 1. `it('returns 409 when deleting a reserved article')` — integration test with `:memory:` DB 2. `it('deletes article and photos when unreserved')` — verify DB row gone + `deleteArtikelPhotos` called 3. `it('returns 400 for invalid artikel_id')` — boundary guard - The load function SQL should be one prepared statement in `lib/db.ts`: ```sql SELECT a.id, a.titel, a.kategorie, f.filename AS thumbnail, r.code_id, c.display_name AS reserviert_von FROM artikel a LEFT JOIN artikel_fotos f ON f.artikel_id = a.id AND f.position = 0 LEFT JOIN reservierungen r ON r.artikel_id = a.id LEFT JOIN codes c ON c.id = r.code_id ORDER BY a.created_at DESC ``` - Map the `form?.error` key in the template to the German message from the acceptance criteria: `"Artikel ist reserviert. Reservierung zuerst aufheben."` — do not render the raw key. - The delete `<form>` must use `method="POST" action="?/deleteArtikel" use:enhance`. The `use:enhance` callback should show a loading state (disable the button) to prevent double-submit. - Bearbeiten-Link: use SvelteKit's `<a href="/admin/inventar/{a.id}/bearbeiten">` — not a button, not a form. An anchor is semantically correct for navigation. ### Open Decisions _(omit if none)_ - **Thumbnail join vs. separate query:** The LEFT JOIN on `artikel_fotos WHERE position = 0` is clean but adds complexity to the main list query. Alternative: load article list, then load all position-0 photos in a second query and merge in the load function. The single-query approach is preferable for this scale, but confirm with Markus whether the prepared statement in `lib/db.ts` for the family gallery is to be reused (with the thumbnail join already present) or if a separate admin statement is warranted.

marcel commented

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

👤 Nora "NullX" Steiner — Application Security Engineer

Observations

IDOR risk on delete action. The acceptance criterion "Löschen nur für nicht reservierte Artikel" is primarily a UI guard. Without a server-side reservation check in the Form Action, any admin with a valid admin_session cookie can POST ?/deleteArtikel with any artikel_id and delete any article — including reserved ones. This is not IDOR in the classical sense (different user accessing another user's data), but it is a bypass of a business rule that could cause data integrity harm.
Auth guard placement. The delete action must begin with if (!locals.admin) redirect(303, '/admin/login'). The +layout.server.ts under /admin/ provides the layout-level guard, but Form Actions in +page.server.ts files must still check locals.admin explicitly — the layout guard does not automatically protect actions.
window.confirm() is client-side only. An attacker (or a script) can POST the delete action without triggering the confirm dialog. The server-side reservation check is the real guard. The confirm dialog is a UX convenience, not a security control.
Input validation on artikel_id: The Form Action must validate that the submitted artikel_id is a positive integer before any DB query. A non-integer or negative value passed to stmtDeleteArtikel.run() could cause unexpected behavior, though with prepared statements the risk is low.
No file path derived from user input. The deleteArtikelPhotos(id) function in lib/photos.ts builds the path as path.join(UPLOAD_DIR, String(id)). Since id comes from the database (not from user input at deletion time), there is no path traversal risk here — the article ID is a trusted integer from the server-side load. Confirm this pattern is maintained.
Redirect after successful delete: After a successful delete, the action should redirect(303, '/admin/inventar') or use SvelteKit's invalidateAll() via use:enhance. A bare return without redirect leaves the form action URL in the browser history, which is not a security issue here but is poor UX and could lead to double-submit attempts.

Recommendations

Add the server-side reservation check as the first business logic check in the delete action, immediately after input validation and auth guard:
```
const reservierung = stmtReservierungByArtikel.get(id);
if (reservierung) {
  return fail(409, { error: 'artikel_reserviert' });
}
```
This is the authoritative guard — the UI "Löschen"-button visibility is only a usability hint.
Add a regression test for the bypass scenario: submit the delete action for a reserved article ID directly (bypassing the UI). Expect fail(409).
Confirm that deleteArtikelPhotos in lib/photos.ts uses path.resolve() to verify the constructed directory path starts with path.resolve(UPLOAD_DIR) before calling rm -rf. Even though id is an integer, a defense-in-depth bounds check costs nothing and prevents surprises if the call signature ever changes.
Do not reflect the artikel_id value in any error message returned to the client.

Open Decisions

Rate limiting on admin delete: Currently there is no rate limiting on Form Actions. For a family-scale admin with 3 known users, this is acceptable. Confirm that no rate limiting is needed for this project scope. (Raised by: Nora)

## 👤 Nora "NullX" Steiner — Application Security Engineer ### Observations - **IDOR risk on delete action.** The acceptance criterion "Löschen nur für nicht reservierte Artikel" is primarily a UI guard. Without a server-side reservation check in the Form Action, any admin with a valid `admin_session` cookie can POST `?/deleteArtikel` with any `artikel_id` and delete any article — including reserved ones. This is not IDOR in the classical sense (different user accessing another user's data), but it is a bypass of a business rule that could cause data integrity harm. - **Auth guard placement.** The delete action must begin with `if (!locals.admin) redirect(303, '/admin/login')`. The `+layout.server.ts` under `/admin/` provides the layout-level guard, but Form Actions in `+page.server.ts` files must still check `locals.admin` explicitly — the layout guard does not automatically protect actions. - **`window.confirm()` is client-side only.** An attacker (or a script) can POST the delete action without triggering the confirm dialog. The server-side reservation check is the real guard. The confirm dialog is a UX convenience, not a security control. - **Input validation on `artikel_id`:** The Form Action must validate that the submitted `artikel_id` is a positive integer before any DB query. A non-integer or negative value passed to `stmtDeleteArtikel.run()` could cause unexpected behavior, though with prepared statements the risk is low. - **No file path derived from user input.** The `deleteArtikelPhotos(id)` function in `lib/photos.ts` builds the path as `path.join(UPLOAD_DIR, String(id))`. Since `id` comes from the database (not from user input at deletion time), there is no path traversal risk here — the article ID is a trusted integer from the server-side load. Confirm this pattern is maintained. - **Redirect after successful delete:** After a successful delete, the action should `redirect(303, '/admin/inventar')` or use SvelteKit's `invalidateAll()` via `use:enhance`. A bare return without redirect leaves the form action URL in the browser history, which is not a security issue here but is poor UX and could lead to double-submit attempts. ### Recommendations - Add the server-side reservation check as the **first business logic check** in the delete action, immediately after input validation and auth guard: ```typescript const reservierung = stmtReservierungByArtikel.get(id); if (reservierung) { return fail(409, { error: 'artikel_reserviert' }); } ``` This is the authoritative guard — the UI "Löschen"-button visibility is only a usability hint. - Add a regression test for the bypass scenario: submit the delete action for a reserved article ID directly (bypassing the UI). Expect `fail(409)`. - Confirm that `deleteArtikelPhotos` in `lib/photos.ts` uses `path.resolve()` to verify the constructed directory path starts with `path.resolve(UPLOAD_DIR)` before calling `rm -rf`. Even though `id` is an integer, a defense-in-depth bounds check costs nothing and prevents surprises if the call signature ever changes. - Do not reflect the `artikel_id` value in any error message returned to the client. ### Open Decisions - **Rate limiting on admin delete:** Currently there is no rate limiting on Form Actions. For a family-scale admin with 3 known users, this is acceptable. Confirm that no rate limiting is needed for this project scope. _(Raised by: Nora)_

marcel commented

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

👤 Sara Holt — QA Engineer & Test Strategist

Observations

Test surface is well-defined. This issue has clear, binary acceptance criteria — each criterion maps directly to a testable behavior. No vague language.
Integration tests needed for the Form Action. The delete action has three distinct outcomes that must each have a dedicated integration test: (1) unreserved article deleted successfully, (2) reserved article returns the correct error, (3) invalid input rejected. These must use a :memory: SQLite database, not mocks.
deleteArtikelPhotos(id) is a side-effectful function — it performs filesystem operations. Unit-testing it requires either a real temp directory or a mock of fs.rm. A real temp directory is preferred (no mocks of Node builtins). Vitest's tmp utilities or os.tmpdir() + mkdtemp work here.
Component test for the table row states: Three visible states exist: (a) article with thumbnail + no reservation → Löschen-button visible; (b) article with thumbnail + reservation → Löschen-button absent; (c) article with no photos → thumbnail placeholder + "—" for title. Each is a vitest-browser-svelte component test.
Empty state: The "no articles" empty state message must have a component test verifying the element renders when artikel = [] is passed.
The window.confirm() call must be tested in Playwright, not Vitest, because it requires a real browser dialog. Playwright's page.on('dialog', dialog => dialog.accept()) handles this.

Recommendations

Add these integration tests in src/routes/admin/inventar/+page.server.test.ts:

it('delete action removes unreserved article from DB')
it('delete action returns 409 when article is reserved')
it('delete action returns 400 for non-integer artikel_id')
it('load function returns all articles with thumbnail and reservation status')
it('load function returns empty array when no articles exist')
it('redirects to /admin/login when admin cookie is absent')

Add component tests in src/routes/admin/inventar/InventarTabelle.test.ts (or inline in the page test):

it('shows Löschen button only for unreserved articles')
it('shows "—" for articles without a title')
it('renders thumbnail img with correct src for articles with photos')
it('shows empty state message when artikel array is empty')

E2E test to add to the Playwright suite:

test('admin can delete an unreserved article with confirmation dialog')
test('admin sees error when attempting to delete a reserved article')

The Playwright test for delete must: (1) dismiss the confirm dialog and verify the article is still in the list; (2) accept the confirm dialog and verify the article disappears from the list.
Do not write E2E tests for the "Bearbeiten" link destination or the "+ Artikel hinzufügen" link — those are covered by #8 and a future issue. A single navigation test that verifies the link href attribute is sufficient here.

Open Decisions (omit if none)

Test coverage for deleteArtikelPhotos with partial failures: If files are partially deleted before an I/O error, the test would need to simulate filesystem failures. This is complex and of low value for this project scope. Recommend skipping partial-failure simulation and testing only the happy path and the "directory does not exist" (no photos) path. Confirm with Markus. (Raised by: Sara)

## 👤 Sara Holt — QA Engineer & Test Strategist ### Observations - **Test surface is well-defined.** This issue has clear, binary acceptance criteria — each criterion maps directly to a testable behavior. No vague language. - **Integration tests needed for the Form Action.** The delete action has three distinct outcomes that must each have a dedicated integration test: (1) unreserved article deleted successfully, (2) reserved article returns the correct error, (3) invalid input rejected. These must use a `:memory:` SQLite database, not mocks. - **`deleteArtikelPhotos(id)` is a side-effectful function** — it performs filesystem operations. Unit-testing it requires either a real temp directory or a mock of `fs.rm`. A real temp directory is preferred (no mocks of Node builtins). Vitest's `tmp` utilities or `os.tmpdir()` + `mkdtemp` work here. - **Component test for the table row states:** Three visible states exist: (a) article with thumbnail + no reservation → Löschen-button visible; (b) article with thumbnail + reservation → Löschen-button absent; (c) article with no photos → thumbnail placeholder + "—" for title. Each is a `vitest-browser-svelte` component test. - **Empty state:** The "no articles" empty state message must have a component test verifying the element renders when `artikel = []` is passed. - **The `window.confirm()` call must be tested in Playwright**, not Vitest, because it requires a real browser dialog. Playwright's `page.on('dialog', dialog => dialog.accept())` handles this. ### Recommendations - Add these integration tests in `src/routes/admin/inventar/+page.server.test.ts`: ```typescript it('delete action removes unreserved article from DB') it('delete action returns 409 when article is reserved') it('delete action returns 400 for non-integer artikel_id') it('load function returns all articles with thumbnail and reservation status') it('load function returns empty array when no articles exist') it('redirects to /admin/login when admin cookie is absent') ``` - Add component tests in `src/routes/admin/inventar/InventarTabelle.test.ts` (or inline in the page test): ```typescript it('shows Löschen button only for unreserved articles') it('shows "—" for articles without a title') it('renders thumbnail img with correct src for articles with photos') it('shows empty state message when artikel array is empty') ``` - E2E test to add to the Playwright suite: ```typescript test('admin can delete an unreserved article with confirmation dialog') test('admin sees error when attempting to delete a reserved article') ``` - The Playwright test for delete must: (1) dismiss the confirm dialog and verify the article is still in the list; (2) accept the confirm dialog and verify the article disappears from the list. - **Do not write E2E tests for the "Bearbeiten" link destination or the "+ Artikel hinzufügen" link** — those are covered by #8 and a future issue. A single navigation test that verifies the link `href` attribute is sufficient here. ### Open Decisions _(omit if none)_ - **Test coverage for `deleteArtikelPhotos` with partial failures:** If files are partially deleted before an I/O error, the test would need to simulate filesystem failures. This is complex and of low value for this project scope. Recommend skipping partial-failure simulation and testing only the happy path and the "directory does not exist" (no photos) path. Confirm with Markus. _(Raised by: Sara)_

marcel commented

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

👤 Leonie Voss — UI/UX Design Lead & Accessibility Advocate

Observations

View 05 in 2026-05-05-erbstuecke-wannsee-views.html is the authoritative spec for this view. The admin table layout is fully specified: sidebar (176px, bg-admin), content area (bg-canvas p-3.5), article table with columns: thumbnail (30×30px, border-radius: 4px), Artikel (title), Kategorie, Status, and action buttons (Bearbeiten + Löschen).
Status colors: "Frei" must use text-status-free (#4A7C5C) with ✓ prefix. "Reserviert" must use text-status-taken (#9B6060) with ● prefix and the reserver's display name. The acceptance criteria correctly reference text-status-taken and text-status-free — confirm these are Tailwind tokens already defined in tailwind.config.js.
30×30px thumbnail: The spec uses .thumb-sm with border-radius: 4px. In Tailwind: w-[30px] h-[30px] rounded object-cover. When no photo exists, render a <div class="w-[30px] h-[30px] rounded bg-line"> placeholder — do not render a broken <img> with no src.
Action buttons: The spec shows small inline buttons: Bearbeiten (act-edit style: muted bg, border) and Löschen (act-del style: #FBF0F0 bg, status-taken border + text). Löschen must be visually disabled (.opacity-35 cursor-not-allowed) for reserved articles — not absent from the DOM. The spec mockup shows it dimmed, not removed.
"+ Hinzufügen" button: Amber accent (bg-accent), white text, positioned top-right of the content area aligned with the page title. Not a link — a styled button that navigates via JS or a standard <a> styled as button.
Admin sidebar: Uses +layout.svelte which should already exist from #4/#8. The Inventar link must have border-l-[3px] border-accent active state on this route.
Font usage: Table text uses Inter (sans). Page title "Inventar" uses Lora (serif, font-serif text-sm font-bold). Table headers are text-[8.5px] font-bold uppercase text-ink-muted.
The spec shows the Löschen button as dimmed (opacity) for reserved articles, not removed. This is an important distinction from the acceptance criterion which says "only visible for unreserved articles." The spec takes precedence visually — use opacity-35 pointer-events-none rather than {#if !reserviert}.

Recommendations

Use opacity-35 pointer-events-none on the Löschen button for reserved articles rather than conditionally rendering it. This matches the spec mockup and gives admins visual feedback that the action exists but is blocked, rather than hiding it entirely. The server still enforces the guard.
Render the thumbnail as <img src="/uploads/{thumbnail}" alt="" class="w-[30px] h-[30px] rounded object-cover flex-shrink-0"> with alt="" (decorative). When thumbnail is null, render <div class="w-[30px] h-[30px] rounded bg-line flex-shrink-0"> instead.
Action button minimum touch target: the spec shows padding: 3px 7px for these small table buttons. On desktop this is fine; confirm the admin interface is desktop-primary (the spec shows this as a sidebar layout, implying desktop use). Touch targets are less critical here than in the family gallery.
The "— Titel nicht vergeben —" display should use text-ink-muted italic styling, not just a dash, to visually distinguish it from a real title. The spec shows "—" but adding muted italic improves clarity.
Use <table> with proper <thead>/<tbody>/<th scope="col"> elements for the article table. The spec uses .atable — in the SvelteKit implementation, use semantic HTML with Tailwind utility classes, not CSS class names from the spec's embedded stylesheet.
The empty state ("Noch keine Artikel vorhanden") should be a <tr><td colspan="5"> row inside the <tbody>, centered, with muted italic text — not an element outside the table.

Open Decisions (omit if none)

Löschen button: hidden vs. dimmed for reserved articles. The acceptance criteria say "only visible for unreserved articles" (implying {#if !reserviert}) but the spec mockup shows it dimmed with reduced opacity (implying always rendered, visually disabled). These conflict. The dimmed approach is better UX (communicates that deletion exists but is blocked) and the server guards the actual operation either way. Recommend the dimmed approach — but this needs explicit confirmation since the acceptance criteria contradict it. (Raised by: Leonie)

## 👤 Leonie Voss — UI/UX Design Lead & Accessibility Advocate ### Observations - **View 05 in `2026-05-05-erbstuecke-wannsee-views.html` is the authoritative spec for this view.** The admin table layout is fully specified: sidebar (176px, `bg-admin`), content area (`bg-canvas p-3.5`), article table with columns: thumbnail (30×30px, `border-radius: 4px`), Artikel (title), Kategorie, Status, and action buttons (Bearbeiten + Löschen). - **Status colors:** "Frei" must use `text-status-free` (`#4A7C5C`) with `✓` prefix. "Reserviert" must use `text-status-taken` (`#9B6060`) with `●` prefix and the reserver's display name. The acceptance criteria correctly reference `text-status-taken` and `text-status-free` — confirm these are Tailwind tokens already defined in `tailwind.config.js`. - **30×30px thumbnail:** The spec uses `.thumb-sm` with `border-radius: 4px`. In Tailwind: `w-[30px] h-[30px] rounded object-cover`. When no photo exists, render a `<div class="w-[30px] h-[30px] rounded bg-line">` placeholder — do not render a broken `<img>` with no `src`. - **Action buttons:** The spec shows small inline buttons: Bearbeiten (`act-edit` style: muted bg, border) and Löschen (`act-del` style: `#FBF0F0` bg, `status-taken` border + text). Löschen must be visually disabled (`.opacity-35 cursor-not-allowed`) for reserved articles — not absent from the DOM. The spec mockup shows it dimmed, not removed. - **"+ Hinzufügen" button:** Amber accent (`bg-accent`), white text, positioned top-right of the content area aligned with the page title. Not a link — a styled button that navigates via JS or a standard `<a>` styled as button. - **Admin sidebar:** Uses `+layout.svelte` which should already exist from #4/#8. The Inventar link must have `border-l-[3px] border-accent` active state on this route. - **Font usage:** Table text uses Inter (sans). Page title "Inventar" uses Lora (serif, `font-serif text-sm font-bold`). Table headers are `text-[8.5px] font-bold uppercase text-ink-muted`. - **The spec shows the Löschen button as dimmed (opacity) for reserved articles, not removed.** This is an important distinction from the acceptance criterion which says "only visible for unreserved articles." The spec takes precedence visually — use `opacity-35 pointer-events-none` rather than `{#if !reserviert}`. ### Recommendations - Use `opacity-35 pointer-events-none` on the Löschen button for reserved articles rather than conditionally rendering it. This matches the spec mockup and gives admins visual feedback that the action exists but is blocked, rather than hiding it entirely. The server still enforces the guard. - Render the thumbnail as `<img src="/uploads/{thumbnail}" alt="" class="w-[30px] h-[30px] rounded object-cover flex-shrink-0">` with `alt=""` (decorative). When `thumbnail` is null, render `<div class="w-[30px] h-[30px] rounded bg-line flex-shrink-0">` instead. - Action button minimum touch target: the spec shows `padding: 3px 7px` for these small table buttons. On desktop this is fine; confirm the admin interface is desktop-primary (the spec shows this as a sidebar layout, implying desktop use). Touch targets are less critical here than in the family gallery. - The "— Titel nicht vergeben —" display should use `text-ink-muted italic` styling, not just a dash, to visually distinguish it from a real title. The spec shows "—" but adding muted italic improves clarity. - Use `<table>` with proper `<thead>/<tbody>/<th scope="col">` elements for the article table. The spec uses `.atable` — in the SvelteKit implementation, use semantic HTML with Tailwind utility classes, not CSS class names from the spec's embedded stylesheet. - The empty state ("Noch keine Artikel vorhanden") should be a `<tr><td colspan="5">` row inside the `<tbody>`, centered, with muted italic text — not an element outside the table. ### Open Decisions _(omit if none)_ - **Löschen button: hidden vs. dimmed for reserved articles.** The acceptance criteria say "only visible for unreserved articles" (implying `{#if !reserviert}`) but the spec mockup shows it dimmed with reduced opacity (implying always rendered, visually disabled). These conflict. The dimmed approach is better UX (communicates that deletion exists but is blocked) and the server guards the actual operation either way. Recommend the dimmed approach — but this needs explicit confirmation since the acceptance criteria contradict it. _(Raised by: Leonie)_

marcel commented

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

👤 Tobias Wendt — DevOps & Platform Engineer

Observations

No new infrastructure required for this issue. The admin inventar route is a pure application-layer feature — no new volumes, no new environment variables, no Dockerfile changes needed.
deleteArtikelPhotos(id) writes to the uploads named volume. The deletion path is UPLOAD_DIR/{id}/ — this is the same volume already mounted in docker-compose.yml. Confirm that UPLOAD_DIR is set to /app/uploads in the container environment and that lib/photos.ts reads this value from process.env.UPLOAD_DIR, not a hardcoded string.
No new environment variables introduced. This issue only uses existing env vars (DATABASE_PATH, UPLOAD_DIR, admin session cookie). No .env.example changes needed.
Health check is unaffected. The /health endpoint is not changed by this feature.
File deletion on the uploads volume is permanent. Once deleteArtikelPhotos(id) runs and the article is deleted, recovery requires restoring from the nightly backup. Confirm the backup script (if already in place) covers the erbstuecke_uploads named volume. If it does not yet exist, this feature introduces a new data-loss scenario that the backup must cover.

Recommendations

Verify lib/photos.ts reads UPLOAD_DIR from process.env.UPLOAD_DIR (with a startup-time fail-fast if absent), not a hardcoded path. This is already implied by the system design but must be confirmed before this issue is implemented.
The deleteArtikelPhotos function should use fs.rm(dir, { recursive: true, force: true }) — the force: true option prevents an error if the directory does not exist (articles with no photos uploaded). This is the correct behavior for a best-effort cleanup.
After implementing this issue: run docker compose exec app ls /app/uploads and verify directories are created and removed correctly. Add this to the post-deploy smoke test if the deploy happens before other issues add articles.
No changes to Dockerfile, docker-compose.yml, or Caddyfile are needed for this issue. If any are proposed during implementation, flag as scope creep.

Open Decisions (omit if none)

Backup before article delete: The nightly backup covers the uploads volume, but if an admin deletes an article between backup runs, the photos are permanently gone. For a family estate app where photos are irreplaceable, consider whether a soft-delete pattern (moving files to an _trash/{id}/ directory rather than rm -rf) is warranted, or whether the nightly backup window is acceptable. This is an architecture-level decision. (Raised by: Tobias)

## 👤 Tobias Wendt — DevOps & Platform Engineer ### Observations - **No new infrastructure required for this issue.** The admin inventar route is a pure application-layer feature — no new volumes, no new environment variables, no Dockerfile changes needed. - **`deleteArtikelPhotos(id)` writes to the `uploads` named volume.** The deletion path is `UPLOAD_DIR/{id}/` — this is the same volume already mounted in `docker-compose.yml`. Confirm that `UPLOAD_DIR` is set to `/app/uploads` in the container environment and that `lib/photos.ts` reads this value from `process.env.UPLOAD_DIR`, not a hardcoded string. - **No new environment variables introduced.** This issue only uses existing env vars (`DATABASE_PATH`, `UPLOAD_DIR`, admin session cookie). No `.env.example` changes needed. - **Health check is unaffected.** The `/health` endpoint is not changed by this feature. - **File deletion on the `uploads` volume is permanent.** Once `deleteArtikelPhotos(id)` runs and the article is deleted, recovery requires restoring from the nightly backup. Confirm the backup script (if already in place) covers the `erbstuecke_uploads` named volume. If it does not yet exist, this feature introduces a new data-loss scenario that the backup must cover. ### Recommendations - Verify `lib/photos.ts` reads `UPLOAD_DIR` from `process.env.UPLOAD_DIR` (with a startup-time fail-fast if absent), not a hardcoded path. This is already implied by the system design but must be confirmed before this issue is implemented. - The `deleteArtikelPhotos` function should use `fs.rm(dir, { recursive: true, force: true })` — the `force: true` option prevents an error if the directory does not exist (articles with no photos uploaded). This is the correct behavior for a best-effort cleanup. - After implementing this issue: run `docker compose exec app ls /app/uploads` and verify directories are created and removed correctly. Add this to the post-deploy smoke test if the deploy happens before other issues add articles. - No changes to `Dockerfile`, `docker-compose.yml`, or `Caddyfile` are needed for this issue. If any are proposed during implementation, flag as scope creep. ### Open Decisions _(omit if none)_ - **Backup before article delete:** The nightly backup covers the `uploads` volume, but if an admin deletes an article between backup runs, the photos are permanently gone. For a family estate app where photos are irreplaceable, consider whether a soft-delete pattern (moving files to an `_trash/{id}/` directory rather than `rm -rf`) is warranted, or whether the nightly backup window is acceptable. This is an architecture-level decision. _(Raised by: Tobias)_

marcel commented

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

👤 Elicit — Requirements Engineer

Observations

US-ADM-003 is well-formed. The user story is clear, the acceptance criteria are specific and binary, dependencies are identified (#8, #4), size is estimated (S), and the spec reference (reservierung-design §5.2) is given. This issue passes the Definition of Ready checklist on almost every point.
One ambiguity in the delete visibility criterion. The AC states "Löschen-Button only visible for unreserved articles" but View 05 in the UI spec shows the button rendered at reduced opacity for reserved articles — it is present but disabled, not absent. These are two different interaction patterns with different UX implications. The requirement as written implies conditional rendering; the spec implies always-rendered-but-disabled. This must be resolved before implementation starts.
"Attempting to delete a reserved article server-side returns error" — the trigger is underspecified. The AC states the error message but not the HTTP status or how it surfaces in the UI. If the button is conditionally hidden (per AC text), how does a user ever see this error? This criterion only makes sense if the button is always rendered (dimmed state) or if someone bypasses the UI. Clarify: is this error for UI display or for API-level integrity only?
deleteArtikelPhotos(id) is referenced without a definition. The AC mentions this function call by name as a specification requirement. It should be defined in lib/photos.ts as per the system design. Confirm this is already implemented via a prior issue or that it is in scope for this issue. If it is new work, this issue is larger than "S".
Empty state message: The AC mentions "empty state message when no articles exist" but does not specify the German text. The spec does not appear to define this either. A concrete string is needed: e.g., "Noch keine Artikel vorhanden. Füge den ersten Artikel hinzu." Recommend specifying this in the AC.
"+ Artikel hinzufügen" button links to /admin/inventar/neu — this route is a dependency. If #8 (which presumably creates this route) is not merged first, this link will be a 404 during development. The "Depends on: #8" note covers this, but it must be enforced in the merge order.

Recommendations

Resolve the "Löschen button hidden vs. dimmed" ambiguity before implementation. Add a clarifying note to the AC: either "button is not rendered for reserved articles" or "button is rendered but visually disabled (opacity-35 pointer-events-none) for reserved articles."
Specify the German empty state string in the AC: "Noch keine Artikel vorhanden." with a link or note pointing to the add-article route.
Confirm whether deleteArtikelPhotos(id) is already implemented by a prior issue. If not, add it explicitly to the AC with a note that it must be implemented as part of this issue (in lib/photos.ts).
Add a note to the "reserved article delete error" AC: this is a server-side integrity guard surfaced as a German inline error message in the table row or page-level error region — not a separate page or modal.
The AC for "After delete: deleteArtikelPhotos(id) removes files from disk, DB row deleted (CASCADE removes artikel_fotos)" should specify the order: file cleanup first, then DB delete — or acknowledge that this order is best-effort and the DB delete is the authoritative operation.

Open Decisions

"Löschen button hidden vs. dimmed" for reserved articles: The AC says "only visible for unreserved articles" (hidden) but the UI spec mockup shows it dimmed. These need to be reconciled into a single explicit requirement before coding starts. Options: (A) always render, use opacity-35 pointer-events-none — better UX, shows admins the action is blocked; (B) conditionally render with {#if !reserviert} — simpler, matches AC text literally. (Raised by: Elicit)
Is deleteArtikelPhotos in scope for this issue? If it was not implemented in #8 or another prior issue, this issue must explicitly include implementing lib/photos.ts::deleteArtikelPhotos. Without this, the delete action cannot meet its AC. (Raised by: Elicit)

## 👤 Elicit — Requirements Engineer ### Observations - **US-ADM-003 is well-formed.** The user story is clear, the acceptance criteria are specific and binary, dependencies are identified (#8, #4), size is estimated (S), and the spec reference (`reservierung-design §5.2`) is given. This issue passes the Definition of Ready checklist on almost every point. - **One ambiguity in the delete visibility criterion.** The AC states "Löschen-Button only visible for unreserved articles" but View 05 in the UI spec shows the button rendered at reduced opacity for reserved articles — it is present but disabled, not absent. These are two different interaction patterns with different UX implications. The requirement as written implies conditional rendering; the spec implies always-rendered-but-disabled. This must be resolved before implementation starts. - **"Attempting to delete a reserved article server-side returns error" — the trigger is underspecified.** The AC states the error message but not the HTTP status or how it surfaces in the UI. If the button is conditionally hidden (per AC text), how does a user ever see this error? This criterion only makes sense if the button is always rendered (dimmed state) or if someone bypasses the UI. Clarify: is this error for UI display or for API-level integrity only? - **`deleteArtikelPhotos(id)` is referenced without a definition.** The AC mentions this function call by name as a specification requirement. It should be defined in `lib/photos.ts` as per the system design. Confirm this is already implemented via a prior issue or that it is in scope for this issue. If it is new work, this issue is larger than "S". - **Empty state message:** The AC mentions "empty state message when no articles exist" but does not specify the German text. The spec does not appear to define this either. A concrete string is needed: e.g., "Noch keine Artikel vorhanden. Füge den ersten Artikel hinzu." Recommend specifying this in the AC. - **"+ Artikel hinzufügen" button links to `/admin/inventar/neu`** — this route is a dependency. If #8 (which presumably creates this route) is not merged first, this link will be a 404 during development. The "Depends on: #8" note covers this, but it must be enforced in the merge order. ### Recommendations - Resolve the "Löschen button hidden vs. dimmed" ambiguity before implementation. Add a clarifying note to the AC: either "button is not rendered for reserved articles" or "button is rendered but visually disabled (`opacity-35 pointer-events-none`) for reserved articles." - Specify the German empty state string in the AC: `"Noch keine Artikel vorhanden."` with a link or note pointing to the add-article route. - Confirm whether `deleteArtikelPhotos(id)` is already implemented by a prior issue. If not, add it explicitly to the AC with a note that it must be implemented as part of this issue (in `lib/photos.ts`). - Add a note to the "reserved article delete error" AC: this is a server-side integrity guard surfaced as a German inline error message in the table row or page-level error region — not a separate page or modal. - The AC for "After delete: `deleteArtikelPhotos(id)` removes files from disk, DB row deleted (CASCADE removes `artikel_fotos`)" should specify the order: file cleanup first, then DB delete — or acknowledge that this order is best-effort and the DB delete is the authoritative operation. ### Open Decisions - **"Löschen button hidden vs. dimmed" for reserved articles:** The AC says "only visible for unreserved articles" (hidden) but the UI spec mockup shows it dimmed. These need to be reconciled into a single explicit requirement before coding starts. Options: (A) always render, use `opacity-35 pointer-events-none` — better UX, shows admins the action is blocked; (B) conditionally render with `{#if !reserviert}` — simpler, matches AC text literally. _(Raised by: Elicit)_ - **Is `deleteArtikelPhotos` in scope for this issue?** If it was not implemented in #8 or another prior issue, this issue must explicitly include implementing `lib/photos.ts::deleteArtikelPhotos`. Without this, the delete action cannot meet its AC. _(Raised by: Elicit)_

marcel commented

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

🗳️ Decision Queue — Action Required

6 decisions need your input before implementation starts.

Delete Button UX: Hidden vs. Dimmed

Löschen button for reserved articles — hidden or dimmed? The acceptance criteria say "only visible for unreserved articles" ({#if !reserviert} conditional rendering), but the UI spec mockup (View 05) shows the button dimmed with reduced opacity — always present, visually disabled. These two approaches conflict. Option A: always render with opacity-35 pointer-events-none (matches spec mockup, communicates to admins that deletion is blocked). Option B: conditionally render with {#if !reserviert} (matches AC text, simpler). The server enforces the guard either way. (Raised by: Leonie, Elicit)

Scope: Is `deleteArtikelPhotos` Already Implemented?

Is lib/photos.ts::deleteArtikelPhotos in scope for this issue or already done? The AC references this function by name. If it was not implemented in a prior issue (#8 or #4), it must be built here — which makes this issue larger than size "S". If it already exists, no change needed. (Raised by: Elicit)

Filesystem Failure Handling

Orphaned uploads on partial delete failure. If deleteArtikelPhotos(id) partially fails (some WebP files deleted, then I/O error), the DB row is then still deleted, leaving a partially-empty uploads/{id}/ directory. Accept this as tolerable (best-effort cleanup, consistent with the upload rollback pattern), or implement a post-delete directory sweep? Recommend: accept as tolerable for this project scope. (Raised by: Markus)
Soft-delete for photos before permanent delete? Since photos of heirloom items may be irreplaceable, consider moving the uploads/{id}/ directory to uploads/_trash/{id}/ instead of rm -rf — allowing recovery before the next backup cycle. The current design assumes rm -rf (permanent). This is an architecture decision. (Raised by: Tobias)

Testing Scope

Partial filesystem failure tests for deleteArtikelPhotos. Testing partial I/O failure scenarios (some files deleted, then error) requires filesystem mocking or complex temp-directory orchestration. Recommended: skip partial-failure simulation, test only happy path and "no-photos directory absent" path. Confirm this is acceptable. (Raised by: Sara)

Security / Operations

Rate limiting on admin delete actions. No rate limiting exists on any Form Action. For 3 known admin users on a family-scale app, this is almost certainly acceptable. Confirm explicitly that rate limiting is out of scope for this project. (Raised by: Nora)

## 🗳️ Decision Queue — Action Required _6 decisions need your input before implementation starts._ --- ### Delete Button UX: Hidden vs. Dimmed - **Löschen button for reserved articles — hidden or dimmed?** The acceptance criteria say "only visible for unreserved articles" (`{#if !reserviert}` conditional rendering), but the UI spec mockup (View 05) shows the button dimmed with reduced opacity — always present, visually disabled. These two approaches conflict. Option A: always render with `opacity-35 pointer-events-none` (matches spec mockup, communicates to admins that deletion is blocked). Option B: conditionally render with `{#if !reserviert}` (matches AC text, simpler). The server enforces the guard either way. _(Raised by: Leonie, Elicit)_ --- ### Scope: Is `deleteArtikelPhotos` Already Implemented? - **Is `lib/photos.ts::deleteArtikelPhotos` in scope for this issue or already done?** The AC references this function by name. If it was not implemented in a prior issue (#8 or #4), it must be built here — which makes this issue larger than size "S". If it already exists, no change needed. _(Raised by: Elicit)_ --- ### Filesystem Failure Handling - **Orphaned uploads on partial delete failure.** If `deleteArtikelPhotos(id)` partially fails (some WebP files deleted, then I/O error), the DB row is then still deleted, leaving a partially-empty `uploads/{id}/` directory. Accept this as tolerable (best-effort cleanup, consistent with the upload rollback pattern), or implement a post-delete directory sweep? Recommend: accept as tolerable for this project scope. _(Raised by: Markus)_ - **Soft-delete for photos before permanent delete?** Since photos of heirloom items may be irreplaceable, consider moving the `uploads/{id}/` directory to `uploads/_trash/{id}/` instead of `rm -rf` — allowing recovery before the next backup cycle. The current design assumes `rm -rf` (permanent). This is an architecture decision. _(Raised by: Tobias)_ --- ### Testing Scope - **Partial filesystem failure tests for `deleteArtikelPhotos`.** Testing partial I/O failure scenarios (some files deleted, then error) requires filesystem mocking or complex temp-directory orchestration. Recommended: skip partial-failure simulation, test only happy path and "no-photos directory absent" path. Confirm this is acceptable. _(Raised by: Sara)_ --- ### Security / Operations - **Rate limiting on admin delete actions.** No rate limiting exists on any Form Action. For 3 known admin users on a family-scale app, this is almost certainly acceptable. Confirm explicitly that rate limiting is out of scope for this project. _(Raised by: Nora)_

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/wannsee-kram#9

Build Admin Inventar — article list and delete #9

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

Acceptance criteria

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

Open Decisions

👤 Sara Holt — QA Engineer & Test Strategist

Observations

Recommendations

Open Decisions (omit if none)

👤 Leonie Voss — UI/UX Design Lead & Accessibility Advocate

Observations

Recommendations

Open Decisions (omit if none)

👤 Tobias Wendt — DevOps & Platform Engineer

Observations

Recommendations

Open Decisions (omit if none)

👤 Elicit — Requirements Engineer

Observations

Recommendations

Open Decisions

🗳️ Decision Queue — Action Required

Delete Button UX: Hidden vs. Dimmed

Scope: Is deleteArtikelPhotos Already Implemented?

Filesystem Failure Handling

Testing Scope

Security / Operations

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

Scope: Is `deleteArtikelPhotos` Already Implemented?