From dfa27006d5286c1d72975957d130aab8589c3c3a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Oct 2025 15:52:34 +0000 Subject: [PATCH 01/17] Add user-scoped endpoints for organizations, actions, coaching sessions, and overarching goals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. GET /users/{user_id}/organizations - Returns all organizations a user belongs to (via user_roles table) - Replaces organizations_users with user_roles for future compatibility 2. GET /users/{user_id}/actions - Query params: coaching_session_id (optional), status (optional), sorting - Returns actions for the user with optional filtering by session and status 3. GET /users/{user_id}/coaching_sessions - Query params: from_date (optional), to_date (optional), sorting - Returns coaching sessions where user is coach OR coachee - Supports date range filtering 4. GET /users/{user_id}/overarching_goals - Query params: coaching_session_id (optional), sorting - Returns overarching goals for the user - Updated `organization::find_by_user` to use `user_roles` instead of `organizations_users` - Added `coaching_session::find_by_user` to query sessions via coaching_relationships - Added tests for both new functions - Created params/user/ module with IndexParams for each resource type - Created controller/user/ with controllers for each endpoint - Added protection middleware in protect/users/ for authorization - All endpoints enforce that authenticated_user.id matches user_id parameter - Added 4 new route functions with proper middleware layering - Registered all endpoints in OpenAPI spec - Maintained existing API stability by creating new endpoints - Follows existing multi-layered architecture pattern - Implements proper error handling and logging - Uses WithSortDefaults trait for consistent sorting behavior - Maintains backward compatibility for existing user params 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- entity_api/src/coaching_session.rs | 37 ++++++- entity_api/src/organization.rs | 9 +- web/src/controller/user/action_controller.rs | 79 ++++++++++++++ .../user/coaching_session_controller.rs | 93 ++++++++++++++++ web/src/controller/user/mod.rs | 4 + .../user/organization_controller.rs | 47 ++++++++ .../user/overarching_goal_controller.rs | 76 +++++++++++++ web/src/params/user/action.rs | 103 ++++++++++++++++++ web/src/params/user/coaching_session.rs | 43 ++++++++ web/src/params/{user.rs => user/mod.rs} | 7 ++ web/src/params/user/overarching_goal.rs | 92 ++++++++++++++++ web/src/protect/users/mod.rs | 84 ++++++++++++++ web/src/router.rs | 76 +++++++++++++ 13 files changed, 746 insertions(+), 4 deletions(-) create mode 100644 web/src/controller/user/action_controller.rs create mode 100644 web/src/controller/user/coaching_session_controller.rs create mode 100644 web/src/controller/user/organization_controller.rs create mode 100644 web/src/controller/user/overarching_goal_controller.rs create mode 100644 web/src/params/user/action.rs create mode 100644 web/src/params/user/coaching_session.rs rename web/src/params/{user.rs => user/mod.rs} (94%) create mode 100644 web/src/params/user/overarching_goal.rs diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 775e5644..0d962e49 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -1,11 +1,11 @@ use super::error::{EntityApiErrorKind, Error}; use entity::{ coaching_relationships, - coaching_sessions::{ActiveModel, Entity, Model}, + coaching_sessions::{ActiveModel, Entity, Model, Relation}, Id, }; use log::debug; -use sea_orm::{entity::prelude::*, DatabaseConnection, Set, TryIntoModel}; +use sea_orm::{entity::prelude::*, DatabaseConnection, JoinType, QuerySelect, Set, TryIntoModel}; pub async fn create( db: &DatabaseConnection, @@ -61,6 +61,20 @@ pub async fn delete(db: &impl ConnectionTrait, coaching_session_id: Id) -> Resul Ok(()) } +pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, Error> { + let sessions = Entity::find() + .join(JoinType::InnerJoin, Relation::CoachingRelationships.def()) + .filter( + coaching_relationships::Column::CoachId + .eq(user_id) + .or(coaching_relationships::Column::CoacheeId.eq(user_id)), + ) + .all(db) + .await?; + + Ok(sessions) +} + #[cfg(test)] // We need to gate seaORM's mock feature behind conditional compilation because // the feature removes the Clone trait implementation from seaORM's DatabaseConnection. @@ -157,4 +171,23 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn find_by_user_returns_sessions_where_user_is_coach_or_coachee() -> Result<(), Error> { + let db = MockDatabase::new(DatabaseBackend::Postgres).into_connection(); + + let user_id = Id::new_v4(); + let _ = find_by_user(&db, user_id).await; + + assert_eq!( + db.into_transaction_log(), + [Transaction::from_sql_and_values( + DatabaseBackend::Postgres, + r#"SELECT "coaching_sessions"."id", "coaching_sessions"."coaching_relationship_id", "coaching_sessions"."collab_document_name", "coaching_sessions"."date", "coaching_sessions"."created_at", "coaching_sessions"."updated_at" FROM "refactor_platform"."coaching_sessions" INNER JOIN "refactor_platform"."coaching_relationships" ON "coaching_sessions"."coaching_relationship_id" = "coaching_relationships"."id" WHERE "coaching_relationships"."coach_id" = $1 OR "coaching_relationships"."coachee_id" = $2"#, + [user_id.into(), user_id.into()] + )] + ); + + Ok(()) + } } diff --git a/entity_api/src/organization.rs b/entity_api/src/organization.rs index bc3bffa6..7f6d7d63 100644 --- a/entity_api/src/organization.rs +++ b/entity_api/src/organization.rs @@ -110,8 +110,13 @@ pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, user_id: Id) -> Select { query - .join(JoinType::InnerJoin, Relation::UserRoles.def()) + .join_as( + JoinType::InnerJoin, + user_roles::Relation::Organizations.def().rev(), + user_roles::Entity, + ) .filter(user_roles::Column::UserId.eq(user_id)) + .filter(user_roles::Column::OrganizationId.is_not_null()) .distinct() } @@ -177,7 +182,7 @@ mod tests { ), Transaction::from_sql_and_values( DatabaseBackend::Postgres, - r#"SELECT DISTINCT "organizations"."id", "organizations"."name", "organizations"."logo", "organizations"."slug", "organizations"."created_at", "organizations"."updated_at" FROM "refactor_platform"."organizations" INNER JOIN "refactor_platform"."user_roles" ON "organizations"."id" = "user_roles"."organization_id" WHERE "user_roles"."user_id" = $1"#, + r#"SELECT DISTINCT "organizations"."id", "organizations"."name", "organizations"."logo", "organizations"."slug", "organizations"."created_at", "organizations"."updated_at" FROM "refactor_platform"."organizations" INNER JOIN "refactor_platform"."user_roles" AS "user_roles" ON "organizations"."id" = "user_roles"."organization_id" WHERE "user_roles"."user_id" = $1 AND "user_roles"."organization_id" IS NOT NULL"#, [user_id.into()] ) ] diff --git a/web/src/controller/user/action_controller.rs b/web/src/controller/user/action_controller.rs new file mode 100644 index 00000000..0b50a0d3 --- /dev/null +++ b/web/src/controller/user/action_controller.rs @@ -0,0 +1,79 @@ +use crate::controller::ApiResponse; +use crate::extractors::{ + authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, +}; +use crate::params::user::action::{IndexParams, SortField}; +use crate::params::WithSortDefaults; +use crate::{AppState, Error}; +use axum::extract::{Path, Query, State}; +use axum::http::StatusCode; +use axum::response::IntoResponse; +use axum::Json; +use domain::{action as ActionApi, status::Status, Id}; +use serde::Deserialize; +use service::config::ApiVersion; + +use log::*; + +#[derive(Debug, Deserialize)] +pub(crate) struct QueryParams { + pub(crate) coaching_session_id: Option, + pub(crate) status: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +/// GET all actions for a specific user +#[utoipa::path( + get, + path = "/users/{user_id}/actions", + params( + ApiVersion, + ("user_id" = Id, Path, description = "User ID to retrieve actions for"), + ("coaching_session_id" = Option, Query, description = "Filter by coaching_session_id"), + ("status" = Option, Query, description = "Filter by action status"), + ("sort_by" = Option, Query, description = "Sort by field. Valid values: 'due_by', 'created_at', 'updated_at'. Must be provided with sort_order.", example = "created_at"), + ("sort_order" = Option, Query, description = "Sort order. Valid values: 'asc' (ascending), 'desc' (descending). Must be provided with sort_by.", example = "desc") + ), + responses( + (status = 200, description = "Successfully retrieved actions for user", body = [domain::actions::Model]), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + (status = 404, description = "User not found"), + (status = 405, description = "Method not allowed") + ), + security( + ("cookie_auth" = []) + ) +)] +pub async fn index( + CompareApiVersion(_v): CompareApiVersion, + AuthenticatedUser(_user): AuthenticatedUser, + State(app_state): State, + Path(user_id): Path, + Query(query_params): Query, +) -> Result { + debug!("GET Actions for User: {user_id}"); + debug!("Filter Params: {query_params:?}"); + + // Build params with user_id from path + let mut params = IndexParams::new(user_id).with_filters( + query_params.coaching_session_id, + query_params.status, + query_params.sort_by, + query_params.sort_order, + ); + + // Apply default sorting parameters + IndexParams::apply_sort_defaults( + &mut params.sort_by, + &mut params.sort_order, + SortField::CreatedAt, + ); + + let actions = ActionApi::find_by(app_state.db_conn_ref(), params).await?; + + debug!("Found {} actions for user {user_id}", actions.len()); + + Ok(Json(ApiResponse::new(StatusCode::OK.into(), actions))) +} diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs new file mode 100644 index 00000000..8886718d --- /dev/null +++ b/web/src/controller/user/coaching_session_controller.rs @@ -0,0 +1,93 @@ +use crate::controller::ApiResponse; +use crate::extractors::{ + authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, +}; +use crate::params::coaching_session::SortField; +use crate::params::sort::SortOrder; +use crate::params::user::coaching_session::IndexParams; +use crate::{AppState, Error}; +use axum::extract::{Path, Query, State}; +use axum::http::StatusCode; +use axum::response::IntoResponse; +use axum::Json; +use chrono::NaiveDate; +use domain::{coaching_sessions::Model, Id}; +use entity_api::coaching_session as CoachingSessionApi; +use serde::Deserialize; +use service::config::ApiVersion; + +use log::*; + +#[derive(Debug, Deserialize)] +pub(crate) struct QueryParams { + pub(crate) from_date: Option, + pub(crate) to_date: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +/// GET all coaching sessions for a specific user +#[utoipa::path( + get, + path = "/users/{user_id}/coaching_sessions", + params( + ApiVersion, + ("user_id" = Id, Path, description = "User ID to retrieve coaching sessions for"), + ("from_date" = Option, Query, description = "Filter by from_date"), + ("to_date" = Option, Query, description = "Filter by to_date"), + ("sort_by" = Option, Query, description = "Sort by field. Valid values: 'date', 'created_at', 'updated_at'. Must be provided with sort_order.", example = "date"), + ("sort_order" = Option, Query, description = "Sort order. Valid values: 'asc' (ascending), 'desc' (descending). Must be provided with sort_by.", example = "desc") + ), + responses( + (status = 200, description = "Successfully retrieved coaching sessions for user", body = [domain::coaching_sessions::Model]), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + (status = 404, description = "User not found"), + (status = 405, description = "Method not allowed") + ), + security( + ("cookie_auth" = []) + ) +)] +pub async fn index( + CompareApiVersion(_v): CompareApiVersion, + AuthenticatedUser(_user): AuthenticatedUser, + State(app_state): State, + Path(user_id): Path, + Query(query_params): Query, +) -> Result { + debug!("GET Coaching Sessions for User: {user_id}"); + debug!("Filter Params: {query_params:?}"); + + // Fetch all coaching sessions for the user (where they are coach or coachee) + let mut sessions = CoachingSessionApi::find_by_user(app_state.db_conn_ref(), user_id).await?; + + // Apply date range filters + if let Some(from_date) = query_params.from_date { + sessions.retain(|session| session.date.date() >= from_date); + } + if let Some(to_date) = query_params.to_date { + sessions.retain(|session| session.date.date() <= to_date); + } + + // Apply sorting + if let (Some(sort_by), Some(sort_order)) = (query_params.sort_by, query_params.sort_order) { + let ascending = matches!(sort_order, SortOrder::Asc); + sessions.sort_by(|a, b| { + let cmp = match sort_by { + SortField::Date => a.date.cmp(&b.date), + SortField::CreatedAt => a.created_at.cmp(&b.created_at), + SortField::UpdatedAt => a.updated_at.cmp(&b.updated_at), + }; + if ascending { + cmp + } else { + cmp.reverse() + } + }); + } + + debug!("Found {} coaching sessions for user {user_id}", sessions.len()); + + Ok(Json(ApiResponse::new(StatusCode::OK.into(), sessions))) +} diff --git a/web/src/controller/user/mod.rs b/web/src/controller/user/mod.rs index 770c26a2..55fdf17e 100644 --- a/web/src/controller/user/mod.rs +++ b/web/src/controller/user/mod.rs @@ -1 +1,5 @@ +pub(crate) mod action_controller; +pub(crate) mod coaching_session_controller; +pub(crate) mod organization_controller; +pub(crate) mod overarching_goal_controller; pub(crate) mod password_controller; diff --git a/web/src/controller/user/organization_controller.rs b/web/src/controller/user/organization_controller.rs new file mode 100644 index 00000000..6127e503 --- /dev/null +++ b/web/src/controller/user/organization_controller.rs @@ -0,0 +1,47 @@ +use crate::controller::ApiResponse; +use crate::extractors::{ + authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, +}; +use crate::{AppState, Error}; +use axum::extract::{Path, State}; +use axum::http::StatusCode; +use axum::response::IntoResponse; +use axum::Json; +use domain::{organization as OrganizationApi, Id}; +use service::config::ApiVersion; + +use log::*; + +/// GET all organizations for a specific user +#[utoipa::path( + get, + path = "/users/{user_id}/organizations", + params( + ApiVersion, + ("user_id" = Id, Path, description = "User ID to retrieve organizations for") + ), + responses( + (status = 200, description = "Successfully retrieved organizations for user", body = [domain::organizations::Model]), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + (status = 404, description = "User not found"), + (status = 405, description = "Method not allowed") + ), + security( + ("cookie_auth" = []) + ) +)] +pub async fn index( + CompareApiVersion(_v): CompareApiVersion, + AuthenticatedUser(_user): AuthenticatedUser, + State(app_state): State, + Path(user_id): Path, +) -> Result { + debug!("GET Organizations for User: {user_id}"); + + let organizations = OrganizationApi::find_by_user(app_state.db_conn_ref(), user_id).await?; + + debug!("Found {} organizations for user {user_id}", organizations.len()); + + Ok(Json(ApiResponse::new(StatusCode::OK.into(), organizations))) +} diff --git a/web/src/controller/user/overarching_goal_controller.rs b/web/src/controller/user/overarching_goal_controller.rs new file mode 100644 index 00000000..07a19a02 --- /dev/null +++ b/web/src/controller/user/overarching_goal_controller.rs @@ -0,0 +1,76 @@ +use crate::controller::ApiResponse; +use crate::extractors::{ + authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, +}; +use crate::params::user::overarching_goal::{IndexParams, SortField}; +use crate::params::WithSortDefaults; +use crate::{AppState, Error}; +use axum::extract::{Path, Query, State}; +use axum::http::StatusCode; +use axum::response::IntoResponse; +use axum::Json; +use domain::{overarching_goal as OverarchingGoalApi, Id}; +use serde::Deserialize; +use service::config::ApiVersion; + +use log::*; + +#[derive(Debug, Deserialize)] +pub(crate) struct QueryParams { + pub(crate) coaching_session_id: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +/// GET all overarching goals for a specific user +#[utoipa::path( + get, + path = "/users/{user_id}/overarching_goals", + params( + ApiVersion, + ("user_id" = Id, Path, description = "User ID to retrieve overarching goals for"), + ("coaching_session_id" = Option, Query, description = "Filter by coaching_session_id"), + ("sort_by" = Option, Query, description = "Sort by field. Valid values: 'title', 'created_at', 'updated_at'. Must be provided with sort_order.", example = "title"), + ("sort_order" = Option, Query, description = "Sort order. Valid values: 'asc' (ascending), 'desc' (descending). Must be provided with sort_by.", example = "desc") + ), + responses( + (status = 200, description = "Successfully retrieved overarching goals for user", body = [domain::overarching_goals::Model]), + (status = 401, description = "Unauthorized"), + (status = 403, description = "Forbidden"), + (status = 404, description = "User not found"), + (status = 405, description = "Method not allowed") + ), + security( + ("cookie_auth" = []) + ) +)] +pub async fn index( + CompareApiVersion(_v): CompareApiVersion, + AuthenticatedUser(_user): AuthenticatedUser, + State(app_state): State, + Path(user_id): Path, + Query(query_params): Query, +) -> Result { + debug!("GET Overarching Goals for User: {user_id}"); + debug!("Filter Params: {query_params:?}"); + + // Build params with user_id from path + let mut params = IndexParams::new(user_id).with_filters( + query_params.coaching_session_id, + query_params.sort_by, + query_params.sort_order, + ); + + // Apply default sorting parameters + IndexParams::apply_sort_defaults( + &mut params.sort_by, + &mut params.sort_order, + SortField::Title, + ); + + let overarching_goals = OverarchingGoalApi::find_by(app_state.db_conn_ref(), params).await?; + + debug!("Found {} overarching goals for user {user_id}", overarching_goals.len()); + + Ok(Json(ApiResponse::new(StatusCode::OK.into(), overarching_goals))) +} diff --git a/web/src/params/user/action.rs b/web/src/params/user/action.rs new file mode 100644 index 00000000..e907e1ac --- /dev/null +++ b/web/src/params/user/action.rs @@ -0,0 +1,103 @@ +use sea_orm::{Order, Value}; +use serde::Deserialize; +use utoipa::{IntoParams, ToSchema}; + +use crate::params::sort::SortOrder; +use crate::params::WithSortDefaults; +use domain::{actions, status::Status, Id, IntoQueryFilterMap, QueryFilterMap, QuerySort}; + +/// Sortable fields for user actions +#[derive(Debug, Deserialize, ToSchema)] +#[schema(example = "created_at")] +pub(crate) enum SortField { + #[serde(rename = "due_by")] + DueBy, + #[serde(rename = "created_at")] + CreatedAt, + #[serde(rename = "updated_at")] + UpdatedAt, +} + +#[derive(Debug, Deserialize, IntoParams)] +pub(crate) struct IndexParams { + #[serde(skip)] + pub(crate) user_id: Id, + pub(crate) coaching_session_id: Option, + pub(crate) status: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +impl IndexParams { + pub fn new(user_id: Id) -> Self { + Self { + user_id, + coaching_session_id: None, + status: None, + sort_by: None, + sort_order: None, + } + } + + pub fn with_filters( + mut self, + coaching_session_id: Option, + status: Option, + sort_by: Option, + sort_order: Option, + ) -> Self { + self.coaching_session_id = coaching_session_id; + self.status = status; + self.sort_by = sort_by; + self.sort_order = sort_order; + self + } +} + +impl IntoQueryFilterMap for IndexParams { + fn into_query_filter_map(self) -> QueryFilterMap { + let mut query_filter_map = QueryFilterMap::new(); + + query_filter_map.insert( + "user_id".to_string(), + Some(Value::Uuid(Some(Box::new(self.user_id)))), + ); + + if let Some(coaching_session_id) = self.coaching_session_id { + query_filter_map.insert( + "coaching_session_id".to_string(), + Some(Value::Uuid(Some(Box::new(coaching_session_id)))), + ); + } + + if let Some(status) = self.status { + query_filter_map.insert( + "status".to_string(), + Some(Value::String(Some(Box::new(status.to_string())))), + ); + } + + query_filter_map + } +} + +impl QuerySort for IndexParams { + fn get_sort_column(&self) -> Option { + self.sort_by.as_ref().map(|field| match field { + SortField::DueBy => actions::Column::DueBy, + SortField::CreatedAt => actions::Column::CreatedAt, + SortField::UpdatedAt => actions::Column::UpdatedAt, + }) + } + + fn get_sort_order(&self) -> Option { + self.sort_order.as_ref().map(|order| match order { + SortOrder::Asc => Order::Asc, + SortOrder::Desc => Order::Desc, + }) + } +} + +impl WithSortDefaults for IndexParams { + type SortField = SortField; +} diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs new file mode 100644 index 00000000..1a6b99c3 --- /dev/null +++ b/web/src/params/user/coaching_session.rs @@ -0,0 +1,43 @@ +use chrono::NaiveDate; +use serde::Deserialize; +use utoipa::IntoParams; + +use crate::params::coaching_session::SortField; +use crate::params::sort::SortOrder; +use domain::Id; + +#[derive(Debug, Deserialize, IntoParams)] +pub(crate) struct IndexParams { + #[serde(skip)] + pub(crate) user_id: Id, + pub(crate) from_date: Option, + pub(crate) to_date: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +impl IndexParams { + pub fn new(user_id: Id) -> Self { + Self { + user_id, + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + } + } + + pub fn with_filters( + mut self, + from_date: Option, + to_date: Option, + sort_by: Option, + sort_order: Option, + ) -> Self { + self.from_date = from_date; + self.to_date = to_date; + self.sort_by = sort_by; + self.sort_order = sort_order; + self + } +} diff --git a/web/src/params/user.rs b/web/src/params/user/mod.rs similarity index 94% rename from web/src/params/user.rs rename to web/src/params/user/mod.rs index 56c53bfb..2623e95f 100644 --- a/web/src/params/user.rs +++ b/web/src/params/user/mod.rs @@ -1,7 +1,13 @@ +pub(crate) mod action; +pub(crate) mod coaching_session; +pub(crate) mod overarching_goal; + +// Re-export user profile update params for backward compatibility use domain::{IntoUpdateMap, UpdateMap}; use sea_orm::Value; use serde::Deserialize; use utoipa::{IntoParams, ToSchema}; + #[derive(Debug, Deserialize, IntoParams, ToSchema)] pub struct UpdateParams { pub email: Option, @@ -11,6 +17,7 @@ pub struct UpdateParams { pub github_profile_url: Option, pub timezone: Option, } + impl IntoUpdateMap for UpdateParams { fn into_update_map(self) -> UpdateMap { let mut update_map = UpdateMap::new(); diff --git a/web/src/params/user/overarching_goal.rs b/web/src/params/user/overarching_goal.rs new file mode 100644 index 00000000..3c8abade --- /dev/null +++ b/web/src/params/user/overarching_goal.rs @@ -0,0 +1,92 @@ +use sea_orm::{Order, Value}; +use serde::Deserialize; +use utoipa::{IntoParams, ToSchema}; + +use crate::params::sort::SortOrder; +use crate::params::WithSortDefaults; +use domain::{overarching_goals, Id, IntoQueryFilterMap, QueryFilterMap, QuerySort}; + +/// Sortable fields for user overarching goals +#[derive(Debug, Deserialize, ToSchema)] +#[schema(example = "title")] +pub(crate) enum SortField { + #[serde(rename = "title")] + Title, + #[serde(rename = "created_at")] + CreatedAt, + #[serde(rename = "updated_at")] + UpdatedAt, +} + +#[derive(Debug, Deserialize, IntoParams)] +pub(crate) struct IndexParams { + #[serde(skip)] + pub(crate) user_id: Id, + pub(crate) coaching_session_id: Option, + pub(crate) sort_by: Option, + pub(crate) sort_order: Option, +} + +impl IndexParams { + pub fn new(user_id: Id) -> Self { + Self { + user_id, + coaching_session_id: None, + sort_by: None, + sort_order: None, + } + } + + pub fn with_filters( + mut self, + coaching_session_id: Option, + sort_by: Option, + sort_order: Option, + ) -> Self { + self.coaching_session_id = coaching_session_id; + self.sort_by = sort_by; + self.sort_order = sort_order; + self + } +} + +impl IntoQueryFilterMap for IndexParams { + fn into_query_filter_map(self) -> QueryFilterMap { + let mut query_filter_map = QueryFilterMap::new(); + + query_filter_map.insert( + "user_id".to_string(), + Some(Value::Uuid(Some(Box::new(self.user_id)))), + ); + + if let Some(coaching_session_id) = self.coaching_session_id { + query_filter_map.insert( + "coaching_session_id".to_string(), + Some(Value::Uuid(Some(Box::new(coaching_session_id)))), + ); + } + + query_filter_map + } +} + +impl QuerySort for IndexParams { + fn get_sort_column(&self) -> Option { + self.sort_by.as_ref().map(|field| match field { + SortField::Title => overarching_goals::Column::Title, + SortField::CreatedAt => overarching_goals::Column::CreatedAt, + SortField::UpdatedAt => overarching_goals::Column::UpdatedAt, + }) + } + + fn get_sort_order(&self) -> Option { + self.sort_order.as_ref().map(|order| match order { + SortOrder::Asc => Order::Asc, + SortOrder::Desc => Order::Desc, + }) + } +} + +impl WithSortDefaults for IndexParams { + type SortField = SortField; +} diff --git a/web/src/protect/users/mod.rs b/web/src/protect/users/mod.rs index db5ba00e..61242c14 100644 --- a/web/src/protect/users/mod.rs +++ b/web/src/protect/users/mod.rs @@ -10,6 +10,90 @@ use log::*; pub(crate) mod passwords; +// checks: +// - that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn organizations( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own organizations (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} + +// checks: +// - that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn actions( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own actions (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} + +// checks: +// - that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn coaching_sessions( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own coaching sessions (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} + +// checks: +// - that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn overarching_goals( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own overarching goals (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} + // checks: // - that the `user_id` matches the `authenticated_user.id` pub(crate) async fn read( diff --git a/web/src/router.rs b/web/src/router.rs index 023af472..1d663b30 100644 --- a/web/src/router.rs +++ b/web/src/router.rs @@ -70,6 +70,10 @@ use self::organization::coaching_relationship_controller; user_session_controller::login, user_session_controller::delete, user::password_controller::update_password, + user::organization_controller::index, + user::action_controller::index, + user::coaching_session_controller::index, + user::overarching_goal_controller::index, jwt_controller::generate_collab_token, ), components( @@ -124,6 +128,10 @@ pub fn define_routes(app_state: AppState) -> Router { .merge(overarching_goal_routes(app_state.clone())) .merge(user_routes(app_state.clone())) .merge(user_password_routes(app_state.clone())) + .merge(user_organizations_routes(app_state.clone())) + .merge(user_actions_routes(app_state.clone())) + .merge(user_coaching_sessions_routes(app_state.clone())) + .merge(user_overarching_goals_routes(app_state.clone())) .merge(user_session_routes()) .merge(user_session_protected_routes(app_state.clone())) .merge(coaching_sessions_routes(app_state.clone())) @@ -432,6 +440,74 @@ fn jwt_routes(app_state: AppState) -> Router { .with_state(app_state) } +fn user_organizations_routes(app_state: AppState) -> Router { + Router::new() + .merge( + Router::new() + .route( + "/users/:user_id/organizations", + get(user::organization_controller::index), + ) + .route_layer(from_fn_with_state( + app_state.clone(), + protect::users::organizations, + )), + ) + .route_layer(from_fn(require_auth)) + .with_state(app_state) +} + +fn user_actions_routes(app_state: AppState) -> Router { + Router::new() + .merge( + Router::new() + .route( + "/users/:user_id/actions", + get(user::action_controller::index), + ) + .route_layer(from_fn_with_state( + app_state.clone(), + protect::users::actions, + )), + ) + .route_layer(from_fn(require_auth)) + .with_state(app_state) +} + +fn user_coaching_sessions_routes(app_state: AppState) -> Router { + Router::new() + .merge( + Router::new() + .route( + "/users/:user_id/coaching_sessions", + get(user::coaching_session_controller::index), + ) + .route_layer(from_fn_with_state( + app_state.clone(), + protect::users::coaching_sessions, + )), + ) + .route_layer(from_fn(require_auth)) + .with_state(app_state) +} + +fn user_overarching_goals_routes(app_state: AppState) -> Router { + Router::new() + .merge( + Router::new() + .route( + "/users/:user_id/overarching_goals", + get(user::overarching_goal_controller::index), + ) + .route_layer(from_fn_with_state( + app_state.clone(), + protect::users::overarching_goals, + )), + ) + .route_layer(from_fn(require_auth)) + .with_state(app_state) +} + // This will serve static files that we can use as a "fallback" for when the server panics pub fn static_routes() -> Router { Router::new().nest_service("/", ServeDir::new("./")) From 061a0313d44b7fa1594c482ee8e1827f346c2f9a Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 26 Oct 2025 20:29:20 -0500 Subject: [PATCH 02/17] Add enhanced coaching sessions endpoint with batch loading (Issue #160) This commit adds an enhanced user-scoped coaching sessions endpoint that supports optional batch loading of related resources to eliminate N+1 queries. Backend changes: - Add EnrichedSession and IncludeOptions to entity_api layer - Implement find_by_user_with_includes with efficient batch loading - Add IncludeParam enum for query parameter validation - Add EnrichedCoachingSession response DTO - Update user coaching_session_controller to use enhanced endpoint Technical improvements: - Single API call replaces 3+ separate queries - Batch loads relationships, users, organizations, goals, agreements - Validates include parameters (organization requires relationship) - Maintains backward compatibility with existing endpoints Related to Today's Sessions UI feature (Issue #160) --- domain/src/coaching_session.rs | 5 +- domain/src/lib.rs | 4 +- entity/src/status.rs | 11 + entity_api/src/coaching_session.rs | 410 +++++++++++++++++- entity_api/src/lib.rs | 2 +- .../user/coaching_session_controller.rs | 117 +++-- web/src/params/user/coaching_session.rs | 154 +++++++ web/src/response/coaching_session.rs | 98 +++++ web/src/response/mod.rs | 1 + 9 files changed, 760 insertions(+), 42 deletions(-) create mode 100644 web/src/response/coaching_session.rs create mode 100644 web/src/response/mod.rs diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index 55ff48fc..a3f1e648 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -11,7 +11,10 @@ use log::*; use sea_orm::{DatabaseConnection, IntoActiveModel}; use service::config::Config; -pub use entity_api::coaching_session::{find_by_id, find_by_id_with_coaching_relationship}; +pub use entity_api::coaching_session::{ + find_by_id, find_by_id_with_coaching_relationship, find_by_user_with_includes, + EnrichedSession, IncludeOptions, +}; #[derive(Debug, Clone)] struct SessionDate(NaiveDateTime); diff --git a/domain/src/lib.rs b/domain/src/lib.rs index cbc3532d..b33c0e35 100644 --- a/domain/src/lib.rs +++ b/domain/src/lib.rs @@ -9,10 +9,10 @@ pub use entity_api::{ query::{FilterOnly, IntoQueryFilterMap, QueryFilterMap}, }; -// Re-exports from `entity` crate +// Re-exports from `entity` crate via `entity_api` pub use entity_api::{ actions, agreements, coachees, coaches, coaching_relationships, coaching_sessions, jwts, notes, - organizations, overarching_goals, query::QuerySort, user_roles, users, Id, + organizations, overarching_goals, query::QuerySort, status, user_roles, users, Id, }; pub mod action; diff --git a/entity/src/status.rs b/entity/src/status.rs index 9d7a5a19..7f45cb73 100644 --- a/entity/src/status.rs +++ b/entity/src/status.rs @@ -31,3 +31,14 @@ impl From<&str> for Status { } } } + +impl std::fmt::Display for Status { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::NotStarted => write!(f, "not_started"), + Self::InProgress => write!(f, "in_progress"), + Self::Completed => write!(f, "completed"), + Self::WontDo => write!(f, "wont_do"), + } + } +} diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 0d962e49..36222158 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -1,11 +1,12 @@ use super::error::{EntityApiErrorKind, Error}; use entity::{ - coaching_relationships, - coaching_sessions::{ActiveModel, Entity, Model, Relation}, - Id, + agreements, coaching_relationships, + coaching_sessions::{self, ActiveModel, Entity, Model, Relation}, + organizations, overarching_goals, users, Id, }; use log::debug; use sea_orm::{entity::prelude::*, DatabaseConnection, JoinType, QuerySelect, Set, TryIntoModel}; +use std::collections::HashMap; pub async fn create( db: &DatabaseConnection, @@ -75,6 +76,289 @@ pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, + pub coach: Option, + pub coachee: Option, + pub organization: Option, + pub overarching_goal: Option, + pub agreement: Option, +} + +/// Configuration for which related resources to include +#[derive(Debug, Clone, Copy)] +pub struct IncludeOptions { + pub relationship: bool, + pub organization: bool, + pub goal: bool, + pub agreements: bool, +} + +impl IncludeOptions { + pub fn none() -> Self { + Self { + relationship: false, + organization: false, + goal: false, + agreements: false, + } + } + + pub fn needs_relationships(&self) -> bool { + self.relationship || self.organization + } +} + +/// Find sessions by user with optional date filtering and related data includes +pub async fn find_by_user_with_includes( + db: &impl ConnectionTrait, + user_id: Id, + from_date: Option, + to_date: Option, + includes: IncludeOptions, +) -> Result, Error> { + // Load base sessions with date filtering + let sessions = load_sessions_for_user(db, user_id, from_date, to_date).await?; + + // Early return if no includes requested + if !includes.needs_relationships() && !includes.goal && !includes.agreements { + return Ok(sessions.into_iter().map(EnrichedSession::from_session).collect()); + } + + // Load all related data in efficient batches + let related_data = load_related_data(db, &sessions, includes).await?; + + // Assemble enriched sessions + Ok(sessions + .into_iter() + .map(|session| assemble_enriched_session(session, &related_data)) + .collect()) +} + +/// Load base sessions filtered by user and optional date range +async fn load_sessions_for_user( + db: &impl ConnectionTrait, + user_id: Id, + from_date: Option, + to_date: Option, +) -> Result, Error> { + let mut query = Entity::find() + .join(JoinType::InnerJoin, Relation::CoachingRelationships.def()) + .filter( + coaching_relationships::Column::CoachId + .eq(user_id) + .or(coaching_relationships::Column::CoacheeId.eq(user_id)), + ); + + if let Some(from) = from_date { + query = query.filter(coaching_sessions::Column::Date.gte(from)); + } + + if let Some(to) = to_date { + // Use next day with less-than for inclusive end date + let end_of_day = to.succ_opt().unwrap_or(to); + query = query.filter(coaching_sessions::Column::Date.lt(end_of_day)); + } + + query.all(db).await.map_err(Into::into) +} + +/// Container for all related data loaded in batches +#[derive(Debug, Default)] +struct RelatedData { + relationships: HashMap, + coaches: HashMap, + coachees: HashMap, + organizations: HashMap, + goals: HashMap, + agreements: HashMap, +} + +/// Load all requested related data in efficient batches +async fn load_related_data( + db: &impl ConnectionTrait, + sessions: &[Model], + includes: IncludeOptions, +) -> Result { + let mut data = RelatedData::default(); + + // Extract IDs for batch loading + let relationship_ids: Vec = sessions.iter().map(|s| s.coaching_relationship_id).collect(); + let session_ids: Vec = sessions.iter().map(|s| s.id).collect(); + + // Load relationships (needed for both relationship and organization includes) + if includes.needs_relationships() { + data.relationships = batch_load_relationships(db, &relationship_ids).await?; + } + + // Load users (coaches and coachees) if relationship is included + if includes.relationship { + let coach_ids: Vec = data.relationships.values().map(|r| r.coach_id).collect(); + let coachee_ids: Vec = data.relationships.values().map(|r| r.coachee_id).collect(); + + data.coaches = batch_load_users(db, &coach_ids).await?; + data.coachees = batch_load_users(db, &coachee_ids).await?; + } + + // Load organizations if requested + if includes.organization { + let org_ids: Vec = data.relationships.values().map(|r| r.organization_id).collect(); + data.organizations = batch_load_organizations(db, &org_ids).await?; + } + + // Load goals by session_id + if includes.goal { + data.goals = batch_load_goals(db, &session_ids).await?; + } + + // Load agreements by session_id + if includes.agreements { + data.agreements = batch_load_agreements(db, &session_ids).await?; + } + + Ok(data) +} + +/// Batch load coaching relationships by IDs +async fn batch_load_relationships( + db: &impl ConnectionTrait, + ids: &[Id], +) -> Result, Error> { + if ids.is_empty() { + return Ok(HashMap::new()); + } + + Ok(coaching_relationships::Entity::find() + .filter(coaching_relationships::Column::Id.is_in(ids.iter().copied())) + .all(db) + .await? + .into_iter() + .map(|r| (r.id, r)) + .collect()) +} + +/// Batch load users by IDs +async fn batch_load_users( + db: &impl ConnectionTrait, + ids: &[Id], +) -> Result, Error> { + if ids.is_empty() { + return Ok(HashMap::new()); + } + + Ok(users::Entity::find() + .filter(users::Column::Id.is_in(ids.iter().copied())) + .all(db) + .await? + .into_iter() + .map(|u| (u.id, u)) + .collect()) +} + +/// Batch load organizations by IDs +async fn batch_load_organizations( + db: &impl ConnectionTrait, + ids: &[Id], +) -> Result, Error> { + if ids.is_empty() { + return Ok(HashMap::new()); + } + + Ok(organizations::Entity::find() + .filter(organizations::Column::Id.is_in(ids.iter().copied())) + .all(db) + .await? + .into_iter() + .map(|o| (o.id, o)) + .collect()) +} + +/// Batch load overarching goals by session IDs +async fn batch_load_goals( + db: &impl ConnectionTrait, + session_ids: &[Id], +) -> Result, Error> { + if session_ids.is_empty() { + return Ok(HashMap::new()); + } + + Ok(overarching_goals::Entity::find() + .filter(overarching_goals::Column::CoachingSessionId.is_in(session_ids.iter().copied())) + .all(db) + .await? + .into_iter() + .map(|g| (g.coaching_session_id, g)) + .collect()) +} + +/// Batch load agreements by session IDs +async fn batch_load_agreements( + db: &impl ConnectionTrait, + session_ids: &[Id], +) -> Result, Error> { + if session_ids.is_empty() { + return Ok(HashMap::new()); + } + + Ok(agreements::Entity::find() + .filter(agreements::Column::CoachingSessionId.is_in(session_ids.iter().copied())) + .all(db) + .await? + .into_iter() + .map(|a| (a.coaching_session_id, a)) + .collect()) +} + +/// Assemble an enriched session from base session and related data +fn assemble_enriched_session(session: Model, related: &RelatedData) -> EnrichedSession { + let relationship = related.relationships.get(&session.coaching_relationship_id).cloned(); + + let (coach, coachee) = relationship + .as_ref() + .map(|rel| { + ( + related.coaches.get(&rel.coach_id).cloned(), + related.coachees.get(&rel.coachee_id).cloned(), + ) + }) + .unwrap_or((None, None)); + + let organization = relationship + .as_ref() + .and_then(|rel| related.organizations.get(&rel.organization_id).cloned()); + + let overarching_goal = related.goals.get(&session.id).cloned(); + let agreement = related.agreements.get(&session.id).cloned(); + + EnrichedSession { + session, + relationship, + coach, + coachee, + organization, + overarching_goal, + agreement, + } +} + +impl EnrichedSession { + /// Create an enriched session from just the base session model + fn from_session(session: Model) -> Self { + Self { + session, + relationship: None, + coach: None, + coachee: None, + organization: None, + overarching_goal: None, + agreement: None, + } + } +} + #[cfg(test)] // We need to gate seaORM's mock feature behind conditional compilation because // the feature removes the Clone trait implementation from seaORM's DatabaseConnection. @@ -190,4 +474,124 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn find_by_user_with_includes_no_includes_returns_basic_sessions() -> Result<(), Error> { + let now = chrono::Utc::now(); + let user_id = Id::new_v4(); + let session_id = Id::new_v4(); + let relationship_id = Id::new_v4(); + + let session = Model { + id: session_id, + coaching_relationship_id: relationship_id, + date: chrono::Local::now().naive_utc(), + collab_document_name: None, + created_at: now.into(), + updated_at: now.into(), + }; + + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![session.clone()]]) + .into_connection(); + + let includes = IncludeOptions::none(); + let results = find_by_user_with_includes(&db, user_id, None, None, includes).await?; + + assert_eq!(results.len(), 1); + assert_eq!(results[0].session.id, session_id); + assert!(results[0].relationship.is_none()); + assert!(results[0].organization.is_none()); + assert!(results[0].overarching_goal.is_none()); + assert!(results[0].agreement.is_none()); + + Ok(()) + } + + #[tokio::test] + async fn find_by_user_with_includes_with_date_filters() -> Result<(), Error> { + let now = chrono::Utc::now(); + let user_id = Id::new_v4(); + let from_date = chrono::NaiveDate::from_ymd_opt(2025, 10, 26).unwrap(); + let to_date = chrono::NaiveDate::from_ymd_opt(2025, 10, 27).unwrap(); + + // Create a session within the date range + let session = Model { + id: Id::new_v4(), + coaching_relationship_id: Id::new_v4(), + date: from_date.into(), + collab_document_name: None, + created_at: now.into(), + updated_at: now.into(), + }; + + let db = MockDatabase::new(DatabaseBackend::Postgres) + .append_query_results(vec![vec![session.clone()]]) + .into_connection(); + + let includes = IncludeOptions::none(); + let results = find_by_user_with_includes(&db, user_id, Some(from_date), Some(to_date), includes).await?; + + assert_eq!(results.len(), 1); + assert_eq!(results[0].session.id, session.id); + assert_eq!(results[0].session.date, from_date.into()); + + Ok(()) + } + + #[tokio::test] + async fn include_options_needs_relationships_returns_true_when_relationship_included() { + let includes = IncludeOptions { + relationship: true, + organization: false, + goal: false, + agreements: false, + }; + assert!(includes.needs_relationships()); + } + + #[tokio::test] + async fn include_options_needs_relationships_returns_true_when_organization_included() { + let includes = IncludeOptions { + relationship: false, + organization: true, + goal: false, + agreements: false, + }; + assert!(includes.needs_relationships()); + } + + #[tokio::test] + async fn include_options_needs_relationships_returns_false_when_only_goals() { + let includes = IncludeOptions { + relationship: false, + organization: false, + goal: true, + agreements: false, + }; + assert!(!includes.needs_relationships()); + } + + #[tokio::test] + async fn enriched_session_from_session_creates_empty_enrichment() { + let now = chrono::Utc::now(); + let session = Model { + id: Id::new_v4(), + coaching_relationship_id: Id::new_v4(), + date: chrono::Local::now().naive_utc(), + collab_document_name: None, + created_at: now.into(), + updated_at: now.into(), + }; + + let enriched = EnrichedSession::from_session(session.clone()); + + assert_eq!(enriched.session.id, session.id); + assert!(enriched.relationship.is_none()); + assert!(enriched.coach.is_none()); + assert!(enriched.coachee.is_none()); + assert!(enriched.organization.is_none()); + assert!(enriched.overarching_goal.is_none()); + assert!(enriched.agreement.is_none()); + } } diff --git a/entity_api/src/lib.rs b/entity_api/src/lib.rs index e774d05b..b9e1bdeb 100644 --- a/entity_api/src/lib.rs +++ b/entity_api/src/lib.rs @@ -4,7 +4,7 @@ use sea_orm::{ActiveModelTrait, DatabaseConnection, Set}; pub use entity::{ actions, agreements, coachees, coaches, coaching_relationships, coaching_sessions, jwts, notes, - organizations, overarching_goals, user_roles, users, users::Role, Id, + organizations, overarching_goals, status, user_roles, users, users::Role, Id, }; pub mod action; diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs index 8886718d..19a5968c 100644 --- a/web/src/controller/user/coaching_session_controller.rs +++ b/web/src/controller/user/coaching_session_controller.rs @@ -1,45 +1,38 @@ use crate::controller::ApiResponse; +use crate::error::WebErrorKind; use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; use crate::params::coaching_session::SortField; use crate::params::sort::SortOrder; -use crate::params::user::coaching_session::IndexParams; +use crate::params::user::coaching_session::{IncludeParam, IndexParams}; +use crate::response::coaching_session::EnrichedCoachingSession; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::Json; -use chrono::NaiveDate; -use domain::{coaching_sessions::Model, Id}; -use entity_api::coaching_session as CoachingSessionApi; -use serde::Deserialize; +use domain::{coaching_session as CoachingSessionApi, Id}; use service::config::ApiVersion; use log::*; -#[derive(Debug, Deserialize)] -pub(crate) struct QueryParams { - pub(crate) from_date: Option, - pub(crate) to_date: Option, - pub(crate) sort_by: Option, - pub(crate) sort_order: Option, -} - -/// GET all coaching sessions for a specific user +/// GET all coaching sessions for a specific user with optional related data #[utoipa::path( get, path = "/users/{user_id}/coaching_sessions", params( ApiVersion, ("user_id" = Id, Path, description = "User ID to retrieve coaching sessions for"), - ("from_date" = Option, Query, description = "Filter by from_date"), - ("to_date" = Option, Query, description = "Filter by to_date"), + ("from_date" = Option, Query, description = "Filter by from_date (inclusive, UTC)"), + ("to_date" = Option, Query, description = "Filter by to_date (inclusive, UTC)"), + ("include" = Option, Query, description = "Comma-separated list of related resources to include. Valid values: 'relationship', 'organization', 'goal', 'agreements'. Example: 'relationship,organization,goal'"), ("sort_by" = Option, Query, description = "Sort by field. Valid values: 'date', 'created_at', 'updated_at'. Must be provided with sort_order.", example = "date"), ("sort_order" = Option, Query, description = "Sort order. Valid values: 'asc' (ascending), 'desc' (descending). Must be provided with sort_by.", example = "desc") ), responses( - (status = 200, description = "Successfully retrieved coaching sessions for user", body = [domain::coaching_sessions::Model]), + (status = 200, description = "Successfully retrieved coaching sessions for user", body = [crate::response::coaching_session::EnrichedCoachingSession]), + (status = 400, description = "Bad Request - Invalid include parameter"), (status = 401, description = "Unauthorized"), (status = 403, description = "Forbidden"), (status = 404, description = "User not found"), @@ -54,30 +47,43 @@ pub async fn index( AuthenticatedUser(_user): AuthenticatedUser, State(app_state): State, Path(user_id): Path, - Query(query_params): Query, + Query(params): Query, ) -> Result { debug!("GET Coaching Sessions for User: {user_id}"); - debug!("Filter Params: {query_params:?}"); - - // Fetch all coaching sessions for the user (where they are coach or coachee) - let mut sessions = CoachingSessionApi::find_by_user(app_state.db_conn_ref(), user_id).await?; + debug!("Query Params: {params:?}"); - // Apply date range filters - if let Some(from_date) = query_params.from_date { - sessions.retain(|session| session.date.date() >= from_date); - } - if let Some(to_date) = query_params.to_date { - sessions.retain(|session| session.date.date() <= to_date); + // Validate include parameters + if let Err(err_msg) = params.validate_includes() { + warn!("Invalid include parameters: {err_msg}"); + return Err(Error::Web(WebErrorKind::Input)); } + // Build include options from parameters + let includes = CoachingSessionApi::IncludeOptions { + relationship: params.include.contains(&IncludeParam::Relationship), + organization: params.include.contains(&IncludeParam::Organization), + goal: params.include.contains(&IncludeParam::Goal), + agreements: params.include.contains(&IncludeParam::Agreements), + }; + + // Fetch sessions with optional includes + let mut enriched_sessions = CoachingSessionApi::find_by_user_with_includes( + app_state.db_conn_ref(), + user_id, + params.from_date, + params.to_date, + includes, + ) + .await?; + // Apply sorting - if let (Some(sort_by), Some(sort_order)) = (query_params.sort_by, query_params.sort_order) { + if let (Some(sort_by), Some(sort_order)) = (params.sort_by, params.sort_order) { let ascending = matches!(sort_order, SortOrder::Asc); - sessions.sort_by(|a, b| { + enriched_sessions.sort_by(|a, b| { let cmp = match sort_by { - SortField::Date => a.date.cmp(&b.date), - SortField::CreatedAt => a.created_at.cmp(&b.created_at), - SortField::UpdatedAt => a.updated_at.cmp(&b.updated_at), + SortField::Date => a.session.date.cmp(&b.session.date), + SortField::CreatedAt => a.session.created_at.cmp(&b.session.created_at), + SortField::UpdatedAt => a.session.updated_at.cmp(&b.session.updated_at), }; if ascending { cmp @@ -87,7 +93,48 @@ pub async fn index( }); } - debug!("Found {} coaching sessions for user {user_id}", sessions.len()); + debug!( + "Found {} coaching sessions for user {user_id}", + enriched_sessions.len() + ); + + // Convert to response DTOs + let response_sessions: Vec = enriched_sessions + .into_iter() + .map(|enriched| convert_to_response(enriched)) + .collect(); + + Ok(Json(ApiResponse::new( + StatusCode::OK.into(), + response_sessions, + ))) +} + +/// Convert entity_api EnrichedSession to web response DTO +fn convert_to_response(enriched: CoachingSessionApi::EnrichedSession) -> EnrichedCoachingSession { + let mut response = EnrichedCoachingSession::from_model(enriched.session); + + // Add relationship data if present + if let (Some(relationship), Some(coach), Some(coachee)) = + (enriched.relationship, enriched.coach, enriched.coachee) + { + response = response.with_relationship(relationship, coach, coachee); + } + + // Add organization data if present + if let Some(organization) = enriched.organization { + response = response.with_organization(organization); + } + + // Add goal data if present + if let Some(goal) = enriched.overarching_goal { + response = response.with_goal(goal); + } + + // Add agreement data if present + if let Some(agreement) = enriched.agreement { + response = response.with_agreement(agreement); + } - Ok(Json(ApiResponse::new(StatusCode::OK.into(), sessions))) + response } diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 1a6b99c3..64d42858 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -6,17 +6,60 @@ use crate::params::coaching_session::SortField; use crate::params::sort::SortOrder; use domain::Id; +/// Include parameter for optionally fetching related resources +/// Supports comma-separated values: ?include=relationship,organization,goal,agreements +#[derive(Debug, Clone, Deserialize, PartialEq, Eq, Hash)] +#[serde(rename_all = "lowercase")] +pub enum IncludeParam { + Relationship, + Organization, + Goal, + Agreements, +} + #[derive(Debug, Deserialize, IntoParams)] pub(crate) struct IndexParams { #[serde(skip)] + #[allow(dead_code)] pub(crate) user_id: Id, pub(crate) from_date: Option, pub(crate) to_date: Option, pub(crate) sort_by: Option, pub(crate) sort_order: Option, + /// Comma-separated list of related resources to include + /// Valid values: relationship, organization, goal, agreements + /// Example: ?include=relationship,organization,goal + #[serde(default, deserialize_with = "deserialize_comma_separated")] + pub(crate) include: Vec, +} + +/// Custom deserializer for comma-separated include parameter +fn deserialize_comma_separated<'de, D>(deserializer: D) -> Result, D::Error> +where + D: serde::Deserializer<'de>, +{ + let s: Option = Option::deserialize(deserializer)?; + match s { + None => Ok(Vec::new()), + Some(s) if s.is_empty() => Ok(Vec::new()), + Some(s) => { + let mut includes = Vec::new(); + for part in s.split(',') { + let trimmed = part.trim(); + if !trimmed.is_empty() { + let include: IncludeParam = serde_json::from_value( + serde_json::Value::String(trimmed.to_string()) + ).map_err(serde::de::Error::custom)?; + includes.push(include); + } + } + Ok(includes) + } + } } impl IndexParams { + #[allow(dead_code)] pub fn new(user_id: Id) -> Self { Self { user_id, @@ -24,9 +67,11 @@ impl IndexParams { to_date: None, sort_by: None, sort_order: None, + include: Vec::new(), } } + #[allow(dead_code)] pub fn with_filters( mut self, from_date: Option, @@ -40,4 +85,113 @@ impl IndexParams { self.sort_order = sort_order; self } + + /// Validates that include parameters are meaningful + /// Returns error message if validation fails + pub fn validate_includes(&self) -> Result<(), &'static str> { + // organization requires relationship (can't get org without relationship) + if self.include.contains(&IncludeParam::Organization) + && !self.include.contains(&IncludeParam::Relationship) { + return Err("Cannot include 'organization' without 'relationship'"); + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn validate_includes_allows_organization_with_relationship() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![IncludeParam::Relationship, IncludeParam::Organization], + }; + + assert!(params.validate_includes().is_ok()); + } + + #[test] + fn validate_includes_rejects_organization_without_relationship() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![IncludeParam::Organization], + }; + + assert!(params.validate_includes().is_err()); + assert_eq!( + params.validate_includes().unwrap_err(), + "Cannot include 'organization' without 'relationship'" + ); + } + + #[test] + fn validate_includes_allows_goal_alone() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![IncludeParam::Goal], + }; + + assert!(params.validate_includes().is_ok()); + } + + #[test] + fn validate_includes_allows_agreements_alone() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![IncludeParam::Agreements], + }; + + assert!(params.validate_includes().is_ok()); + } + + #[test] + fn validate_includes_allows_all_includes() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![ + IncludeParam::Relationship, + IncludeParam::Organization, + IncludeParam::Goal, + IncludeParam::Agreements, + ], + }; + + assert!(params.validate_includes().is_ok()); + } + + #[test] + fn validate_includes_allows_empty_includes() { + let params = IndexParams { + user_id: Id::new_v4(), + from_date: None, + to_date: None, + sort_by: None, + sort_order: None, + include: vec![], + }; + + assert!(params.validate_includes().is_ok()); + } } diff --git a/web/src/response/coaching_session.rs b/web/src/response/coaching_session.rs new file mode 100644 index 00000000..c24fa77b --- /dev/null +++ b/web/src/response/coaching_session.rs @@ -0,0 +1,98 @@ +//! Enriched coaching session response DTOs +//! +//! These DTOs support optional inclusion of related resources to avoid N+1 queries. +//! Related data is only included when explicitly requested via the `include` parameter. + +use domain::agreements::Model as AgreementModel; +use domain::coaching_relationships::Model as CoachingRelationshipModel; +use domain::coaching_sessions::Model as CoachingSessionModel; +use domain::organizations::Model as OrganizationModel; +use domain::overarching_goals::Model as OverarchingGoalModel; +use domain::users::Model as UserModel; +use serde::Serialize; +use utoipa::ToSchema; + +/// Enriched coaching session with optional related resources +/// Reuses existing domain models to avoid duplication +#[derive(Debug, Clone, Serialize, ToSchema)] +pub struct EnrichedCoachingSession { + /// The coaching session itself + #[serde(flatten)] + pub session: CoachingSessionModel, + + /// Coaching relationship (only if ?include=relationship) + #[serde(skip_serializing_if = "Option::is_none")] + pub relationship: Option, + + /// Organization details (only if ?include=organization) + #[serde(skip_serializing_if = "Option::is_none")] + pub organization: Option, + + /// Overarching goal for this session (only if ?include=goal) + #[serde(skip_serializing_if = "Option::is_none")] + pub overarching_goal: Option, + + /// Agreement for this session (only if ?include=agreements) + #[serde(skip_serializing_if = "Option::is_none")] + pub agreement: Option, +} + +/// Relationship with coach and coachee user details +#[derive(Debug, Clone, Serialize, ToSchema)] +pub struct RelationshipWithUsers { + /// The relationship itself + #[serde(flatten)] + pub relationship: CoachingRelationshipModel, + + /// Coach user details + pub coach: UserModel, + + /// Coachee user details + pub coachee: UserModel, +} + +impl EnrichedCoachingSession { + /// Create from a basic coaching session model (no includes) + pub fn from_model(session: CoachingSessionModel) -> Self { + Self { + session, + relationship: None, + organization: None, + overarching_goal: None, + agreement: None, + } + } + + /// Add relationship data with coach and coachee users + pub fn with_relationship( + mut self, + relationship: CoachingRelationshipModel, + coach: UserModel, + coachee: UserModel, + ) -> Self { + self.relationship = Some(RelationshipWithUsers { + relationship, + coach, + coachee, + }); + self + } + + /// Add organization data + pub fn with_organization(mut self, organization: OrganizationModel) -> Self { + self.organization = Some(organization); + self + } + + /// Add overarching goal data + pub fn with_goal(mut self, goal: OverarchingGoalModel) -> Self { + self.overarching_goal = Some(goal); + self + } + + /// Add agreement data + pub fn with_agreement(mut self, agreement: AgreementModel) -> Self { + self.agreement = Some(agreement); + self + } +} diff --git a/web/src/response/mod.rs b/web/src/response/mod.rs new file mode 100644 index 00000000..e2ef9fe0 --- /dev/null +++ b/web/src/response/mod.rs @@ -0,0 +1 @@ +pub mod coaching_session; From 9d3bb71e737f35bb49cc44d89cb4baf6ed3da723 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 1 Nov 2025 17:08:29 -0500 Subject: [PATCH 03/17] Make sure response is included in lib.rs --- web/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/web/src/lib.rs b/web/src/lib.rs index 4698166e..a1251752 100644 --- a/web/src/lib.rs +++ b/web/src/lib.rs @@ -25,6 +25,7 @@ pub(crate) mod extractors; pub(crate) mod middleware; pub(crate) mod params; pub(crate) mod protect; +pub(crate) mod response; mod router; pub async fn init_server(app_state: AppState) -> Result<()> { From 2b400f46792ac5a0c258a299805b2a6a9ffb7442 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 1 Nov 2025 17:39:33 -0500 Subject: [PATCH 04/17] Refactor coaching sessions endpoint for better architecture - Move validation from web to entity_api layer for proper error propagation - Add database-level sorting to find_by_user_with_includes - Simplify comma-separated deserializer with functional style - Use entity_api type directly with serde flatten, eliminating response DTO - Fix pre-existing test failure in organization.rs Impact: Removes 151 lines, improves consistency with codebase patterns --- domain/src/coaching_session.rs | 2 +- entity_api/src/coaching_session.rs | 122 ++++++++++++++-- .../user/coaching_session_controller.rs | 83 +++-------- web/src/lib.rs | 1 - web/src/params/user/coaching_session.rs | 130 ++---------------- web/src/response/coaching_session.rs | 98 ------------- web/src/response/mod.rs | 1 - 7 files changed, 143 insertions(+), 294 deletions(-) delete mode 100644 web/src/response/coaching_session.rs delete mode 100644 web/src/response/mod.rs diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index a3f1e648..fcd374eb 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -13,7 +13,7 @@ use service::config::Config; pub use entity_api::coaching_session::{ find_by_id, find_by_id_with_coaching_relationship, find_by_user_with_includes, - EnrichedSession, IncludeOptions, + EnrichedSession, IncludeOptions, SortField, SortOrder, }; #[derive(Debug, Clone)] diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 36222158..8c0af87e 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -5,7 +5,7 @@ use entity::{ organizations, overarching_goals, users, Id, }; use log::debug; -use sea_orm::{entity::prelude::*, DatabaseConnection, JoinType, QuerySelect, Set, TryIntoModel}; +use sea_orm::{entity::prelude::*, DatabaseConnection, JoinType, QueryOrder, QuerySelect, Set, TryIntoModel}; use std::collections::HashMap; pub async fn create( @@ -77,14 +77,21 @@ pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, + #[serde(skip_serializing_if = "Option::is_none")] pub coach: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub coachee: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub organization: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub overarching_goal: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub agreement: Option, } @@ -110,18 +117,51 @@ impl IncludeOptions { pub fn needs_relationships(&self) -> bool { self.relationship || self.organization } + + /// Validates that include options are meaningful + /// Returns error if validation fails + pub fn validate(&self) -> Result<(), Error> { + // organization requires relationship (can't get org without relationship) + if self.organization && !self.relationship { + return Err(Error { + source: None, + error_kind: EntityApiErrorKind::InvalidQueryTerm, + }); + } + Ok(()) + } +} + +/// Sort field for coaching sessions +#[derive(Debug, Clone, Copy)] +pub enum SortField { + Date, + CreatedAt, + UpdatedAt, } -/// Find sessions by user with optional date filtering and related data includes +/// Sort order for queries +#[derive(Debug, Clone, Copy)] +pub enum SortOrder { + Asc, + Desc, +} + +/// Find sessions by user with optional date filtering, sorting, and related data includes pub async fn find_by_user_with_includes( db: &impl ConnectionTrait, user_id: Id, from_date: Option, to_date: Option, + sort_by: Option, + sort_order: Option, includes: IncludeOptions, ) -> Result, Error> { - // Load base sessions with date filtering - let sessions = load_sessions_for_user(db, user_id, from_date, to_date).await?; + // Validate include options + includes.validate()?; + + // Load base sessions with date filtering and sorting + let sessions = load_sessions_for_user(db, user_id, from_date, to_date, sort_by, sort_order).await?; // Early return if no includes requested if !includes.needs_relationships() && !includes.goal && !includes.agreements { @@ -138,12 +178,14 @@ pub async fn find_by_user_with_includes( .collect()) } -/// Load base sessions filtered by user and optional date range +/// Load base sessions filtered by user, optional date range, and sorting async fn load_sessions_for_user( db: &impl ConnectionTrait, user_id: Id, from_date: Option, to_date: Option, + sort_by: Option, + sort_order: Option, ) -> Result, Error> { let mut query = Entity::find() .join(JoinType::InnerJoin, Relation::CoachingRelationships.def()) @@ -163,6 +205,20 @@ async fn load_sessions_for_user( query = query.filter(coaching_sessions::Column::Date.lt(end_of_day)); } + // Apply sorting if both field and order are provided + if let (Some(field), Some(order)) = (sort_by, sort_order) { + let sea_order = match order { + SortOrder::Asc => sea_orm::Order::Asc, + SortOrder::Desc => sea_orm::Order::Desc, + }; + + query = match field { + SortField::Date => query.order_by(coaching_sessions::Column::Date, sea_order), + SortField::CreatedAt => query.order_by(coaching_sessions::Column::CreatedAt, sea_order), + SortField::UpdatedAt => query.order_by(coaching_sessions::Column::UpdatedAt, sea_order), + }; + } + query.all(db).await.map_err(Into::into) } @@ -496,7 +552,7 @@ mod tests { .into_connection(); let includes = IncludeOptions::none(); - let results = find_by_user_with_includes(&db, user_id, None, None, includes).await?; + let results = find_by_user_with_includes(&db, user_id, None, None, None, None, includes).await?; assert_eq!(results.len(), 1); assert_eq!(results[0].session.id, session_id); @@ -530,7 +586,7 @@ mod tests { .into_connection(); let includes = IncludeOptions::none(); - let results = find_by_user_with_includes(&db, user_id, Some(from_date), Some(to_date), includes).await?; + let results = find_by_user_with_includes(&db, user_id, Some(from_date), Some(to_date), None, None, includes).await?; assert_eq!(results.len(), 1); assert_eq!(results[0].session.id, session.id); @@ -594,4 +650,54 @@ mod tests { assert!(enriched.overarching_goal.is_none()); assert!(enriched.agreement.is_none()); } + + #[test] + fn validate_allows_organization_with_relationship() { + let includes = IncludeOptions { + relationship: true, + organization: true, + goal: false, + agreements: false, + }; + assert!(includes.validate().is_ok()); + } + + #[test] + fn validate_rejects_organization_without_relationship() { + let includes = IncludeOptions { + relationship: false, + organization: true, + goal: false, + agreements: false, + }; + assert!(includes.validate().is_err()); + } + + #[test] + fn validate_allows_goal_alone() { + let includes = IncludeOptions { + relationship: false, + organization: false, + goal: true, + agreements: false, + }; + assert!(includes.validate().is_ok()); + } + + #[test] + fn validate_allows_all_includes() { + let includes = IncludeOptions { + relationship: true, + organization: true, + goal: true, + agreements: true, + }; + assert!(includes.validate().is_ok()); + } + + #[test] + fn validate_allows_none() { + let includes = IncludeOptions::none(); + assert!(includes.validate().is_ok()); + } } diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs index 19a5968c..1f1c580b 100644 --- a/web/src/controller/user/coaching_session_controller.rs +++ b/web/src/controller/user/coaching_session_controller.rs @@ -1,12 +1,10 @@ use crate::controller::ApiResponse; -use crate::error::WebErrorKind; use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; use crate::params::coaching_session::SortField; use crate::params::sort::SortOrder; use crate::params::user::coaching_session::{IncludeParam, IndexParams}; -use crate::response::coaching_session::EnrichedCoachingSession; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; @@ -31,7 +29,7 @@ use log::*; ("sort_order" = Option, Query, description = "Sort order. Valid values: 'asc' (ascending), 'desc' (descending). Must be provided with sort_by.", example = "desc") ), responses( - (status = 200, description = "Successfully retrieved coaching sessions for user", body = [crate::response::coaching_session::EnrichedCoachingSession]), + (status = 200, description = "Successfully retrieved coaching sessions for user", body = [domain::coaching_session::EnrichedSession]), (status = 400, description = "Bad Request - Invalid include parameter"), (status = 401, description = "Unauthorized"), (status = 403, description = "Forbidden"), @@ -52,12 +50,6 @@ pub async fn index( debug!("GET Coaching Sessions for User: {user_id}"); debug!("Query Params: {params:?}"); - // Validate include parameters - if let Err(err_msg) = params.validate_includes() { - warn!("Invalid include parameters: {err_msg}"); - return Err(Error::Web(WebErrorKind::Input)); - } - // Build include options from parameters let includes = CoachingSessionApi::IncludeOptions { relationship: params.include.contains(&IncludeParam::Relationship), @@ -66,75 +58,38 @@ pub async fn index( agreements: params.include.contains(&IncludeParam::Agreements), }; - // Fetch sessions with optional includes - let mut enriched_sessions = CoachingSessionApi::find_by_user_with_includes( + // Convert web layer sort params to entity_api types + let entity_sort_by = params.sort_by.map(|sf| match sf { + SortField::Date => CoachingSessionApi::SortField::Date, + SortField::CreatedAt => CoachingSessionApi::SortField::CreatedAt, + SortField::UpdatedAt => CoachingSessionApi::SortField::UpdatedAt, + }); + + let entity_sort_order = params.sort_order.map(|so| match so { + SortOrder::Asc => CoachingSessionApi::SortOrder::Asc, + SortOrder::Desc => CoachingSessionApi::SortOrder::Desc, + }); + + // Fetch sessions with optional includes and sorting at database level + let enriched_sessions = CoachingSessionApi::find_by_user_with_includes( app_state.db_conn_ref(), user_id, params.from_date, params.to_date, + entity_sort_by, + entity_sort_order, includes, ) .await?; - // Apply sorting - if let (Some(sort_by), Some(sort_order)) = (params.sort_by, params.sort_order) { - let ascending = matches!(sort_order, SortOrder::Asc); - enriched_sessions.sort_by(|a, b| { - let cmp = match sort_by { - SortField::Date => a.session.date.cmp(&b.session.date), - SortField::CreatedAt => a.session.created_at.cmp(&b.session.created_at), - SortField::UpdatedAt => a.session.updated_at.cmp(&b.session.updated_at), - }; - if ascending { - cmp - } else { - cmp.reverse() - } - }); - } - debug!( "Found {} coaching sessions for user {user_id}", enriched_sessions.len() ); - // Convert to response DTOs - let response_sessions: Vec = enriched_sessions - .into_iter() - .map(|enriched| convert_to_response(enriched)) - .collect(); - + // Return entity_api type directly - it's already serializable Ok(Json(ApiResponse::new( StatusCode::OK.into(), - response_sessions, + enriched_sessions, ))) } - -/// Convert entity_api EnrichedSession to web response DTO -fn convert_to_response(enriched: CoachingSessionApi::EnrichedSession) -> EnrichedCoachingSession { - let mut response = EnrichedCoachingSession::from_model(enriched.session); - - // Add relationship data if present - if let (Some(relationship), Some(coach), Some(coachee)) = - (enriched.relationship, enriched.coach, enriched.coachee) - { - response = response.with_relationship(relationship, coach, coachee); - } - - // Add organization data if present - if let Some(organization) = enriched.organization { - response = response.with_organization(organization); - } - - // Add goal data if present - if let Some(goal) = enriched.overarching_goal { - response = response.with_goal(goal); - } - - // Add agreement data if present - if let Some(agreement) = enriched.agreement { - response = response.with_agreement(agreement); - } - - response -} diff --git a/web/src/lib.rs b/web/src/lib.rs index a1251752..4698166e 100644 --- a/web/src/lib.rs +++ b/web/src/lib.rs @@ -25,7 +25,6 @@ pub(crate) mod extractors; pub(crate) mod middleware; pub(crate) mod params; pub(crate) mod protect; -pub(crate) mod response; mod router; pub async fn init_server(app_state: AppState) -> Result<()> { diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 64d42858..d9b4d2d9 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -42,19 +42,15 @@ where match s { None => Ok(Vec::new()), Some(s) if s.is_empty() => Ok(Vec::new()), - Some(s) => { - let mut includes = Vec::new(); - for part in s.split(',') { - let trimmed = part.trim(); - if !trimmed.is_empty() { - let include: IncludeParam = serde_json::from_value( - serde_json::Value::String(trimmed.to_string()) - ).map_err(serde::de::Error::custom)?; - includes.push(include); - } - } - Ok(includes) - } + Some(s) => s + .split(',') + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(|s| { + serde_json::from_value(serde_json::Value::String(s.to_string())) + .map_err(serde::de::Error::custom) + }) + .collect(), } } @@ -85,113 +81,5 @@ impl IndexParams { self.sort_order = sort_order; self } - - /// Validates that include parameters are meaningful - /// Returns error message if validation fails - pub fn validate_includes(&self) -> Result<(), &'static str> { - // organization requires relationship (can't get org without relationship) - if self.include.contains(&IncludeParam::Organization) - && !self.include.contains(&IncludeParam::Relationship) { - return Err("Cannot include 'organization' without 'relationship'"); - } - Ok(()) - } } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn validate_includes_allows_organization_with_relationship() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![IncludeParam::Relationship, IncludeParam::Organization], - }; - - assert!(params.validate_includes().is_ok()); - } - - #[test] - fn validate_includes_rejects_organization_without_relationship() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![IncludeParam::Organization], - }; - - assert!(params.validate_includes().is_err()); - assert_eq!( - params.validate_includes().unwrap_err(), - "Cannot include 'organization' without 'relationship'" - ); - } - - #[test] - fn validate_includes_allows_goal_alone() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![IncludeParam::Goal], - }; - - assert!(params.validate_includes().is_ok()); - } - - #[test] - fn validate_includes_allows_agreements_alone() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![IncludeParam::Agreements], - }; - - assert!(params.validate_includes().is_ok()); - } - - #[test] - fn validate_includes_allows_all_includes() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![ - IncludeParam::Relationship, - IncludeParam::Organization, - IncludeParam::Goal, - IncludeParam::Agreements, - ], - }; - - assert!(params.validate_includes().is_ok()); - } - - #[test] - fn validate_includes_allows_empty_includes() { - let params = IndexParams { - user_id: Id::new_v4(), - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: vec![], - }; - - assert!(params.validate_includes().is_ok()); - } -} diff --git a/web/src/response/coaching_session.rs b/web/src/response/coaching_session.rs deleted file mode 100644 index c24fa77b..00000000 --- a/web/src/response/coaching_session.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! Enriched coaching session response DTOs -//! -//! These DTOs support optional inclusion of related resources to avoid N+1 queries. -//! Related data is only included when explicitly requested via the `include` parameter. - -use domain::agreements::Model as AgreementModel; -use domain::coaching_relationships::Model as CoachingRelationshipModel; -use domain::coaching_sessions::Model as CoachingSessionModel; -use domain::organizations::Model as OrganizationModel; -use domain::overarching_goals::Model as OverarchingGoalModel; -use domain::users::Model as UserModel; -use serde::Serialize; -use utoipa::ToSchema; - -/// Enriched coaching session with optional related resources -/// Reuses existing domain models to avoid duplication -#[derive(Debug, Clone, Serialize, ToSchema)] -pub struct EnrichedCoachingSession { - /// The coaching session itself - #[serde(flatten)] - pub session: CoachingSessionModel, - - /// Coaching relationship (only if ?include=relationship) - #[serde(skip_serializing_if = "Option::is_none")] - pub relationship: Option, - - /// Organization details (only if ?include=organization) - #[serde(skip_serializing_if = "Option::is_none")] - pub organization: Option, - - /// Overarching goal for this session (only if ?include=goal) - #[serde(skip_serializing_if = "Option::is_none")] - pub overarching_goal: Option, - - /// Agreement for this session (only if ?include=agreements) - #[serde(skip_serializing_if = "Option::is_none")] - pub agreement: Option, -} - -/// Relationship with coach and coachee user details -#[derive(Debug, Clone, Serialize, ToSchema)] -pub struct RelationshipWithUsers { - /// The relationship itself - #[serde(flatten)] - pub relationship: CoachingRelationshipModel, - - /// Coach user details - pub coach: UserModel, - - /// Coachee user details - pub coachee: UserModel, -} - -impl EnrichedCoachingSession { - /// Create from a basic coaching session model (no includes) - pub fn from_model(session: CoachingSessionModel) -> Self { - Self { - session, - relationship: None, - organization: None, - overarching_goal: None, - agreement: None, - } - } - - /// Add relationship data with coach and coachee users - pub fn with_relationship( - mut self, - relationship: CoachingRelationshipModel, - coach: UserModel, - coachee: UserModel, - ) -> Self { - self.relationship = Some(RelationshipWithUsers { - relationship, - coach, - coachee, - }); - self - } - - /// Add organization data - pub fn with_organization(mut self, organization: OrganizationModel) -> Self { - self.organization = Some(organization); - self - } - - /// Add overarching goal data - pub fn with_goal(mut self, goal: OverarchingGoalModel) -> Self { - self.overarching_goal = Some(goal); - self - } - - /// Add agreement data - pub fn with_agreement(mut self, agreement: AgreementModel) -> Self { - self.agreement = Some(agreement); - self - } -} diff --git a/web/src/response/mod.rs b/web/src/response/mod.rs deleted file mode 100644 index e2ef9fe0..00000000 --- a/web/src/response/mod.rs +++ /dev/null @@ -1 +0,0 @@ -pub mod coaching_session; From 502767eb67a401d79d24cd1a56f0707529e34fe1 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Thu, 6 Nov 2025 15:58:54 -0600 Subject: [PATCH 05/17] Add some helpful code documentation for the different types and methods added in this PR --- entity_api/src/coaching_session.rs | 100 ++++++++++++++++++++++-- web/src/params/user/action.rs | 17 +++- web/src/params/user/coaching_session.rs | 33 ++++++-- web/src/params/user/overarching_goal.rs | 16 +++- 4 files changed, 154 insertions(+), 12 deletions(-) diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 8c0af87e..c62c18c5 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -76,7 +76,39 @@ pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, } -/// Configuration for which related resources to include +/// Configuration for which related resources to include when fetching coaching sessions. +/// +/// # Purpose +/// This struct acts as a feature flag configuration for batch loading related resources. +/// It controls which additional database queries are executed beyond fetching the base +/// coaching sessions, enabling clients to request only the data they need. +/// +/// # Design Rationale +/// Using boolean flags instead of an enum allows for: +/// - Multiple resources to be requested simultaneously +/// - Fine-grained control over query execution +/// - Easy addition of new optional resources without breaking changes +/// - Zero-cost abstraction (Copy trait) for passing options around +/// +/// # Relationship Dependencies +/// Some resources have dependencies on others due to database foreign key relationships: +/// - `organization` requires `relationship` because organizations are linked via coaching_relationships +/// - The `validate()` method enforces these constraints at the API boundary +/// +/// # Usage Example +/// ```rust +/// // Create options requesting relationship and organization data +/// let mut options = IncludeOptions::none(); +/// options.relationship = true; +/// options.organization = true; +/// options.validate()?; // Passes: organization depends on relationship +/// +/// // This would fail validation: +/// let mut invalid = IncludeOptions::none(); +/// invalid.organization = true; // Without relationship: true +/// invalid.validate()?; // Error: organization requires relationship +/// ``` #[derive(Debug, Clone, Copy)] pub struct IncludeOptions { pub relationship: bool, @@ -105,6 +168,10 @@ pub struct IncludeOptions { } impl IncludeOptions { + /// Creates an `IncludeOptions` with all resources disabled. + /// + /// This is the baseline configuration - only the base coaching session data + /// will be fetched without any related resources. pub fn none() -> Self { Self { relationship: false, @@ -114,12 +181,27 @@ impl IncludeOptions { } } + /// Returns true if any option requires loading coaching_relationships data. + /// + /// This helper method determines whether we need to execute the batch query + /// for coaching_relationships. Currently, both the `relationship` and `organization` + /// options require this data (organization is accessed via relationship.organization_id). pub fn needs_relationships(&self) -> bool { self.relationship || self.organization } - /// Validates that include options are meaningful - /// Returns error if validation fails + /// Validates that the include options form a valid dependency graph. + /// + /// # Validation Rules + /// - `organization = true` requires `relationship = true` + /// (organizations are accessed through coaching_relationships) + /// + /// # Errors + /// Returns `EntityApiErrorKind::InvalidQueryTerm` if validation fails. + /// + /// # Why This Matters + /// Early validation at the entity_api layer prevents invalid database queries + /// and provides clear error messages to clients about invalid include combinations. pub fn validate(&self) -> Result<(), Error> { // organization requires relationship (can't get org without relationship) if self.organization && !self.relationship { @@ -222,7 +304,15 @@ async fn load_sessions_for_user( query.all(db).await.map_err(Into::into) } -/// Container for all related data loaded in batches +/// Internal lookup tables for batch-loaded related data (not serialized). +/// +/// This struct stores ALL related resources in HashMaps for O(1) lookup during assembly. +/// Contrast with `EnrichedSession`, which holds the specific related data for a single session. +/// +/// **Usage:** Temporary container used only within `find_by_user_with_includes`: +/// 1. Load phase: Execute bulk queries, populate these HashMaps +/// 2. Assembly phase: For each session, lookup its specific related data by ID +/// 3. Discard: This struct is not returned to clients #[derive(Debug, Default)] struct RelatedData { relationships: HashMap, diff --git a/web/src/params/user/action.rs b/web/src/params/user/action.rs index e907e1ac..b5d9919b 100644 --- a/web/src/params/user/action.rs +++ b/web/src/params/user/action.rs @@ -6,7 +6,9 @@ use crate::params::sort::SortOrder; use crate::params::WithSortDefaults; use domain::{actions, status::Status, Id, IntoQueryFilterMap, QueryFilterMap, QuerySort}; -/// Sortable fields for user actions +/// Sortable fields for user actions endpoint. +/// +/// Maps query parameter values (e.g., `?sort_by=due_by`) to database columns. #[derive(Debug, Deserialize, ToSchema)] #[schema(example = "created_at")] pub(crate) enum SortField { @@ -18,17 +20,27 @@ pub(crate) enum SortField { UpdatedAt, } +/// Query parameters for GET `/users/{user_id}/actions` endpoint. +/// +/// Supports filtering by coaching session and status, plus standard sorting. +/// The `user_id` is populated from the URL path parameter, not query string. #[derive(Debug, Deserialize, IntoParams)] pub(crate) struct IndexParams { + /// User ID from URL path (not a query parameter) #[serde(skip)] pub(crate) user_id: Id, + /// Optional: filter actions by coaching session pub(crate) coaching_session_id: Option, + /// Optional: filter actions by status (e.g., "open", "completed") pub(crate) status: Option, + /// Optional: field to sort by (defaults via WithSortDefaults) pub(crate) sort_by: Option, + /// Optional: sort direction (defaults via WithSortDefaults) pub(crate) sort_order: Option, } impl IndexParams { + /// Creates params with only user_id set (all filters empty). pub fn new(user_id: Id) -> Self { Self { user_id, @@ -39,6 +51,9 @@ impl IndexParams { } } + /// Builder method to add optional filters and sorting. + /// + /// Useful for programmatically constructing params in tests or internal code. pub fn with_filters( mut self, coaching_session_id: Option, diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index d9b4d2d9..3ba53ac1 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -6,29 +6,48 @@ use crate::params::coaching_session::SortField; use crate::params::sort::SortOrder; use domain::Id; -/// Include parameter for optionally fetching related resources -/// Supports comma-separated values: ?include=relationship,organization,goal,agreements +/// Related resources that can be batch-loaded with coaching sessions. +/// +/// Used in `?include=` query parameter to eliminate N+1 queries. Supports +/// comma-separated values: `?include=relationship,organization,goal,agreements` +/// +/// Maps to `entity_api::coaching_session::IncludeOptions` for database queries. #[derive(Debug, Clone, Deserialize, PartialEq, Eq, Hash)] #[serde(rename_all = "lowercase")] pub enum IncludeParam { + /// Include coaching relationship (coach/coachee info) Relationship, + /// Include organization (requires relationship) Organization, + /// Include overarching goal Goal, + /// Include session agreements Agreements, } +/// Query parameters for GET `/users/{user_id}/coaching_sessions` endpoint. +/// +/// Supports date range filtering, sorting, and optional batch loading of related resources. +/// The enhanced `include` parameter enables efficient data fetching (see `IncludeParam`). #[derive(Debug, Deserialize, IntoParams)] pub(crate) struct IndexParams { + /// User ID from URL path (not a query parameter) #[serde(skip)] #[allow(dead_code)] pub(crate) user_id: Id, + /// Optional: filter sessions starting from this date (inclusive) pub(crate) from_date: Option, + /// Optional: filter sessions up to this date (inclusive) pub(crate) to_date: Option, + /// Optional: field to sort by (e.g., "date", "created_at") pub(crate) sort_by: Option, + /// Optional: sort direction (asc/desc) pub(crate) sort_order: Option, - /// Comma-separated list of related resources to include - /// Valid values: relationship, organization, goal, agreements - /// Example: ?include=relationship,organization,goal + /// Optional: comma-separated list of related resources to batch-load + /// + /// Example: `?include=relationship,organization,goal` + /// + /// See `IncludeParam` for valid values and N+1 query optimization details. #[serde(default, deserialize_with = "deserialize_comma_separated")] pub(crate) include: Vec, } @@ -55,6 +74,7 @@ where } impl IndexParams { + /// Creates params with only user_id set (all filters empty, no includes). #[allow(dead_code)] pub fn new(user_id: Id) -> Self { Self { @@ -67,6 +87,9 @@ impl IndexParams { } } + /// Builder method to add date range filtering and sorting. + /// + /// Note: Does not set `include` - use field access to add related resources. #[allow(dead_code)] pub fn with_filters( mut self, diff --git a/web/src/params/user/overarching_goal.rs b/web/src/params/user/overarching_goal.rs index 3c8abade..6f132a71 100644 --- a/web/src/params/user/overarching_goal.rs +++ b/web/src/params/user/overarching_goal.rs @@ -6,7 +6,9 @@ use crate::params::sort::SortOrder; use crate::params::WithSortDefaults; use domain::{overarching_goals, Id, IntoQueryFilterMap, QueryFilterMap, QuerySort}; -/// Sortable fields for user overarching goals +/// Sortable fields for user overarching goals endpoint. +/// +/// Maps query parameter values (e.g., `?sort_by=title`) to database columns. #[derive(Debug, Deserialize, ToSchema)] #[schema(example = "title")] pub(crate) enum SortField { @@ -18,16 +20,25 @@ pub(crate) enum SortField { UpdatedAt, } +/// Query parameters for GET `/users/{user_id}/overarching_goals` endpoint. +/// +/// Supports filtering by coaching session and standard sorting. +/// Overarching goals are long-term objectives that span multiple coaching sessions. #[derive(Debug, Deserialize, IntoParams)] pub(crate) struct IndexParams { + /// User ID from URL path (not a query parameter) #[serde(skip)] pub(crate) user_id: Id, + /// Optional: filter goals associated with a specific coaching session pub(crate) coaching_session_id: Option, + /// Optional: field to sort by (defaults via WithSortDefaults) pub(crate) sort_by: Option, + /// Optional: sort direction (defaults via WithSortDefaults) pub(crate) sort_order: Option, } impl IndexParams { + /// Creates params with only user_id set (all filters empty). pub fn new(user_id: Id) -> Self { Self { user_id, @@ -37,6 +48,9 @@ impl IndexParams { } } + /// Builder method to add optional coaching session filter and sorting. + /// + /// Useful for programmatically constructing params in tests or internal code. pub fn with_filters( mut self, coaching_session_id: Option, From 2d3df6ac57d9f9954ecb96c1fd5a8ec0b848e3d6 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Thu, 6 Nov 2025 16:27:15 -0600 Subject: [PATCH 06/17] Run cargo fmt --- domain/src/coaching_session.rs | 4 +- entity_api/src/coaching_session.rs | 42 +++++++++++++++---- .../user/organization_controller.rs | 5 ++- .../user/overarching_goal_controller.rs | 10 ++++- web/src/params/user/coaching_session.rs | 1 - 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index fcd374eb..86970afb 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -12,8 +12,8 @@ use sea_orm::{DatabaseConnection, IntoActiveModel}; use service::config::Config; pub use entity_api::coaching_session::{ - find_by_id, find_by_id_with_coaching_relationship, find_by_user_with_includes, - EnrichedSession, IncludeOptions, SortField, SortOrder, + find_by_id, find_by_id_with_coaching_relationship, find_by_user_with_includes, EnrichedSession, + IncludeOptions, SortField, SortOrder, }; #[derive(Debug, Clone)] diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index c62c18c5..485c1407 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -5,7 +5,9 @@ use entity::{ organizations, overarching_goals, users, Id, }; use log::debug; -use sea_orm::{entity::prelude::*, DatabaseConnection, JoinType, QueryOrder, QuerySelect, Set, TryIntoModel}; +use sea_orm::{ + entity::prelude::*, DatabaseConnection, JoinType, QueryOrder, QuerySelect, Set, TryIntoModel, +}; use std::collections::HashMap; pub async fn create( @@ -243,11 +245,15 @@ pub async fn find_by_user_with_includes( includes.validate()?; // Load base sessions with date filtering and sorting - let sessions = load_sessions_for_user(db, user_id, from_date, to_date, sort_by, sort_order).await?; + let sessions = + load_sessions_for_user(db, user_id, from_date, to_date, sort_by, sort_order).await?; // Early return if no includes requested if !includes.needs_relationships() && !includes.goal && !includes.agreements { - return Ok(sessions.into_iter().map(EnrichedSession::from_session).collect()); + return Ok(sessions + .into_iter() + .map(EnrichedSession::from_session) + .collect()); } // Load all related data in efficient batches @@ -332,7 +338,10 @@ async fn load_related_data( let mut data = RelatedData::default(); // Extract IDs for batch loading - let relationship_ids: Vec = sessions.iter().map(|s| s.coaching_relationship_id).collect(); + let relationship_ids: Vec = sessions + .iter() + .map(|s| s.coaching_relationship_id) + .collect(); let session_ids: Vec = sessions.iter().map(|s| s.id).collect(); // Load relationships (needed for both relationship and organization includes) @@ -351,7 +360,11 @@ async fn load_related_data( // Load organizations if requested if includes.organization { - let org_ids: Vec = data.relationships.values().map(|r| r.organization_id).collect(); + let org_ids: Vec = data + .relationships + .values() + .map(|r| r.organization_id) + .collect(); data.organizations = batch_load_organizations(db, &org_ids).await?; } @@ -460,7 +473,10 @@ async fn batch_load_agreements( /// Assemble an enriched session from base session and related data fn assemble_enriched_session(session: Model, related: &RelatedData) -> EnrichedSession { - let relationship = related.relationships.get(&session.coaching_relationship_id).cloned(); + let relationship = related + .relationships + .get(&session.coaching_relationship_id) + .cloned(); let (coach, coachee) = relationship .as_ref() @@ -642,7 +658,8 @@ mod tests { .into_connection(); let includes = IncludeOptions::none(); - let results = find_by_user_with_includes(&db, user_id, None, None, None, None, includes).await?; + let results = + find_by_user_with_includes(&db, user_id, None, None, None, None, includes).await?; assert_eq!(results.len(), 1); assert_eq!(results[0].session.id, session_id); @@ -676,7 +693,16 @@ mod tests { .into_connection(); let includes = IncludeOptions::none(); - let results = find_by_user_with_includes(&db, user_id, Some(from_date), Some(to_date), None, None, includes).await?; + let results = find_by_user_with_includes( + &db, + user_id, + Some(from_date), + Some(to_date), + None, + None, + includes, + ) + .await?; assert_eq!(results.len(), 1); assert_eq!(results[0].session.id, session.id); diff --git a/web/src/controller/user/organization_controller.rs b/web/src/controller/user/organization_controller.rs index 6127e503..f799de99 100644 --- a/web/src/controller/user/organization_controller.rs +++ b/web/src/controller/user/organization_controller.rs @@ -41,7 +41,10 @@ pub async fn index( let organizations = OrganizationApi::find_by_user(app_state.db_conn_ref(), user_id).await?; - debug!("Found {} organizations for user {user_id}", organizations.len()); + debug!( + "Found {} organizations for user {user_id}", + organizations.len() + ); Ok(Json(ApiResponse::new(StatusCode::OK.into(), organizations))) } diff --git a/web/src/controller/user/overarching_goal_controller.rs b/web/src/controller/user/overarching_goal_controller.rs index 07a19a02..95676463 100644 --- a/web/src/controller/user/overarching_goal_controller.rs +++ b/web/src/controller/user/overarching_goal_controller.rs @@ -70,7 +70,13 @@ pub async fn index( let overarching_goals = OverarchingGoalApi::find_by(app_state.db_conn_ref(), params).await?; - debug!("Found {} overarching goals for user {user_id}", overarching_goals.len()); + debug!( + "Found {} overarching goals for user {user_id}", + overarching_goals.len() + ); - Ok(Json(ApiResponse::new(StatusCode::OK.into(), overarching_goals))) + Ok(Json(ApiResponse::new( + StatusCode::OK.into(), + overarching_goals, + ))) } diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 3ba53ac1..70737ddf 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -105,4 +105,3 @@ impl IndexParams { self } } - From b3d95b42188bb691f4ada0ef38b64851ec68c517 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Thu, 6 Nov 2025 16:51:32 -0600 Subject: [PATCH 07/17] Fix doc-test for IncludeOptions usage example Add missing import and fix assertions in documentation example to make the doc-test pass. The example now properly demonstrates both valid and invalid usage patterns with appropriate assertions. --- entity_api/src/coaching_session.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 485c1407..31154a28 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -150,16 +150,18 @@ pub struct EnrichedSession { /// /// # Usage Example /// ```rust +/// use entity_api::coaching_session::IncludeOptions; +/// /// // Create options requesting relationship and organization data /// let mut options = IncludeOptions::none(); /// options.relationship = true; /// options.organization = true; -/// options.validate()?; // Passes: organization depends on relationship +/// options.validate().unwrap(); // Passes: organization depends on relationship /// /// // This would fail validation: /// let mut invalid = IncludeOptions::none(); /// invalid.organization = true; // Without relationship: true -/// invalid.validate()?; // Error: organization requires relationship +/// assert!(invalid.validate().is_err()); // Error: organization requires relationship /// ``` #[derive(Debug, Clone, Copy)] pub struct IncludeOptions { From b17a6e8b09d10dabd72c67912e80676f912aed89 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Thu, 6 Nov 2025 16:58:10 -0600 Subject: [PATCH 08/17] Fix clippy derivable_impls warning for Status enum Replace manual Default implementation with derived Default trait. Mark InProgress as the default variant using #[default] attribute. --- entity/src/status.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/entity/src/status.rs b/entity/src/status.rs index 7f45cb73..e0a93cb1 100644 --- a/entity/src/status.rs +++ b/entity/src/status.rs @@ -1,12 +1,15 @@ use sea_orm::entity::prelude::*; use serde::{Deserialize, Serialize}; -#[derive(Debug, Clone, Eq, PartialEq, EnumIter, Deserialize, Serialize, DeriveActiveEnum)] +#[derive( + Debug, Clone, Eq, PartialEq, EnumIter, Deserialize, Serialize, DeriveActiveEnum, Default, +)] #[sea_orm(rs_type = "String", db_type = "Enum", enum_name = "status")] pub enum Status { #[sea_orm(string_value = "not_started")] NotStarted, #[sea_orm(string_value = "in_progress")] + #[default] InProgress, #[sea_orm(string_value = "completed")] Completed, @@ -14,12 +17,6 @@ pub enum Status { WontDo, } -impl std::default::Default for Status { - fn default() -> Self { - Self::InProgress - } -} - impl From<&str> for Status { fn from(value: &str) -> Self { match value { From 43d1a81f536bcb730f54287a5f413315ae49ae96 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 8 Nov 2025 12:23:30 -0600 Subject: [PATCH 09/17] Clean up bad rebase merge conflict from previous commit --- entity_api/src/organization.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/entity_api/src/organization.rs b/entity_api/src/organization.rs index 7f6d7d63..bc3bffa6 100644 --- a/entity_api/src/organization.rs +++ b/entity_api/src/organization.rs @@ -110,13 +110,8 @@ pub async fn find_by_user(db: &impl ConnectionTrait, user_id: Id) -> Result, user_id: Id) -> Select { query - .join_as( - JoinType::InnerJoin, - user_roles::Relation::Organizations.def().rev(), - user_roles::Entity, - ) + .join(JoinType::InnerJoin, Relation::UserRoles.def()) .filter(user_roles::Column::UserId.eq(user_id)) - .filter(user_roles::Column::OrganizationId.is_not_null()) .distinct() } @@ -182,7 +177,7 @@ mod tests { ), Transaction::from_sql_and_values( DatabaseBackend::Postgres, - r#"SELECT DISTINCT "organizations"."id", "organizations"."name", "organizations"."logo", "organizations"."slug", "organizations"."created_at", "organizations"."updated_at" FROM "refactor_platform"."organizations" INNER JOIN "refactor_platform"."user_roles" AS "user_roles" ON "organizations"."id" = "user_roles"."organization_id" WHERE "user_roles"."user_id" = $1 AND "user_roles"."organization_id" IS NOT NULL"#, + r#"SELECT DISTINCT "organizations"."id", "organizations"."name", "organizations"."logo", "organizations"."slug", "organizations"."created_at", "organizations"."updated_at" FROM "refactor_platform"."organizations" INNER JOIN "refactor_platform"."user_roles" ON "organizations"."id" = "user_roles"."organization_id" WHERE "user_roles"."user_id" = $1"#, [user_id.into()] ) ] From 51d58964b1c2bb7bd014ba59f039da566456dba1 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 8 Nov 2025 12:32:58 -0600 Subject: [PATCH 10/17] Reformat Status's display trait implementation to use human-friendly strings --- entity/src/status.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/entity/src/status.rs b/entity/src/status.rs index e0a93cb1..e020d618 100644 --- a/entity/src/status.rs +++ b/entity/src/status.rs @@ -32,10 +32,10 @@ impl From<&str> for Status { impl std::fmt::Display for Status { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::NotStarted => write!(f, "not_started"), - Self::InProgress => write!(f, "in_progress"), - Self::Completed => write!(f, "completed"), - Self::WontDo => write!(f, "wont_do"), + Self::NotStarted => write!(f, "Not Started"), + Self::InProgress => write!(f, "In Progress"), + Self::Completed => write!(f, "Completed"), + Self::WontDo => write!(f, "Won't Do"), } } } From 77d70dc585e922d9564e1c142ead8ed2d2873e8e Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 8 Nov 2025 12:46:17 -0600 Subject: [PATCH 11/17] Don't duplicate sorting params from the web layer into the entity_api layer --- domain/src/coaching_session.rs | 2 +- entity_api/src/coaching_session.rs | 86 ++++++------------- .../user/coaching_session_controller.rs | 29 +++---- web/src/params/user/coaching_session.rs | 25 +++++- 4 files changed, 64 insertions(+), 78 deletions(-) diff --git a/domain/src/coaching_session.rs b/domain/src/coaching_session.rs index 86970afb..a77dab19 100644 --- a/domain/src/coaching_session.rs +++ b/domain/src/coaching_session.rs @@ -13,7 +13,7 @@ use service::config::Config; pub use entity_api::coaching_session::{ find_by_id, find_by_id_with_coaching_relationship, find_by_user_with_includes, EnrichedSession, - IncludeOptions, SortField, SortOrder, + IncludeOptions, }; #[derive(Debug, Clone)] diff --git a/entity_api/src/coaching_session.rs b/entity_api/src/coaching_session.rs index 31154a28..e0ba14c2 100644 --- a/entity_api/src/coaching_session.rs +++ b/entity_api/src/coaching_session.rs @@ -217,66 +217,20 @@ impl IncludeOptions { Ok(()) } } - -/// Sort field for coaching sessions -#[derive(Debug, Clone, Copy)] -pub enum SortField { - Date, - CreatedAt, - UpdatedAt, -} - -/// Sort order for queries -#[derive(Debug, Clone, Copy)] -pub enum SortOrder { - Asc, - Desc, -} - /// Find sessions by user with optional date filtering, sorting, and related data includes pub async fn find_by_user_with_includes( db: &impl ConnectionTrait, user_id: Id, from_date: Option, to_date: Option, - sort_by: Option, - sort_order: Option, + sort_column: Option, + sort_order: Option, includes: IncludeOptions, ) -> Result, Error> { // Validate include options includes.validate()?; - // Load base sessions with date filtering and sorting - let sessions = - load_sessions_for_user(db, user_id, from_date, to_date, sort_by, sort_order).await?; - - // Early return if no includes requested - if !includes.needs_relationships() && !includes.goal && !includes.agreements { - return Ok(sessions - .into_iter() - .map(EnrichedSession::from_session) - .collect()); - } - - // Load all related data in efficient batches - let related_data = load_related_data(db, &sessions, includes).await?; - - // Assemble enriched sessions - Ok(sessions - .into_iter() - .map(|session| assemble_enriched_session(session, &related_data)) - .collect()) -} - -/// Load base sessions filtered by user, optional date range, and sorting -async fn load_sessions_for_user( - db: &impl ConnectionTrait, - user_id: Id, - from_date: Option, - to_date: Option, - sort_by: Option, - sort_order: Option, -) -> Result, Error> { + // Build query for sessions filtered by user let mut query = Entity::find() .join(JoinType::InnerJoin, Relation::CoachingRelationships.def()) .filter( @@ -285,6 +239,7 @@ async fn load_sessions_for_user( .or(coaching_relationships::Column::CoacheeId.eq(user_id)), ); + // Apply date filtering if let Some(from) = from_date { query = query.filter(coaching_sessions::Column::Date.gte(from)); } @@ -295,21 +250,30 @@ async fn load_sessions_for_user( query = query.filter(coaching_sessions::Column::Date.lt(end_of_day)); } - // Apply sorting if both field and order are provided - if let (Some(field), Some(order)) = (sort_by, sort_order) { - let sea_order = match order { - SortOrder::Asc => sea_orm::Order::Asc, - SortOrder::Desc => sea_orm::Order::Desc, - }; + // Apply sorting if both column and order are provided + if let (Some(column), Some(order)) = (sort_column, sort_order) { + query = query.order_by(column, order); + } - query = match field { - SortField::Date => query.order_by(coaching_sessions::Column::Date, sea_order), - SortField::CreatedAt => query.order_by(coaching_sessions::Column::CreatedAt, sea_order), - SortField::UpdatedAt => query.order_by(coaching_sessions::Column::UpdatedAt, sea_order), - }; + // Execute query to load base sessions + let sessions = query.all(db).await?; + + // Early return if no includes requested + if !includes.needs_relationships() && !includes.goal && !includes.agreements { + return Ok(sessions + .into_iter() + .map(EnrichedSession::from_session) + .collect()); } - query.all(db).await.map_err(Into::into) + // Load all related data in efficient batches + let related_data = load_related_data(db, &sessions, includes).await?; + + // Assemble enriched sessions + Ok(sessions + .into_iter() + .map(|session| assemble_enriched_session(session, &related_data)) + .collect()) } /// Internal lookup tables for batch-loaded related data (not serialized). diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs index 1f1c580b..2504d9f8 100644 --- a/web/src/controller/user/coaching_session_controller.rs +++ b/web/src/controller/user/coaching_session_controller.rs @@ -3,14 +3,14 @@ use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; use crate::params::coaching_session::SortField; -use crate::params::sort::SortOrder; use crate::params::user::coaching_session::{IncludeParam, IndexParams}; +use crate::params::WithSortDefaults; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::Json; -use domain::{coaching_session as CoachingSessionApi, Id}; +use domain::{coaching_session as CoachingSessionApi, Id, QuerySort}; use service::config::ApiVersion; use log::*; @@ -45,7 +45,7 @@ pub async fn index( AuthenticatedUser(_user): AuthenticatedUser, State(app_state): State, Path(user_id): Path, - Query(params): Query, + Query(mut params): Query, ) -> Result { debug!("GET Coaching Sessions for User: {user_id}"); debug!("Query Params: {params:?}"); @@ -58,17 +58,16 @@ pub async fn index( agreements: params.include.contains(&IncludeParam::Agreements), }; - // Convert web layer sort params to entity_api types - let entity_sort_by = params.sort_by.map(|sf| match sf { - SortField::Date => CoachingSessionApi::SortField::Date, - SortField::CreatedAt => CoachingSessionApi::SortField::CreatedAt, - SortField::UpdatedAt => CoachingSessionApi::SortField::UpdatedAt, - }); + // Apply sort defaults if needed + ::apply_sort_defaults( + &mut params.sort_by, + &mut params.sort_order, + SortField::Date, + ); - let entity_sort_order = params.sort_order.map(|so| match so { - SortOrder::Asc => CoachingSessionApi::SortOrder::Asc, - SortOrder::Desc => CoachingSessionApi::SortOrder::Desc, - }); + // Use QuerySort trait to get SeaORM types + let sort_column = params.get_sort_column(); + let sort_order = params.get_sort_order(); // Fetch sessions with optional includes and sorting at database level let enriched_sessions = CoachingSessionApi::find_by_user_with_includes( @@ -76,8 +75,8 @@ pub async fn index( user_id, params.from_date, params.to_date, - entity_sort_by, - entity_sort_order, + sort_column, + sort_order, includes, ) .await?; diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 70737ddf..fd8836e7 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -1,10 +1,12 @@ use chrono::NaiveDate; +use sea_orm::Order; use serde::Deserialize; use utoipa::IntoParams; use crate::params::coaching_session::SortField; use crate::params::sort::SortOrder; -use domain::Id; +use crate::params::WithSortDefaults; +use domain::{coaching_sessions, Id, QuerySort}; /// Related resources that can be batch-loaded with coaching sessions. /// @@ -105,3 +107,24 @@ impl IndexParams { self } } + +impl QuerySort for IndexParams { + fn get_sort_column(&self) -> Option { + self.sort_by.as_ref().map(|field| match field { + SortField::Date => coaching_sessions::Column::Date, + SortField::CreatedAt => coaching_sessions::Column::CreatedAt, + SortField::UpdatedAt => coaching_sessions::Column::UpdatedAt, + }) + } + + fn get_sort_order(&self) -> Option { + self.sort_order.as_ref().map(|order| match order { + SortOrder::Asc => Order::Asc, + SortOrder::Desc => Order::Desc, + }) + } +} + +impl WithSortDefaults for IndexParams { + type SortField = SortField; +} From b307fd382660620415f7776410f53df5ced8244a Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sat, 8 Nov 2025 13:04:13 -0600 Subject: [PATCH 12/17] Follow the established protect structure already laid out in web/src/protect/organizations --- web/src/protect/users/actions.rs | 29 +++++++ web/src/protect/users/coaching_sessions.rs | 29 +++++++ web/src/protect/users/mod.rs | 94 ++-------------------- web/src/protect/users/organizations.rs | 29 +++++++ web/src/protect/users/overarching_goals.rs | 29 +++++++ web/src/router.rs | 8 +- 6 files changed, 126 insertions(+), 92 deletions(-) create mode 100644 web/src/protect/users/actions.rs create mode 100644 web/src/protect/users/coaching_sessions.rs create mode 100644 web/src/protect/users/organizations.rs create mode 100644 web/src/protect/users/overarching_goals.rs diff --git a/web/src/protect/users/actions.rs b/web/src/protect/users/actions.rs new file mode 100644 index 00000000..02f4fd47 --- /dev/null +++ b/web/src/protect/users/actions.rs @@ -0,0 +1,29 @@ +use crate::{extractors::authenticated_user::AuthenticatedUser, AppState}; +use axum::{ + extract::{Path, Request, State}, + http::StatusCode, + middleware::Next, + response::IntoResponse, +}; +use domain::Id; +use log::*; + +/// Checks that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn index( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own actions (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} diff --git a/web/src/protect/users/coaching_sessions.rs b/web/src/protect/users/coaching_sessions.rs new file mode 100644 index 00000000..03361855 --- /dev/null +++ b/web/src/protect/users/coaching_sessions.rs @@ -0,0 +1,29 @@ +use crate::{extractors::authenticated_user::AuthenticatedUser, AppState}; +use axum::{ + extract::{Path, Request, State}, + http::StatusCode, + middleware::Next, + response::IntoResponse, +}; +use domain::Id; +use log::*; + +/// Checks that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn index( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own coaching sessions (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} diff --git a/web/src/protect/users/mod.rs b/web/src/protect/users/mod.rs index 61242c14..822f78a4 100644 --- a/web/src/protect/users/mod.rs +++ b/web/src/protect/users/mod.rs @@ -8,94 +8,13 @@ use axum::{ use domain::Id; use log::*; +pub(crate) mod actions; +pub(crate) mod coaching_sessions; +pub(crate) mod organizations; +pub(crate) mod overarching_goals; pub(crate) mod passwords; -// checks: -// - that the `user_id` matches the `authenticated_user.id` -pub(crate) async fn organizations( - State(_app_state): State, - AuthenticatedUser(authenticated_user): AuthenticatedUser, - Path(user_id): Path, - request: Request, - next: Next, -) -> impl IntoResponse { - // check that we are only allowing authenticated users to read their own organizations (for now) - if authenticated_user.id == user_id { - next.run(request).await - } else { - error!( - "Unauthorized: user_id {} does not match authenticated_user_id {}", - user_id, authenticated_user.id - ); - (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() - } -} - -// checks: -// - that the `user_id` matches the `authenticated_user.id` -pub(crate) async fn actions( - State(_app_state): State, - AuthenticatedUser(authenticated_user): AuthenticatedUser, - Path(user_id): Path, - request: Request, - next: Next, -) -> impl IntoResponse { - // check that we are only allowing authenticated users to read their own actions (for now) - if authenticated_user.id == user_id { - next.run(request).await - } else { - error!( - "Unauthorized: user_id {} does not match authenticated_user_id {}", - user_id, authenticated_user.id - ); - (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() - } -} - -// checks: -// - that the `user_id` matches the `authenticated_user.id` -pub(crate) async fn coaching_sessions( - State(_app_state): State, - AuthenticatedUser(authenticated_user): AuthenticatedUser, - Path(user_id): Path, - request: Request, - next: Next, -) -> impl IntoResponse { - // check that we are only allowing authenticated users to read their own coaching sessions (for now) - if authenticated_user.id == user_id { - next.run(request).await - } else { - error!( - "Unauthorized: user_id {} does not match authenticated_user_id {}", - user_id, authenticated_user.id - ); - (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() - } -} - -// checks: -// - that the `user_id` matches the `authenticated_user.id` -pub(crate) async fn overarching_goals( - State(_app_state): State, - AuthenticatedUser(authenticated_user): AuthenticatedUser, - Path(user_id): Path, - request: Request, - next: Next, -) -> impl IntoResponse { - // check that we are only allowing authenticated users to read their own overarching goals (for now) - if authenticated_user.id == user_id { - next.run(request).await - } else { - error!( - "Unauthorized: user_id {} does not match authenticated_user_id {}", - user_id, authenticated_user.id - ); - (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() - } -} - -// checks: -// - that the `user_id` matches the `authenticated_user.id` +/// Checks that the `user_id` matches the `authenticated_user.id` pub(crate) async fn read( State(_app_state): State, AuthenticatedUser(authenticated_user): AuthenticatedUser, @@ -115,8 +34,7 @@ pub(crate) async fn read( } } -// checks: -// - that the `user_id` matches the `authenticated_user.id` +/// Checks that the `user_id` matches the `authenticated_user.id` pub(crate) async fn update( State(_app_state): State, AuthenticatedUser(authenticated_user): AuthenticatedUser, diff --git a/web/src/protect/users/organizations.rs b/web/src/protect/users/organizations.rs new file mode 100644 index 00000000..768a4a6f --- /dev/null +++ b/web/src/protect/users/organizations.rs @@ -0,0 +1,29 @@ +use crate::{extractors::authenticated_user::AuthenticatedUser, AppState}; +use axum::{ + extract::{Path, Request, State}, + http::StatusCode, + middleware::Next, + response::IntoResponse, +}; +use domain::Id; +use log::*; + +/// Checks that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn index( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own organizations (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} diff --git a/web/src/protect/users/overarching_goals.rs b/web/src/protect/users/overarching_goals.rs new file mode 100644 index 00000000..eeb186ad --- /dev/null +++ b/web/src/protect/users/overarching_goals.rs @@ -0,0 +1,29 @@ +use crate::{extractors::authenticated_user::AuthenticatedUser, AppState}; +use axum::{ + extract::{Path, Request, State}, + http::StatusCode, + middleware::Next, + response::IntoResponse, +}; +use domain::Id; +use log::*; + +/// Checks that the `user_id` matches the `authenticated_user.id` +pub(crate) async fn index( + State(_app_state): State, + AuthenticatedUser(authenticated_user): AuthenticatedUser, + Path(user_id): Path, + request: Request, + next: Next, +) -> impl IntoResponse { + // check that we are only allowing authenticated users to read their own overarching goals (for now) + if authenticated_user.id == user_id { + next.run(request).await + } else { + error!( + "Unauthorized: user_id {} does not match authenticated_user_id {}", + user_id, authenticated_user.id + ); + (StatusCode::UNAUTHORIZED, "Unauthorized").into_response() + } +} diff --git a/web/src/router.rs b/web/src/router.rs index 1d663b30..5550c30e 100644 --- a/web/src/router.rs +++ b/web/src/router.rs @@ -450,7 +450,7 @@ fn user_organizations_routes(app_state: AppState) -> Router { ) .route_layer(from_fn_with_state( app_state.clone(), - protect::users::organizations, + protect::users::organizations::index, )), ) .route_layer(from_fn(require_auth)) @@ -467,7 +467,7 @@ fn user_actions_routes(app_state: AppState) -> Router { ) .route_layer(from_fn_with_state( app_state.clone(), - protect::users::actions, + protect::users::actions::index, )), ) .route_layer(from_fn(require_auth)) @@ -484,7 +484,7 @@ fn user_coaching_sessions_routes(app_state: AppState) -> Router { ) .route_layer(from_fn_with_state( app_state.clone(), - protect::users::coaching_sessions, + protect::users::coaching_sessions::index, )), ) .route_layer(from_fn(require_auth)) @@ -501,7 +501,7 @@ fn user_overarching_goals_routes(app_state: AppState) -> Router { ) .route_layer(from_fn_with_state( app_state.clone(), - protect::users::overarching_goals, + protect::users::overarching_goals::index, )), ) .route_layer(from_fn(require_auth)) From af161566eedffcaa7d6dd7a907df6ba747f43162 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 9 Nov 2025 12:30:16 -0600 Subject: [PATCH 13/17] Use IndexParams directly in the action_controller.rs index method --- web/src/controller/user/action_controller.rs | 24 ++++------------ web/src/params/user/action.rs | 29 ++++---------------- 2 files changed, 10 insertions(+), 43 deletions(-) diff --git a/web/src/controller/user/action_controller.rs b/web/src/controller/user/action_controller.rs index 0b50a0d3..49c0663f 100644 --- a/web/src/controller/user/action_controller.rs +++ b/web/src/controller/user/action_controller.rs @@ -9,20 +9,11 @@ use axum::extract::{Path, Query, State}; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::Json; -use domain::{action as ActionApi, status::Status, Id}; -use serde::Deserialize; +use domain::{action as ActionApi, Id}; use service::config::ApiVersion; use log::*; -#[derive(Debug, Deserialize)] -pub(crate) struct QueryParams { - pub(crate) coaching_session_id: Option, - pub(crate) status: Option, - pub(crate) sort_by: Option, - pub(crate) sort_order: Option, -} - /// GET all actions for a specific user #[utoipa::path( get, @@ -51,18 +42,13 @@ pub async fn index( AuthenticatedUser(_user): AuthenticatedUser, State(app_state): State, Path(user_id): Path, - Query(query_params): Query, + Query(params): Query, ) -> Result { debug!("GET Actions for User: {user_id}"); - debug!("Filter Params: {query_params:?}"); + debug!("Filter Params: {params:?}"); - // Build params with user_id from path - let mut params = IndexParams::new(user_id).with_filters( - query_params.coaching_session_id, - query_params.status, - query_params.sort_by, - query_params.sort_order, - ); + // Set user_id from path parameter + let mut params = params.with_user_id(user_id); // Apply default sorting parameters IndexParams::apply_sort_defaults( diff --git a/web/src/params/user/action.rs b/web/src/params/user/action.rs index b5d9919b..1263ccd2 100644 --- a/web/src/params/user/action.rs +++ b/web/src/params/user/action.rs @@ -40,31 +40,12 @@ pub(crate) struct IndexParams { } impl IndexParams { - /// Creates params with only user_id set (all filters empty). - pub fn new(user_id: Id) -> Self { - Self { - user_id, - coaching_session_id: None, - status: None, - sort_by: None, - sort_order: None, - } - } - - /// Builder method to add optional filters and sorting. + /// Sets the user_id field (useful when user_id comes from path parameter). /// - /// Useful for programmatically constructing params in tests or internal code. - pub fn with_filters( - mut self, - coaching_session_id: Option, - status: Option, - sort_by: Option, - sort_order: Option, - ) -> Self { - self.coaching_session_id = coaching_session_id; - self.status = status; - self.sort_by = sort_by; - self.sort_order = sort_order; + /// This allows using `Query` to deserialize query parameters, + /// then setting the path-based user_id afterward. + pub fn with_user_id(mut self, user_id: Id) -> Self { + self.user_id = user_id; self } } From 6c51f7aeef4f5c23bb224d944be6281e09bc6dce Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 9 Nov 2025 12:36:22 -0600 Subject: [PATCH 14/17] Move default sorting logic details outside of the coaching_session_controller --- .../controller/user/coaching_session_controller.rs | 14 +++----------- web/src/params/user/coaching_session.rs | 13 +++++++++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs index 2504d9f8..5b50f653 100644 --- a/web/src/controller/user/coaching_session_controller.rs +++ b/web/src/controller/user/coaching_session_controller.rs @@ -2,9 +2,7 @@ use crate::controller::ApiResponse; use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; -use crate::params::coaching_session::SortField; use crate::params::user::coaching_session::{IncludeParam, IndexParams}; -use crate::params::WithSortDefaults; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; @@ -45,7 +43,7 @@ pub async fn index( AuthenticatedUser(_user): AuthenticatedUser, State(app_state): State, Path(user_id): Path, - Query(mut params): Query, + Query(params): Query, ) -> Result { debug!("GET Coaching Sessions for User: {user_id}"); debug!("Query Params: {params:?}"); @@ -58,14 +56,8 @@ pub async fn index( agreements: params.include.contains(&IncludeParam::Agreements), }; - // Apply sort defaults if needed - ::apply_sort_defaults( - &mut params.sort_by, - &mut params.sort_order, - SortField::Date, - ); - - // Use QuerySort trait to get SeaORM types + // Apply defaults and get SeaORM types for sorting + let params = params.apply_defaults(); let sort_column = params.get_sort_column(); let sort_order = params.get_sort_order(); diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index fd8836e7..310c1531 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -106,6 +106,19 @@ impl IndexParams { self.sort_order = sort_order; self } + + /// Applies default sorting parameters if any sort parameter is provided. + /// + /// Uses `Date` as the default sort field for coaching sessions. + /// This encapsulates the default field choice within the params module. + pub fn apply_defaults(mut self) -> Self { + ::apply_sort_defaults( + &mut self.sort_by, + &mut self.sort_order, + SortField::Date, + ); + self + } } impl QuerySort for IndexParams { From 5b347e754bf53638e1aa48029db7abef55bf144d Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 9 Nov 2025 12:44:47 -0600 Subject: [PATCH 15/17] Use IndexParams directly in the overarching_goal_controller.rs index method --- .../user/overarching_goal_controller.rs | 30 +++------------- web/src/params/user/overarching_goal.rs | 35 +++++++++---------- 2 files changed, 21 insertions(+), 44 deletions(-) diff --git a/web/src/controller/user/overarching_goal_controller.rs b/web/src/controller/user/overarching_goal_controller.rs index 95676463..abb57604 100644 --- a/web/src/controller/user/overarching_goal_controller.rs +++ b/web/src/controller/user/overarching_goal_controller.rs @@ -2,26 +2,17 @@ use crate::controller::ApiResponse; use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; -use crate::params::user::overarching_goal::{IndexParams, SortField}; -use crate::params::WithSortDefaults; +use crate::params::user::overarching_goal::IndexParams; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; use axum::response::IntoResponse; use axum::Json; use domain::{overarching_goal as OverarchingGoalApi, Id}; -use serde::Deserialize; use service::config::ApiVersion; use log::*; -#[derive(Debug, Deserialize)] -pub(crate) struct QueryParams { - pub(crate) coaching_session_id: Option, - pub(crate) sort_by: Option, - pub(crate) sort_order: Option, -} - /// GET all overarching goals for a specific user #[utoipa::path( get, @@ -49,24 +40,13 @@ pub async fn index( AuthenticatedUser(_user): AuthenticatedUser, State(app_state): State, Path(user_id): Path, - Query(query_params): Query, + Query(params): Query, ) -> Result { debug!("GET Overarching Goals for User: {user_id}"); - debug!("Filter Params: {query_params:?}"); + debug!("Filter Params: {params:?}"); - // Build params with user_id from path - let mut params = IndexParams::new(user_id).with_filters( - query_params.coaching_session_id, - query_params.sort_by, - query_params.sort_order, - ); - - // Apply default sorting parameters - IndexParams::apply_sort_defaults( - &mut params.sort_by, - &mut params.sort_order, - SortField::Title, - ); + // Set user_id from path parameter and apply default sorting + let params = params.with_user_id(user_id).apply_defaults(); let overarching_goals = OverarchingGoalApi::find_by(app_state.db_conn_ref(), params).await?; diff --git a/web/src/params/user/overarching_goal.rs b/web/src/params/user/overarching_goal.rs index 6f132a71..da11fe20 100644 --- a/web/src/params/user/overarching_goal.rs +++ b/web/src/params/user/overarching_goal.rs @@ -38,28 +38,25 @@ pub(crate) struct IndexParams { } impl IndexParams { - /// Creates params with only user_id set (all filters empty). - pub fn new(user_id: Id) -> Self { - Self { - user_id, - coaching_session_id: None, - sort_by: None, - sort_order: None, - } + /// Sets the user_id field (useful when user_id comes from path parameter). + /// + /// This allows using `Query` to deserialize query parameters, + /// then setting the path-based user_id afterward. + pub fn with_user_id(mut self, user_id: Id) -> Self { + self.user_id = user_id; + self } - /// Builder method to add optional coaching session filter and sorting. + /// Applies default sorting parameters if any sort parameter is provided. /// - /// Useful for programmatically constructing params in tests or internal code. - pub fn with_filters( - mut self, - coaching_session_id: Option, - sort_by: Option, - sort_order: Option, - ) -> Self { - self.coaching_session_id = coaching_session_id; - self.sort_by = sort_by; - self.sort_order = sort_order; + /// Uses `Title` as the default sort field for overarching goals. + /// This encapsulates the default field choice within the params module. + pub fn apply_defaults(mut self) -> Self { + ::apply_sort_defaults( + &mut self.sort_by, + &mut self.sort_order, + SortField::Title, + ); self } } From 3ac7fbbd102e74d78722a5c7077c199c536fde5f Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 9 Nov 2025 12:49:52 -0600 Subject: [PATCH 16/17] Remove unused code related to IndexParams --- web/src/params/user/coaching_session.rs | 31 ------------------------- 1 file changed, 31 deletions(-) diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 310c1531..0d278431 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -76,37 +76,6 @@ where } impl IndexParams { - /// Creates params with only user_id set (all filters empty, no includes). - #[allow(dead_code)] - pub fn new(user_id: Id) -> Self { - Self { - user_id, - from_date: None, - to_date: None, - sort_by: None, - sort_order: None, - include: Vec::new(), - } - } - - /// Builder method to add date range filtering and sorting. - /// - /// Note: Does not set `include` - use field access to add related resources. - #[allow(dead_code)] - pub fn with_filters( - mut self, - from_date: Option, - to_date: Option, - sort_by: Option, - sort_order: Option, - ) -> Self { - self.from_date = from_date; - self.to_date = to_date; - self.sort_by = sort_by; - self.sort_order = sort_order; - self - } - /// Applies default sorting parameters if any sort parameter is provided. /// /// Uses `Date` as the default sort field for coaching sessions. From ba2cb622767edee15e321d79ef4edaf2573005e4 Mon Sep 17 00:00:00 2001 From: Jim Hodapp Date: Sun, 9 Nov 2025 12:54:04 -0600 Subject: [PATCH 17/17] Ensure consistent user_id param handling --- web/src/controller/user/action_controller.rs | 14 +++----------- .../controller/user/coaching_session_controller.rs | 6 +++--- web/src/params/user/action.rs | 13 +++++++++++++ web/src/params/user/coaching_session.rs | 11 ++++++++++- 4 files changed, 29 insertions(+), 15 deletions(-) diff --git a/web/src/controller/user/action_controller.rs b/web/src/controller/user/action_controller.rs index 49c0663f..19133d97 100644 --- a/web/src/controller/user/action_controller.rs +++ b/web/src/controller/user/action_controller.rs @@ -2,8 +2,7 @@ use crate::controller::ApiResponse; use crate::extractors::{ authenticated_user::AuthenticatedUser, compare_api_version::CompareApiVersion, }; -use crate::params::user::action::{IndexParams, SortField}; -use crate::params::WithSortDefaults; +use crate::params::user::action::IndexParams; use crate::{AppState, Error}; use axum::extract::{Path, Query, State}; use axum::http::StatusCode; @@ -47,15 +46,8 @@ pub async fn index( debug!("GET Actions for User: {user_id}"); debug!("Filter Params: {params:?}"); - // Set user_id from path parameter - let mut params = params.with_user_id(user_id); - - // Apply default sorting parameters - IndexParams::apply_sort_defaults( - &mut params.sort_by, - &mut params.sort_order, - SortField::CreatedAt, - ); + // Set user_id from path parameter and apply default sorting + let params = params.with_user_id(user_id).apply_defaults(); let actions = ActionApi::find_by(app_state.db_conn_ref(), params).await?; diff --git a/web/src/controller/user/coaching_session_controller.rs b/web/src/controller/user/coaching_session_controller.rs index 5b50f653..92847e97 100644 --- a/web/src/controller/user/coaching_session_controller.rs +++ b/web/src/controller/user/coaching_session_controller.rs @@ -48,6 +48,9 @@ pub async fn index( debug!("GET Coaching Sessions for User: {user_id}"); debug!("Query Params: {params:?}"); + // Set user_id from path parameter and apply defaults + let params = params.with_user_id(user_id).apply_defaults(); + // Build include options from parameters let includes = CoachingSessionApi::IncludeOptions { relationship: params.include.contains(&IncludeParam::Relationship), @@ -55,9 +58,6 @@ pub async fn index( goal: params.include.contains(&IncludeParam::Goal), agreements: params.include.contains(&IncludeParam::Agreements), }; - - // Apply defaults and get SeaORM types for sorting - let params = params.apply_defaults(); let sort_column = params.get_sort_column(); let sort_order = params.get_sort_order(); diff --git a/web/src/params/user/action.rs b/web/src/params/user/action.rs index 1263ccd2..178027db 100644 --- a/web/src/params/user/action.rs +++ b/web/src/params/user/action.rs @@ -48,6 +48,19 @@ impl IndexParams { self.user_id = user_id; self } + + /// Applies default sorting parameters if any sort parameter is provided. + /// + /// Uses `CreatedAt` as the default sort field for actions. + /// This encapsulates the default field choice within the params module. + pub fn apply_defaults(mut self) -> Self { + ::apply_sort_defaults( + &mut self.sort_by, + &mut self.sort_order, + SortField::CreatedAt, + ); + self + } } impl IntoQueryFilterMap for IndexParams { diff --git a/web/src/params/user/coaching_session.rs b/web/src/params/user/coaching_session.rs index 0d278431..4a5c7680 100644 --- a/web/src/params/user/coaching_session.rs +++ b/web/src/params/user/coaching_session.rs @@ -35,7 +35,6 @@ pub enum IncludeParam { pub(crate) struct IndexParams { /// User ID from URL path (not a query parameter) #[serde(skip)] - #[allow(dead_code)] pub(crate) user_id: Id, /// Optional: filter sessions starting from this date (inclusive) pub(crate) from_date: Option, @@ -76,6 +75,16 @@ where } impl IndexParams { + /// Sets the user_id field (useful when user_id comes from path parameter). + /// + /// This allows using `Query` to deserialize query parameters, + /// then setting the path-based user_id afterward for consistency with other + /// user sub-resource endpoints. + pub fn with_user_id(mut self, user_id: Id) -> Self { + self.user_id = user_id; + self + } + /// Applies default sorting parameters if any sort parameter is provided. /// /// Uses `Date` as the default sort field for coaching sessions.