Skip to content

Goals PR2 companion — relationship-scoped goals with per-session display#330

Merged
jhodapp merged 10 commits into
mainfrom
feat/goals-pr2-companion
Mar 13, 2026
Merged

Goals PR2 companion — relationship-scoped goals with per-session display#330
jhodapp merged 10 commits into
mainfrom
feat/goals-pr2-companion

Conversation

@jhodapp

@jhodapp jhodapp commented Mar 10, 2026

Copy link
Copy Markdown
Member

Description

Frontend companion for backend PR2 (goals relationship scoping). Updates the Goal type, API layer, and UI components to work with the new relationship-scoped goals and coaching_sessions_goals join table.

Coordinated deploy: must merge alongside backend PR #242.

Changes

  • Update Goal interface: remove coaching_session_id, add coaching_relationship_id (required), created_in_session_id (nullable), target_date (nullable)
  • Update GoalApi.list() to send coaching_relationship_id query param
  • Add GoalApi.listBySession() and useGoalsBySession hook for GET /coaching_sessions/{id}/goals (join table endpoint)
  • Fix per-session goal display in dashboard session cards and coaching session goal drawer
  • Send created_in_session_id on goal creation to trigger backend auto-link
  • Apply render guard pattern in GoalContainer
  • Add 38 new tests: Goal type (27), GoalApi/hooks (11)

Screenshots / Videos Showing UI Changes (if applicable)

None

Testing Strategy

  • 663 tests passing (38 new)
  • Manual verification: dashboard shows correct per-session goals, goal drawer shows session-specific goal, creating a goal from session context auto-links it

Concerns

  • include=goal on enriched sessions endpoint still works for existing data but will become incomplete as new join table links are created — migration to join table endpoint is done in this PR

…n display

Update Goal type from session-scoped (coaching_session_id) to relationship-scoped
(coaching_relationship_id, created_in_session_id nullable, target_date nullable).

Use GET /coaching_sessions/{id}/goals join table endpoint for per-session goal
display in dashboard cards and coaching session goal drawer. Send
created_in_session_id on goal creation to trigger backend auto-link.
@jhodapp jhodapp changed the title feat: goals PR2 companion — relationship-scoped goals with per-session display Goals PR2 companion — relationship-scoped goals with per-session display Mar 10, 2026
@jhodapp jhodapp self-assigned this Mar 10, 2026
@jhodapp jhodapp added the enhancement Improves existing functionality or feature label Mar 10, 2026
@jhodapp jhodapp moved this to 🏗 In progress in Refactor Coaching Platform Mar 10, 2026
@jhodapp jhodapp added this to the 1.0.0-beta3 milestone Mar 10, 2026
Goal SSE events (created/updated/deleted) now also invalidate
/coaching_sessions SWR keys, covering the GET /coaching_sessions/{id}/goals
join table endpoint used for per-session goal display.
Extract DEFAULT_GOAL_TITLE constant and goalTitle() helper in goal.ts
to consistently handle empty/null goal titles across 6 UI call sites.
@jhodapp jhodapp marked this pull request as ready for review March 10, 2026 00:58
@jhodapp jhodapp moved this from 🏗 In progress to Review in Refactor Coaching Platform Mar 10, 2026
…sion

- Add missing field checks (title, body, target_date, status_changed_at,
  completed_at) to isGoal type guard to prevent malformed data at boundary
- Conditionally construct URL in useGoalsBySession to avoid embedding
  "null" string when session ID is absent
- Remove as-any casts in goal API tests, remove test that asserted
  null-in-URL implementation detail
invalidateEndpoint('/coaching_sessions') was too broad — it revalidated
every SWR key containing that path, causing the coaching session title
to flash on goal updates. Replace with a targeted invalidator that only
matches /coaching_sessions/{id}/goals keys.

@calebbourg calebbourg left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of thoughts. Not sure if they're valid

// any other local code using the KeyedMutator will also update with this new data.
refresh();
}
if (goal.id) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is goal here the goal that's defined on line 30? If so would it make sense to pass it in as an argument here? I've seen referencing variables from outside of the function scope in JS before but not sure what the best practice is.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. Fixed in 49e71a9.

Comment thread src/lib/api/goals.ts Outdated
* @param coachingSessionId The ID of the coaching session
* @returns Promise resolving to an array of Goal objects linked to the session
*/
listBySession: async (coachingSessionId: Id): Promise<Goal[]> => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more conventional to this app for this to be called listNested similar to createNested

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 49e71a9.

Address PR review feedback: use listNested convention with
EntityApi.listNestedFn and make handleGoalChange accept the current
goal explicitly instead of closing over component scope.
jhodapp added 2 commits March 11, 2026 15:47
Surface a destructive toast when the backend returns HTTP 409
because the coaching relationship already has 3 active goals.
Adds ActiveGoalLimitError types and extractActiveGoalLimitError
helper for reuse in future swap dialog work.
"Active" means InProgress only — NotStarted does not count.
Use max_active_goals from 409 response instead of hardcoding.
Rename ACTIVE_GOAL_LIMIT to DEFAULT_MAX_ACTIVE_GOALS. Return
ActiveGoalLimitInfo (with limit + goals) from the extractor.
jhodapp added a commit that referenced this pull request Mar 12, 2026
Mark backend PR1/PR2 as merged with links to PRs (#242, #330). Update
API gap analysis with PR2 completion status. Correct active goal limit
from frontend-only to backend-enforced (409 Conflict). Update join table
endpoints to nested routes. Add Layer 2 PR2 companion deliverables and
remaining work blockers. Document auto-link on creation interim behavior.
@github-actions

Copy link
Copy Markdown
Contributor

🚀 PR Preview Environment Deployed!

🔗 Access URLs

Service URL
Frontend http://neo/pr-330/
Backend API http://neo/pr-330/api
Health Check http://neo/pr-330/health
Base Path /pr-330/

📊 Environment Details

🧪 Testing

# Health check
curl http://neo/pr-330/health

# Frontend
curl http://neo/pr-330/

# API test
curl http://neo/pr-330/api/v1/users

🧹 Cleanup

Environment auto-cleaned when PR closes/merges


Deployed: 2026-03-13T16:48:47.808Z
Architecture: Native ARM64 build on Neo + Multi-tier caching

@jhodapp jhodapp merged commit 9ae55f0 into main Mar 13, 2026
12 checks passed
@github-project-automation github-project-automation Bot moved this from Review to ✅ Done in Refactor Coaching Platform Mar 13, 2026
@jhodapp jhodapp deleted the feat/goals-pr2-companion branch March 13, 2026 17:00
jhodapp added a commit that referenced this pull request Mar 15, 2026
Mark backend PR1/PR2 as merged with links to PRs (#242, #330). Update
API gap analysis with PR2 completion status. Correct active goal limit
from frontend-only to backend-enforced (409 Conflict). Update join table
endpoints to nested routes. Add Layer 2 PR2 companion deliverables and
remaining work blockers. Document auto-link on creation interim behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improves existing functionality or feature

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants