diff --git a/src/application/routes/bags.rs b/src/application/routes/bags.rs index 6ce12f4..97e8eac 100644 --- a/src/application/routes/bags.rs +++ b/src/application/routes/bags.rs @@ -217,10 +217,10 @@ pub(crate) async fn update_bag( finished_at: body_update.finished_at.or(update_params.finished_at), }; - if let Some(true) = update.closed { - if update.finished_at.is_none() { - update.finished_at = Some(chrono::Utc::now().date_naive()); - } + if let Some(true) = update.closed + && update.finished_at.is_none() + { + update.finished_at = Some(chrono::Utc::now().date_naive()); } let bag = state @@ -231,28 +231,28 @@ pub(crate) async fn update_bag( if let Some(true) = update.closed { // Fetch roast and roaster for timeline event - if let Ok(roast) = state.roast_repo.get(bag.roast_id).await { - if let Ok(roaster) = state.roaster_repo.get(roast.roaster_id).await { - let event = NewTimelineEvent { - entity_type: "bag".to_string(), - entity_id: bag.id.into_inner(), - action: "finished".to_string(), - occurred_at: chrono::Utc::now(), - title: format!("{}", roast.name), - details: vec![ - TimelineEventDetail { - label: "Roaster".to_string(), - value: roaster.name, - }, - TimelineEventDetail { - label: "Amount".to_string(), - value: format!("{}g", bag.amount), - }, - ], - tasting_notes: vec![], - }; - let _ = state.timeline_repo.insert(event).await; - } + if let Ok(roast) = state.roast_repo.get(bag.roast_id).await + && let Ok(roaster) = state.roaster_repo.get(roast.roaster_id).await + { + let event = NewTimelineEvent { + entity_type: "bag".to_string(), + entity_id: bag.id.into_inner(), + action: "finished".to_string(), + occurred_at: chrono::Utc::now(), + title: roast.name.to_string(), + details: vec![ + TimelineEventDetail { + label: "Roaster".to_string(), + value: roaster.name, + }, + TimelineEventDetail { + label: "Amount".to_string(), + value: format!("{}g", bag.amount), + }, + ], + tasting_notes: vec![], + }; + let _ = state.timeline_repo.insert(event).await; } } diff --git a/src/infrastructure/repositories/bags.rs b/src/infrastructure/repositories/bags.rs index ba406e5..4327855 100644 --- a/src/infrastructure/repositories/bags.rs +++ b/src/infrastructure/repositories/bags.rs @@ -1,7 +1,8 @@ use async_trait::async_trait; use chrono::{DateTime, NaiveDate, Utc}; -use sqlx::query_as; +use sqlx::{QueryBuilder, query_as}; +use super::macros::push_update_field; use crate::domain::RepositoryError; use crate::domain::bags::{Bag, BagSortKey, BagWithRoast, NewBag, UpdateBag}; use crate::domain::ids::{BagId, RoastId}; @@ -29,6 +30,23 @@ impl SqlBagRepository { Self { pool } } + fn order_clause(request: &ListRequest) -> String { + let sort_column = match request.sort_key { + BagSortKey::RoastDate => "b.roast_date", + BagSortKey::CreatedAt => "b.created_at", + BagSortKey::Roaster => "rr.name", + BagSortKey::Roast => "r.name", + BagSortKey::FinishedAt => "b.finished_at", + }; + + let direction = match request.sort_direction { + SortDirection::Asc => "ASC", + SortDirection::Desc => "DESC", + }; + + format!("{} {}", sort_column, direction) + } + fn to_domain(record: BagRecord) -> Bag { Bag { id: BagId::new(record.id), @@ -106,21 +124,7 @@ impl BagRepository for SqlBagRepository { &self, request: &ListRequest, ) -> Result, RepositoryError> { - let sort_column = match request.sort_key { - BagSortKey::RoastDate => "b.roast_date", - BagSortKey::CreatedAt => "b.created_at", - BagSortKey::Roaster => "rr.name", - BagSortKey::Roast => "r.name", - BagSortKey::FinishedAt => "b.finished_at", - }; - - let direction = match request.sort_direction { - SortDirection::Asc => "ASC", - SortDirection::Desc => "DESC", - }; - - let order_clause = format!("{} {}", sort_column, direction); - + let order_clause = Self::order_clause(request); let count_query = "SELECT COUNT(*) FROM bags"; crate::infrastructure::repositories::pagination::paginate( @@ -153,47 +157,20 @@ impl BagRepository for SqlBagRepository { } async fn update(&self, id: BagId, changes: UpdateBag) -> Result { - let mut query = "UPDATE bags SET updated_at = CURRENT_TIMESTAMP".to_string(); - let mut has_changes = false; + let mut builder = QueryBuilder::new("UPDATE bags SET updated_at = CURRENT_TIMESTAMP"); + let mut sep = true; // Already have updated_at - if changes.remaining.is_some() { - query.push_str(", remaining = ?"); - has_changes = true; - } + push_update_field!(builder, sep, "remaining", changes.remaining); + push_update_field!(builder, sep, "closed", changes.closed); + push_update_field!(builder, sep, "finished_at", changes.finished_at); + let _ = sep; // Suppress unused_assignments warning from macro - if changes.closed.is_some() { - query.push_str(", closed = ?"); - has_changes = true; - } + builder.push(" WHERE id = "); + builder.push_bind(id.into_inner()); + builder.push(" RETURNING id, roast_id, roast_date, amount, remaining, closed, finished_at, created_at, updated_at"); - if changes.finished_at.is_some() { - query.push_str(", finished_at = ?"); - has_changes = true; - } - - if !has_changes { - return self.get(id).await; - } - - query.push_str(" WHERE id = ? RETURNING id, roast_id, roast_date, amount, remaining, closed, finished_at, created_at, updated_at"); - - let mut q = query_as::<_, BagRecord>(&query); - - if let Some(remaining) = changes.remaining { - q = q.bind(remaining); - } - - if let Some(closed) = changes.closed { - q = q.bind(closed); - } - - if let Some(finished_at) = changes.finished_at { - q = q.bind(finished_at); - } - - q = q.bind(id.into_inner()); - - let record = q + let record = builder + .build_query_as::() .fetch_optional(&self.pool) .await .map_err(|err| RepositoryError::unexpected(err.to_string()))? @@ -239,23 +216,8 @@ impl BagRepository for SqlBagRepository { &self, request: &ListRequest, ) -> Result, RepositoryError> { - let sort_column = match request.sort_key { - BagSortKey::RoastDate => "b.roast_date", - BagSortKey::CreatedAt => "b.created_at", - BagSortKey::Roaster => "rr.name", - BagSortKey::Roast => "r.name", - BagSortKey::FinishedAt => "b.finished_at", - }; - - let direction = match request.sort_direction { - SortDirection::Asc => "ASC", - SortDirection::Desc => "DESC", - }; - - let order_clause = format!("{} {}", sort_column, direction); - + let order_clause = Self::order_clause(request); let base_query = format!("{} WHERE b.closed = TRUE", BASE_SELECT); - let count_query = "SELECT COUNT(*) FROM bags WHERE closed = TRUE"; crate::infrastructure::repositories::pagination::paginate( diff --git a/src/infrastructure/repositories/macros.rs b/src/infrastructure/repositories/macros.rs new file mode 100644 index 0000000..1268136 --- /dev/null +++ b/src/infrastructure/repositories/macros.rs @@ -0,0 +1,26 @@ +/// Helper macro for building dynamic UPDATE queries with QueryBuilder. +/// +/// Handles the common pattern of conditionally adding SET clauses with proper +/// comma separation. +/// +/// # Example +/// ```ignore +/// let mut builder = QueryBuilder::new("UPDATE users SET "); +/// let mut sep = false; +/// push_update_field!(builder, sep, "name", changes.name); +/// push_update_field!(builder, sep, "email", changes.email); +/// ``` +macro_rules! push_update_field { + ($builder:expr, $separator:expr, $field:literal, $value:expr) => { + if let Some(value) = $value { + if $separator { + $builder.push(", "); + } + $separator = true; + $builder.push(concat!($field, " = ")); + $builder.push_bind(value); + } + }; +} + +pub(super) use push_update_field; diff --git a/src/infrastructure/repositories/mod.rs b/src/infrastructure/repositories/mod.rs index a08ad8f..9446da8 100644 --- a/src/infrastructure/repositories/mod.rs +++ b/src/infrastructure/repositories/mod.rs @@ -1,4 +1,5 @@ pub mod bags; +mod macros; pub mod pagination; pub mod roasters; pub mod roasts; diff --git a/src/infrastructure/repositories/roasters.rs b/src/infrastructure/repositories/roasters.rs index ec7c311..de424ee 100644 --- a/src/infrastructure/repositories/roasters.rs +++ b/src/infrastructure/repositories/roasters.rs @@ -2,6 +2,7 @@ use async_trait::async_trait; use chrono::{DateTime, Utc}; use sqlx::{QueryBuilder, query, query_as}; +use super::macros::push_update_field; use crate::domain::RepositoryError; use crate::domain::ids::RoasterId; use crate::domain::listing::{ListRequest, Page, SortDirection}; @@ -20,7 +21,7 @@ impl SqlRoasterRepository { Self { pool } } - fn sort_clause(request: &ListRequest) -> String { + fn order_clause(request: &ListRequest) -> String { let dir_sql = match request.sort_direction() { SortDirection::Asc => "ASC", SortDirection::Desc => "DESC", @@ -184,7 +185,7 @@ impl RoasterRepository for SqlRoasterRepository { &self, request: &ListRequest, ) -> Result, RepositoryError> { - let order_clause = Self::sort_clause(request); + let order_clause = Self::order_clause(request); let base_query = "SELECT id, name, slug, country, city, homepage, notes, created_at FROM roasters"; let count_query = "SELECT COUNT(*) FROM roasters"; @@ -206,50 +207,15 @@ impl RoasterRepository for SqlRoasterRepository { changes: UpdateRoaster, ) -> Result { let mut builder = QueryBuilder::new("UPDATE roasters SET "); - let mut wrote_field = false; + let mut sep = false; - if let Some(name) = changes.name { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("name = "); - builder.push_bind(name); - } - if let Some(country) = changes.country { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("country = "); - builder.push_bind(country); - } - if let Some(city) = changes.city { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("city = "); - builder.push_bind(city); - } - if let Some(homepage) = changes.homepage { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("homepage = "); - builder.push_bind(homepage); - } - if let Some(notes) = changes.notes { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("notes = "); - builder.push_bind(notes); - } + push_update_field!(builder, sep, "name", changes.name); + push_update_field!(builder, sep, "country", changes.country); + push_update_field!(builder, sep, "city", changes.city); + push_update_field!(builder, sep, "homepage", changes.homepage); + push_update_field!(builder, sep, "notes", changes.notes); - if !wrote_field { + if !sep { return Err(RepositoryError::unexpected( "No fields provided for update".to_string(), )); diff --git a/src/infrastructure/repositories/roasts.rs b/src/infrastructure/repositories/roasts.rs index 4a61d95..e524701 100644 --- a/src/infrastructure/repositories/roasts.rs +++ b/src/infrastructure/repositories/roasts.rs @@ -3,6 +3,7 @@ use chrono::{DateTime, Utc}; use serde_json::{from_str, to_string}; use sqlx::{Error as SqlxError, QueryBuilder, query, query_as, query_scalar}; +use super::macros::push_update_field; use crate::domain::RepositoryError; use crate::domain::ids::{RoastId, RoasterId}; use crate::domain::listing::{ListRequest, Page, SortDirection}; @@ -261,96 +262,52 @@ impl RoastRepository for SqlRoastRepository { .await .map_err(|err| RepositoryError::unexpected(err.to_string()))?; - let UpdateRoast { - roaster_id, - name, - origin, - region, - producer, - tasting_notes, - process, - } = changes; - let mut builder = QueryBuilder::new("UPDATE roasts SET "); - let mut wrote_field = false; + let mut sep = false; - if let Some(roaster_id) = roaster_id { - if wrote_field { - builder.push(", "); - } - wrote_field = true; + // Handle roaster_id specially due to type conversion + if let Some(roaster_id) = changes.roaster_id { + sep = true; builder.push("roaster_id = "); builder.push_bind(i64::from(roaster_id)); } - if let Some(name) = name { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("name = "); - builder.push_bind(name); - } - if let Some(origin) = origin { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("origin = "); - builder.push_bind(origin); - } - if let Some(region) = region { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("region = "); - builder.push_bind(region); - } - if let Some(producer) = producer { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("producer = "); - builder.push_bind(producer); - } - if let Some(process) = process { - if wrote_field { - builder.push(", "); - } - wrote_field = true; - builder.push("process = "); - builder.push_bind(process); - } - if let Some(tasting_notes) = tasting_notes { + + push_update_field!(builder, sep, "name", changes.name); + push_update_field!(builder, sep, "origin", changes.origin); + push_update_field!(builder, sep, "region", changes.region); + push_update_field!(builder, sep, "producer", changes.producer); + push_update_field!(builder, sep, "process", changes.process); + + // Handle tasting_notes specially due to JSON encoding + if let Some(tasting_notes) = changes.tasting_notes { let notes_json = Self::encode_notes(&tasting_notes)?; - if wrote_field { + if sep { builder.push(", "); } - wrote_field = true; + sep = true; builder.push("tasting_notes = "); builder.push_bind(notes_json); } - if wrote_field { - builder.push(" WHERE id = "); - builder.push_bind(i64::from(id)); - - let result = builder - .build() - .execute(&mut *tx) - .await - .map_err(|err| RepositoryError::unexpected(err.to_string()))?; - - if result.rows_affected() == 0 { - return Err(RepositoryError::NotFound); - } - } else { + if !sep { return Err(RepositoryError::unexpected( "No fields provided for update".to_string(), )); } + builder.push(" WHERE id = "); + builder.push_bind(i64::from(id)); + + let result = builder + .build() + .execute(&mut *tx) + .await + .map_err(|err| RepositoryError::unexpected(err.to_string()))?; + + if result.rows_affected() == 0 { + return Err(RepositoryError::NotFound); + } + tx.commit() .await .map_err(|err| RepositoryError::unexpected(err.to_string()))?; diff --git a/src/presentation/cli/mod.rs b/src/presentation/cli/mod.rs index c57e237..a9a6460 100644 --- a/src/presentation/cli/mod.rs +++ b/src/presentation/cli/mod.rs @@ -1,5 +1,5 @@ -mod macros; pub mod bags; +mod macros; pub mod roasters; pub mod roasts; pub mod tokens; diff --git a/src/presentation/cli/roasters.rs b/src/presentation/cli/roasters.rs index 5fdb512..bf039be 100644 --- a/src/presentation/cli/roasters.rs +++ b/src/presentation/cli/roasters.rs @@ -73,4 +73,10 @@ pub async fn update_roaster(client: &BrewlogClient, command: UpdateRoasterComman print_json(&roaster) } -define_delete_command!(DeleteRoasterCommand, delete_roaster, RoasterId, roasters, "roaster"); +define_delete_command!( + DeleteRoasterCommand, + delete_roaster, + RoasterId, + roasters, + "roaster" +);