160 feature enhanced coaching sessions endpoint #202

jhodapp · 2025-11-01T22:09:39Z

Description

This PR adds enhanced user-scoped endpoints for coaching sessions with optional batch loading to eliminate N+1 queries, plus additional user-scoped endpoints for organizations, actions, and overarching goals.

GitHub Issue: Relates to #160

Changes

Enhanced Coaching Sessions Endpoint: Added /users/{user_id}/coaching_sessions with optional include query parameter supporting batch loading of related
resources (relationships, users, organizations, goals, agreements)
User-Scoped Resource Endpoints: Added endpoints for organizations, actions, and overarching goals scoped to specific users
Architecture Improvements:
- Moved validation from web to entity_api layer for proper error propagation
- Added database-level sorting to find_by_user_with_includes
- Eliminated response DTO by using entity_api type directly with serde flatten
Data Model Updates: Added organizations_users entity back to support batch loading while maintaining user_roles as primary relationship table
Authorization: All user-scoped endpoints enforce that authenticated_user.id matches user_id parameter

Testing Strategy

Test enhanced coaching sessions endpoint with various include parameters:
- /users/{user_id}/coaching_sessions?include=relationship
- /users/{user_id}/coaching_sessions?include=relationship,organization
- Verify single query loads all related data efficiently
Test user-scoped endpoints for organizations, actions, and overarching goals
Verify date range filtering and status filtering work correctly
Verify authorization checks prevent users from accessing other users' data
Run existing test suite to ensure backward compatibility

Concerns

The organizations_users table was added back after being previously scheduled for removal. This decision supports efficient batch loading while keeping
user_roles as the primary source of truth for user-organization relationships.

★ Insight ─────────────────────────────────────

N+1 Query Elimination: This PR implements a sophisticated batch loading pattern that allows clients to request related data in a single API call, replacing what would have been 3+ separate queries
Layered Validation: Moving validation from the web layer to entity_api improves error handling by allowing domain-specific errors to propagate properly through the stack
Net Code Reduction: Despite adding significant functionality, the refactoring actually removes 151 lines of code, demonstrating cleaner architecture
─────────────────────────────────────────────────

entity_api/src/organization.rs

…ns, and overarching goals 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 <noreply@anthropic.com>

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)

- 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

…ds added in this PR

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.

Replace manual Default implementation with derived Default trait. Mark InProgress as the default variant using #[default] attribute.

calebbourg

This is a lot of really awesome stuff!

I'm impressed with the "includes" concept and where it's going. Very clever and awesome thinking.

I left a few comments calling out some things I think can be modified to be inline with some of our historical design decisions and patterns but there are some larger architectural things that I anticipate may resolve the other comments as well:

We have existing controllers for many of the ones defined here in this PR. I see that you have scoped the controllers within the user domain. (/{user_id}/{resource}). This isn't a bad idea and is worth considering. There becomes a lot of duplicated code thought, between the existing controllers and the new controllers. Just something to think about here pretty deeply as it's a large design choice. If we decide to use the existing controllers we would need to pass the user ID in as a query parameter and add some logic to check in the protect modules. Example. Your approach may actually be cleaner so just bringing this up to think about. The goal in me bringing it up is to reuse as much code as possible.
Finding a way to use our existing query module, I believe, is paramount as it was specifically designed to remove the need for the many if conditionals by allowing the web layer to describe what query parameters are possible through the use of things like IntoQueryFilterMap. IntoQueryFilterMap is exposed from entity_api into web as a way for web to provide information back down into entity_api functions without having a circular dependency between the two crates. We can change the mechanics of query or even move the includes concept into a more generic concept similar to query but I thing we need to maintain the larger concepts to preserve decoupling and dependency hierarchy. Very happy to think through this with you.

calebbourg · 2025-11-08T12:41:44Z

entity/src/status.rs

    #[sea_orm(string_value = "not_started")]
    NotStarted,
    #[sea_orm(string_value = "in_progress")]
+    #[default]


entity/src/status.rs

entity_api/src/organization.rs

calebbourg · 2025-11-08T13:14:00Z

entity_api/src/coaching_session.rs

