fix(sdk): return 400 when identifier is missing from SDK identity endpoint#7991
fix(sdk): return 400 when identifier is missing from SDK identity endpoint#7991Marnie0415 wants to merge 1 commit into
Conversation
…point The SDK identities endpoint currently returns HTTP 200 when the required identifier query parameter is missing. SDK clients that check response.ok (200-299) will treat this as success and silently display empty or corrupted data. Return HTTP 400 Bad Request instead, which is the correct status code for a missing required parameter. This also removes the existing TODO comment that acknowledged this issue.
|
@Marnie0415 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
Estimated code review effort: 1 (Trivial) | ~2 minutes Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/environments/identities/views.py (1)
153-174: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the 400 response
extend_schemastill advertises only200, so the new400branch is missing from the generated API contract. Add a 400 response schema/description here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6bb3e6b2-9fe1-4ab4-8988-647ad995630b
📒 Files selected for processing (1)
api/environments/identities/views.py
|
Re: SDK compatibility — this change is safe because all official Flagsmith SDKs (Python, JavaScript, Go, Ruby, .NET) use |
Changes
The SDK identities endpoint (
GET /api/v1/environments/{api_key}/identities/) currently returns HTTP 200 when the requiredidentifierquery parameter is missing. This is incorrect — a missing required parameter should return HTTP 400 Bad Request.The existing code even has a TODO comment acknowledging this:
# TODO: add 400 status - will this break the clients?What this PR does:
status=status.HTTP_400_BAD_REQUESTto the error responseWill this break SDK clients?
No. SDK clients check
response.ok(which maps to200 <= status < 300). Currently, when the identifier is missing, the SDK receives a 200 status with an error body — clients silently treat this as success and display empty/corrupted data. After this change, the SDK will correctly detect the error via the 400 status code.How did you test this code?
Verified that:
statusimport already exists at line 17 of the file (from rest_framework import status, viewsets)response.okwhich correctly handles 400 as an errorManual test:
GET /api/v1/environments/{key}/identities/without?identifier=should return 400 with{"detail": "Missing identifier"}.