Step 3-4: Rewrite db/bulletins.rs with pure SQL + fix BulletinService

- Complete rewrite of db/bulletins.rs using direct SQL queries only
- Remove all dependencies on dead *Operations patterns
- Enhanced error handling with specific error types (bulletin_not_found, duplicate_entry)
- Fix BulletinService to call db::bulletins:: directly instead of dead BulletinOperations
- Clean SQL with proper logging and error context
- Part of nuclear consolidation to Handler → Service → DB pattern
This commit is contained in:
Benjamin Slingo 2025-08-28 21:41:01 -04:00
parent da06dae89d
commit b55b9f0abe
2 changed files with 220 additions and 83 deletions

View file

@ -1,15 +1,14 @@
use sqlx::PgPool; use sqlx::PgPool;
use uuid::Uuid; use uuid::Uuid;
use chrono::NaiveDate;
use crate::{ use crate::{
error::{ApiError, Result}, error::{ApiError, Result},
models::{Bulletin, CreateBulletinRequest}, models::{Bulletin, CreateBulletinRequest},
utils::{ utils::sanitize::strip_html_tags,
sanitize::strip_html_tags,
db_operations::{DbOperations, BulletinOperations},
},
}; };
/// List bulletins with pagination
pub async fn list( pub async fn list(
pool: &PgPool, pool: &PgPool,
page: i32, page: i32,
@ -18,30 +17,133 @@ pub async fn list(
) -> Result<(Vec<Bulletin>, i64)> { ) -> Result<(Vec<Bulletin>, i64)> {
let offset = ((page - 1) as i64) * per_page; let offset = ((page - 1) as i64) * per_page;
// Use shared database operations // Get bulletins with pagination
BulletinOperations::list_paginated(pool, offset, per_page, active_only).await let bulletins = if active_only {
sqlx::query_as!(
Bulletin,
r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at
FROM bulletins
WHERE is_active = true
ORDER BY date DESC
LIMIT $1 OFFSET $2"#,
per_page,
offset
)
} else {
sqlx::query_as!(
Bulletin,
r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at
FROM bulletins
ORDER BY date DESC
LIMIT $1 OFFSET $2"#,
per_page,
offset
)
}
.fetch_all(pool)
.await
.map_err(|e| {
tracing::error!("Failed to list bulletins: {}", e);
ApiError::DatabaseError(e)
})?;
// Get total count
let total = if active_only {
sqlx::query_scalar!(
"SELECT COUNT(*) FROM bulletins WHERE is_active = true"
)
} else {
sqlx::query_scalar!(
"SELECT COUNT(*) FROM bulletins"
)
}
.fetch_one(pool)
.await
.map_err(|e| {
tracing::error!("Failed to count bulletins: {}", e);
ApiError::DatabaseError(e)
})?
.unwrap_or(0);
Ok((bulletins, total))
} }
/// Get current bulletin (active and date <= today)
pub async fn get_current(pool: &PgPool) -> Result<Option<Bulletin>> { pub async fn get_current(pool: &PgPool) -> Result<Option<Bulletin>> {
// Use shared database operations sqlx::query_as!(
BulletinOperations::get_current(pool).await
}
pub async fn get_by_id(pool: &PgPool, id: &Uuid) -> Result<Option<Bulletin>> {
// Use shared database operations
DbOperations::get_by_id(pool, "bulletins", id).await
}
pub async fn get_by_date(pool: &PgPool, date: chrono::NaiveDate) -> Result<Option<Bulletin>> {
let bulletin = sqlx::query_as!(
Bulletin, Bulletin,
"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school, r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path, divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at created_at, updated_at
FROM bulletins FROM bulletins
WHERE date = $1 AND is_active = true WHERE is_active = true
ORDER BY created_at DESC AND date <= (NOW() AT TIME ZONE 'America/New_York')::date
LIMIT 1", ORDER BY date DESC
LIMIT 1"#
)
.fetch_optional(pool)
.await
.map_err(|e| {
tracing::error!("Failed to get current bulletin: {}", e);
ApiError::DatabaseError(e)
})
}
/// Get next bulletin (active and date > today)
pub async fn get_next(pool: &PgPool) -> Result<Option<Bulletin>> {
sqlx::query_as!(
Bulletin,
r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at
FROM bulletins
WHERE is_active = true
AND date > (NOW() AT TIME ZONE 'America/New_York')::date
ORDER BY date ASC
LIMIT 1"#
)
.fetch_optional(pool)
.await
.map_err(|e| {
tracing::error!("Failed to get next bulletin: {}", e);
ApiError::DatabaseError(e)
})
}
/// Get bulletin by ID
pub async fn get_by_id(pool: &PgPool, id: &Uuid) -> Result<Option<Bulletin>> {
sqlx::query_as!(
Bulletin,
r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at
FROM bulletins
WHERE id = $1"#,
id
)
.fetch_optional(pool)
.await
.map_err(|e| {
tracing::error!("Failed to get bulletin by id {}: {}", id, e);
ApiError::DatabaseError(e)
})
}
/// Get bulletin by date
pub async fn get_by_date(pool: &PgPool, date: NaiveDate) -> Result<Option<Bulletin>> {
sqlx::query_as!(
Bulletin,
r#"SELECT id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school,
divine_worship, scripture_reading, sunset, cover_image, pdf_path,
created_at, updated_at
FROM bulletins
WHERE date = $1 AND is_active = true
ORDER BY created_at DESC
LIMIT 1"#,
date date
) )
.fetch_optional(pool) .fetch_optional(pool)
@ -49,72 +151,107 @@ pub async fn get_by_date(pool: &PgPool, date: chrono::NaiveDate) -> Result<Optio
.map_err(|e| { .map_err(|e| {
tracing::error!("Failed to get bulletin by date {}: {}", date, e); tracing::error!("Failed to get bulletin by date {}: {}", date, e);
ApiError::DatabaseError(e) ApiError::DatabaseError(e)
})?; })
Ok(bulletin)
} }
pub async fn create(pool: &PgPool, req: CreateBulletinRequest) -> Result<Bulletin> { /// Create new bulletin
let bulletin = sqlx::query_as!( pub async fn create(pool: &PgPool, bulletin: &CreateBulletinRequest) -> Result<Bulletin> {
let id = Uuid::new_v4();
let clean_title = strip_html_tags(&bulletin.title);
sqlx::query_as!(
Bulletin, Bulletin,
"INSERT INTO bulletins (title, date, url, cover_image, sabbath_school, divine_worship, scripture_reading, sunset, is_active) r#"INSERT INTO bulletins (
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) id, title, date, url, pdf_url, is_active, pdf_file,
RETURNING id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school, divine_worship, sabbath_school, divine_worship, scripture_reading,
scripture_reading, sunset, cover_image, pdf_path, created_at, updated_at", sunset, cover_image, pdf_path, created_at, updated_at
strip_html_tags(&req.title), ) VALUES (
req.date, $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, NOW(), NOW()
req.url.as_ref().map(|s| strip_html_tags(s)), ) RETURNING *"#,
req.cover_image.as_ref().map(|s| strip_html_tags(s)), id,
req.sabbath_school.as_ref().map(|s| strip_html_tags(s)), clean_title,
req.divine_worship.as_ref().map(|s| strip_html_tags(s)), bulletin.date,
req.scripture_reading.as_ref().map(|s| strip_html_tags(s)), bulletin.url,
req.sunset.as_ref().map(|s| strip_html_tags(s)), bulletin.pdf_url,
req.is_active.unwrap_or(true) bulletin.is_active.unwrap_or(true),
bulletin.pdf_file,
bulletin.sabbath_school,
bulletin.divine_worship,
bulletin.scripture_reading,
bulletin.sunset,
bulletin.cover_image,
bulletin.pdf_path
) )
.fetch_one(pool) .fetch_one(pool)
.await?; .await
.map_err(|e| {
Ok(bulletin) tracing::error!("Failed to create bulletin: {}", e);
match e {
sqlx::Error::Database(db_err) if db_err.constraint().is_some() => {
ApiError::duplicate_entry("Bulletin", &bulletin.date)
}
_ => ApiError::DatabaseError(e)
}
})
} }
pub async fn update( /// Update bulletin
pool: &PgPool, pub async fn update(pool: &PgPool, id: &Uuid, bulletin: &CreateBulletinRequest) -> Result<Bulletin> {
id: &Uuid, let clean_title = strip_html_tags(&bulletin.title);
req: CreateBulletinRequest,
) -> Result<Option<Bulletin>> { sqlx::query_as!(
let bulletin = sqlx::query_as!(
Bulletin, Bulletin,
"UPDATE bulletins r#"UPDATE bulletins SET
SET title = $1, date = $2, url = $3, cover_image = $4, sabbath_school = $5, divine_worship = $6, title = $2, date = $3, url = $4, pdf_url = $5, is_active = $6,
scripture_reading = $7, sunset = $8, is_active = $9, updated_at = NOW() pdf_file = $7, sabbath_school = $8, divine_worship = $9,
WHERE id = $10 scripture_reading = $10, sunset = $11, cover_image = $12,
RETURNING id, title, date, url, pdf_url, is_active, pdf_file, sabbath_school, divine_worship, pdf_path = $13, updated_at = NOW()
scripture_reading, sunset, cover_image, pdf_path, created_at, updated_at", WHERE id = $1
strip_html_tags(&req.title), RETURNING *"#,
req.date, id,
req.url.as_ref().map(|s| strip_html_tags(s)), clean_title,
req.cover_image.as_ref().map(|s| strip_html_tags(s)), bulletin.date,
req.sabbath_school.as_ref().map(|s| strip_html_tags(s)), bulletin.url,
req.divine_worship.as_ref().map(|s| strip_html_tags(s)), bulletin.pdf_url,
req.scripture_reading.as_ref().map(|s| strip_html_tags(s)), bulletin.is_active.unwrap_or(true),
req.sunset.as_ref().map(|s| strip_html_tags(s)), bulletin.pdf_file,
req.is_active.unwrap_or(true), bulletin.sabbath_school,
bulletin.divine_worship,
bulletin.scripture_reading,
bulletin.sunset,
bulletin.cover_image,
bulletin.pdf_path
)
.fetch_one(pool)
.await
.map_err(|e| {
tracing::error!("Failed to update bulletin {}: {}", id, e);
match e {
sqlx::Error::RowNotFound => ApiError::bulletin_not_found(id),
sqlx::Error::Database(db_err) if db_err.constraint().is_some() => {
ApiError::duplicate_entry("Bulletin", &bulletin.date)
}
_ => ApiError::DatabaseError(e)
}
})
}
/// Delete bulletin
pub async fn delete(pool: &PgPool, id: &Uuid) -> Result<()> {
let result = sqlx::query!(
"DELETE FROM bulletins WHERE id = $1",
id id
) )
.fetch_optional(pool) .execute(pool)
.await?; .await
.map_err(|e| {
Ok(bulletin) tracing::error!("Failed to delete bulletin {}: {}", id, e);
} ApiError::DatabaseError(e)
})?;
pub async fn delete(pool: &PgPool, id: &Uuid) -> Result<()> {
let result = sqlx::query!("DELETE FROM bulletins WHERE id = $1", id)
.execute(pool)
.await?;
if result.rows_affected() == 0 { if result.rows_affected() == 0 {
return Err(ApiError::NotFound("Bulletin not found".to_string())); return Err(ApiError::bulletin_not_found(id));
} }
Ok(()) Ok(())
} }

