refactor: simplify overly nested and duplicated code across five modules

- Extract 35-line filter_map closure into parse_cafe() in foursquare.rs
- Unify triple-duplicated search branching in pagination.rs into
  fetch_records() and fetch_count() helpers
- DRY three identical gear-loading blocks in brews.rs via
  load_gear_options() helper
- Flatten nested if/else-if in timeline map_details() using match,
  extract shared is_blank() helper
- Deduplicate JSON decode pattern in backup.rs with decode_json_vec()
  and decode_json_opt() helpers
This commit is contained in:
Jon Seager 2026-02-04 19:25:35 +00:00
parent 939743a224
commit 47c89bd725
No known key found for this signature in database
5 changed files with 161 additions and 176 deletions

View file

@ -57,50 +57,10 @@ async fn load_brew_form_data(state: &AppState) -> Result<BrewFormData, AppError>
let gear_request = ListRequest::show_all(GearSortKey::Make, SortDirection::Asc); let gear_request = ListRequest::show_all(GearSortKey::Make, SortDirection::Asc);
let grinders = state let grinder_options = load_gear_options(state, GearCategory::Grinder, &gear_request).await?;
.gear_repo let brewer_options = load_gear_options(state, GearCategory::Brewer, &gear_request).await?;
.list( let filter_paper_options =
GearFilter::for_category(GearCategory::Grinder), load_gear_options(state, GearCategory::FilterPaper, &gear_request).await?;
&gear_request,
None,
)
.await
.map_err(AppError::from)?;
let grinder_options: Vec<GearOptionView> = grinders
.items
.into_iter()
.map(GearOptionView::from)
.collect();
let brewers = state
.gear_repo
.list(
GearFilter::for_category(GearCategory::Brewer),
&gear_request,
None,
)
.await
.map_err(AppError::from)?;
let brewer_options: Vec<GearOptionView> = brewers
.items
.into_iter()
.map(GearOptionView::from)
.collect();
let filter_papers = state
.gear_repo
.list(
GearFilter::for_category(GearCategory::FilterPaper),
&gear_request,
None,
)
.await
.map_err(AppError::from)?;
let filter_paper_options: Vec<GearOptionView> = filter_papers
.items
.into_iter()
.map(GearOptionView::from)
.collect();
let last_brew_request = ListRequest::new( let last_brew_request = ListRequest::new(
1, 1,
@ -129,6 +89,19 @@ async fn load_brew_form_data(state: &AppState) -> Result<BrewFormData, AppError>
}) })
} }
async fn load_gear_options(
state: &AppState,
category: GearCategory,
request: &ListRequest<GearSortKey>,
) -> Result<Vec<GearOptionView>, AppError> {
let page = state
.gear_repo
.list(GearFilter::for_category(category), request, None)
.await
.map_err(AppError::from)?;
Ok(page.items.into_iter().map(GearOptionView::from).collect())
}
#[tracing::instrument(skip(state))] #[tracing::instrument(skip(state))]
async fn load_brew_page( async fn load_brew_page(
state: &AppState, state: &AppState,

View file

@ -12,9 +12,33 @@ use crate::domain::gear::{Gear, GearCategory};
use crate::domain::ids::{BagId, BrewId, CafeId, GearId, RoastId, RoasterId, TimelineEventId}; use crate::domain::ids::{BagId, BrewId, CafeId, GearId, RoastId, RoasterId, TimelineEventId};
use crate::domain::roasters::Roaster; use crate::domain::roasters::Roaster;
use crate::domain::roasts::Roast; use crate::domain::roasts::Roast;
use crate::domain::timeline::{TimelineBrewData, TimelineEvent, TimelineEventDetail}; use crate::domain::timeline::TimelineEvent;
use crate::infrastructure::database::{DatabasePool, DatabaseTransaction}; use crate::infrastructure::database::{DatabasePool, DatabaseTransaction};
fn decode_json_vec<T: serde::de::DeserializeOwned>(
raw: Option<String>,
label: &str,
) -> anyhow::Result<Vec<T>> {
match raw {
Some(s) if !s.is_empty() => {
from_str(&s).with_context(|| format!("failed to decode {label}: {s}"))
}
_ => Ok(Vec::new()),
}
}
fn decode_json_opt<T: serde::de::DeserializeOwned>(
raw: Option<String>,
label: &str,
) -> anyhow::Result<Option<T>> {
match raw {
Some(s) if !s.is_empty() => from_str(&s)
.map(Some)
.with_context(|| format!("failed to decode {label}: {s}")),
_ => Ok(None),
}
}
#[derive(Debug, Serialize, Deserialize)] #[derive(Debug, Serialize, Deserialize)]
pub struct BackupData { pub struct BackupData {
pub version: u32, pub version: u32,
@ -480,11 +504,7 @@ struct RoastRecord {
impl RoastRecord { impl RoastRecord {
fn into_domain(self) -> anyhow::Result<Roast> { fn into_domain(self) -> anyhow::Result<Roast> {
let tasting_notes = match self.tasting_notes { let tasting_notes = decode_json_vec(self.tasting_notes, "tasting notes")?;
Some(raw) => from_str::<Vec<String>>(&raw)
.with_context(|| format!("failed to decode tasting notes: {raw}"))?,
None => Vec::new(),
};
Ok(Roast { Ok(Roast {
id: RoastId::from(self.id), id: RoastId::from(self.id),
@ -611,25 +631,9 @@ struct TimelineEventRecord {
impl TimelineEventRecord { impl TimelineEventRecord {
fn into_domain(self) -> anyhow::Result<TimelineEvent> { fn into_domain(self) -> anyhow::Result<TimelineEvent> {
let details = match self.details_json { let details = decode_json_vec(self.details_json, "timeline event details")?;
Some(raw) if !raw.is_empty() => from_str::<Vec<TimelineEventDetail>>(&raw) let tasting_notes = decode_json_vec(self.tasting_notes_json, "timeline tasting notes")?;
.with_context(|| format!("failed to decode timeline event details: {raw}"))?, let brew_data = decode_json_opt(self.brew_data_json, "timeline brew data")?;
_ => Vec::new(),
};
let tasting_notes = match self.tasting_notes_json {
Some(raw) if !raw.is_empty() => from_str::<Vec<String>>(&raw)
.with_context(|| format!("failed to decode timeline tasting notes: {raw}"))?,
_ => Vec::new(),
};
let brew_data = match self.brew_data_json {
Some(raw) if !raw.is_empty() => Some(
from_str::<TimelineBrewData>(&raw)
.with_context(|| format!("failed to decode timeline brew data: {raw}"))?,
),
_ => None,
};
Ok(TimelineEvent { Ok(TimelineEvent {
id: TimelineEventId::from(self.id), id: TimelineEventId::from(self.id),

View file

@ -84,46 +84,44 @@ pub async fn search_nearby(
let cafes = result let cafes = result
.results .results
.into_iter() .into_iter()
.filter_map(|place| { .filter_map(|place| parse_cafe(place, location))
if place.name.is_empty() {
return None;
}
let place_lat = place.latitude?;
let place_lng = place.longitude?;
let place_location = place.location.unwrap_or_default();
let country = place_location
.country
.as_deref()
.map(country_name)
.unwrap_or_default();
let distance = place.distance.unwrap_or_else(|| {
if let SearchLocation::Coordinates { lat, lng } = location {
haversine_distance(*lat, *lng, place_lat, place_lng) as u32
} else {
0
}
});
let website = place.website.filter(|w| !w.trim().is_empty());
Some(NearbyCafe {
name: place.name,
latitude: place_lat,
longitude: place_lng,
city: place_location.locality.unwrap_or_default(),
country,
website,
distance_meters: distance,
})
})
.collect(); .collect();
Ok(cafes) Ok(cafes)
} }
fn parse_cafe(place: FoursquarePlace, location: &SearchLocation) -> Option<NearbyCafe> {
if place.name.is_empty() {
return None;
}
let lat = place.latitude?;
let lng = place.longitude?;
let loc = place.location.unwrap_or_default();
let country = loc.country.as_deref().map(country_name).unwrap_or_default();
let distance = place.distance.unwrap_or_else(|| match location {
SearchLocation::Coordinates {
lat: ref_lat,
lng: ref_lng,
} => haversine_distance(*ref_lat, *ref_lng, lat, lng) as u32,
SearchLocation::Near(_) => 0,
});
let website = place.website.filter(|w| !w.trim().is_empty());
Some(NearbyCafe {
name: place.name,
latitude: lat,
longitude: lng,
city: loc.locality.unwrap_or_default(),
country,
website,
distance_meters: distance,
})
}
/// Converts a 2-letter ISO 3166-1 alpha-2 country code to a full country name. /// Converts a 2-letter ISO 3166-1 alpha-2 country code to a full country name.
/// Falls back to the raw code if the lookup fails. /// Falls back to the raw code if the lookup fails.
fn country_name(code: &str) -> String { fn country_name(code: &str) -> String {

View file

@ -1,4 +1,4 @@
use sqlx::{FromRow, QueryBuilder, query_as, query_scalar}; use sqlx::{FromRow, QueryBuilder, query_scalar};
use crate::domain::RepositoryError; use crate::domain::RepositoryError;
use crate::domain::listing::{ListRequest, Page, PageSize, SortKey}; use crate::domain::listing::{ListRequest, Page, PageSize, SortKey};
@ -42,22 +42,7 @@ where
{ {
match request.page_size() { match request.page_size() {
PageSize::All => { PageSize::All => {
let records: Vec<R> = if let Some(sf) = search { let records = fetch_records::<R>(pool, base_query, order_clause, search, None).await?;
let mut qb = QueryBuilder::new(base_query);
append_search_condition(&mut qb, base_query, sf);
qb.push(" ORDER BY ");
qb.push(order_clause);
qb.build_query_as()
.fetch_all(pool)
.await
.map_err(|err| RepositoryError::unexpected(err.to_string()))?
} else {
let query = format!("{base_query} ORDER BY {order_clause}");
query_as::<_, R>(&query)
.fetch_all(pool)
.await
.map_err(|err| RepositoryError::unexpected(err.to_string()))?
};
let mut items = Vec::with_capacity(records.len()); let mut items = Vec::with_capacity(records.len());
for record in records { for record in records {
@ -72,31 +57,29 @@ where
let mut page = request.page(); let mut page = request.page();
let offset = i64::from(page - 1).saturating_mul(limit); let offset = i64::from(page - 1).saturating_mul(limit);
let total: i64 = if let Some(sf) = search { let total = fetch_count(pool, count_query, search).await?;
let mut count_qb = QueryBuilder::new(count_query);
append_search_condition(&mut count_qb, count_query, sf);
let row: (i64,) = count_qb
.build_query_as()
.fetch_one(pool)
.await
.map_err(|err| RepositoryError::unexpected(err.to_string()))?;
row.0
} else {
query_scalar(count_query)
.fetch_one(pool)
.await
.map_err(|err| RepositoryError::unexpected(err.to_string()))?
};
let mut records = let mut records = fetch_records::<R>(
fetch_page::<R>(pool, base_query, order_clause, search, limit, offset).await?; pool,
base_query,
order_clause,
search,
Some((limit, offset)),
)
.await?;
if page > 1 && records.is_empty() && total > 0 { if page > 1 && records.is_empty() && total > 0 {
let last_page = ((total + limit - 1) / limit) as u32; let last_page = ((total + limit - 1) / limit) as u32;
page = last_page.max(1); page = last_page.max(1);
let offset = i64::from(page - 1).saturating_mul(limit); let offset = i64::from(page - 1).saturating_mul(limit);
records = records = fetch_records::<R>(
fetch_page::<R>(pool, base_query, order_clause, search, limit, offset).await?; pool,
base_query,
order_clause,
search,
Some((limit, offset)),
)
.await?;
} }
let mut items = Vec::with_capacity(records.len()); let mut items = Vec::with_capacity(records.len());
@ -109,36 +92,51 @@ where
} }
} }
async fn fetch_page<R>( async fn fetch_records<R>(
pool: &DatabasePool, pool: &DatabasePool,
base_query: &str, base_query: &str,
order_clause: &str, order_clause: &str,
search: Option<&SearchFilter>, search: Option<&SearchFilter>,
limit: i64, limit_offset: Option<(i64, i64)>,
offset: i64,
) -> Result<Vec<R>, RepositoryError> ) -> Result<Vec<R>, RepositoryError>
where where
R: for<'r> FromRow<'r, DatabaseRow> + Send + Unpin, R: for<'r> FromRow<'r, DatabaseRow> + Send + Unpin,
{ {
let mut qb = QueryBuilder::new(base_query);
if let Some(sf) = search { if let Some(sf) = search {
let mut qb = QueryBuilder::new(base_query);
append_search_condition(&mut qb, base_query, sf); append_search_condition(&mut qb, base_query, sf);
qb.push(" ORDER BY "); }
qb.push(order_clause); qb.push(" ORDER BY ");
qb.push(order_clause);
if let Some((limit, offset)) = limit_offset {
qb.push(" LIMIT "); qb.push(" LIMIT ");
qb.push_bind(limit); qb.push_bind(limit);
qb.push(" OFFSET "); qb.push(" OFFSET ");
qb.push_bind(offset); qb.push_bind(offset);
qb.build_query_as() }
.fetch_all(pool) qb.build_query_as()
.fetch_all(pool)
.await
.map_err(|err| RepositoryError::unexpected(err.to_string()))
}
async fn fetch_count(
pool: &DatabasePool,
count_query: &str,
search: Option<&SearchFilter>,
) -> Result<i64, RepositoryError> {
if let Some(sf) = search {
let mut qb = QueryBuilder::new(count_query);
append_search_condition(&mut qb, count_query, sf);
let row: (i64,) = qb
.build_query_as()
.fetch_one(pool)
.await .await
.map_err(|err| RepositoryError::unexpected(err.to_string())) .map_err(|err| RepositoryError::unexpected(err.to_string()))?;
Ok(row.0)
} else { } else {
let query_sql = format!("{base_query} ORDER BY {order_clause} LIMIT ? OFFSET ?"); query_scalar(count_query)
query_as::<_, R>(&query_sql) .fetch_one(pool)
.bind(limit)
.bind(offset)
.fetch_all(pool)
.await .await
.map_err(|err| RepositoryError::unexpected(err.to_string())) .map_err(|err| RepositoryError::unexpected(err.to_string()))
} }

View file

@ -2,6 +2,11 @@ use crate::domain::timeline::{TimelineEvent, TimelineEventDetail};
use super::relative_date; use super::relative_date;
/// Returns `true` if the value is empty or just an em-dash placeholder.
fn is_blank(value: &str) -> bool {
value.is_empty() || value == "\u{2014}"
}
#[derive(Clone)] #[derive(Clone)]
pub struct TimelineEventDetailView { pub struct TimelineEventDetailView {
pub label: String, pub label: String,
@ -141,7 +146,7 @@ impl TimelineEventView {
.iter() .iter()
.find(|d| d.label.eq_ignore_ascii_case(label)) .find(|d| d.label.eq_ignore_ascii_case(label))
.map(|d| d.value.trim()) .map(|d| d.value.trim())
.filter(|v| !v.is_empty() && *v != "\u{2014}") .filter(|v| !is_blank(v))
}; };
let picks: &[&str] = match entity_type { let picks: &[&str] = match entity_type {
@ -157,7 +162,7 @@ impl TimelineEventView {
.iter() .iter()
.take(3) .take(3)
.map(|d| d.value.trim()) .map(|d| d.value.trim())
.filter(|v| !v.is_empty() && *v != "\u{2014}") .filter(|v| !is_blank(v))
.collect() .collect()
} else { } else {
picks.iter().filter_map(|l| find_value(l)).collect() picks.iter().filter_map(|l| find_value(l)).collect()
@ -175,34 +180,41 @@ impl TimelineEventView {
) -> (Vec<TimelineEventDetailView>, Option<String>) { ) -> (Vec<TimelineEventDetailView>, Option<String>) {
let mut mapped = Vec::new(); let mut mapped = Vec::new();
let mut external_link = None; let mut external_link = None;
for detail in details { for detail in details {
if detail.label.eq_ignore_ascii_case("homepage") let label_lower = detail.label.to_ascii_lowercase();
|| detail.label.eq_ignore_ascii_case("website") match label_lower.as_str() {
{ // Website/homepage links are extracted, not shown as detail rows
let trimmed = detail.value.trim(); "homepage" | "website" => {
if !trimmed.is_empty() && trimmed != "" { let trimmed = detail.value.trim();
external_link = Some(trimmed.to_string()); if !is_blank(trimmed) {
external_link = Some(trimmed.to_string());
}
} }
} else if detail.label.eq_ignore_ascii_case("position") { // Position values become clickable map links
let trimmed = detail.value.trim(); "position" => {
if !trimmed.is_empty() { let trimmed = detail.value.trim();
let display = trimmed if !trimmed.is_empty() {
.strip_prefix("https://www.google.com/maps?q=") let display = trimmed
.unwrap_or(trimmed); .strip_prefix("https://www.google.com/maps?q=")
.unwrap_or(trimmed);
mapped.push(TimelineEventDetailView {
label: detail.label,
value: display.to_string(),
link: Some(trimmed.to_string()),
});
}
}
_ => {
mapped.push(TimelineEventDetailView { mapped.push(TimelineEventDetailView {
label: detail.label, label: detail.label,
value: display.to_string(), value: detail.value,
link: Some(trimmed.to_string()), link: None,
}); });
} }
} else {
mapped.push(TimelineEventDetailView {
label: detail.label,
value: detail.value,
link: None,
});
} }
} }
(mapped, external_link) (mapped, external_link)
} }
} }