fix(admin): report real outcome of ten admin mutations (BUNYIP-398) #391

Merged

nrupard merged 1 commit from fix/BUNYIP-398-admin-false-success into main

2026-07-24 17:54:30 +02:00

nrupard commented

2026-07-24 16:19:53 +02:00

Owner

What

Ten admin mutation handlers in bunyip-web/src/handlers/admin.rs now report the real outcome of their API call instead of always redirecting as success.

Why

Each of these handlers did let _ = admin_api::...(...).await; followed by an unconditional success redirect_cookies(...). If the API or transport call failed (network error, downstream 5xx, validation rejection) the operator saw a success redirect while the destructive operation (delete user, revoke lifetime, revoke membership, delete application group, delete doc, etc.) never happened, with no error shown and no log at the site. Silent false-success on destructive operations is a real trust and correctness problem (BUNYIP-398).

Changes

Converted the discard-and-redirect pattern at the ten cited sites into let target = match admin_api::...(...).await { Ok(_) => <existing success target>, Err(e) => { tracing::warn!(...); format!("...?toast_err={}", urlenc("...")) } }; then redirect_cookies(&target, &c.set_cookies); the Ok arm preserves the exact prior success redirect so successful calls are unchanged.
Error surfacing reuses the module's existing ?toast_err= flash convention (drained by the global toast system in bunyip-web/src/views/layout.rs), matching the closest non-discarding neighbors in the same module (user_email, user_verify_email, user_reset_2fa, unban_ip, reset_rate_limit).
Logging uses tracing::warn!(<id> = %<id>, error = ?e, "admin <op> failed") with the primary identifier of the mutated entity; error = ?e because ApiError derives Debug (not Display) and its Debug output carries status, code, and message.
Sites covered: user_role (update_user_role), user_delete, user_suspend, user_revoke_lifetime, membership_revoke, application_field (update_application), application_group_delete, application_doc_create, application_doc_update, application_doc_delete.
Deliberately out of scope: the seven other let _ = admin_api:: sites in this module that the issue does not enumerate (reactivate_user, admin_reset_password, grant_lifetime, grant_membership, swap_application_order, set_application_group, set_application_restricted) are left unchanged.

Tests

Ran the pinned rust-builder gate (ghcr.io/niceguyit/rust-builder-glibc:v1.0.1-rust1.94-trixie, SQLX_OFFLINE=true): cargo fmt --all --check clean, cargo clippy --workspace --all-targets -- -D warnings clean, cargo test --workspace --all-targets green (493 tests passed, 0 failed). No live Postgres required.

## What Ten admin mutation handlers in `bunyip-web/src/handlers/admin.rs` now report the real outcome of their API call instead of always redirecting as success. ## Why Each of these handlers did `let _ = admin_api::...(...).await;` followed by an unconditional success `redirect_cookies(...)`. If the API or transport call failed (network error, downstream 5xx, validation rejection) the operator saw a success redirect while the destructive operation (delete user, revoke lifetime, revoke membership, delete application group, delete doc, etc.) never happened, with no error shown and no log at the site. Silent false-success on destructive operations is a real trust and correctness problem (BUNYIP-398). ## Changes - Converted the discard-and-redirect pattern at the ten cited sites into `let target = match admin_api::...(...).await { Ok(_) => <existing success target>, Err(e) => { tracing::warn!(...); format!("...?toast_err={}", urlenc("...")) } };` then `redirect_cookies(&target, &c.set_cookies)`; the Ok arm preserves the exact prior success redirect so successful calls are unchanged. - Error surfacing reuses the module's existing `?toast_err=` flash convention (drained by the global toast system in `bunyip-web/src/views/layout.rs`), matching the closest non-discarding neighbors in the same module (`user_email`, `user_verify_email`, `user_reset_2fa`, `unban_ip`, `reset_rate_limit`). - Logging uses `tracing::warn!(<id> = %<id>, error = ?e, "admin <op> failed")` with the primary identifier of the mutated entity; `error = ?e` because `ApiError` derives `Debug` (not `Display`) and its Debug output carries status, code, and message. - Sites covered: `user_role` (update_user_role), `user_delete`, `user_suspend`, `user_revoke_lifetime`, `membership_revoke`, `application_field` (update_application), `application_group_delete`, `application_doc_create`, `application_doc_update`, `application_doc_delete`. - Deliberately out of scope: the seven other `let _ = admin_api::` sites in this module that the issue does not enumerate (reactivate_user, admin_reset_password, grant_lifetime, grant_membership, swap_application_order, set_application_group, set_application_restricted) are left unchanged. ## Tests - Ran the pinned rust-builder gate (`ghcr.io/niceguyit/rust-builder-glibc:v1.0.1-rust1.94-trixie`, `SQLX_OFFLINE=true`): `cargo fmt --all --check` clean, `cargo clippy --workspace --all-targets -- -D warnings` clean, `cargo test --workspace --all-targets` green (493 tests passed, 0 failed). No live Postgres required.

nrupard added 1 commit

2026-07-24 16:19:53 +02:00

fix(admin): report real outcome of ten admin mutations

E2E / Playwright against deployment (pull_request) Successful in 33s

Details

Check / fmt + clippy + build + tests (pull_request) Successful in 13m51s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

2e69f8db3e

Ten admin mutation handlers in bunyip-web discarded the API call's Result with `let _ = ...` and then redirected unconditionally as success, so a failed call (network error, downstream 5xx, validation rejection) showed the operator a success redirect while the destructive operation never happened, with no error surfaced and no log at the site.

Each site now matches the Result: on Ok it keeps the existing success redirect; on Err it logs the failure with tracing::warn! and redirects with a `?toast_err=` flash (the module's existing toast convention, drained by the global toast system in views/layout.rs) so the operator sees the real outcome. Covers update_user_role, delete_user, suspend_user, revoke_lifetime, revoke_membership, update_application, delete_application_group, and app-doc create/update/delete. The seven other discard sites not listed in the issue are left unchanged.

#BUNYIP-398

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>