From 8b1f58ba23fd8ee3aebc78f7cb86b4657097322c Mon Sep 17 00:00:00 2001 From: Brummel Date: Tue, 21 Apr 2026 11:20:17 +0200 Subject: [PATCH] fix: preserve show_closed view + ungrey closed-case controls Three UX polish items for the show-closed listing, grouped because they are the same root problem in three surfaces: * handle_reopen_case now redirects via resolve_return_path(&headers) instead of hard-coding /web/cases, so a reopen from the ?show_closed=1 listing stays in that view. Matches the pattern already used by analyze/reset/delete-recording. * handle_close_case now redirects via the new helper resolve_list_return_path(&headers), which wraps resolve_return_path and additionally collapses a /web/cases/{uuid} detail path to /web/cases while keeping the query string. A close from the detail page previously would have tried to redirect back to the now-404 detail, or when triggered from ?show_closed=1 would have dropped the query. * Closed cases in my_cases.html now render the checkbox, Analysieren/Neu analysieren and Reset controls in disabled state instead of being hidden. Layout stays consistent with mixed-state listings, and the user can see what actions would be available after reopen. The handlers remain the authoritative guard via locate_case_or_404 -> 404 for closed cases, so the HTML disabled is a hint only. Adds four unit tests for resolve_list_return_path and two integration tests for the new redirect behaviour (close from listing with query, close from detail with query). --- server/src/routes/case_actions.rs | 74 ++++++++++++++++++- server/templates/my_cases.html | 6 +- server/tests/analyze_test.rs | 118 ++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 7 deletions(-) diff --git a/server/src/routes/case_actions.rs b/server/src/routes/case_actions.rs index 3b67bfa..9316531 100644 --- a/server/src/routes/case_actions.rs +++ b/server/src/routes/case_actions.rs @@ -122,6 +122,30 @@ fn resolve_return_path(headers: &HeaderMap) -> String { } } +/// Like `resolve_return_path`, but collapses a `/web/cases/{uuid}` detail +/// path to `/web/cases` while preserving the query string. Used by +/// handlers that remove the case from the detail view (close, purge) — +/// a redirect back to the now-404 detail page would be a dead end, but +/// losing `?show_closed=1` would eject the user from the show-closed +/// view they were in. +fn resolve_list_return_path(headers: &HeaderMap) -> String { + let raw = resolve_return_path(headers); + let Some(tail) = raw.strip_prefix("/web/cases/") else { + return raw; + }; + let (case_part, query) = match tail.split_once('?') { + Some((c, q)) => (c, Some(q)), + None => (tail, None), + }; + if uuid::Uuid::parse_str(case_part).is_err() { + return raw; + } + match query { + Some(q) => format!("/web/cases?{q}"), + None => "/web/cases".into(), + } +} + /// Build an `AnalysisInput` for the given case directory. /// Collects all `.m4a` (excluding `.m4a.failed`), reads the matching /// `.transcript.txt` for each, skips blank transcripts, and computes @@ -282,11 +306,17 @@ pub(crate) async fn reset_case_artefacts(case_dir: &Path) -> std::io::Result<()> /// The case immediately disappears from listings and detail views (404). /// Reversible via `handle_reopen_case` (explicit button) or by uploading /// a new recording for the same case (automatic via `handle_upload`). +/// +/// Redirects back to the `Referer`-derived list path (see +/// `resolve_list_return_path`) so a close from the `?show_closed=1` +/// view keeps the user there — but drops the case-id segment if the +/// referer was the case detail, since that page 404s after the close. pub async fn handle_close_case( user: AuthenticatedWebUser, State(config): State>, State(events_tx): State, CaseIdPath(case_id): CaseIdPath, + headers: HeaderMap, ) -> Result { let user_root = config.data_path.join(&user.slug); let case_dir = locate_case_or_404(&user_root, &case_id, &user.slug, "close").await?; @@ -310,7 +340,7 @@ pub async fn handle_close_case( CaseEventKind::CaseClosed, ); - Ok(Redirect::to("/web/cases")) + Ok(Redirect::to(&resolve_list_return_path(&headers))) } /// POST /web/cases/{case_id}/reopen @@ -319,14 +349,20 @@ pub async fn handle_close_case( /// immediately back in the default listing. A reopened case is /// indistinguishable from one that was never closed — no status /// remainders on disk. Idempotent: an already-open case is a no-op. +/// +/// Redirects back to the `Referer`-path (filtered via +/// `resolve_return_path`) so a reopen from the `?show_closed=1` view +/// stays in that view instead of jumping back to the default listing. pub async fn handle_reopen_case( user: AuthenticatedWebUser, State(config): State>, State(events_tx): State, CaseIdPath(case_id): CaseIdPath, + headers: HeaderMap, ) -> Result { let user_root = config.data_path.join(&user.slug); let case_dir = user_root.join(case_id.to_string()); + let return_path = resolve_return_path(&headers); if !crate::paths::is_closed(&case_dir).await { info!( @@ -334,7 +370,7 @@ pub async fn handle_reopen_case( case_id = %case_id, "reopen requested but case is not closed" ); - return Ok(Redirect::to("/web/cases")); + return Ok(Redirect::to(&return_path)); } match tokio::fs::remove_file(case_dir.join(CLOSE_MARKER)).await { @@ -356,7 +392,7 @@ pub async fn handle_reopen_case( CaseEventKind::CaseReopened, ); - Ok(Redirect::to("/web/cases")) + Ok(Redirect::to(&return_path)) } /// POST /web/cases/{case_id}/reset @@ -547,7 +583,7 @@ async fn remove_if_exists(path: &Path) -> Result<(), AppError> { #[cfg(test)] mod tests { - use super::resolve_return_path; + use super::{resolve_list_return_path, resolve_return_path}; use axum::http::{HeaderMap, HeaderValue, header::REFERER}; fn headers_with_referer(value: &str) -> HeaderMap { @@ -605,4 +641,34 @@ mod tests { h.insert(REFERER, HeaderValue::from_bytes(&[0xff, 0xfe]).unwrap()); assert_eq!(resolve_return_path(&h), "/web/cases"); } + + // `resolve_list_return_path` is `resolve_return_path` plus a case- + // detail collapse. It leaves non-detail paths alone and strips the + // `/{uuid}` segment from a detail path while preserving the query. + + #[test] + fn list_return_path_preserves_list_referer() { + let h = headers_with_referer("/web/cases?show_closed=1"); + assert_eq!(resolve_list_return_path(&h), "/web/cases?show_closed=1"); + } + + #[test] + fn list_return_path_collapses_detail_with_query() { + let h = + headers_with_referer("/web/cases/11111111-1111-1111-1111-111111111111?show_closed=1"); + assert_eq!(resolve_list_return_path(&h), "/web/cases?show_closed=1"); + } + + #[test] + fn list_return_path_collapses_bare_detail() { + let h = headers_with_referer("/web/cases/11111111-1111-1111-1111-111111111111"); + assert_eq!(resolve_list_return_path(&h), "/web/cases"); + } + + #[test] + fn list_return_path_leaves_non_uuid_segment_alone() { + // Non-UUID `/web/cases/` is not a detail page — don't touch it. + let h = headers_with_referer("/web/cases/undo-delete"); + assert_eq!(resolve_list_return_path(&h), "/web/cases/undo-delete"); + } } diff --git a/server/templates/my_cases.html b/server/templates/my_cases.html index 35d8628..e802224 100644 --- a/server/templates/my_cases.html +++ b/server/templates/my_cases.html @@ -92,7 +92,7 @@ try {
    {% for case in g.cases %}
  • -
    {% if !case.is_closed %}{% endif %}
    +
    — {% call ol::render(case.oneliner) %}
    @@ -101,8 +101,8 @@ try {
    -{% if !case.is_closed && is_admin %} -{% endif %} +{% if is_admin %} +{% endif %} {% if case.is_closed %} {% else %} diff --git a/server/tests/analyze_test.rs b/server/tests/analyze_test.rs index 00db39d..7d0e545 100644 --- a/server/tests/analyze_test.rs +++ b/server/tests/analyze_test.rs @@ -851,6 +851,124 @@ async fn closed_case_detail_with_show_closed_returns_200() { ); } +#[tokio::test] +async fn close_redirects_back_to_referer_query() { + // Close from the show-closed listing must keep ?show_closed=1 so + // the user stays in the view they were in. + let config = config_with_llm(unique_tmp("close-ref-q"), "http://unused".into()); + let case_id = "11111111-1111-1111-1111-111111111111"; + let case_dir = seed_case(&config.data_path, "dr_a", case_id); + seed_recording(&case_dir, "10-00-00", Some("ok")); + + let app = doctate_server::create_router(config); + let cookie = login(app.clone(), "dr_a").await; + + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri(format!("/web/cases/{case_id}/close")) + .header(header::COOKIE, &cookie) + .header(header::REFERER, "/web/cases?show_closed=1") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + assert_eq!( + resp.headers() + .get(header::LOCATION) + .unwrap() + .to_str() + .unwrap(), + "/web/cases?show_closed=1" + ); +} + +#[tokio::test] +async fn close_from_detail_redirects_to_list_preserving_query() { + // Close from /web/cases/{uuid}?show_closed=1 must NOT redirect back + // to the detail page (it 404s after the close). The uuid segment + // is stripped; the query survives so the user lands in show-closed. + let config = config_with_llm(unique_tmp("close-ref-det"), "http://unused".into()); + let case_id = "11111111-1111-1111-1111-111111111111"; + let case_dir = seed_case(&config.data_path, "dr_a", case_id); + seed_recording(&case_dir, "10-00-00", Some("ok")); + + let app = doctate_server::create_router(config); + let cookie = login(app.clone(), "dr_a").await; + + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri(format!("/web/cases/{case_id}/close")) + .header(header::COOKIE, &cookie) + .header( + header::REFERER, + format!("/web/cases/{case_id}?show_closed=1"), + ) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + assert_eq!( + resp.headers() + .get(header::LOCATION) + .unwrap() + .to_str() + .unwrap(), + "/web/cases?show_closed=1", + "close from detail must redirect to the LIST with the query intact" + ); +} + +#[tokio::test] +async fn reopen_redirects_back_to_referer() { + // User reopens a case from the show-closed listing. The redirect + // must preserve `?show_closed=1` so the user stays in that view + // instead of being ejected back to the default open-only listing. + let config = config_with_llm(unique_tmp("reopen-ref"), "http://unused".into()); + let case_id = "11111111-1111-1111-1111-111111111111"; + let case_dir = seed_case(&config.data_path, "dr_a", case_id); + seed_recording(&case_dir, "10-00-00", Some("ok")); + + let app = doctate_server::create_router(config); + let cookie = login(app.clone(), "dr_a").await; + + let _ = app + .clone() + .oneshot(close_request(case_id, &cookie)) + .await + .unwrap(); + + let resp = app + .oneshot( + Request::builder() + .method("POST") + .uri(format!("/web/cases/{case_id}/reopen")) + .header(header::COOKIE, &cookie) + .header(header::REFERER, "/web/cases?show_closed=1") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(resp.status(), StatusCode::SEE_OTHER); + assert_eq!( + resp.headers() + .get(header::LOCATION) + .unwrap() + .to_str() + .unwrap(), + "/web/cases?show_closed=1", + "reopen must redirect back to the referer path (preserving the query)" + ); +} + #[tokio::test] async fn reopen_is_noop_on_open_case() { let config = config_with_llm(unique_tmp("reopen-2"), "http://unused".into());