From 3a9fb167932133611d19a0f1583af62d787948e2 Mon Sep 17 00:00:00 2001 From: Jon Seager Date: Mon, 2 Feb 2026 14:15:18 +0000 Subject: [PATCH] refactor(bags): replace multiple list methods with composable filter - Add BagFilter struct with constructor methods (all, open, closed, for_roast) - Replace 5 repository methods with single list(filter, request) method - Add build_where_clause helper for dynamic WHERE clause construction - Update all callers in bags and roasts routes This eliminates method explosion when adding new filters - now only BagFilter and build_where_clause need updating instead of adding new repository methods. --- src/application/routes/bags.rs | 35 +++++--- src/application/routes/roasts.rs | 9 +- src/domain/bags.rs | 38 ++++++++ src/domain/repositories.rs | 10 +-- src/infrastructure/repositories/bags.rs | 110 ++++++++---------------- 5 files changed, 107 insertions(+), 95 deletions(-) diff --git a/src/application/routes/bags.rs b/src/application/routes/bags.rs index 3225b4d..be22575 100644 --- a/src/application/routes/bags.rs +++ b/src/application/routes/bags.rs @@ -12,7 +12,7 @@ use crate::application::routes::support::{ FlexiblePayload, ListQuery, PayloadSource, is_datastar_request, }; use crate::application::server::AppState; -use crate::domain::bags::{Bag, BagSortKey, BagWithRoast, NewBag, UpdateBag}; +use crate::domain::bags::{Bag, BagFilter, BagSortKey, BagWithRoast, NewBag, UpdateBag}; use crate::domain::ids::{BagId, RoastId}; use crate::domain::listing::{ListRequest, SortDirection}; use crate::domain::roasters::RoasterSortKey; @@ -34,12 +34,21 @@ async fn load_bag_page( state: &AppState, request: ListRequest, ) -> Result { - let open_bags = state.bag_repo.list_open().await.map_err(AppError::from)?; - let open_bags_view = open_bags.into_iter().map(BagView::from_domain).collect(); + let open_request = ListRequest::show_all(BagSortKey::RoastDate, SortDirection::Desc); + let open_page = state + .bag_repo + .list(BagFilter::open(), &open_request) + .await + .map_err(AppError::from)?; + let open_bags_view = open_page + .items + .into_iter() + .map(BagView::from_domain) + .collect(); let page = state .bag_repo - .list_closed(&request) + .list(BagFilter::closed(), &request) .await .map_err(AppError::from)?; @@ -174,15 +183,17 @@ pub(crate) async fn list_bags( State(state): State, Query(params): Query, ) -> Result>, ApiError> { - let bags = match params.roast_id { - Some(roast_id) => state - .bag_repo - .list_by_roast(roast_id) - .await - .map_err(AppError::from)?, - None => state.bag_repo.list_all().await.map_err(AppError::from)?, + let filter = match params.roast_id { + Some(roast_id) => BagFilter::for_roast(roast_id), + None => BagFilter::all(), }; - Ok(Json(bags)) + let request = ListRequest::show_all(BagSortKey::RoastDate, SortDirection::Desc); + let page = state + .bag_repo + .list(filter, &request) + .await + .map_err(AppError::from)?; + Ok(Json(page.items)) } define_get_handler!(get_bag, BagId, Bag, bag_repo); diff --git a/src/application/routes/roasts.rs b/src/application/routes/roasts.rs index 1a975a9..3c4eb06 100644 --- a/src/application/routes/roasts.rs +++ b/src/application/routes/roasts.rs @@ -12,6 +12,7 @@ use crate::application::routes::support::{ FlexiblePayload, ListQuery, PayloadSource, is_datastar_request, }; use crate::application::server::AppState; +use crate::domain::bags::{BagFilter, BagSortKey}; use crate::domain::ids::{RoastId, RoasterId}; use crate::domain::listing::{ListRequest, SortDirection}; use crate::domain::roasters::RoasterSortKey; @@ -105,13 +106,15 @@ pub(crate) async fn roast_page( .await .map_err(|err| map_app_error(AppError::from(err)))?; - let bags = state + let bag_request = ListRequest::show_all(BagSortKey::RoastDate, SortDirection::Desc); + let bags_page = state .bag_repo - .list_by_roast(roast.id) + .list(BagFilter::for_roast(roast.id), &bag_request) .await .map_err(|err| map_app_error(AppError::from(err)))?; - let bag_views = bags + let bag_views = bags_page + .items .into_iter() .map(crate::presentation::web::views::BagView::from_domain) .collect(); diff --git a/src/domain/bags.rs b/src/domain/bags.rs index 913be17..e98ccfe 100644 --- a/src/domain/bags.rs +++ b/src/domain/bags.rs @@ -41,6 +41,44 @@ pub struct UpdateBag { pub finished_at: Option, } +/// Filter criteria for bag queries. +#[derive(Debug, Default, Clone)] +pub struct BagFilter { + pub closed: Option, + pub roast_id: Option, +} + +impl BagFilter { + /// No filter - returns all bags. + pub fn all() -> Self { + Self::default() + } + + /// Filter for open (unclosed) bags only. + pub fn open() -> Self { + Self { + closed: Some(false), + ..Default::default() + } + } + + /// Filter for closed bags only. + pub fn closed() -> Self { + Self { + closed: Some(true), + ..Default::default() + } + } + + /// Filter for bags of a specific roast. + pub fn for_roast(roast_id: RoastId) -> Self { + Self { + roast_id: Some(roast_id), + ..Default::default() + } + } +} + #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum BagSortKey { RoastDate, diff --git a/src/domain/repositories.rs b/src/domain/repositories.rs index ff2a3d8..02e3074 100644 --- a/src/domain/repositories.rs +++ b/src/domain/repositories.rs @@ -1,7 +1,7 @@ use super::RepositoryError; use crate::domain::listing::{ListRequest, Page, SortDirection, SortKey}; -use crate::domain::bags::{Bag, BagSortKey, BagWithRoast, NewBag, UpdateBag}; +use crate::domain::bags::{Bag, BagFilter, BagSortKey, BagWithRoast, NewBag, UpdateBag}; use crate::domain::ids::{BagId, RoastId, RoasterId, SessionId, TokenId, UserId}; use crate::domain::roasters::RoasterSortKey; use crate::domain::roasters::{NewRoaster, Roaster, UpdateRoaster}; @@ -126,15 +126,9 @@ pub trait BagRepository: Send + Sync { async fn get(&self, id: BagId) -> Result; async fn list( &self, + filter: BagFilter, request: &ListRequest, ) -> Result, RepositoryError>; - async fn list_by_roast(&self, roast_id: RoastId) -> Result, RepositoryError>; async fn update(&self, id: BagId, changes: UpdateBag) -> Result; async fn delete(&self, id: BagId) -> Result<(), RepositoryError>; - async fn list_open(&self) -> Result, RepositoryError>; - async fn list_closed( - &self, - request: &ListRequest, - ) -> Result, RepositoryError>; - async fn list_all(&self) -> Result, RepositoryError>; } diff --git a/src/infrastructure/repositories/bags.rs b/src/infrastructure/repositories/bags.rs index 4327855..ce5ff9e 100644 --- a/src/infrastructure/repositories/bags.rs +++ b/src/infrastructure/repositories/bags.rs @@ -4,7 +4,7 @@ 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::bags::{Bag, BagFilter, BagSortKey, BagWithRoast, NewBag, UpdateBag}; use crate::domain::ids::{BagId, RoastId}; use crate::domain::listing::{ListRequest, Page, SortDirection}; use crate::domain::repositories::BagRepository; @@ -80,6 +80,27 @@ impl SqlBagRepository { roaster_slug: record.roaster_slug, } } + + fn build_where_clause(filter: &BagFilter) -> Option { + let mut conditions = Vec::new(); + + if let Some(closed) = filter.closed { + conditions.push(format!( + "b.closed = {}", + if closed { "TRUE" } else { "FALSE" } + )); + } + + if let Some(roast_id) = filter.roast_id { + conditions.push(format!("b.roast_id = {}", roast_id.into_inner())); + } + + if conditions.is_empty() { + None + } else { + Some(conditions.join(" AND ")) + } + } } #[async_trait] @@ -122,40 +143,35 @@ impl BagRepository for SqlBagRepository { async fn list( &self, + filter: BagFilter, request: &ListRequest, ) -> Result, RepositoryError> { let order_clause = Self::order_clause(request); - let count_query = "SELECT COUNT(*) FROM bags"; + + // Build WHERE clause from filter + let where_clause = Self::build_where_clause(&filter); + + let base_query = match &where_clause { + Some(w) => format!("{} WHERE {}", BASE_SELECT, w), + None => BASE_SELECT.to_string(), + }; + + let count_query = match &where_clause { + Some(w) => format!("SELECT COUNT(*) FROM bags b WHERE {}", w), + None => "SELECT COUNT(*) FROM bags".to_string(), + }; crate::infrastructure::repositories::pagination::paginate( &self.pool, request, - BASE_SELECT, - count_query, + &base_query, + &count_query, &order_clause, |record| Ok(Self::to_domain_with_roast(record)), ) .await } - async fn list_by_roast(&self, roast_id: RoastId) -> Result, RepositoryError> { - let query = format!( - "{} WHERE b.roast_id = ? ORDER BY b.roast_date DESC", - BASE_SELECT - ); - - let records = query_as::<_, BagWithRoastRecord>(&query) - .bind(roast_id.into_inner()) - .fetch_all(&self.pool) - .await - .map_err(|err| RepositoryError::unexpected(err.to_string()))?; - - Ok(records - .into_iter() - .map(Self::to_domain_with_roast) - .collect()) - } - async fn update(&self, id: BagId, changes: UpdateBag) -> Result { let mut builder = QueryBuilder::new("UPDATE bags SET updated_at = CURRENT_TIMESTAMP"); let mut sep = true; // Already have updated_at @@ -194,56 +210,6 @@ impl BagRepository for SqlBagRepository { Ok(()) } - - async fn list_open(&self) -> Result, RepositoryError> { - let query = format!( - "{} WHERE b.closed = FALSE ORDER BY b.roast_date DESC", - BASE_SELECT - ); - - let records = query_as::<_, BagWithRoastRecord>(&query) - .fetch_all(&self.pool) - .await - .map_err(|err| RepositoryError::unexpected(err.to_string()))?; - - Ok(records - .into_iter() - .map(Self::to_domain_with_roast) - .collect()) - } - - async fn list_closed( - &self, - request: &ListRequest, - ) -> Result, RepositoryError> { - 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( - &self.pool, - request, - &base_query, - count_query, - &order_clause, - |record| Ok(Self::to_domain_with_roast(record)), - ) - .await - } - - async fn list_all(&self) -> Result, RepositoryError> { - let query = format!("{} ORDER BY b.roast_date DESC", BASE_SELECT); - - let records = query_as::<_, BagWithRoastRecord>(&query) - .fetch_all(&self.pool) - .await - .map_err(|err| RepositoryError::unexpected(err.to_string()))?; - - Ok(records - .into_iter() - .map(Self::to_domain_with_roast) - .collect()) - } } #[derive(sqlx::FromRow)]