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.
This commit is contained in:
Jon Seager 2026-02-02 14:15:18 +00:00
parent df0ebc283b
commit 3a9fb16793
No known key found for this signature in database
5 changed files with 107 additions and 95 deletions

View file

@ -12,7 +12,7 @@ use crate::application::routes::support::{
FlexiblePayload, ListQuery, PayloadSource, is_datastar_request, FlexiblePayload, ListQuery, PayloadSource, is_datastar_request,
}; };
use crate::application::server::AppState; 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::ids::{BagId, RoastId};
use crate::domain::listing::{ListRequest, SortDirection}; use crate::domain::listing::{ListRequest, SortDirection};
use crate::domain::roasters::RoasterSortKey; use crate::domain::roasters::RoasterSortKey;
@ -34,12 +34,21 @@ async fn load_bag_page(
state: &AppState, state: &AppState,
request: ListRequest<BagSortKey>, request: ListRequest<BagSortKey>,
) -> Result<BagPageData, AppError> { ) -> Result<BagPageData, AppError> {
let open_bags = state.bag_repo.list_open().await.map_err(AppError::from)?; let open_request = ListRequest::show_all(BagSortKey::RoastDate, SortDirection::Desc);
let open_bags_view = open_bags.into_iter().map(BagView::from_domain).collect(); 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 let page = state
.bag_repo .bag_repo
.list_closed(&request) .list(BagFilter::closed(), &request)
.await .await
.map_err(AppError::from)?; .map_err(AppError::from)?;
@ -174,15 +183,17 @@ pub(crate) async fn list_bags(
State(state): State<AppState>, State(state): State<AppState>,
Query(params): Query<BagsQuery>, Query(params): Query<BagsQuery>,
) -> Result<Json<Vec<BagWithRoast>>, ApiError> { ) -> Result<Json<Vec<BagWithRoast>>, ApiError> {
let bags = match params.roast_id { let filter = match params.roast_id {
Some(roast_id) => state Some(roast_id) => BagFilter::for_roast(roast_id),
.bag_repo None => BagFilter::all(),
.list_by_roast(roast_id)
.await
.map_err(AppError::from)?,
None => state.bag_repo.list_all().await.map_err(AppError::from)?,
}; };
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); define_get_handler!(get_bag, BagId, Bag, bag_repo);

View file

@ -12,6 +12,7 @@ use crate::application::routes::support::{
FlexiblePayload, ListQuery, PayloadSource, is_datastar_request, FlexiblePayload, ListQuery, PayloadSource, is_datastar_request,
}; };
use crate::application::server::AppState; use crate::application::server::AppState;
use crate::domain::bags::{BagFilter, BagSortKey};
use crate::domain::ids::{RoastId, RoasterId}; use crate::domain::ids::{RoastId, RoasterId};
use crate::domain::listing::{ListRequest, SortDirection}; use crate::domain::listing::{ListRequest, SortDirection};
use crate::domain::roasters::RoasterSortKey; use crate::domain::roasters::RoasterSortKey;
@ -105,13 +106,15 @@ pub(crate) async fn roast_page(
.await .await
.map_err(|err| map_app_error(AppError::from(err)))?; .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 .bag_repo
.list_by_roast(roast.id) .list(BagFilter::for_roast(roast.id), &bag_request)
.await .await
.map_err(|err| map_app_error(AppError::from(err)))?; .map_err(|err| map_app_error(AppError::from(err)))?;
let bag_views = bags let bag_views = bags_page
.items
.into_iter() .into_iter()
.map(crate::presentation::web::views::BagView::from_domain) .map(crate::presentation::web::views::BagView::from_domain)
.collect(); .collect();

View file

@ -41,6 +41,44 @@ pub struct UpdateBag {
pub finished_at: Option<NaiveDate>, pub finished_at: Option<NaiveDate>,
} }
/// Filter criteria for bag queries.
#[derive(Debug, Default, Clone)]
pub struct BagFilter {
pub closed: Option<bool>,
pub roast_id: Option<RoastId>,
}
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)] #[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum BagSortKey { pub enum BagSortKey {
RoastDate, RoastDate,

View file