+pub enum SortField {
+    Date,
+    CreatedAt,
+    UpdatedAt,
+}
+
+/// Sort order for queries
+#[derive(Debug, Clone, Copy)]
+pub enum SortOrder {
+    Asc,
+    Desc,
+}


I believe these should belong in web as they are the concerns of incoming API requests.

We already have a SortOrder defined here and I think we should use that if we can.

If we have a good argument for moving this concept into entity_api then we will need to find a more generic solution to port the other domains that use SortOrder (in web) to the new pattern.

Having two separate patterns, I think, might come back to bite us.

Addressed in commit 77d70dc

calebbourg · 2025-11-08T13:16:54Z

entity_api/src/coaching_session.rs

+    from_date: Option<chrono::NaiveDate>,
+    to_date: Option<chrono::NaiveDate>,
+    sort_by: Option<SortField>,
+    sort_order: Option<SortOrder>,


I think we should find a way to use the query module here.

https://github.com/refactor-group/refactor-platform-rs/blob/main/entity_api/src/query.rs

Good thought... I've partially addressed this by updating the function signature to accept Option<coaching_sessions::Column> and Option<sea_orm::Order> directly (the same SeaORM types that query::find_by uses internally).

However, we can't fully use query::find_by here because it only supports simple column-based equality filtering. Our function needs complex JOINs with coaching_relationships (to filter by user as coach OR coachee), custom date range logic, and batch loading of related data.

The web layer now uses the QuerySort trait to convert params to these SeaORM types, which matches the pattern used by query::find_by - we just can't delegate to it directly due to the query complexity.

calebbourg · 2025-11-08T13:17:31Z

entity_api/src/coaching_session.rs

+    to_date: Option<chrono::NaiveDate>,
+    sort_by: Option<SortField>,
+    sort_order: Option<SortOrder>,


similarly use query
https://github.com/refactor-group/refactor-platform-rs/blob/main/entity_api/src/query.rs

Unfortunately, we can't use query::find_by here either. The find_by_user function needs to JOIN with the coaching_relationships table and filter with an OR condition (coach_id = user_id OR coachee_id = user_id). The query::find_by helper only supports simple column-based equality filtering - it iterates through entity columns and applies column.eq(value) filters. It doesn't support JOINs or OR conditions, so we need to manually build the query for this use case.

calebbourg · 2025-11-08T13:21:01Z

entity_api/src/coaching_session.rs

+    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));
+    }
+
+    // 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,
+        };


using query, I believe will remove the need for all of these conditionals. The conditional logic is moved upstream by having params related structs implement IntoQueryFilterMap which is defined here.

We already have an example of this for Coaching Sessions which we may be able to use out of the box here and then define functions that use this mechanism and allow for the includes concept.

Will hold off on this until you verify my findings for not being able to currently use query.

calebbourg · 2025-11-08T13:44:27Z

web/src/protect/users/mod.rs


 pub(crate) mod passwords;

+// checks:


Based on our current patterns, these should be extracted into their own modules and the function names should match the controller action names. See organizations.

Fixed in commit b307fd3

…strings

… layer

…protect/organizations

jhodapp · 2025-11-08T19:06:41Z

We have existing controllers for many of the ones defined here in this PR. I see that you have scoped the controllers within the user domain. (/{user_id}/{resource}). This isn't a bad idea and is worth considering. There becomes a lot of duplicated code thought, between the existing controllers and the new controllers. Just something to think about here pretty deeply as it's a large design choice. If we decide to use the existing controllers we would need to pass the user ID in as a query parameter and add some logic to check in the protect modules. Example. Your approach may actually be cleaner so just bringing this up to think about. The goal in me bringing it up is to reuse as much code as possible.

Thanks. I thought about this pattern too but decided on the path I took because I thought you had already established this pattern in web/src/controller/organization/coaching_relationship_controller.rs? Am I mistaken here?

calebbourg

This is looking really good!

Thank you for looking into using the generic query function. I see what you're describing where it doesn't allow for more complex situations that are required here. I appreciate you looking into it and for your patience with the reviews and other suggestions! Maybe we can come back and see if we can find a way to make query more flexible.

I also think your choice for the new controllers is the right one. Thanks for accommodating my needing to think it through a bit as well. This is cleaner.

