From 7fd96a7b418367edbe04be3a436ca4be65f5a552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sergio=20Casta=C3=B1o=20Arteaga?= Date: Sun, 3 May 2026 11:47:26 +0200 Subject: [PATCH] Handle missing group and event pages with 404s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sergio Castaño Arteaga --- ocg-server/src/db/event.rs | 6 +-- ocg-server/src/db/group.rs | 11 +++-- ocg-server/src/db/mock.rs | 4 +- ocg-server/src/handlers/error.rs | 5 +++ ocg-server/src/handlers/error/tests.rs | 8 ++++ ocg-server/src/handlers/event.rs | 6 ++- ocg-server/src/handlers/event/tests.rs | 44 ++++++++++++++++++- ocg-server/src/handlers/group.rs | 3 +- ocg-server/src/handlers/group/tests.rs | 60 +++++++++++++++++++++++++- 9 files changed, 132 insertions(+), 15 deletions(-) diff --git a/ocg-server/src/db/event.rs b/ocg-server/src/db/event.rs index f28887f84..a5a04640e 100644 --- a/ocg-server/src/db/event.rs +++ b/ocg-server/src/db/event.rs @@ -58,7 +58,7 @@ pub(crate) trait DBEvent { community_id: Uuid, group_slug: &str, event_slug: &str, - ) -> Result; + ) -> Result>; /// Retrieves summary event information by its identifier. async fn get_event_summary_by_id(&self, community_id: Uuid, event_id: Uuid) -> Result; @@ -175,8 +175,8 @@ impl DBEvent for PgDB { community_id: Uuid, group_slug: &str, event_slug: &str, - ) -> Result { - self.fetch_json_one( + ) -> Result> { + self.fetch_json_opt( "select get_event_full_by_slug($1::uuid, $2::text, $3::text)", &[&community_id, &group_slug, &event_slug], ) diff --git a/ocg-server/src/db/group.rs b/ocg-server/src/db/group.rs index ba994b7e3..a408274d8 100644 --- a/ocg-server/src/db/group.rs +++ b/ocg-server/src/db/group.rs @@ -17,7 +17,8 @@ use crate::{ #[async_trait] pub(crate) trait DBGroup { /// Retrieves group information. - async fn get_group_full_by_slug(&self, community_id: Uuid, group_slug: &str) -> Result; + async fn get_group_full_by_slug(&self, community_id: Uuid, group_slug: &str) + -> Result>; /// Retrieves past events for a specific group. async fn get_group_past_events( @@ -51,8 +52,12 @@ pub(crate) trait DBGroup { impl DBGroup for PgDB { /// [`DBGroup::get_group_full_by_slug`] #[instrument(skip(self), err)] - async fn get_group_full_by_slug(&self, community_id: Uuid, group_slug: &str) -> Result { - self.fetch_json_one( + async fn get_group_full_by_slug( + &self, + community_id: Uuid, + group_slug: &str, + ) -> Result> { + self.fetch_json_opt( "select get_group_full_by_slug($1::uuid, $2::text)", &[&community_id, &group_slug], ) diff --git a/ocg-server/src/db/mock.rs b/ocg-server/src/db/mock.rs index b7d051ac5..1e10f820f 100644 --- a/ocg-server/src/db/mock.rs +++ b/ocg-server/src/db/mock.rs @@ -671,7 +671,7 @@ mock! { community_id: Uuid, group_slug: &str, event_slug: &str, - ) -> Result; + ) -> Result>; async fn get_event_summary_by_id( &self, community_id: Uuid, @@ -714,7 +714,7 @@ mock! { &self, community_id: Uuid, group_slug: &str, - ) -> Result; + ) -> Result>; async fn get_group_past_events( &self, community_id: Uuid, diff --git a/ocg-server/src/handlers/error.rs b/ocg-server/src/handlers/error.rs index 70a387e7a..cf6ddd82c 100644 --- a/ocg-server/src/handlers/error.rs +++ b/ocg-server/src/handlers/error.rs @@ -31,6 +31,10 @@ pub(crate) enum HandlerError { #[error("forbidden")] Forbidden, + /// Resource was not found. + #[error("not found")] + NotFound, + /// Any other error, wrapped in `anyhow::Error` for flexibility. #[error(transparent)] Other(anyhow::Error), @@ -61,6 +65,7 @@ impl IntoResponse for HandlerError { (StatusCode::UNPROCESSABLE_ENTITY, msg).into_response() } HandlerError::Forbidden => StatusCode::FORBIDDEN.into_response(), + HandlerError::NotFound => StatusCode::NOT_FOUND.into_response(), HandlerError::Validation(report) => { (StatusCode::UNPROCESSABLE_ENTITY, report.to_string()).into_response() } diff --git a/ocg-server/src/handlers/error/tests.rs b/ocg-server/src/handlers/error/tests.rs index e9b7ee5de..dd7242dad 100644 --- a/ocg-server/src/handlers/error/tests.rs +++ b/ocg-server/src/handlers/error/tests.rs @@ -22,3 +22,11 @@ async fn test_non_db_anyhow_error_returns_500() { assert_eq!(parts.status, StatusCode::INTERNAL_SERVER_ERROR); } + +#[tokio::test] +async fn test_not_found_error_returns_404() { + let response = HandlerError::NotFound.into_response(); + let parts = response.into_parts().0; + + assert_eq!(parts.status, StatusCode::NOT_FOUND); +} diff --git a/ocg-server/src/handlers/event.rs b/ocg-server/src/handlers/event.rs index f14d5d3e3..e900cb538 100644 --- a/ocg-server/src/handlers/event.rs +++ b/ocg-server/src/handlers/event.rs @@ -60,10 +60,11 @@ pub(crate) async fn page( uri: Uri, ) -> Result { // Prepare template - let (mut event, site_settings) = tokio::try_join!( + let (event, site_settings) = tokio::try_join!( db.get_event_full_by_slug(community_id, &group_slug, &event_slug), db.get_site_settings() )?; + let mut event = event.ok_or(HandlerError::NotFound)?; trim_public_gallery_images(&mut event.photos_urls); let template = Page { event, @@ -163,7 +164,8 @@ pub(crate) async fn availability( // Get current public event availability let event = db .get_event_full_by_slug(community_id, &group_slug, &event_slug) - .await?; + .await? + .ok_or(HandlerError::NotFound)?; // Prevent volatile seat availability from being cached let mut headers = HeaderMap::new(); diff --git a/ocg-server/src/handlers/event/tests.rs b/ocg-server/src/handlers/event/tests.rs index f7dbacbcf..b6d3cbb58 100644 --- a/ocg-server/src/handlers/event/tests.rs +++ b/ocg-server/src/handlers/event/tests.rs @@ -68,7 +68,7 @@ async fn test_availability_success() { .withf(move |id, group_slug, event_slug| { *id == community_id && group_slug == "test-group" && event_slug == "test-event" }) - .returning(move |_, _, _| Ok(event.clone())); + .returning(move |_, _, _| Ok(Some(event.clone()))); // Setup router and send request let router = TestRouterBuilder::new(db, MockNotificationsManager::new()) @@ -124,7 +124,7 @@ async fn test_page_success() { .withf(move |id, group_slug, event_slug| { *id == community_id && group_slug == "test-group" && event_slug == "test-event" }) - .returning(move |_, _, _| Ok(sample_event_full(community_id, event_id, group_id))); + .returning(move |_, _, _| Ok(Some(sample_event_full(community_id, event_id, group_id)))); db.expect_get_site_settings() .times(1) .returning(|| Ok(sample_site_settings())); @@ -156,6 +156,46 @@ async fn test_page_success() { assert!(!bytes.is_empty()); } +#[tokio::test] +async fn test_page_not_found() { + // Setup identifiers and data structures + let community_id = Uuid::new_v4(); + + // Setup database mock + let mut db = MockDB::new(); + db.expect_get_community_id_by_name() + .times(1) + .withf(|name| name == "test-community") + .returning(move |_| Ok(Some(community_id))); + db.expect_get_event_full_by_slug() + .times(1) + .withf(move |id, group_slug, event_slug| { + *id == community_id && group_slug == "test-group" && event_slug == "missing-event" + }) + .returning(move |_, _, _| Ok(None)); + db.expect_get_site_settings() + .times(1) + .returning(|| Ok(sample_site_settings())); + + // Setup notifications manager mock + let nm = MockNotificationsManager::new(); + + // Setup router and send request + let router = TestRouterBuilder::new(db, nm).build().await; + let request = Request::builder() + .method("GET") + .uri("/test-community/group/test-group/event/missing-event") + .body(Body::empty()) + .unwrap(); + let response = router.oneshot(request).await.unwrap(); + let (parts, body) = response.into_parts(); + let bytes = to_bytes(body, usize::MAX).await.unwrap(); + + // Check response matches expectations + assert_eq!(parts.status, StatusCode::NOT_FOUND); + assert!(bytes.is_empty()); +} + #[tokio::test] async fn test_page_db_error() { // Setup identifiers and data structures diff --git a/ocg-server/src/handlers/group.rs b/ocg-server/src/handlers/group.rs index 321f33cf9..e057f884e 100644 --- a/ocg-server/src/handlers/group.rs +++ b/ocg-server/src/handlers/group.rs @@ -44,12 +44,13 @@ pub(crate) async fn page( ) -> Result { // Fetch the group page data let event_kinds = vec![EventKind::InPerson, EventKind::Virtual, EventKind::Hybrid]; - let (mut group, past_events, site_settings, upcoming_events) = tokio::try_join!( + let (group, past_events, site_settings, upcoming_events) = tokio::try_join!( db.get_group_full_by_slug(community_id, &group_slug), db.get_group_past_events(community_id, &group_slug, event_kinds.clone(), 9), db.get_site_settings(), db.get_group_upcoming_events(community_id, &group_slug, event_kinds, 9) )?; + let mut group = group.ok_or(HandlerError::NotFound)?; // Trim gallery media trim_public_gallery_images(&mut group.photos_urls); diff --git a/ocg-server/src/handlers/group/tests.rs b/ocg-server/src/handlers/group/tests.rs index c142b0d9f..ed70ac016 100644 --- a/ocg-server/src/handlers/group/tests.rs +++ b/ocg-server/src/handlers/group/tests.rs @@ -45,7 +45,7 @@ async fn test_page_success() { db.expect_get_group_full_by_slug() .times(1) .withf(move |id, slug| *id == community_id && slug == "test-group") - .returning(move |_, _| Ok(group.clone())); + .returning(move |_, _| Ok(Some(group.clone()))); db.expect_get_group_upcoming_events() .times(1) .withf(move |id, slug, kinds, limit| { @@ -95,6 +95,62 @@ async fn test_page_success() { assert!(!bytes.is_empty()); } +#[tokio::test] +async fn test_page_not_found() { + // Setup identifiers and data structures + let community_id = Uuid::new_v4(); + + // Setup database mock + let mut db = MockDB::new(); + db.expect_get_community_id_by_name() + .times(1) + .withf(|name| name == "test-community") + .returning(move |_| Ok(Some(community_id))); + db.expect_get_group_full_by_slug() + .times(1) + .withf(move |id, slug| *id == community_id && slug == "missing-group") + .returning(move |_, _| Ok(None)); + db.expect_get_group_upcoming_events() + .times(1) + .withf(move |id, slug, kinds, limit| { + *id == community_id + && slug == "missing-group" + && kinds == &vec![EventKind::InPerson, EventKind::Virtual, EventKind::Hybrid] + && *limit == 9 + }) + .returning(move |_, _, _, _| Ok(vec![])); + db.expect_get_group_past_events() + .times(1) + .withf(move |id, slug, kinds, limit| { + *id == community_id + && slug == "missing-group" + && kinds == &vec![EventKind::InPerson, EventKind::Virtual, EventKind::Hybrid] + && *limit == 9 + }) + .returning(move |_, _, _, _| Ok(vec![])); + db.expect_get_site_settings() + .times(1) + .returning(|| Ok(sample_site_settings())); + + // Setup notifications manager mock + let nm = MockNotificationsManager::new(); + + // Setup router and send request + let router = TestRouterBuilder::new(db, nm).build().await; + let request = Request::builder() + .method("GET") + .uri("/test-community/group/missing-group") + .body(Body::empty()) + .unwrap(); + let response = router.oneshot(request).await.unwrap(); + let (parts, body) = response.into_parts(); + let bytes = to_bytes(body, usize::MAX).await.unwrap(); + + // Check response matches expectations + assert_eq!(parts.status, StatusCode::NOT_FOUND); + assert!(bytes.is_empty()); +} + #[tokio::test] async fn test_page_db_error() { // Setup identifiers and data structures @@ -110,7 +166,7 @@ async fn test_page_db_error() { db.expect_get_group_full_by_slug() .times(1) .withf(move |id, slug| *id == community_id && slug == "test-group") - .returning(move |_, _| Ok(sample_group_full(community_id, group_id))); + .returning(move |_, _| Ok(Some(sample_group_full(community_id, group_id)))); db.expect_get_group_past_events() .times(1) .withf(move |id, slug, kinds, limit| {