Add GET /v1/goals/{goal_id}: fetch a single goal by id#9386
Conversation
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the focused addition — the endpoint and DB helper look straightforward, and the new unit coverage matches the intended 404/normalization behavior.
I do need to request one fix before this can merge: the committed app-client OpenAPI contract is stale. I reproduced the failing CI locally with:
cd backend
env -i PATH="$PATH" HOME="$HOME" OPENAPI_RUNNER_VENV="/tmp/omi-pr9386-review/.openapi-venv" \
./scripts/openapi_runner.sh scripts/export_openapi.py --surface app-client --check ../docs/api-reference/app-client-openapi.json
which reports:
OpenAPI contract check failed: ../docs/api-reference/app-client-openapi.json is stale; run backend/scripts/export_openapi.py --write ../docs/api-reference/app-client-openapi.json
Please regenerate and commit docs/api-reference/app-client-openapi.json for the new GET /v1/goals/{goal_id} route, then the contract check should be able to go green. I’m not seeing a security/supply-chain concern in the code itself; this is mainly a public API contract artifact issue on an authenticated goal read endpoint.
The goals API already lets a client update, delete, set progress on, and read the history/advice of a goal by its id, and lists goals via /v1/goals and /v1/goals/all, but there was no plain read of one goal by id: a client wanting to display or refresh a specific goal had to page the capped list and filter client-side. Adds get_goal(uid, goal_id) in the DB layer (mirroring update_goal's read) and the matching endpoint, returning 404 when the goal does not exist or belongs to another user. Declared after the static /v1/goals/* routes so those still match first.
…_id}
The Public Developer API contract check flagged docs/api-reference/app-client-openapi.json
as stale after the goal-by-id route was added. Regenerated via
backend/scripts/export_openapi.py --surface app-client --write; the only change is the new
GET /v1/goals/{goal_id} operation.
e07b1e6 to
25ebd33
Compare
|
Done. Regenerated |
kodjima33
left a comment
There was a problem hiding this comment.
Backend read endpoint GET /v1/goals/{goal_id}; mirrors existing goal routes. Approve (feature, backend deploy is Nik's).
…r GET /v1/goals/{goal_id}
Adds the route-policy manifest entry (legacy_unreviewed) for the new goal-by-id route and
regenerates the app-client TypeScript and Swift client schemas, so the Public Developer API
contract check passes. Only the new operation is added.
… client
The app-client contract regen for GET /v1/goals/{goal_id} updates the generated Swift and
Windows API client schemas (desktop files), which the desktop changelog Hygiene check requires
a fragment for.
Summary
Adds
GET /v1/goals/{goal_id}to fetch a single goal by id.Motivation
The goals API already lets a client update, delete, set progress on, and read the history and advice of a goal by its id, and lists goals via
/v1/goalsand/v1/goals/all. But there was no plain read of one goal by id. A client that wants to display or refresh a specific goal (for example after an edit, or when deep-linking to a goal) had to fetch the whole capped list and filter client-side.Change
database/goals.py:get_goal(uid, goal_id)returns the goal dict, orNoneif it does not exist. It mirrors the readupdate_goalalready does (document(goal_id).get(), then_goal_dictto ensure theidis present).routers/goals.py:GET /v1/goals/{goal_id}returns the normalized goal, or404when the goal does not exist or belongs to another user (the lookup is scoped to the caller's own uid). It is declared after the static/v1/goals/*routes (/all,/suggest,/advice,/extract-progress) so those match first and are not captured as agoal_id.The change is additive and does not touch existing routes.
Testing
tests/unit/test_goal_get_by_id.py: the endpoint returns the normalized goal and returns 404 when the goal is absent; the DB helper returnsNonefor a missing document and fillsidfrom the document id when present.