I left a few comments relating to the use of IndexParams. If we could move as much of the logic from the controllers into the respective modules where the IndexParams structs are defined that would be the icing on the cake here.

For context on my thought process: My preference is to try to keep as much actual logic out of controllers as possible so any opportunity to keep the logic paired with where the data structures are defined is a win.

I don't think it's the end of the world to have logic in controllers and some is certainly necessary. Just more of a trying to combat clutter wherever possible as, probably, a futile goal :)

I realized after the fact that you had already defined new params modules for these new controllers. That's totally the right approach I think. The functions for adding user_id to the IndexParams structs can go there, not in the params modules relating to the other existing controllers.

If we can make those changes (if they are possible and make sense) I think this will be good to go!

web/src/controller/user/action_controller.rs

web/src/controller/user/coaching_session_controller.rs

web/src/controller/user/overarching_goal_controller.rs

web/src/params/user/coaching_session.rs

…ntroller

…method

calebbourg

Beautiful! Thanks for working through all of it

jhodapp added this to the 1.0.0-beta2 milestone Nov 1, 2025

jhodapp requested a review from calebbourg November 1, 2025 22:09

jhodapp self-assigned this Nov 1, 2025

jhodapp added the feature work Specifically implementing a new feature label Nov 1, 2025

jhodapp added this to Refactor Coaching Platform Nov 1, 2025

jhodapp moved this to 🏗 In progress in Refactor Coaching Platform Nov 1, 2025

jhodapp commented Nov 6, 2025

View reviewed changes

entity_api/src/organization.rs Outdated Show resolved Hide resolved

claude and others added 5 commits November 6, 2025 16:04

Make sure response is included in lib.rs

9d3bb71

Add some helpful code documentation for the different types and metho…

502767e

…ds added in this PR

jhodapp force-pushed the 160-feature-enhanced-coaching-sessions-endpoint branch from ed975a1 to 502767e Compare November 6, 2025 22:15

jhodapp moved this from 🏗 In progress to Review in Refactor Coaching Platform Nov 6, 2025

jhodapp added 3 commits November 6, 2025 16:27

Run cargo fmt

2d3df6a

Fix doc-test for IncludeOptions usage example

b3d95b4

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.

Fix clippy derivable_impls warning for Status enum

b17a6e8

Replace manual Default implementation with derived Default trait. Mark InProgress as the default variant using #[default] attribute.

jhodapp marked this pull request as ready for review November 6, 2025 23:33

calebbourg requested changes Nov 8, 2025

View reviewed changes

jhodapp added 4 commits November 8, 2025 12:23

Clean up bad rebase merge conflict from previous commit

43d1a81

Reformat Status's display trait implementation to use human-friendly …

51d5896

…strings

Don't duplicate sorting params from the web layer into the entity_api…

77d70dc

… layer

Follow the established protect structure already laid out in web/src/…

b307fd3

…protect/organizations

calebbourg requested changes Nov 9, 2025

View reviewed changes

jhodapp added 5 commits November 9, 2025 12:30

Use IndexParams directly in the action_controller.rs index method

af16156

Move default sorting logic details outside of the coaching_session_co…

6c51f7a

…ntroller

Use IndexParams directly in the overarching_goal_controller.rs index …

5b347e7

…method

Remove unused code related to IndexParams

3ac7fbb

Ensure consistent user_id param handling

ba2cb62

calebbourg approved these changes Nov 9, 2025

View reviewed changes

jhodapp merged commit 55a299c into main Nov 10, 2025
5 checks passed

jhodapp deleted the 160-feature-enhanced-coaching-sessions-endpoint branch November 10, 2025 00:13

github-project-automation bot moved this from Review to ✅ Done in Refactor Coaching Platform Nov 10, 2025

160 feature enhanced coaching sessions endpoint #202

160 feature enhanced coaching sessions endpoint #202

Uh oh!

Conversation

jhodapp commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

GitHub Issue: Relates to #160

Changes

Testing Strategy

Concerns

Uh oh!

Uh oh!

calebbourg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhodapp Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhodapp commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

calebbourg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

calebbourg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jhodapp commented Nov 1, 2025 •

edited

Loading

jhodapp Nov 8, 2025 •

edited

Loading

jhodapp commented Nov 8, 2025 •

edited

Loading