fix: adjust project routes to use consistent path structure#285
Conversation
There was a problem hiding this comment.
Important
This change correctly fixes the chi shadowing issue, but the regressed route has no test coverage. Please add a regression test for /projects/workspace-stats before merging.
Reviewed changes — Switches project collection routes from r.Route("/projects") to r.Group(), registering full /projects paths so the static collection routes are not shadowed by the later /projects/{projectId} mount.
services/api/internal/transport/http/router/router.go— registers list, workspace-stats, and create with full/projectspaths under aGroupto coexist with the parameterized project router.
⚠️ Missing regression coverage for /projects/workspace-stats
No existing test or integration case hits GET /api/v1/projects/workspace-stats. Since this PR exists specifically because chi was shadowing that route with /projects/{projectId}, a regression test is the best way to keep the bug from reappearing if the route tree is reorganized again.
Technical details
# Missing regression coverage for /projects/workspace-stats
## Affected sites
- `services/api/test/integration/project_test.go` — project integration tests cover list/create/get but not workspace-stats.
- `services/api/internal/transport/http/router/router_test.go` — router tests currently do not include a `Project` handler, so they also miss this path.
## Required outcome
- An authenticated request to `GET /api/v1/projects/workspace-stats` reaches `Project.GetWorkspaceStats` and returns a non-404/non-parameter-error response.
- The test should fail if the static route is again shadowed by `/projects/{projectId}`.
## Suggested approach
- Add a case in `services/api/test/integration/project_test.go` (or another appropriate router/integration test) that creates a project, then hits `/api/v1/projects/workspace-stats` and verifies `200 OK` and the expected stats payload shape.
- If `router_test.go` is the preferred place, wire a minimal `Project` stub in `Deps` and assert routing/middleware behavior; otherwise an integration-level test already has the real handler available.Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — The new commit wires a project service stub into the router tests and adds regression coverage for the chi route-shadowing fix.
- Added
stubProjectSvcand wiredProjecthandler inrouter_test.go— implements the project-domain service interface and registershandler.NewProjectHandlerinnewTestRouterWithStoreso the project collection routes can be exercised in isolation. - Replaced the inline
authz.NewAuthorizer(store)call with a sharedauthorizervariable — the handler and middlewareAuthorizernow point to the same instance insidenewTestRouterWithStore. - Added
TestProjectsRoute_WorkspaceStats_NotShadowedByProjectIDRoute— asserts thatGET /api/v1/projects/workspace-statsreturns200 OKand a body containingopen_task_count, covering the shadowing regression flagged in the prior review. - Added
TestProjectsRoute_GetByID_StillParsesProjectID— asserts thatGET /api/v1/projects/{uuid}still reaches the project-ID route and returns404for an unknown project, ensuring the new group registration did not break parameterized project routes.
Kimi K2 (free via Pullfrog for OSS) | 𝕏

No description provided.