View file

@ -38,7 +38,7 @@ impl BulletinService {
/// Get current bulletin with V1 timezone conversion (EST) /// Get current bulletin with V1 timezone conversion (EST)
pub async fn get_current_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<Bulletin>> { pub async fn get_current_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<Bulletin>> {
let mut bulletin = BulletinOperations::get_current(pool).await?; let mut bulletin = db::bulletins::get_current(pool).await?;
if let Some(ref mut bulletin_data) = bulletin { if let Some(ref mut bulletin_data) = bulletin {
process_single_bulletin(pool, bulletin_data).await?; process_single_bulletin(pool, bulletin_data).await?;
@ -56,7 +56,7 @@ impl BulletinService {
/// Get next bulletin with V1 timezone conversion (EST) /// Get next bulletin with V1 timezone conversion (EST)
pub async fn get_next_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<Bulletin>> { pub async fn get_next_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<Bulletin>> {
let mut bulletin = BulletinOperations::get_next(pool).await?; let mut bulletin = db::bulletins::get_next(pool).await?;
if let Some(ref mut bulletin_data) = bulletin { if let Some(ref mut bulletin_data) = bulletin {
process_single_bulletin(pool, bulletin_data).await?; process_single_bulletin(pool, bulletin_data).await?;
@ -145,7 +145,7 @@ impl BulletinService {
/// Get next bulletin with V2 format (UTC timestamps) /// Get next bulletin with V2 format (UTC timestamps)
pub async fn get_next_v2(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<BulletinV2>> { pub async fn get_next_v2(pool: &PgPool, url_builder: &UrlBuilder) -> Result<Option<BulletinV2>> {
let bulletin = BulletinOperations::get_next(pool).await?; let bulletin = db::bulletins::get_next(pool).await?;
match bulletin { match bulletin {
Some(b) => { Some(b) => {