[CSM Portal] Replace project-scoped search endpoints with flat POST /cases/search and POST /deployments/search - #835
Conversation
📝 WalkthroughWalkthroughThe PR migrates the CSM Portal backend case search from a project-scoped endpoint ( ChangesUnified Case Search Endpoint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/csm-portal/backend/openapi.yaml (1)
324-353:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
/cases/searchresponse contract is missing404, but handler behavior still returns it.
SearchCasesstill maps upstream404viamapUpstreamError, andTestSearchCasesexplicitly asserts this mapping. That makes the OpenAPI response set incomplete for current runtime behavior.📄 Suggested OpenAPI fix
responses: "200": description: Ok @@ "403": description: Forbidden content: application/json: schema: $ref: '`#/components/schemas/ErrorPayload`' + "404": + description: NotFound + content: + application/json: + schema: + $ref: '`#/components/schemas/ErrorPayload`' "413": description: RequestEntityTooLarge🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/backend/openapi.yaml` around lines 324 - 353, The OpenAPI spec for the /cases/search endpoint is missing a 404 response even though the runtime handler SearchCases (and its test TestSearchCases) maps upstream 404 via mapUpstreamError and asserts that behavior; update the responses block in openapi.yaml for /cases/search to include a "404" response with description (e.g., NotFound) and the same ErrorPayload schema used by 400/401/403/413/500 so the contract matches runtime behavior and tests.
🧹 Nitpick comments (1)
apps/csm-portal/backend/internal/handler/cases_test.go (1)
196-257: ⚡ Quick winAdd an explicit test for search requests without
projectIds.The new contract’s key behavior is “no
projectIds=> all accessible cases”; this path is not directly asserted yet.🧪 Suggested test addition
func TestSearchCases(t *testing.T) { + t.Run("forwards body without projectIds filter", func(t *testing.T) { + var capturedBody []byte + client := &mockEntityCaseClient{ + searchCasesFn: func(_ context.Context, body []byte) ([]byte, error) { + capturedBody = body + return []byte(`{"cases":[],"total":0}`), nil + }, + } + h := NewCaseHandler(client) + r := withUser(httptest.NewRequest(http.MethodPost, "/cases/search", + strings.NewReader(`{"stateKeys":["open"],"pagination":{"limit":10,"offset":0}}`))) + w := httptest.NewRecorder() + h.SearchCases(w, r) + + assertStatus(t, w, http.StatusOK) + var sent map[string]json.RawMessage + if err := json.Unmarshal(capturedBody, &sent); err != nil { + t.Fatalf("upstream received invalid JSON: %v", err) + } + if _, exists := sent["projectIds"]; exists { + t.Errorf("upstream body unexpectedly contains projectIds: %s", string(sent["projectIds"])) + } + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/csm-portal/backend/internal/handler/cases_test.go` around lines 196 - 257, Add a new subtest in TestSearchCases that verifies the "no projectIds => all accessible cases" path: create a mockEntityCaseClient with searchCasesFn capturing the forwarded body, call NewCaseHandler(...).SearchCases with a request body that omits projectIds (e.g., {"stateKeys":["open"],"pagination":{"limit":10}}), assert the handler returns http.StatusOK and "application/json", unmarshal the capturedBody and assert the "projectIds" key is absent (or not present) in the JSON sent upstream; use the existing decodeJSON helper and assertions consistent with the other subtests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/csm-portal/backend/internal/handler/cases.go`:
- Around line 189-193: The handler currently forwards the incoming request body
(variable body) directly to h.entity.SearchCases, allowing client-controlled
projectIds; update the handler in cases.go to enforce authorization by
validating or replacing body.ProjectIds with the authenticated user's allowed
project IDs (derive allowed IDs from the authenticated user context or an
authorization helper) before calling h.entity.SearchCases, or remove projectIds
from body and pass only the server-determined project scope; ensure you modify
the call site that references h.entity.SearchCases and use user (e.g.,
user.UserID or an auth helper) to compute the permitted projects and inject
those into the request payload sent to the entity.
---
Outside diff comments:
In `@apps/csm-portal/backend/openapi.yaml`:
- Around line 324-353: The OpenAPI spec for the /cases/search endpoint is
missing a 404 response even though the runtime handler SearchCases (and its test
TestSearchCases) maps upstream 404 via mapUpstreamError and asserts that
behavior; update the responses block in openapi.yaml for /cases/search to
include a "404" response with description (e.g., NotFound) and the same
ErrorPayload schema used by 400/401/403/413/500 so the contract matches runtime
behavior and tests.
---
Nitpick comments:
In `@apps/csm-portal/backend/internal/handler/cases_test.go`:
- Around line 196-257: Add a new subtest in TestSearchCases that verifies the
"no projectIds => all accessible cases" path: create a mockEntityCaseClient with
searchCasesFn capturing the forwarded body, call NewCaseHandler(...).SearchCases
with a request body that omits projectIds (e.g.,
{"stateKeys":["open"],"pagination":{"limit":10}}), assert the handler returns
http.StatusOK and "application/json", unmarshal the capturedBody and assert the
"projectIds" key is absent (or not present) in the JSON sent upstream; use the
existing decodeJSON helper and assertions consistent with the other subtests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 06784521-6fb5-4aa9-bd5d-d796438eb31f
📒 Files selected for processing (4)
apps/csm-portal/backend/cmd/server/main.goapps/csm-portal/backend/internal/handler/cases.goapps/csm-portal/backend/internal/handler/cases_test.goapps/csm-portal/backend/openapi.yaml
…s/search Callers now pass projectIds directly in the request body instead of the URL path. The handler is a raw passthrough — no server-side injection is needed, so injectProjectID is removed. The OpenAPI schema is renamed from ProjectCaseSearchPayload to CaseSearchPayload and gains a projectIds filter field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
c939d48 to
5d3f3ec
Compare
- Add 404 response to /cases/search OpenAPI spec; mapUpstreamError can return it and the upstream error test table asserts this mapping - Add TestSearchCases subtest verifying that a body without projectIds is forwarded unchanged (projectIds key absent upstream) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… /deployments/search Project IDs are now passed in the request body via the existing projectIds field in DeploymentSearchPayload. Removes the unused path param extraction from the handler and adds 403/404 responses to the spec (mapUpstreamError can return both). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Flattens two project-scoped search endpoints so callers supply filter IDs directly in the request body instead of the URL path.
POST /cases/search (replaces POST /projects/{id}/cases/search)
injectProjectIDhelper — body forwarded as-is (raw passthrough)ProjectCaseSearchPayload→CaseSearchPayload; adds optionalprojectIdsfilter field404response (missing from old spec;mapUpstreamErrorcan return it)POST /deployments/search (replaces POST /projects/{id}/deployments/search)
DeploymentSearchPayloadalready hadprojectIds403and404responses (both missing from old spec)Test plan
make testpassesPOST /cases/searchwith{"projectIds":["<uuid>"]}returns matching casesPOST /cases/searchwith noprojectIdsreturns all accessible casesPOST /deployments/searchwith{"projectIds":["<uuid>"]}returns matching deployments🤖 Generated with Claude Code