diff --git a/CLAUDE.md b/CLAUDE.md index 645fb80..7d4b59f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -119,7 +119,7 @@ If/when multi-tenancy is on the roadmap, both #54 and #16 should be re-opened to - **Error handling:** `AppError` enum with `IntoResponse`. Conflict = 409, Unauthorized = 303 redirect with HX-Redirect. - **Logging:** `tracing` macros only — no `println!`. -- **i18n:** `rust_i18n::t!("key")` for ALL user-facing text. Keys in `locales/en.yml` + `locales/fr.yml`. JS strings: read `` and use embedded string map. **CRITICAL YAML FORMAT: locale files must NOT have `en:` or `fr:` as top-level wrapper — the filename determines the locale. Keys start at root level (e.g., `nav:` not `en: nav:`). After adding/changing keys, run `touch src/lib.rs` before `cargo build` to force proc macro recompilation.** +- **i18n:** `rust_i18n::t!("key")` for ALL user-facing text. Keys in **all four** locale files — `locales/{en,fr,de,it}.yml`. `tests/locale_parity.rs` fails the build on any key present in one file and missing from another, so a key added to en/fr alone turns the DB-integration job red; run `cargo test --test locale_parity` after touching a locale. JS strings: read `` and use embedded string map. **CRITICAL YAML FORMAT: locale files must NOT have `en:` or `fr:` as top-level wrapper — the filename determines the locale. Keys start at root level (e.g., `nav:` not `en: nav:`). After adding/changing keys, run `touch src/lib.rs` before `cargo build` to force proc macro recompilation.** - **DB queries:** MUST include `deleted_at IS NULL` in every SELECT/JOIN on entity tables. Every entity table has `deleted_at`, `version`, `created_at`, `updated_at` columns. **MariaDB type gotchas:** (1) `JSON` columns are stored as `BLOB` — use `CAST(col AS CHAR)` to read as String. (2) `BIGINT UNSIGNED NULL` columns — use `CAST(col AS SIGNED)` and read as `Option`, then convert to `u64`. (3) Never use `CAST(... AS UNSIGNED)` in SELECT — SQLx can't decode `BIGINT UNSIGNED` into Rust integers reliably. (4) `TIMESTAMP` columns in dynamic queries (`sqlx::query()`) — use `CAST(col AS DATETIME) AS col` to read as `NaiveDateTime`. Without CAST, SQLx returns a type mismatch error. This does NOT affect typed macros (`sqlx::query!`) which handle conversion automatically. - **Optimistic locking:** UPDATE with `WHERE id = ? AND version = ?`, then `check_update_result()` from `services/locking.rs`. - **Soft delete:** `services/soft_delete.rs` with table whitelist. Never hard-delete. diff --git a/locales/de.yml b/locales/de.yml index da21e79..c74a64f 100644 --- a/locales/de.yml +++ b/locales/de.yml @@ -823,6 +823,7 @@ admin: days_remaining_warning: "%{days} Tage" days_remaining_critical: "<7 Tage" restore_success: "Wiederhergestellt: %{name}" + restore_error_not_found: "Dieser Eintrag liegt nicht mehr im Papierkorb. Er wurde vermutlich schon wiederhergestellt oder endgültig gelöscht." delete_permanent_success: "Endgültig gelöscht: %{name}" delete_permanent_modal_title: "Endgültig löschen?" delete_permanent_modal_warning: "Dies kann nicht rückgängig gemacht werden." diff --git a/locales/en.yml b/locales/en.yml index c7a284a..58d8b91 100644 --- a/locales/en.yml +++ b/locales/en.yml @@ -838,6 +838,7 @@ admin: days_remaining_warning: "%{days} day" days_remaining_critical: "<7 days" restore_success: "Restored: %{name}" + restore_error_not_found: "That item is no longer in the trash. It may have been restored or purged already." delete_permanent_success: "Deleted permanently: %{name}" delete_permanent_modal_title: "Delete permanently?" delete_permanent_modal_warning: "This cannot be undone." diff --git a/locales/fr.yml b/locales/fr.yml index 9dcd2f7..0898cca 100644 --- a/locales/fr.yml +++ b/locales/fr.yml @@ -835,6 +835,7 @@ admin: days_remaining_warning: "%{days} jour" days_remaining_critical: "<7 jours" restore_success: "Restauré : %{name}" + restore_error_not_found: "Cet élément n'est plus dans la corbeille. Il a peut-être déjà été restauré ou purgé." delete_permanent_success: "Supprimé définitivement : %{name}" delete_permanent_modal_title: "Supprimer définitivement ?" delete_permanent_modal_warning: "Ceci ne peut pas être annulé." diff --git a/locales/it.yml b/locales/it.yml index 572fab9..4df0ce7 100644 --- a/locales/it.yml +++ b/locales/it.yml @@ -811,6 +811,7 @@ admin: days_remaining_warning: "%{days} giorno" days_remaining_critical: "<7 giorni" restore_success: "Ripristinato: %{name}" + restore_error_not_found: "Questo elemento non è più nel cestino. Probabilmente è già stato ripristinato o eliminato definitivamente." delete_permanent_success: "Eliminato definitivamente: %{name}" delete_permanent_modal_title: "Eliminare definitivamente?" delete_permanent_modal_warning: "Questa azione non può essere annullata." diff --git a/src/routes/admin.rs b/src/routes/admin.rs index 514d52b..1696871 100644 --- a/src/routes/admin.rs +++ b/src/routes/admin.rs @@ -225,6 +225,21 @@ struct AdminTrashPermanentDeleteModal { inner_form_html: String, } +#[derive(Template)] +#[template(path = "fragments/admin_trash_restore_modal.html")] +struct AdminTrashRestoreModal { + /// Issue #478 — thin wrapper around the same UX-DR8 macro as the + /// permanent-delete modal. Shown only on the conflict path; a + /// conflict-free restore never opens a dialog. + title: String, + body_html: String, + confirm_label: String, + cancel_label: String, + action_url: String, + csrf_token: String, + version: i32, +} + #[derive(Template)] #[template(path = "fragments/admin_trash_panel.html")] struct AdminTrashPanel { @@ -554,6 +569,24 @@ pub struct PermanentDeleteConfirmQuery { pub page: Option, } +/// Query for `POST /admin/trash/{table}/{id}/restore` (issue #478). +/// `version` drives the optimistic lock; `clear_conflicts` marks the +/// second pass, after the admin confirmed the conflict modal; the rest +/// are the panel filters threaded through so the post-restore re-render +/// lands on the same view (same contract as `PermanentDeleteConfirmQuery`). +#[derive(Debug, Deserialize)] +pub struct RestoreQuery { + pub version: Option, + /// Typed as a string, not a `bool`: `serde_urlencoded` accepts only + /// `true` / `false` for a bool, so a hand-edited `?clear_conflicts=1` + /// would 400 instead of doing the obvious thing. Parsed through the + /// strict accept-set the project uses for its flags elsewhere. + pub clear_conflicts: Option, + pub entity_type: Option, + pub search: Option, + pub page: Option, +} + pub async fn admin_trash_permanent_delete_confirm( State(state): State, session: Session, @@ -808,6 +841,218 @@ pub(crate) async fn render_admin_for_reference_data( render_admin(state, session, loc, uri, is_htmx, AdminTab::ReferenceData, None).await } +/// Restore a soft-deleted item from the Trash (issue #478). +/// +/// The Restore button had been rendering a URL to a route that was never +/// registered, so a click produced a 404 that HTMX does not swap — no +/// restore, no error, nothing. Everything below it already existed: +/// `TrashService::restore`, `detect_restore_conflicts`, +/// `restore_with_conflicts_cleared`, and the `admin.trash.restore_*` +/// locale keys. This handler is the missing wiring. +/// +/// **POST, not GET.** The button used to emit `hx-get`. A state-changing +/// GET sits outside the CSRF middleware (story 8-2 guards +/// POST/PUT/PATCH/DELETE only) and is fair game for any prefetcher, so +/// the route is registered as POST and the button posts. +/// +/// Two passes when relationships changed while the item sat in the trash: +/// the first returns the conflict modal (retargeted into `#modal-slot`), +/// its Confirm re-posts with `clear_conflicts=1`. A conflict-free restore +/// takes the single-click path. +pub async fn admin_trash_restore( + State(state): State, + session: Session, + Extension(locale): Extension, + axum::extract::Path((table, id)): axum::extract::Path<(String, u64)>, + Query(params): Query, +) -> Result { + session.require_role_with_return(Role::Admin, "/admin?tab=trash", locale.0)?; + let loc = locale.0; + + let version = params + .version + .ok_or(AppError::BadRequest("Missing or invalid version".to_string()))?; + + // `get_trash_entry` validates `table` against ALLOWED_TABLES and + // filters on `deleted_at IS NOT NULL`, so a purged (or already + // restored) row lands here rather than in the service layer. + let entry = crate::models::trash::TrashModel::get_trash_entry(&state.pool, &table, id) + .await? + .ok_or_else(|| { + AppError::NotFound( + rust_i18n::t!("admin.trash.restore_error_not_found", locale = loc).to_string(), + ) + })?; + + let clear_conflicts = matches!( + params.clear_conflicts.as_deref(), + Some("1" | "true" | "TRUE") + ); + let conflicts = + crate::services::trash::TrashService::detect_restore_conflicts(&state.pool, &table, id) + .await?; + + if !conflicts.is_empty() && !clear_conflicts { + // The version in the URL is the one the admin's page carried, not + // a fresh read: a stale panel must still lose the optimistic lock + // when the modal's Confirm comes back. + let action_url = format!( + "/admin/trash/{}/{}/restore?version={}&clear_conflicts=1&entity_type={}&search={}&page={}", + crate::utils::html_escape(&table), + id, + version, + crate::utils::url_encode(params.entity_type.as_deref().unwrap_or("")), + crate::utils::url_encode(params.search.as_deref().unwrap_or("")), + params.page.unwrap_or(1).max(1), + ); + let modal_html = render_restore_conflict_modal( + &session, + loc, + &action_url, + version, + &entry.item_name, + &conflicts, + )?; + + // The click came from the panel's Restore button, whose + // `hx-target` is `#admin-trash-panel`. Retarget so the dialog + // lands in the stable `#modal-slot` (polish-1: outside any + // HTMX-swappable region) instead of replacing the panel with a + // modal. Not stripped by `ModalConfirmRetargetGuard` — that + // layer only touches responses to requests carrying + // `X-Modal-Confirm`, which a panel button never sends. + let mut response = Html(modal_html).into_response(); + response.headers_mut().insert( + axum::http::HeaderName::from_static("hx-retarget"), + axum::http::HeaderValue::from_static("#modal-slot"), + ); + response.headers_mut().insert( + axum::http::HeaderName::from_static("hx-reswap"), + axum::http::HeaderValue::from_static("innerHTML"), + ); + return Ok(response); + } + + let restored = if clear_conflicts { + crate::services::trash::TrashService::restore_with_conflicts_cleared( + &state.pool, + &table, + id, + version, + ) + .await? + } else { + crate::services::trash::TrashService::restore(&state.pool, &table, id, version).await? + }; + + // Forensics, mirroring `permanent_delete_from_trash` — restoring a + // row is the inverse of purging it and belongs in the same trail. + // Best-effort on purpose: the restore already committed, so a failed + // audit INSERT must not turn a successful action into an error page. + if let Some(user_id) = session.user_id { + let actor_username: String = sqlx::query_scalar("SELECT username FROM users WHERE id = ?") + .bind(user_id) + .fetch_optional(&state.pool) + .await + .ok() + .flatten() + .unwrap_or_else(|| format!("user-{}", user_id)); + + if let Err(e) = crate::models::admin_audit::AdminAuditModel::create( + &state.pool, + user_id, + "restore_from_trash", + Some(&table), + Some(id), + Some(serde_json::json!({ + "user_username": actor_username, + "user_role": session.role.to_string(), + "item_name": restored.item_name, + "conflicts_cleared": clear_conflicts, + })), + ) + .await + { + tracing::warn!(error = %e, table = %table, id = id, "restore audit entry failed"); + } + } + + let success_msg = + rust_i18n::t!("admin.trash.restore_success", locale = loc, name = &restored.item_name) + .to_string(); + let feedback = feedback_html("success", &success_msg, ""); + + // Re-render with the filters the admin was looking at, exactly as the + // permanent-delete handler does (patch P12). + let filters = TrashQuery { + entity_type: params.entity_type, + search: params.search, + page: params.page, + }; + let panel_html = render_trash_panel(&state, loc, &filters).await?; + + // `HX-Trigger: modal-close` closes the conflict modal on the second + // pass. Harmless on the single-click path: modal.js only closes when + // the finished Confirm came from its own slot. + Ok(HtmxResponse { + main: panel_html, + oob: vec![OobUpdate { + swap_mode: Default::default(), + target: "feedback-list".to_string(), + content: feedback, + }], + } + .into_response_with_hx_trigger("modal-close")) +} + +/// Build the conflict modal for [`admin_trash_restore`]. Kept separate so +/// the handler reads as one flow. Every interpolated value carrying user +/// data goes through `html_escape` — the macro consumes `body_html` with +/// `|safe` (CSP-clean, no inline script/style). +fn render_restore_conflict_modal( + session: &Session, + loc: &'static str, + action_url: &str, + version: i32, + item_name: &str, + conflicts: &[crate::services::trash::ConflictInfo], +) -> Result { + let explanation = rust_i18n::t!("admin.trash.restore_modal_explanation", locale = loc).to_string(); + let conflicts_label = rust_i18n::t!("admin.trash.restore_modal_conflicts", locale = loc).to_string(); + + let items = conflicts + .iter() + .map(|c| format!("
  • {}
    • ", crate::utils::html_escape(&c.description))) + .collect::>() + .join(""); + + let body_html = format!( + r##"

    {explanation}

    +

    {item_name}

    +

    {conflicts_label}

    +
      {items}
    "##, + explanation = crate::utils::html_escape(&explanation), + item_name = crate::utils::html_escape(item_name), + conflicts_label = crate::utils::html_escape(&conflicts_label), + items = items, + ); + + let modal = AdminTrashRestoreModal { + title: rust_i18n::t!("admin.trash.restore_modal_title", locale = loc).to_string(), + body_html, + confirm_label: rust_i18n::t!("admin.trash.restore_modal_clear_conflicts", locale = loc) + .to_string(), + cancel_label: rust_i18n::t!("admin.trash.restore_modal_cancel", locale = loc).to_string(), + action_url: action_url.to_string(), + csrf_token: session.csrf_token.clone(), + version, + }; + + modal + .render() + .map_err(|_| AppError::Internal("Modal render failed".to_string())) +} + pub async fn admin_trash_panel( State(state): State, session: Session, diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 9221e99..585991a 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -589,6 +589,11 @@ pub fn build_router(state: AppState) -> Router { ) .route("/admin/trash", axum::routing::get(admin::admin_trash_panel)) .route("/admin/trash/{table}/{id}/permanent-delete", axum::routing::get(admin::admin_trash_permanent_delete_confirm).post(admin::admin_trash_permanent_delete)) + // Issue #478: the panel's Restore button rendered this URL from + // story 8-6 on, but the route was never registered — every click + // 404'd, and HTMX does not swap a 4xx, so nothing happened at all. + // POST rather than GET so the CSRF layer (story 8-2) covers it. + .route("/admin/trash/{table}/{id}/restore", axum::routing::post(admin::admin_trash_restore)) // Admin → System settings (story 8-5). 4 routes — 1 GET panel + 3 // POST per-form saves. All Admin-gated, all CSRF-protected via the // 8-2 middleware (none added to CSRF_EXEMPT_ROUTES). diff --git a/templates/fragments/admin_trash_panel.html b/templates/fragments/admin_trash_panel.html index 1d609df..0990d4b 100644 --- a/templates/fragments/admin_trash_panel.html +++ b/templates/fragments/admin_trash_panel.html @@ -70,7 +70,13 @@

    {{ heading }}

    {{ entry.deleted_at.format("%Y-%m-%d %H:%M") }} {{ entry.days_remaining }} - + {# Issue #478: POST (the route mutates, so it rides the CSRF layer), + and the current filters / page are threaded through so the + post-restore re-render lands the admin back on the same view — + same contract as the permanent-delete button below. + `data-modal-trigger` is a focus anchor for the conflict modal + this may open; a conflict-free restore opens no dialog. #} + {# R3-N1: thread current filters / page through the confirm-modal URL so the post-delete re-render lands the admin back on the same view. #} diff --git a/templates/fragments/admin_trash_restore_modal.html b/templates/fragments/admin_trash_restore_modal.html new file mode 100644 index 0000000..341efee --- /dev/null +++ b/templates/fragments/admin_trash_restore_modal.html @@ -0,0 +1,26 @@ +{# Issue #478 — restore-conflict modal. Shown only when + `TrashService::detect_restore_conflicts` reports that relationships + changed while the item sat in the trash (series reassigned, contributor + role removed). The Confirm button re-posts the same restore URL with + `clear_conflicts=1`, which routes to + `TrashService::restore_with_conflicts_cleared`. + + Variant "warning" (indigo), not "delete" — restoring is not destructive; + what the Confirm clears is the stale relationship rows, and the body + lists them so the admin knows what they are agreeing to. Cancel keeps + the item deleted, per the `restore_modal_cancel` copy. #} +{% import "components/modal.html" as modal %} +{% call modal::modal( + "warning", + title, + body_html, + confirm_label, + cancel_label, + action_url, + "POST", + csrf_token, + "#admin-trash-panel", + "outerHTML", + version, + "", +) %}{% endcall %} diff --git a/tests/admin_trash_restore.rs b/tests/admin_trash_restore.rs new file mode 100644 index 0000000..b3cc88c --- /dev/null +++ b/tests/admin_trash_restore.rs @@ -0,0 +1,475 @@ +//! Issue #478 — HTTP-level coverage for `POST /admin/trash/{table}/{id}/restore`. +//! +//! The Trash panel had rendered a Restore button since story 8-6, but the +//! route behind it was never registered: every click returned 404, and HTMX +//! does not swap a 4xx, so the button did nothing at all — no restore, no +//! error, no feedback. `TrashService::restore` was green the whole time +//! because its only callers were `#[cfg(test)]`. These tests exercise the +//! route over HTTP so the wiring itself is covered, not just the service. +//! +//! Run locally: +//! docker compose -f tests/docker-compose.rust-test.yml up -d +//! SQLX_OFFLINE=true DATABASE_URL='mysql://root:root_test@localhost:3307/mybibli_rust_test' \ +//! cargo test --test admin_trash_restore + +use std::path::PathBuf; +use std::sync::{Arc, RwLock}; + +use axum::body::Body; +use axum::http::{Method, Request, StatusCode, header}; +use sqlx::MySqlPool; +use tower::ServiceExt; + +use mybibli::AppState; +use mybibli::config::AppSettings; +use mybibli::metadata::registry::ProviderRegistry; +use mybibli::routes::build_router; +use mybibli::services::admin_health::new_mariadb_version_cache; +use mybibli::tasks::provider_health::new_provider_health_map; + +fn build_state(pool: MySqlPool) -> AppState { + AppState { + pool, + settings: Arc::new(RwLock::new(AppSettings::default())), + http_client: reqwest::Client::new(), + registry: Arc::new(ProviderRegistry::new()), + covers_dir: PathBuf::from("/tmp/mybibli-test-covers"), + provider_health: new_provider_health_map(), + mariadb_version_cache: new_mariadb_version_cache(), + setup_gate: Arc::new(RwLock::new( + mybibli::middleware::setup_gate::SetupGateState::default(), + )), + bulk_cover_fetch: Arc::new(RwLock::new( + mybibli::services::bulk_cover_fetch::BulkCoverFetchStatus::default(), + )), + log_level_reloader: mybibli::noop_log_level_reloader(), + } +} + +const TEST_CSRF_TOKEN: &str = "trash_restore_test_csrf_token_abcdef1234567890"; + +fn rand_suffix() -> String { + use base64::Engine; + let bytes: [u8; 8] = rand::random(); + base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(bytes) +} + +/// Seed a session for a user of the given role. Returns (user_id, token). +async fn seed_session(pool: &MySqlPool, role: &str) -> (u64, String) { + let username = format!("{}-{}", &role[..1], rand_suffix()); + let user_id: u64 = sqlx::query_scalar( + "INSERT INTO users (username, password_hash, role) VALUES (?, '$argon2id$v=19$m=65536,t=3,p=4$placeholder$placeholder', ?) RETURNING id", + ) + .bind(&username) + .bind(role) + .fetch_one(pool) + .await + .expect("insert user"); + + // sessions.token is VARCHAR(44) so keep the prefix short. + let token = format!("tr-{}", rand_suffix()); + sqlx::query("INSERT INTO sessions (token, user_id, csrf_token, data) VALUES (?, ?, ?, '{}')") + .bind(&token) + .bind(user_id) + .bind(TEST_CSRF_TOKEN) + .execute(pool) + .await + .expect("insert session"); + + (user_id, token) +} + +/// Insert a soft-deleted title as the restore target. Returns (id, name, version). +async fn seed_soft_deleted_title(pool: &MySqlPool) -> (u64, String, i32) { + let name = format!("Trashed Title {}", rand_suffix()); + let id = sqlx::query( + "INSERT INTO titles (title, media_type, genre_id, version, deleted_at) \ + VALUES (?, 'book', 1, 1, NOW())", + ) + .bind(&name) + .execute(pool) + .await + .expect("insert soft-deleted title") + .last_insert_id(); + + (id, name, 1) +} + +async fn deleted_at_of(pool: &MySqlPool, table: &str, id: u64) -> Option { + sqlx::query_scalar(&format!( + "SELECT CAST(deleted_at AS DATETIME) FROM {table} WHERE id = ?" + )) + .bind(id) + .fetch_one(pool) + .await + .expect("read deleted_at") +} + +async fn body_text(resp: axum::response::Response) -> String { + let bytes = axum::body::to_bytes(resp.into_body(), 1024 * 1024) + .await + .expect("body bytes"); + String::from_utf8(bytes.to_vec()).expect("utf-8 body") +} + +/// A restore request as the browser sends it: HTMX `hx-post` with the CSRF +/// token in the header (`static/js/csrf.js`), no body. +fn restore_request(uri: String, token: &str, csrf: Option<&str>) -> Request { + let mut builder = Request::builder() + .method(Method::POST) + .uri(uri) + .header(header::COOKIE, format!("session={token}")) + .header("hx-request", "true"); + if let Some(csrf) = csrf { + builder = builder.header("x-csrf-token", csrf); + } + builder.body(Body::empty()).unwrap() +} + +// ─── The route exists and restores ───────────────────────────────── + +#[sqlx::test(migrations = "./migrations")] +async fn restore_clears_deleted_at_and_reports_success(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + let (title_id, title_name, version) = seed_soft_deleted_title(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/titles/{title_id}/restore?version={version}"), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!( + resp.status(), + StatusCode::OK, + "the restore route must be registered — a 404 here is the #478 bug itself" + ); + // The conflict modal closes on success; harmless on the single-click path. + assert_eq!( + resp.headers() + .get("hx-trigger") + .and_then(|v| v.to_str().ok()), + Some("modal-close"), + ); + + let html = body_text(resp).await; + assert!( + html.contains("admin-trash-panel"), + "response should re-render the trash panel" + ); + assert!( + html.contains(&title_name), + "the success FeedbackEntry should name the restored item" + ); + + assert!( + deleted_at_of(&pool, "titles", title_id).await.is_none(), + "deleted_at should be NULL after a restore" + ); + let new_version: i32 = sqlx::query_scalar("SELECT version FROM titles WHERE id = ?") + .bind(title_id) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(new_version, version + 1, "optimistic-lock version should bump"); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_writes_an_audit_row(pool: MySqlPool) { + let (admin_id, admin_token) = seed_session(&pool, "admin").await; + let (title_id, _title_name, version) = seed_soft_deleted_title(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/titles/{title_id}/restore?version={version}"), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let (actor, entity_id): (Option, Option) = sqlx::query_as( + "SELECT CAST(user_id AS SIGNED), CAST(entity_id AS SIGNED) FROM admin_audit \ + WHERE action = 'restore_from_trash' AND entity_type = 'titles'", + ) + .fetch_one(&pool) + .await + .expect("a restore should leave an audit row"); + + assert_eq!(actor, Some(admin_id as i64)); + assert_eq!(entity_id, Some(title_id as i64)); +} + +// ─── Guards ──────────────────────────────────────────────────────── + +#[sqlx::test(migrations = "./migrations")] +async fn restore_without_a_csrf_token_is_rejected(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + let (title_id, _name, version) = seed_soft_deleted_title(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/titles/{title_id}/restore?version={version}"), + &admin_token, + None, + )) + .await + .unwrap(); + + // The reason the route is POST rather than the GET the button used to + // emit: state-changing verbs ride the story 8-2 CSRF layer. + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + assert_eq!( + resp.headers() + .get("hx-trigger") + .and_then(|v| v.to_str().ok()), + Some("csrf-rejected"), + ); + assert!( + deleted_at_of(&pool, "titles", title_id).await.is_some(), + "a rejected request must not restore anything" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_is_refused_to_a_librarian(pool: MySqlPool) { + let (_lib_id, lib_token) = seed_session(&pool, "librarian").await; + let (title_id, _name, version) = seed_soft_deleted_title(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/titles/{title_id}/restore?version={version}"), + &lib_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::FORBIDDEN); + assert!( + deleted_at_of(&pool, "titles", title_id).await.is_some(), + "a librarian must not be able to restore" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_with_a_stale_version_returns_409(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + let (title_id, _name, version) = seed_soft_deleted_title(&pool).await; + + // Someone else touched the row after the panel was rendered. + sqlx::query("UPDATE titles SET version = version + 1 WHERE id = ?") + .bind(title_id) + .execute(&pool) + .await + .unwrap(); + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/titles/{title_id}/restore?version={version}"), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::CONFLICT); + assert!( + deleted_at_of(&pool, "titles", title_id).await.is_some(), + "a lost optimistic lock must leave the row in the trash" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_of_a_purged_row_returns_404(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + + let app = build_router(build_state(pool)); + let resp = app + .oneshot(restore_request( + "/admin/trash/titles/999999/restore?version=1".to_string(), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + let html = body_text(resp).await; + // i18n-aware: the test stack's default language is FR, and the copy + // must be the operator-facing sentence in either language — never a + // bare 404 page. + assert!( + html.contains("no longer in the trash") || html.contains("plus dans la corbeille"), + "the operator should get the friendly copy, not a bare 404: {html}" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_rejects_a_table_outside_the_whitelist(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + + let app = build_router(build_state(pool)); + let resp = app + .oneshot(restore_request( + "/admin/trash/sessions/1/restore?version=1".to_string(), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "`table` is interpolated into SQL — only ALLOWED_TABLES may pass" + ); +} + +// ─── Conflict path ───────────────────────────────────────────────── + +/// Seed the issue-#66 shape: a soft-deleted series whose title has since +/// been reassigned to a live series. Returns the soft-deleted series id. +async fn seed_series_conflict(pool: &MySqlPool) -> u64 { + let old_series_id = sqlx::query("INSERT INTO series (name, version, deleted_at) VALUES (?, 1, NOW())") + .bind(format!("Old Series {}", rand_suffix())) + .execute(pool) + .await + .unwrap() + .last_insert_id(); + + let new_series_id = sqlx::query("INSERT INTO series (name, version) VALUES (?, 1)") + .bind(format!("New Series {}", rand_suffix())) + .execute(pool) + .await + .unwrap() + .last_insert_id(); + + let title_id = sqlx::query( + "INSERT INTO titles (title, media_type, genre_id, version) VALUES (?, 'book', 1, 1)", + ) + .bind("Reassigned Title") + .execute(pool) + .await + .unwrap() + .last_insert_id(); + + sqlx::query( + "INSERT INTO title_series (title_id, series_id, position_number, deleted_at) \ + VALUES (?, ?, 1, NOW())", + ) + .bind(title_id) + .bind(old_series_id) + .execute(pool) + .await + .unwrap(); + + sqlx::query("INSERT INTO title_series (title_id, series_id, position_number) VALUES (?, ?, 1)") + .bind(title_id) + .bind(new_series_id) + .execute(pool) + .await + .unwrap(); + + old_series_id +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_with_conflicts_returns_the_modal_and_changes_nothing(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + let series_id = seed_series_conflict(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/series/{series_id}/restore?version=1"), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + // The panel button targets #admin-trash-panel; the dialog belongs in + // the stable #modal-slot, so the response retargets. + assert_eq!( + resp.headers() + .get("hx-retarget") + .and_then(|v| v.to_str().ok()), + Some("#modal-slot"), + ); + assert_eq!( + resp.headers().get("hx-reswap").and_then(|v| v.to_str().ok()), + Some("innerHTML"), + ); + + let html = body_text(resp).await; + assert!(html.contains("data-modal-confirm"), "UX-DR8 macro shape expected"); + assert!(html.contains("data-modal-cancel"), "Cancel must be present"); + assert!( + html.contains("clear_conflicts=1"), + "Confirm should re-post the same route with the conflicts flag" + ); + assert!( + html.contains("Reassigned Title"), + "the modal should name the conflicting title: {html}" + ); + assert!( + html.contains(TEST_CSRF_TOKEN), + "story 8-2 invariant: the modal form carries the CSRF token" + ); + + assert!( + deleted_at_of(&pool, "series", series_id).await.is_some(), + "showing the conflict modal must not restore anything yet" + ); +} + +#[sqlx::test(migrations = "./migrations")] +async fn restore_with_clear_conflicts_restores_and_drops_the_stale_link(pool: MySqlPool) { + let (_admin_id, admin_token) = seed_session(&pool, "admin").await; + let series_id = seed_series_conflict(&pool).await; + + let app = build_router(build_state(pool.clone())); + let resp = app + .oneshot(restore_request( + format!("/admin/trash/series/{series_id}/restore?version=1&clear_conflicts=1"), + &admin_token, + Some(TEST_CSRF_TOKEN), + )) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + assert_eq!( + resp.headers() + .get("hx-trigger") + .and_then(|v| v.to_str().ok()), + Some("modal-close"), + "the confirm pass must close the modal" + ); + + assert!( + deleted_at_of(&pool, "series", series_id).await.is_none(), + "the series should be restored" + ); + let stale_links: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM title_series WHERE series_id = ?") + .bind(series_id) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!( + stale_links, 0, + "the shadowed assignment to the restored series should be cleared" + ); +} diff --git a/tests/e2e/specs/journeys/admin-permanent-delete.spec.ts b/tests/e2e/specs/journeys/admin-permanent-delete.spec.ts index cf88718..f68a656 100644 --- a/tests/e2e/specs/journeys/admin-permanent-delete.spec.ts +++ b/tests/e2e/specs/journeys/admin-permanent-delete.spec.ts @@ -61,7 +61,12 @@ test.describe("Story 8-7: Permanent Delete & Auto-Purge", () => { const row = await openTrashAndFindRow(page, name); // Click "Delete permanently" on our row → modal opens - const deleteBtn = row.locator("button[data-modal-trigger]"); + // Issue #478 gave the Restore button `data-modal-trigger` too (it can + // open the restore-conflict modal), so the delete button is addressed + // by its accessible name rather than by the marker attribute. + const deleteBtn = row.getByRole("button", { + name: /delete permanently|supprimer définitivement/i, + }); await expect(deleteBtn).toBeVisible(); await deleteBtn.click(); @@ -97,7 +102,9 @@ test.describe("Story 8-7: Permanent Delete & Auto-Purge", () => { const name = await seedSoftDeletedSeries(page, "SC2"); const row = await openTrashAndFindRow(page, name); - await row.locator("button[data-modal-trigger]").click(); + await row + .getByRole("button", { name: /delete permanently|supprimer définitivement/i }) + .click(); const modal = page.locator("#modal-slot dialog[open]"); await expect(modal).toBeVisible({ timeout: 5000 }); diff --git a/tests/e2e/specs/journeys/admin-trash-restore.spec.ts b/tests/e2e/specs/journeys/admin-trash-restore.spec.ts new file mode 100644 index 0000000..c043d1b --- /dev/null +++ b/tests/e2e/specs/journeys/admin-trash-restore.spec.ts @@ -0,0 +1,116 @@ +import { test, expect, Page } from "@playwright/test"; +import { loginAs } from "../../helpers/auth"; +import { createBorrower } from "../../helpers/loans"; + +/** + * Issue #478 — the Trash panel's Restore journey. + * + * The button existed since story 8-6 but pointed at a route that was never + * registered: HTMX does not swap a 4xx, so a click produced nothing at all + * — no restore, no error, no feedback entry. Nothing in the suite noticed, + * because `TrashService::restore` was only ever called from `#[cfg(test)]`. + * + * The journey uses a **borrower**, not a series, on purpose: restoring + * leaves the entity live, and `empty-states.spec.ts` asserts that the + * /series list is empty. A live borrower collides with nothing. + */ + +/** Create a borrower, soft-delete it through its modal, return the name. */ +async function seedDeletedBorrower(page: Page, slug: string): Promise { + const name = `TR-${slug}-${Date.now() % 1000000}`; + await createBorrower(page, name); + + const link = page + .locator('tbody a[href^="/borrower/"]') + .filter({ hasText: new RegExp(`^\\s*${name}\\s*$`) }); + await link.click(); + await expect(page.locator("h1")).toContainText(name, { timeout: 5000 }); + + await page.locator("button[data-modal-trigger]").click(); + await expect(page.locator("#modal-slot dialog[open]")).toBeVisible({ + timeout: 5000, + }); + await page.locator("[data-modal-confirm]").click(); + await page.waitForURL("**/borrowers", { timeout: 10000 }); + return name; +} + +/** Open the Trash tab filtered to borrowers and return the row for `name`. */ +async function openTrashRow(page: Page, name: string) { + await page.goto("/admin?tab=trash", { waitUntil: "domcontentloaded" }); + await expect( + page.locator('section[aria-labelledby="admin-trash-heading"]'), + ).toBeVisible({ timeout: 10000 }); + await page.locator("#filter-entity-type").selectOption("borrowers"); + const row = page.locator("tbody tr").filter({ hasText: name }); + await expect(row).toBeVisible({ timeout: 10000 }); + return row; +} + +const restoreButton = (row: ReturnType) => + row.getByRole("button", { name: /^(restore|restaurer)$/i }); + +test.describe("Issue #478: restore from the Trash", () => { + test.beforeEach(async ({ page }) => { + await loginAs(page, "admin"); + }); + + test("delete a borrower, restore it from the Trash, find it back in the list", async ({ + page, + }) => { + const name = await seedDeletedBorrower(page, "SC1"); + const row = await openTrashRow(page, name); + + await restoreButton(row).click(); + + // The panel re-renders and the OOB feedback entry names the item. + await expect(page.locator(".feedback-entry").first()).toContainText( + new RegExp(`(Restored|Restauré).*${name}`, "i"), + { timeout: 10000 }, + ); + + // The row is gone from the Trash — the restore actually committed. + await expect(page.locator("tbody tr").filter({ hasText: name })).toHaveCount( + 0, + { timeout: 10000 }, + ); + + // And the borrower is back in its own list. + await page.goto("/borrowers"); + await expect( + page.locator('tbody a[href^="/borrower/"]').filter({ hasText: name }), + ).toBeVisible({ timeout: 10000 }); + }); + + test("replaying a restore from a stale panel says so instead of failing silently", async ({ + page, + }) => { + const name = await seedDeletedBorrower(page, "SC2"); + const row = await openTrashRow(page, name); + + // Capture the exact request the panel would re-send if the admin left + // the tab open and clicked twice. + const url = await restoreButton(row).getAttribute("hx-post"); + expect(url).toBeTruthy(); + + await restoreButton(row).click(); + await expect(page.locator(".feedback-entry").first()).toContainText( + new RegExp(`(Restored|Restauré).*${name}`, "i"), + { timeout: 10000 }, + ); + + // The row is no longer in the trash, so the replay gets the friendly + // "already gone" copy — never a silent no-op, which is the #478 + // symptom this spec exists to prevent. + const csrf = await page + .locator('meta[name="csrf-token"]') + .getAttribute("content"); + const replay = await page.request.post(url as string, { + headers: { "X-CSRF-Token": csrf as string, "HX-Request": "true" }, + }); + expect(replay.status()).toBe(404); + expect(await replay.text()).toMatch( + /no longer in the trash|plus dans la corbeille/i, + ); + }); +});