From bc449e520ed0ebe3f0bf99dfa16f9d7f64f9142c Mon Sep 17 00:00:00 2001 From: Jon Seager Date: Tue, 10 Feb 2026 18:11:15 +0000 Subject: [PATCH] feat: upgrade update handlers to FlexiblePayload with three-way response All five existing update handlers (roaster, roast, cafe, gear, bag) now accept FlexiblePayload with UpdateSubmission types that separate image data from domain structs. Each returns Datastar redirect scripts, form redirects, or JSON depending on request type. Image save support added to all update paths. --- src/application/routes/api/coffee/bags.rs | 47 ++++++++++- src/application/routes/api/coffee/cafes.rs | 81 +++++++++++++++--- src/application/routes/api/coffee/gear.rs | 58 +++++++++++-- src/application/routes/api/coffee/roasters.rs | 73 ++++++++++++++--- src/application/routes/api/coffee/roasts.rs | 82 ++++++++++++++++--- tests/server/datastar.rs | 23 +++--- 6 files changed, 306 insertions(+), 58 deletions(-) diff --git a/src/application/routes/api/coffee/bags.rs b/src/application/routes/api/coffee/bags.rs index 9a2ebd0..22c704c 100644 --- a/src/application/routes/api/coffee/bags.rs +++ b/src/application/routes/api/coffee/bags.rs @@ -8,6 +8,7 @@ use tracing::info; use crate::application::auth::AuthenticatedUser; use crate::application::errors::{ApiError, AppError}; +use crate::application::routes::api::images::save_deferred_image; use crate::application::routes::api::macros::{define_delete_handler, define_enriched_get_handler}; use crate::application::routes::support::{ FlexiblePayload, ListQuery, PayloadSource, is_datastar_request, @@ -120,6 +121,41 @@ pub(crate) async fn list_bags( define_enriched_get_handler!(get_bag, BagId, BagWithRoast, bag_repo, get_with_roast); +#[derive(Debug, Deserialize)] +pub(crate) struct UpdateBagSubmission { + #[serde(default)] + roast_id: Option, + #[serde(default)] + roast_date: Option, + #[serde(default)] + amount: Option, + #[serde(default)] + remaining: Option, + #[serde(default)] + closed: Option, + #[serde(default)] + finished_at: Option, + #[serde(default)] + created_at: Option>, + #[serde(default)] + image: Option, +} + +impl UpdateBagSubmission { + fn into_parts(self) -> (UpdateBag, Option) { + let update = UpdateBag { + roast_id: self.roast_id, + roast_date: self.roast_date, + amount: self.amount, + remaining: self.remaining, + closed: self.closed, + finished_at: self.finished_at, + created_at: self.created_at, + }; + (update, self.image) + } +} + #[tracing::instrument(skip(state, _auth_user, headers, query))] pub(crate) async fn update_bag( State(state): State, @@ -128,11 +164,11 @@ pub(crate) async fn update_bag( Path(id): Path, Query(query): Query, Query(update_params): Query, - payload: Option>, + payload: FlexiblePayload, ) -> Result { let (request, search) = query.into_request_and_search::(); - - let body_update = payload.map_or(UpdateBag::default(), |Json(p)| p); + let (submission, source) = payload.into_parts(); + let (body_update, image_data_url) = submission.into_parts(); let update = UpdateBag { roast_id: body_update.roast_id.or(update_params.roast_id), @@ -161,6 +197,8 @@ pub(crate) async fn update_bag( info!(%id, closed = ?update.closed, "bag updated"); state.stats_invalidator.invalidate(); + save_deferred_image(&state, "bag", i64::from(bag.id), image_data_url.as_deref()).await; + if is_datastar_request(&headers) { let from_bag_page = headers .get("referer") @@ -176,6 +214,9 @@ pub(crate) async fn update_bag( crate::application::routes::support::render_redirect_script(&detail_url) .map_err(ApiError::from) } + } else if matches!(source, PayloadSource::Form) { + let detail_url = format!("/bags/{id}"); + Ok(Redirect::to(&detail_url).into_response()) } else { let enriched = state .bag_repo diff --git a/src/application/routes/api/coffee/cafes.rs b/src/application/routes/api/coffee/cafes.rs index c6c3f89..ec47e17 100644 --- a/src/application/routes/api/coffee/cafes.rs +++ b/src/application/routes/api/coffee/cafes.rs @@ -140,20 +140,60 @@ pub(crate) async fn create_cafe( define_get_handler!(get_cafe, CafeId, Cafe, cafe_repo); -#[tracing::instrument(skip(state, _auth_user))] +#[derive(Debug, Deserialize)] +pub(crate) struct UpdateCafeSubmission { + #[serde(default)] + name: Option, + #[serde(default)] + city: Option, + #[serde(default)] + country: Option, + #[serde(default)] + latitude: Option, + #[serde(default)] + longitude: Option, + #[serde(default)] + website: Option, + #[serde(default)] + created_at: Option>, + #[serde(default)] + image: Option, +} + +impl UpdateCafeSubmission { + fn into_parts(self) -> (UpdateCafe, Option) { + let update = UpdateCafe { + name: self.name, + city: self.city, + country: self.country, + latitude: self.latitude, + longitude: self.longitude, + website: self.website, + created_at: self.created_at, + }; + (update, self.image) + } +} + +#[tracing::instrument(skip(state, _auth_user, headers))] pub(crate) async fn update_cafe( State(state): State, _auth_user: AuthenticatedUser, + headers: HeaderMap, Path(id): Path, - Json(payload): Json, -) -> Result, ApiError> { - let has_changes = payload.name.is_some() - || payload.city.is_some() - || payload.country.is_some() - || payload.latitude.is_some() - || payload.longitude.is_some() - || payload.website.is_some() - || payload.created_at.is_some(); + payload: FlexiblePayload, +) -> Result { + let (submission, source) = payload.into_parts(); + let (update, image_data_url) = submission.into_parts(); + + let has_changes = update.name.is_some() + || update.city.is_some() + || update.country.is_some() + || update.latitude.is_some() + || update.longitude.is_some() + || update.website.is_some() + || update.created_at.is_some() + || image_data_url.is_some(); if !has_changes { return Err(AppError::validation("no changes provided").into()); @@ -161,12 +201,29 @@ pub(crate) async fn update_cafe( let cafe = state .cafe_repo - .update(id, payload) + .update(id, update) .await .map_err(AppError::from)?; info!(%id, "cafe updated"); state.stats_invalidator.invalidate(); - Ok(Json(cafe)) + + save_deferred_image( + &state, + "cafe", + i64::from(cafe.id), + image_data_url.as_deref(), + ) + .await; + + let detail_url = format!("/cafes/{}", cafe.slug); + + if is_datastar_request(&headers) { + render_redirect_script(&detail_url).map_err(ApiError::from) + } else if matches!(source, PayloadSource::Form) { + Ok(Redirect::to(&detail_url).into_response()) + } else { + Ok(Json(cafe).into_response()) + } } define_delete_handler!( diff --git a/src/application/routes/api/coffee/gear.rs b/src/application/routes/api/coffee/gear.rs index 74f8509..81e92cc 100644 --- a/src/application/routes/api/coffee/gear.rs +++ b/src/application/routes/api/coffee/gear.rs @@ -125,30 +125,72 @@ pub(crate) async fn list_gear( define_get_handler!(get_gear, GearId, Gear, gear_repo); -#[tracing::instrument(skip(state, _auth_user, headers, query))] +#[derive(Debug, Deserialize)] +pub(crate) struct UpdateGearSubmission { + #[serde(default)] + make: Option, + #[serde(default)] + model: Option, + #[serde(default)] + created_at: Option>, + #[serde(default)] + image: Option, +} + +impl UpdateGearSubmission { + fn into_parts(self) -> (UpdateGear, Option) { + let update = UpdateGear { + make: self.make, + model: self.model, + created_at: self.created_at, + }; + (update, self.image) + } +} + +#[tracing::instrument(skip(state, _auth_user, headers))] pub(crate) async fn update_gear( State(state): State, _auth_user: AuthenticatedUser, headers: HeaderMap, Path(id): Path, - Query(query): Query, - payload: Json, + payload: FlexiblePayload, ) -> Result { - let (request, search) = query.into_request_and_search::(); + let (submission, source) = payload.into_parts(); + let (update, image_data_url) = submission.into_parts(); + + let has_changes = update.make.is_some() + || update.model.is_some() + || update.created_at.is_some() + || image_data_url.is_some(); + + if !has_changes { + return Err(AppError::validation("no changes provided").into()); + } let gear = state .gear_repo - .update(id, payload.0) + .update(id, update) .await .map_err(AppError::from)?; info!(%id, "gear updated"); state.stats_invalidator.invalidate(); + save_deferred_image( + &state, + "gear", + i64::from(gear.id), + image_data_url.as_deref(), + ) + .await; + + let detail_url = format!("/gear/{}", gear.id); + if is_datastar_request(&headers) { - render_gear_list_fragment(state, request, search, true) - .await - .map_err(ApiError::from) + render_redirect_script(&detail_url).map_err(ApiError::from) + } else if matches!(source, PayloadSource::Form) { + Ok(Redirect::to(&detail_url).into_response()) } else { Ok(Json(gear).into_response()) } diff --git a/src/application/routes/api/coffee/roasters.rs b/src/application/routes/api/coffee/roasters.rs index 61f3b14..a3e1a2d 100644 --- a/src/application/routes/api/coffee/roasters.rs +++ b/src/application/routes/api/coffee/roasters.rs @@ -139,19 +139,53 @@ pub(crate) async fn create_roaster( define_get_handler!(get_roaster, RoasterId, Roaster, roaster_repo); -#[tracing::instrument(skip(state, _auth_user))] +#[derive(Debug, Deserialize)] +pub(crate) struct UpdateRoasterSubmission { + #[serde(default)] + name: Option, + #[serde(default)] + country: Option, + #[serde(default)] + city: Option, + #[serde(default)] + homepage: Option, + #[serde(default)] + created_at: Option>, + #[serde(default)] + image: Option, +} + +impl UpdateRoasterSubmission { + fn into_parts(self) -> (UpdateRoaster, Option) { + let update = UpdateRoaster { + name: self.name, + country: self.country, + city: self.city, + homepage: self.homepage, + created_at: self.created_at, + }; + (update, self.image) + } +} + +#[tracing::instrument(skip(state, _auth_user, headers))] pub(crate) async fn update_roaster( State(state): State, _auth_user: AuthenticatedUser, + headers: HeaderMap, Path(id): Path, - Json(payload): Json, -) -> Result, ApiError> { - let payload = payload.normalize(); - let has_changes = payload.name.is_some() - || payload.country.is_some() - || payload.city.is_some() - || payload.homepage.is_some() - || payload.created_at.is_some(); + payload: FlexiblePayload, +) -> Result { + let (submission, source) = payload.into_parts(); + let (update, image_data_url) = submission.into_parts(); + let update = update.normalize(); + + let has_changes = update.name.is_some() + || update.country.is_some() + || update.city.is_some() + || update.homepage.is_some() + || update.created_at.is_some() + || image_data_url.is_some(); if !has_changes { return Err(AppError::validation("no changes provided").into()); @@ -159,12 +193,29 @@ pub(crate) async fn update_roaster( let roaster = state .roaster_repo - .update(id, payload) + .update(id, update) .await .map_err(AppError::from)?; info!(%id, "roaster updated"); state.stats_invalidator.invalidate(); - Ok(Json(roaster)) + + save_deferred_image( + &state, + "roaster", + i64::from(roaster.id), + image_data_url.as_deref(), + ) + .await; + + let detail_url = format!("/roasters/{}", roaster.slug); + + if is_datastar_request(&headers) { + render_redirect_script(&detail_url).map_err(ApiError::from) + } else if matches!(source, PayloadSource::Form) { + Ok(Redirect::to(&detail_url).into_response()) + } else { + Ok(Json(roaster).into_response()) + } } define_delete_handler!( diff --git a/src/application/routes/api/coffee/roasts.rs b/src/application/routes/api/coffee/roasts.rs index bd40822..3acca15 100644 --- a/src/application/routes/api/coffee/roasts.rs +++ b/src/application/routes/api/coffee/roasts.rs @@ -176,21 +176,64 @@ define_delete_handler!( image_type: "roast" ); -#[tracing::instrument(skip(state, _auth_user))] +#[derive(Debug, Deserialize)] +pub(crate) struct UpdateRoastSubmission { + #[serde(default)] + roaster_id: Option, + #[serde(default)] + name: Option, + #[serde(default)] + origin: Option, + #[serde(default)] + region: Option, + #[serde(default)] + producer: Option, + #[serde(default)] + tasting_notes: Option, + #[serde(default)] + process: Option, + #[serde(default)] + created_at: Option>, + #[serde(default)] + image: Option, +} + +impl UpdateRoastSubmission { + fn into_parts(self) -> (UpdateRoast, Option) { + let update = UpdateRoast { + roaster_id: self.roaster_id, + name: self.name, + origin: self.origin, + region: self.region, + producer: self.producer, + tasting_notes: self.tasting_notes.map(TastingNotesInput::into_vec), + process: self.process, + created_at: self.created_at, + }; + (update, self.image) + } +} + +#[tracing::instrument(skip(state, _auth_user, headers))] pub(crate) async fn update_roast( State(state): State, _auth_user: AuthenticatedUser, + headers: HeaderMap, Path(id): Path, - Json(payload): Json, -) -> Result, ApiError> { - let has_changes = payload.roaster_id.is_some() - || payload.name.is_some() - || payload.origin.is_some() - || payload.region.is_some() - || payload.producer.is_some() - || payload.tasting_notes.is_some() - || payload.process.is_some() - || payload.created_at.is_some(); + payload: FlexiblePayload, +) -> Result { + let (submission, source) = payload.into_parts(); + let (update, image_data_url) = submission.into_parts(); + + let has_changes = update.roaster_id.is_some() + || update.name.is_some() + || update.origin.is_some() + || update.region.is_some() + || update.producer.is_some() + || update.tasting_notes.is_some() + || update.process.is_some() + || update.created_at.is_some() + || image_data_url.is_some(); if !has_changes { return Err(AppError::validation("no changes provided").into()); @@ -198,20 +241,33 @@ pub(crate) async fn update_roast( state .roast_repo - .update(id, payload) + .update(id, update) .await .map_err(AppError::from)?; info!(%id, "roast updated"); state.stats_invalidator.invalidate(); + save_deferred_image(&state, "roast", i64::from(id), image_data_url.as_deref()).await; + let enriched = state .roast_repo .get_with_roaster(id) .await .map_err(AppError::from)?; - Ok(Json(enriched)) + let detail_url = format!( + "/roasters/{}/roasts/{}", + enriched.roaster_slug, enriched.roast.slug + ); + + if is_datastar_request(&headers) { + render_redirect_script(&detail_url).map_err(ApiError::from) + } else if matches!(source, PayloadSource::Form) { + Ok(Redirect::to(&detail_url).into_response()) + } else { + Ok(Json(enriched).into_response()) + } } #[derive(Debug, Deserialize)] diff --git a/tests/server/datastar.rs b/tests/server/datastar.rs index 1ae1ca6..d8c18f6 100644 --- a/tests/server/datastar.rs +++ b/tests/server/datastar.rs @@ -4,9 +4,9 @@ //! Datastar headers when the `datastar-request: true` header is present. use crate::helpers::{ - TestApp, assert_datastar_headers, assert_full_page, assert_html_fragment, create_default_bag, - create_default_cafe, create_default_gear, create_default_roast, create_default_roaster, - spawn_app_with_auth, + TestApp, assert_datastar_headers, assert_datastar_headers_with_mode, assert_full_page, + assert_html_fragment, create_default_bag, create_default_cafe, create_default_gear, + create_default_roast, create_default_roaster, spawn_app_with_auth, }; use crate::test_macros::define_datastar_entity_tests; use brewlog::domain::bags::UpdateBag; @@ -459,7 +459,7 @@ async fn gear_create_without_datastar_header_returns_json() { } #[tokio::test] -async fn gear_update_with_datastar_header_returns_fragment() { +async fn gear_update_with_datastar_header_returns_redirect_script() { let app = spawn_app_with_auth().await; let client = Client::new(); @@ -479,11 +479,9 @@ async fn gear_update_with_datastar_header_returns_fragment() { let gear: brewlog::domain::gear::Gear = create_response.json().await.unwrap(); - let update = brewlog::domain::gear::UpdateGear { - make: Some("Updated Make".to_string()), - model: None, - created_at: None, - }; + let update = serde_json::json!({ + "make": "Updated Make", + }); let response = client .put(app.api_url(&format!("/gear/{}", gear.id))) @@ -495,10 +493,13 @@ async fn gear_update_with_datastar_header_returns_fragment() { .expect("failed to update gear"); assert_eq!(response.status(), 200); - assert_datastar_headers(&response, "#gear-list"); + assert_datastar_headers_with_mode(&response, "body", "append"); let body = response.text().await.expect("failed to read body"); - assert_html_fragment(&body); + assert!( + body.contains("window.location"), + "Expected redirect script in body" + ); } #[tokio::test]