Skip to content

Add GET /v1/focus-sessions/{session_id}: fetch a single focus session#8987

Open
ZachL111 wants to merge 5 commits into
BasedHardware:mainfrom
ZachL111:zach/focus-session-get
Open

Add GET /v1/focus-sessions/{session_id}: fetch a single focus session#8987
ZachL111 wants to merge 5 commits into
BasedHardware:mainfrom
ZachL111:zach/focus-session-get

Conversation

@ZachL111

@ZachL111 ZachL111 commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What

Adds GET /v1/focus-sessions/{session_id}, which fetches a single focus session by id.

Why

Focus sessions can be created (POST /v1/focus-sessions), listed (GET /v1/focus-sessions), and deleted (DELETE /v1/focus-sessions/{session_id}), but there is no way to fetch one by id. A client that already has a session id (from the list or from a create response) cannot re-read that single session, for example to refresh a detail view after an edit. This adds the missing read.

Details

  • Returns the stored document (with its id), or 404 when the session does not exist, mirroring the get-by-id convention in sibling routers.
  • New db helper get_focus_session(uid, session_id) reuses the module's db/_user_col seam and the existing ref.get().exists idiom.
  • No new imports beyond HTTPException and typing.Optional; import-clean.
  • The route coexists unambiguously with the existing DELETE /v1/focus-sessions/{session_id} and the segmentless list route.

Test

backend/tests/unit/test_focus_session_get.py: the router handler returns the session when found and raises 404 when missing; the db helper returns None when the doc is absent and injects id when present. 4 passed locally. Uses the sanctioned seams (import the modules normally and patch.object on the singletons, no sys.modules mutation).

Review in cubic

Focus sessions can be created, listed, and deleted, but there is no way to
fetch a single one by id. Add the missing GET-by-id: it returns the stored
document (with its id) and 404 when the session is absent, mirroring the
get-by-id convention in sibling routers. Reuses the module's db/_user_col
seam and the existing ref.get().exists idiom.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@kodjima33 kodjima33 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.

Reviewed: scoped backend addition, approve-only per policy (features/new endpoints are Nik's merge call).

@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Thanks for the focused backend addition here. I did a static maintainer review of the route/database change and the new unit coverage.

Positive signal from my side: the implementation is small and coherently scoped, keeps the lookup under users/{uid}/focus_sessions, returns 404 for missing sessions, and the added tests cover the router pass-through/404 behavior plus the database helper's absent/present cases. The current CI checks are also green.

I’m not submitting a formal approval from automation because this adds a new authenticated user-data read endpoint, which falls under the maintainer policy’s data-access/security-sensitive gate. I don’t see a blocking implementation issue, but final approval/merge should remain a human maintainer call.

@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Thanks for the update on this PR. I re-reviewed the current head (b50f75b1) including the new route-policy manifest entry.

Positive signal from my side: the implementation remains small and coherently scoped. The new GET handler is authenticated through auth.get_current_user_uid, reads from users/{uid}/focus_sessions/{session_id}, returns 404 for missing sessions, and the focused unit tests cover the router found/missing behavior plus the database helper’s absent/present cases. I also ran the new unit test file locally in an isolated dependency environment: 4 passed.

I’m still not submitting a formal automation approval because this exposes a new authenticated user-data read endpoint and the monitor policy gates data-access/security-sensitive surfaces for human maintainer judgment. I don’t see a blocking implementation issue in this diff; final approval/merge should remain with maintainers.

@ZachL111 ZachL111 force-pushed the zach/focus-session-get branch from d5ca62a to b50f75b Compare July 11, 2026 00:51
@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Thanks for keeping this endpoint focused. I re-reviewed the current head (2fb0e0d) after the update/merge from main.

Positive signal from my side: the implementation is still small and scoped to the missing single-session read. The handler stays behind auth.get_current_user_uid, the database helper reads only from users/{uid}/focus_sessions/{session_id}, missing documents return a 404, and the route-policy manifest now includes the new endpoint. I also ran the new focused unit test file locally with provider/GitHub/cloud credentials stripped: 4 passed.

I’m not submitting a formal automation approval because this is a new authenticated user-data read endpoint, and the maintainer policy gates security/data-access surfaces for human maintainer judgment. I don’t see a blocking implementation issue in this diff; final approval/merge should remain with maintainers.

@ZachL111

Copy link
Copy Markdown
Contributor Author

Thanks David. Nothing outstanding on my side; agreed a human maintainer should make the final call on the new authenticated read surface. The gate that was red is now fixed on main (#9462), so this should go green on rerun.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants