From b55b9f0abe597a61f3bfa43916e9107cf3d35f41 Mon Sep 17 00:00:00 2001 From: Benjamin Slingo Date: Thu, 28 Aug 2025 21:41:01 -0400 Subject: [PATCH] Step 3-4: Rewrite db/bulletins.rs with pure SQL + fix BulletinService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- src/db/bulletins.rs | 297 ++++++++++++++++++++++++++++---------- src/services/bulletins.rs | 6 +- 2 files changed, 220 insertions(+), 83 deletions(-) diff --git a/src/db/bulletins.rs b/src/db/bulletins.rs index 6e69487..5c5b411 100644 --- a/src/db/bulletins.rs +++ b/src/db/bulletins.rs @@ -1,15 +1,14 @@ use sqlx::PgPool; use uuid::Uuid; +use chrono::NaiveDate; use crate::{ error::{ApiError, Result}, models::{Bulletin, CreateBulletinRequest}, - utils::{ - sanitize::strip_html_tags, - db_operations::{DbOperations, BulletinOperations}, - }, + utils::sanitize::strip_html_tags, }; +/// List bulletins with pagination pub async fn list( pool: &PgPool, page: i32, @@ -18,30 +17,133 @@ pub async fn list( ) -> Result<(Vec, i64)> { let offset = ((page - 1) as i64) * per_page; - // Use shared database operations - BulletinOperations::list_paginated(pool, offset, per_page, active_only).await + // Get bulletins with pagination + 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> { - // Use shared database operations - BulletinOperations::get_current(pool).await -} - -pub async fn get_by_id(pool: &PgPool, id: &Uuid) -> Result> { - // Use shared database operations - DbOperations::get_by_id(pool, "bulletins", id).await -} - -pub async fn get_by_date(pool: &PgPool, date: chrono::NaiveDate) -> Result> { - let bulletin = sqlx::query_as!( + sqlx::query_as!( Bulletin, - "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", + 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 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> { + 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> { + 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> { + 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 ) .fetch_optional(pool) @@ -49,72 +151,107 @@ pub async fn get_by_date(pool: &PgPool, date: chrono::NaiveDate) -> Result Result { - let bulletin = sqlx::query_as!( +/// Create new bulletin +pub async fn create(pool: &PgPool, bulletin: &CreateBulletinRequest) -> Result { + let id = Uuid::new_v4(); + let clean_title = strip_html_tags(&bulletin.title); + + sqlx::query_as!( Bulletin, - "INSERT INTO bulletins (title, date, url, cover_image, sabbath_school, divine_worship, scripture_reading, sunset, is_active) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) - RETURNING 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", - strip_html_tags(&req.title), - req.date, - req.url.as_ref().map(|s| strip_html_tags(s)), - req.cover_image.as_ref().map(|s| strip_html_tags(s)), - req.sabbath_school.as_ref().map(|s| strip_html_tags(s)), - req.divine_worship.as_ref().map(|s| strip_html_tags(s)), - req.scripture_reading.as_ref().map(|s| strip_html_tags(s)), - req.sunset.as_ref().map(|s| strip_html_tags(s)), - req.is_active.unwrap_or(true) + r#"INSERT INTO bulletins ( + 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 + ) VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, NOW(), NOW() + ) RETURNING *"#, + id, + clean_title, + bulletin.date, + bulletin.url, + bulletin.pdf_url, + 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) - .await?; - - Ok(bulletin) + .await + .map_err(|e| { + 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( - pool: &PgPool, - id: &Uuid, - req: CreateBulletinRequest, -) -> Result> { - let bulletin = sqlx::query_as!( +/// Update bulletin +pub async fn update(pool: &PgPool, id: &Uuid, bulletin: &CreateBulletinRequest) -> Result { + let clean_title = strip_html_tags(&bulletin.title); + + sqlx::query_as!( Bulletin, - "UPDATE bulletins - SET title = $1, date = $2, url = $3, cover_image = $4, sabbath_school = $5, divine_worship = $6, - scripture_reading = $7, sunset = $8, is_active = $9, updated_at = NOW() - WHERE id = $10 - RETURNING 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", - strip_html_tags(&req.title), - req.date, - req.url.as_ref().map(|s| strip_html_tags(s)), - req.cover_image.as_ref().map(|s| strip_html_tags(s)), - req.sabbath_school.as_ref().map(|s| strip_html_tags(s)), - req.divine_worship.as_ref().map(|s| strip_html_tags(s)), - req.scripture_reading.as_ref().map(|s| strip_html_tags(s)), - req.sunset.as_ref().map(|s| strip_html_tags(s)), - req.is_active.unwrap_or(true), + r#"UPDATE bulletins SET + title = $2, date = $3, url = $4, pdf_url = $5, is_active = $6, + pdf_file = $7, sabbath_school = $8, divine_worship = $9, + scripture_reading = $10, sunset = $11, cover_image = $12, + pdf_path = $13, updated_at = NOW() + WHERE id = $1 + RETURNING *"#, + id, + clean_title, + bulletin.date, + bulletin.url, + bulletin.pdf_url, + 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) + .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 ) - .fetch_optional(pool) - .await?; - - Ok(bulletin) -} - -pub async fn delete(pool: &PgPool, id: &Uuid) -> Result<()> { - let result = sqlx::query!("DELETE FROM bulletins WHERE id = $1", id) - .execute(pool) - .await?; - + .execute(pool) + .await + .map_err(|e| { + tracing::error!("Failed to delete bulletin {}: {}", id, e); + ApiError::DatabaseError(e) + })?; + if result.rows_affected() == 0 { - return Err(ApiError::NotFound("Bulletin not found".to_string())); + return Err(ApiError::bulletin_not_found(id)); } - + Ok(()) -} +} \ No newline at end of file diff --git a/src/services/bulletins.rs b/src/services/bulletins.rs index eac3a17..e58079a 100644 --- a/src/services/bulletins.rs +++ b/src/services/bulletins.rs @@ -38,7 +38,7 @@ impl BulletinService { /// Get current bulletin with V1 timezone conversion (EST) pub async fn get_current_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result> { - 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 { process_single_bulletin(pool, bulletin_data).await?; @@ -56,7 +56,7 @@ impl BulletinService { /// Get next bulletin with V1 timezone conversion (EST) pub async fn get_next_v1(pool: &PgPool, url_builder: &UrlBuilder) -> Result> { - 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 { process_single_bulletin(pool, bulletin_data).await?; @@ -145,7 +145,7 @@ impl BulletinService { /// Get next bulletin with V2 format (UTC timestamps) pub async fn get_next_v2(pool: &PgPool, url_builder: &UrlBuilder) -> Result> { - let bulletin = BulletinOperations::get_next(pool).await?; + let bulletin = db::bulletins::get_next(pool).await?; match bulletin { Some(b) => {