Skip to content

fix(#478): register the restore route the Trash panel has been calling - #485

Merged
guycorbaz merged 2 commits into
mainfrom
fix/478-trash-restore-route
Sep 10, 2026
Merged

guycorbaz merged 2 commits into
mainfrom
fix/478-trash-restore-route

Conversation

@guycorbaz

Copy link
Copy Markdown
Owner

Closes #478.

The bug

templates/fragments/admin_trash_panel.html has rendered a Restore button since story 8-6, calling /admin/trash/{table}/{id}/restore. That route was never registered. The click returned 404, HTMX does not swap a 4xx — so it produced nothing: no restore, no error, no feedback entry.

The suite never noticed because TrashService::restore and restore_with_conflicts_cleared had no caller outside #[cfg(test)]. Meanwhile CLAUDE.md promises soft delete with the Trash as the recovery path, and auto_purge hard-deletes after 30 days — so in practice nothing deleted by mistake was recoverable through the UI at all.

Landing this required #480 first (merged): with the seed gate only soft-deleting, a working Restore button would have handed back the admin/admin account documented in SECURITY.md.

The change

  • POST /admin/trash/{table}/{id}/restore, not the GET the button emitted. A state-changing GET sits outside the story 8-2 CSRF layer (which guards POST/PUT/PATCH/DELETE) and is fair game for any prefetcher. The button posts; csrf.js supplies the header.
  • The panel filters and page ride the URL, so the post-restore re-render lands the admin back on the view they were on — same contract as the permanent-delete sibling (patch P12).
  • Conflict path. When relationships changed while the item sat in the trash, the first pass returns the UX-DR8 conflict modal, retargeted into #modal-slot, listing exactly what the Confirm would clear; the Confirm re-posts with clear_conflicts=1 → restore_with_conflicts_cleared. The admin.trash.restore_modal_* keys for this flow had been sitting unused in both locale files since story 8-6. A conflict-free restore stays one click.
  • Audit. A restore writes an admin_audit row (restore_from_trash, with actor username/role and whether conflicts were cleared) — the inverse of a purge belongs in the same trail. Best-effort: the restore has already committed, so a failed audit INSERT must not turn a successful action into an error page.
  • One new locale key in EN + FR for the "already gone" case; everything else was already translated.
  • The Restore button gained data-modal-trigger (focus anchor for the conflict modal), which made button[data-modal-trigger] ambiguous inside a trash row — admin-permanent-delete.spec.ts now addresses its button by accessible name, which the selector policy prefers anyway.

Coverage

Rust — tests/admin_trash_restore.rs, nine tests over HTTP through build_router, so the wiring is covered and not just the service: the route restores and reports; a librarian is refused; a request with no CSRF token is rejected (the reason POST matters); a stale version is 409 and leaves the row in the trash; a purged row is 404 with the operator-facing copy; a table outside ALLOWED_TABLES is 400; an audit row is written; the conflict modal is returned with HX-Retarget: #modal-slot and changes nothing; the confirm pass restores and drops the stale link.

Browser — tests/e2e/specs/journeys/admin-trash-restore.spec.ts: delete a borrower → find it in the Trash → Restore → the feedback names it, the row disappears, the borrower is back in its list. A second test replays the same request from a stale panel and asserts the operator gets the "already gone" copy rather than the silence #478 is about. The journey uses a borrower rather than a series on purpose: a restore leaves the entity live, and empty-states.spec.ts asserts the /series list is empty.

Testing

  • cargo clippy --all-targets -- -D warnings — clean.
  • cargo test --lib — 1171 passed (includes templates_audit).
  • cargo test --test admin_trash_restore — 9 passed.
  • CI=true npx playwright test --workers=2 on a freshly reset stack — 332 passed, 2 skipped, 0 failed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D5cV9QRqQAFpiNKZwZfoNX

guycorbaz and others added 2 commits September 11, 2026 00:17
The Trash panel has rendered a Restore button since story 8-6, pointing at
`/admin/trash/{table}/{id}/restore`. That route was never registered, so
every click returned 404 — and HTMX does not swap a 4xx, so the click did
nothing at all: no restore, no error, no feedback entry. The suite stayed
green because `TrashService::restore` and
`restore_with_conflicts_cleared` were only ever called from `#[cfg(test)]`.

Everything under the button already existed; this is the missing wiring.
The handler registers as **POST**, not the GET the button used to emit: a
state-changing GET sits outside the story 8-2 CSRF layer, which guards
POST/PUT/PATCH/DELETE only. The button now posts, with the panel filters
threaded through so the post-restore re-render lands the admin back on the
view they were on — the same contract as its permanent-delete sibling.

When relationships changed while the item sat in the trash (a series whose
title was reassigned, a contributor role removed), the first pass returns
the UX-DR8 conflict modal — retargeted into #modal-slot — listing what the
Confirm would clear; the Confirm re-posts with `clear_conflicts=1`. The
`admin.trash.restore_modal_*` locale keys for exactly this flow have been
sitting unused in both locale files since story 8-6. A conflict-free
restore stays a single click.

A restore also writes an `admin_audit` row, best-effort: restoring is the
inverse of purging and belongs in the same trail, but the restore has
already committed by then, so a failed audit INSERT must not turn a
successful action into an error page.

Refs #478

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D5cV9QRqQAFpiNKZwZfoNX
`tests/locale_parity.rs` requires every key to exist in all four locale
files; `restore_error_not_found` had landed in en and fr only, which
turned the DB-integration job red. CLAUDE.md said the locales were
"en.yml + fr.yml" — stale since German and Italian were added, and the
reason the omission looked correct while writing it. Fixed there too,
with the parity test named so the next person runs it.

Refs #478

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D5cV9QRqQAFpiNKZwZfoNX
@guycorbaz
guycorbaz marked this pull request as ready for review September 10, 2026 22:50
@guycorbaz
guycorbaz merged commit f00eec8 into main Sep 10, 2026
8 checks passed
@guycorbaz
guycorbaz deleted the fix/478-trash-restore-route branch September 10, 2026 22:50
guycorbaz added a commit that referenced this pull request Sep 11, 2026
Cut the v1.19.0 release documentation: version bump in Cargo.toml /
Cargo.lock, and every version-bearing surface required by Foundation
Rule 19 brought to 1.19.0.

- Manual EN/FR: title-page version, install snippets, a "What's new in
  1.19.0" section in chapter 8 (the Restore button that never worked,
  the seeded accounts leaving no trace, the bounded cover decode, and
  an explicit "nothing to do on upgrade"); the release-notes section
  now states that the PDFs are attached to the release rather than
  promising it once CI is wired. PDFs rebuilt.
- README: status line, live-install label, image-size badge, current-
  release paragraph.
- ROADMAP: current-stable header, a v1.19.0 shipped section, and the
  "merged but not released" block retired now that it has shipped.
- docs/dockerhub-overview.md: tags list.
- website/: index (nav badge, hero, JSON-LD softwareVersion, the hero
  paragraph rewritten around this release), about (nav badge), roadmap
  (meta descriptions, JSON-LD, nav badge, both narrative paragraphs),
  sitemap lastmod — stale since May, and the surface Rule 19 names as
  the easiest to forget.
- sprint-status.yaml: last_updated header.

No source change. The release carries #478, #480 and #479 (merged in
#485, #484 and #486), the supply-chain CI work (#481), the network-
posture documentation (#482), the community-health files (#477) and the
documentation audit (#487). No migration.


Claude-Session: https://claude.ai/code/session_01D5cV9QRqQAFpiNKZwZfoNX

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CR] The Trash panel's Restore button points at a route that does not exist

1 participant