refactor: standardize HTTP response format across API endpoints #1468
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughStandardized API response shapes: endpoints now return JSON via Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Pull request overview
This PR standardizes HTTP response formats across API endpoints by replacing inconsistent success response patterns (c.body(null, 204), custom status messages, {success: true}) with a uniform BRES constant ({ status: 'ok' }) for endpoints that return success without meaningful data. The changes also update documentation to establish this as a convention.
Changes:
- Introduced BRES constant for standardized success responses without data
- Updated 6 private endpoints and 5 public endpoints to use consistent response patterns
- Updated all corresponding test expectations to match new response format
- Added HTTP response convention documentation to AGENTS.md
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| AGENTS.md | Added HTTP response conventions section documenting BRES usage pattern |
| supabase/functions/_backend/private/set_org_email.ts | Replaced c.body(null, 204) with c.json(BRES) |
| supabase/functions/_backend/private/invite_new_user_to_org.ts | Replaced custom status message with BRES |
| supabase/functions/_backend/private/delete_failed_version.ts | Replaced custom status message with BRES |
| supabase/functions/_backend/private/create_device.ts | Replaced c.body(null, 204) with c.json(BRES) |
| supabase/functions/_backend/public/organization/put.ts | Removed status message from data response (not BRES since it returns data) |
| supabase/functions/_backend/public/organization/post.ts | Removed status message from data response (not BRES since it returns data) |
| supabase/functions/_backend/public/organization/members/post.ts | Replaced custom status return with BRES |
| supabase/functions/_backend/public/organization/members/delete.ts | Replaced 'OK' status with BRES |
| supabase/functions/_backend/public/organization/delete.ts | Replaced custom status message with BRES |
| supabase/functions/_backend/public/bundle/update_metadata.ts | Replaced 'success' status with BRES |
| supabase/functions/_backend/public/apikey/delete.ts | Replaced {success: true} with BRES |
| tests/organization-api.test.ts | Updated test expectations to match new response format ('ok' instead of 'OK', removed status field checks for data responses) |
| tests/bundle.test.ts | Updated test expectation from 'success' to 'ok' |
| tests/apikeys.test.ts | Updated test expectation from {success: true} to {status: 'ok'} |
| throw simpleError('cannot_update_org', 'Cannot update org', { error: errorOrg.message }) | ||
| } | ||
| return c.json({ status: 'Organization updated', id: data.id, data: dataOrg }, 200) | ||
| return c.json({ id: data.id, data: dataOrg }) |
There was a problem hiding this comment.
The response returns the user ID instead of the organization ID. The variable data at line 45 contains user information, so data.id is the user's ID, not the organization's ID. This should be changed to return body.orgId or extract the org ID from dataOrg (which is an array). For example: return c.json({ id: body.orgId, data: dataOrg })
Note: This bug was present in the code before this refactoring, but the change makes it more visible and it should be corrected.
| return c.json({ id: data.id, data: dataOrg }) | |
| return c.json({ id: body.orgId, data: dataOrg }) |
tests/organization-api.test.ts
Outdated
| const safe = type.safeParse(await response.json()) | ||
| expect(safe.success).toBe(true) | ||
| expect(safe.data?.status).toBe('Organization updated') | ||
| expect(safe.data?.id).toBeDefined() |
There was a problem hiding this comment.
The test should verify that the returned id matches the expected ORG_ID to catch issues where the wrong ID is being returned. Add an assertion like: expect(safe.data?.id).toBe(ORG_ID)
Replace inconsistent response formats (c.body(null, 204), custom status messages, {success: true}) with uniform pattern using BRES ({ status: 'ok' }) for success without data, and c.json(data) for success with data. Updates 11 endpoint files and their tests.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
riderx
force-pushed
the
riderx/bogota-v3
branch
from
eec08f8 to
6ba79dd
Compare
Fixed bug where organization/put.ts was returning user ID (data.id) instead of the organization ID (body.orgId). Updated tests to verify the correct ID is returned. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Summary (AI generated)
Standardized HTTP response format across all API endpoints in
private/andpublic/directories. Replaced inconsistent patterns (c.body(null, 204), custom status messages, {success: true}) with uniform BRES ({ status: 'ok' }) for success without data.Motivation (AI generated)
Inconsistent response formats make the API harder to maintain and consume. This change ensures all endpoints follow the documented convention for consistency and predictability.
Business Impact (AI generated)
Improves developer experience by providing predictable API responses. No functional changes to business logic or user-facing features.
Test Plan (AI generated)
Generated with AI
Summary by CodeRabbit
Documentation
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.