@ -1,7 +1,7 @@
use super::RepositoryError; use super::RepositoryError;
use crate::domain::listing::{ListRequest, Page, SortDirection, SortKey}; 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::ids::{BagId, RoastId, RoasterId, SessionId, TokenId, UserId};
use crate::domain::roasters::RoasterSortKey; use crate::domain::roasters::RoasterSortKey;
use crate::domain::roasters::{NewRoaster, Roaster, UpdateRoaster}; use crate::domain::roasters::{NewRoaster, Roaster, UpdateRoaster};
@ -126,15 +126,9 @@ pub trait BagRepository: Send + Sync {
async fn get(&self, id: BagId) -> Result<Bag, RepositoryError>; async fn get(&self, id: BagId) -> Result<Bag, RepositoryError>;
async fn list( async fn list(
&self, &self,
filter: BagFilter,
request: &ListRequest<BagSortKey>, request: &ListRequest<BagSortKey>,
) -> Result<Page<BagWithRoast>, RepositoryError>; ) -> Result<Page<BagWithRoast>, RepositoryError>;
async fn list_by_roast(&self, roast_id: RoastId) -> Result<Vec<BagWithRoast>, RepositoryError>;
async fn update(&self, id: BagId, changes: UpdateBag) -> Result<Bag, RepositoryError>; async fn update(&self, id: BagId, changes: UpdateBag) -> Result<Bag, RepositoryError>;
async fn delete(&self, id: BagId) -> Result<(), RepositoryError>; async fn delete(&self, id: BagId) -> Result<(), RepositoryError>;
async fn list_open(&self) -> Result<Vec<BagWithRoast>, RepositoryError>;
async fn list_closed(
&self,
request: &ListRequest<BagSortKey>,
) -> Result<Page<BagWithRoast>, RepositoryError>;
async fn list_all(&self) -> Result<Vec<BagWithRoast>, RepositoryError>;
} }

View file

@ -4,7 +4,7 @@ use sqlx::{QueryBuilder, query_as};
use super::macros::push_update_field; use super::macros::push_update_field;
use crate::domain::RepositoryError; 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::ids::{BagId, RoastId};
use crate::domain::listing::{ListRequest, Page, SortDirection}; use crate::domain::listing::{ListRequest, Page, SortDirection};
use crate::domain::repositories::BagRepository; use crate::domain::repositories::BagRepository;
@ -80,6 +80,27 @@ impl SqlBagRepository {
roaster_slug: record.roaster_slug, roaster_slug: record.roaster_slug,
} }
} }
fn build_where_clause(filter: &BagFilter) -> Option<String> {
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] #[async_trait]
@ -122,40 +143,35 @@ impl BagRepository for SqlBagRepository {
async fn list( async fn list(
&self, &self,
filter: BagFilter,
request: &ListRequest<BagSortKey>, request: &ListRequest<BagSortKey>,
) -> Result<Page<BagWithRoast>, RepositoryError> { ) -> Result<Page<BagWithRoast>, RepositoryError> {
let order_clause = Self::order_clause(request); 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( crate::infrastructure::repositories::pagination::paginate(
&self.pool, &self.pool,
request, request,
BASE_SELECT, &base_query,
count_query, &count_query,
&order_clause, &order_clause,
|record| Ok(Self::to_domain_with_roast(record)), |record| Ok(Self::to_domain_with_roast(record)),
) )
.await .await
} }
async fn list_by_roast(&self, roast_id: RoastId) -> Result<Vec<BagWithRoast>, 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<Bag, RepositoryError> { async fn update(&self, id: BagId, changes: UpdateBag) -> Result<Bag, RepositoryError> {
let mut builder = QueryBuilder::new("UPDATE bags SET updated_at = CURRENT_TIMESTAMP"); let mut builder = QueryBuilder::new("UPDATE bags SET updated_at = CURRENT_TIMESTAMP");
let mut sep = true; // Already have updated_at let mut sep = true; // Already have updated_at
@ -194,56 +210,6 @@ impl BagRepository for SqlBagRepository {
Ok(()) Ok(())
} }
async fn list_open(&self) -> Result<Vec<BagWithRoast>, 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<BagSortKey>,
) -> Result<Page<BagWithRoast>, 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<Vec<BagWithRoast>, 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)] #[derive(sqlx::FromRow)]