Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR updates multiple OpenAPI spec files to standardize error response schemas and examples, add/replace several schemas (permission/resource and sandbox message models), refine parameter formats and server descriptions, and replace inline responses with explicit schema references across account-management, contacts, email-sending, sandbox, and templates specs. ChangesOpenAPI specs — error-schema consolidation, examples, and domain model updates
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specs/account-management.openapi.yml (1)
1047-1053: ⚡ Quick winDuplicate inline 403 responses — consider extracting a shared response component.
The
403block underGET /api/organizations/{organization_id}/sub_accounts(lines 1047–1053) andPOST /api/organizations/{organization_id}/sub_accounts(lines 1109–1115) are byte-for-byte identical. Since they intentionally differ from the sharedPERMISSION_DENIEDcomponent (differentdescriptionand example value), they can't reuse that component as-is — but they can share a new one:♻️ Suggested refactor
Add to
components/responses:+ ORG_PERMISSION_DENIED: + description: Insufficient organization permissions or wrong organization. + content: + application/json: + schema: + $ref: '#/components/schemas/PermissionsDeniedResponse' + example: + errors: Access forbiddenThen replace both inline 403 blocks:
- '403': - description: Insufficient organization permissions or wrong organization. - content: - application/json: - schema: - $ref: '#/components/schemas/PermissionsDeniedResponse' - example: - errors: Access forbidden + '403': + $ref: '#/components/responses/ORG_PERMISSION_DENIED'(apply to both the
GETandPOSToperations)Also applies to: 1109-1115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/account-management.openapi.yml` around lines 1047 - 1053, Two operations (GET /api/organizations/{organization_id}/sub_accounts and POST /api/organizations/{organization_id}/sub_accounts) contain identical inline 403 responses that differ from the existing PERMISSION_DENIED component; extract them into a single new response component (e.g., ORGANIZATION_PERMISSIONS_DENIED) under components/responses preserving the description and example, then replace the inline 403 blocks in both operations with a $ref to that new component and remove the duplicated inline definitions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@specs/account-management.openapi.yml`:
- Around line 1047-1053: Two operations (GET
/api/organizations/{organization_id}/sub_accounts and POST
/api/organizations/{organization_id}/sub_accounts) contain identical inline 403
responses that differ from the existing PERMISSION_DENIED component; extract
them into a single new response component (e.g.,
ORGANIZATION_PERMISSIONS_DENIED) under components/responses preserving the
description and example, then replace the inline 403 blocks in both operations
with a $ref to that new component and remove the duplicated inline definitions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 638f6324-2549-4286-8c26-52005091f87e
📒 Files selected for processing (8)
specs/account-management.openapi.ymlspecs/contacts.openapi.ymlspecs/email-sending-bulk.openapi.ymlspecs/email-sending-transactional.openapi.ymlspecs/email-sending.openapi.ymlspecs/sandbox-sending.openapi.ymlspecs/sandbox.openapi.ymlspecs/templates.openapi.yml
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)
specs/email-sending.openapi.yml (1)
2970-2977:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSplit empty-body and JSON 400s into separate response components.
The
BadRequestschema explicitly states: "Some endpoints return an empty body with HTTP 400; when present, a message may use anerrorfield." However, the sharedBAD_REQUESTresponse still unconditionally declaresapplication/jsoncontent. This forces all 7 operations that reference it to be documented as always returning a JSON body, contradicting the schema's documented behavior. Either removecontentfromBAD_REQUESTor introduce aBAD_REQUEST_JSONcomponent for endpoints with guaranteed JSON payloads, ensuring the spec accurately reflects actual behavior and compliance with OpenAPI 3.1.Possible direction
responses: BAD_REQUEST: - description: Bad request - invalid parameters. - content: - application/json: - schema: - $ref: '#/components/schemas/BadRequest' - example: - error: Bad Request + description: Bad request - invalid parameters. Some endpoints may return an empty body. + + BAD_REQUEST_JSON: + description: Bad request - invalid parameters. + content: + application/json: + schema: + $ref: '#/components/schemas/BadRequest' + example: + error: Bad Request🤖 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 `@specs/email-sending.openapi.yml` around lines 2970 - 2977, The shared BAD_REQUEST response currently declares application/json content but the referenced components/schemas/BadRequest allows empty 400 bodies; update the OpenAPI responses so they reflect both variants: create a new response component BAD_REQUEST_JSON that includes the application/json content and schema (referencing components/schemas/BadRequest), and change the existing BAD_REQUEST component to have no content (only description) for endpoints that may return an empty body; then update the seven operations that reference BAD_REQUEST to point to BAD_REQUEST_JSON only where JSON is guaranteed and leave the rest using the empty-body BAD_REQUEST component.
🤖 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 `@specs/contacts.openapi.yml`:
- Around line 2790-2804: The UnprocessableEntity schema defines errors.<field>
as string[] but several 422 response examples violate this; update the examples
for the endpoints PATCH /contacts/{contact_identifier}
(errors.list_id_included/0), POST /contacts/fields (errors.merge_tag/0 and
errors.name/0), and POST /contacts/exports (errors.filters) so each error value
is an array of strings (e.g., ["contains invalid data"] or ["invalid"]) to match
the UnprocessableEntity schema, or alternatively update the UnprocessableEntity
schema to accept nested arrays/scalars if you intentionally want those
shapes—ensure the chosen fix is applied consistently and the specs remain
OpenAPI 3.1 compliant.
---
Outside diff comments:
In `@specs/email-sending.openapi.yml`:
- Around line 2970-2977: The shared BAD_REQUEST response currently declares
application/json content but the referenced components/schemas/BadRequest allows
empty 400 bodies; update the OpenAPI responses so they reflect both variants:
create a new response component BAD_REQUEST_JSON that includes the
application/json content and schema (referencing components/schemas/BadRequest),
and change the existing BAD_REQUEST component to have no content (only
description) for endpoints that may return an empty body; then update the seven
operations that reference BAD_REQUEST to point to BAD_REQUEST_JSON only where
JSON is guaranteed and leave the rest using the empty-body BAD_REQUEST
component.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: fad0e953-86a1-42fd-a1b3-6d8f74aa69ef
📒 Files selected for processing (3)
specs/account-management.openapi.ymlspecs/contacts.openapi.ymlspecs/email-sending.openapi.yml
| title: UnprocessableEntity | ||
| type: object | ||
| properties: | ||
| errors: | ||
| type: object | ||
| description: Validation errors per attribute. Entire record errors are under `base` key. | ||
| additionalProperties: | ||
| type: array | ||
| items: | ||
| type: string | ||
| example: | ||
| errors: | ||
| email: | ||
| - can't be blank | ||
| examples: {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python -m pip install --quiet pyyaml jsonschema >/dev/null
python - <<'PY'
import yaml
from jsonschema import Draft202012Validator
from pathlib import Path
spec = yaml.safe_load(Path("specs/contacts.openapi.yml").read_text())
schema = spec["components"]["schemas"]["UnprocessableEntity"]
validator = Draft202012Validator(schema)
targets = [
(
"PATCH /api/accounts/{account_id}/contacts/{contact_identifier} 422",
spec["paths"]["/api/accounts/{account_id}/contacts/{contact_identifier}"]["patch"]["responses"]["422"]["content"]["application/json"]["example"],
),
(
"POST /api/accounts/{account_id}/contacts/{contact_identifier}/events 422",
spec["paths"]["/api/accounts/{account_id}/contacts/{contact_identifier}/events"]["post"]["responses"]["422"]["content"]["application/json"]["example"],
),
(
"POST /api/accounts/{account_id}/contacts/exports 422",
spec["paths"]["/api/accounts/{account_id}/contacts/exports"]["post"]["responses"]["422"]["content"]["application/json"]["example"],
),
(
"POST /api/accounts/{account_id}/contacts/fields 422",
spec["paths"]["/api/accounts/{account_id}/contacts/fields"]["post"]["responses"]["422"]["content"]["application/json"]["example"],
),
(
"PATCH /api/accounts/{account_id}/contacts/fields/{field_id} 422",
spec["paths"]["/api/accounts/{account_id}/contacts/fields/{field_id}"]["patch"]["responses"]["422"]["content"]["application/json"]["example"],
),
]
for name, example in targets:
errors = list(validator.iter_errors(example))
if not errors:
print(f"{name}: OK")
continue
print(f"{name}: INVALID")
for err in errors[:3]:
path = "/".join(str(p) for p in err.path) or "<root>"
print(f" - {path}: {err.message}")
PYRepository: mailtrap/mailtrap-openapi
Length of output: 1393
Fix incompatibility between UnprocessableEntity schema and 422 response examples
The UnprocessableEntity schema restricts errors.<field> to string[], but multiple 422 response examples in this file violate this constraint. For example:
- PATCH /contacts/{contact_identifier}:
errors.list_id_included/0shown as nested array['contains invalid data']instead of string - POST /contacts/fields:
errors.merge_tag/0anderrors.name/0shown as nested arrays - POST /contacts/exports:
errors.filtersshown as scalar string'invalid'instead of array
Normalize all 422 examples to conform to the schema, or adjust the schema to accept the current structures. Per coding guidelines, specifications must be compliant with OpenAPI 3.1 standard.
🤖 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 `@specs/contacts.openapi.yml` around lines 2790 - 2804, The UnprocessableEntity
schema defines errors.<field> as string[] but several 422 response examples
violate this; update the examples for the endpoints PATCH
/contacts/{contact_identifier} (errors.list_id_included/0), POST
/contacts/fields (errors.merge_tag/0 and errors.name/0), and POST
/contacts/exports (errors.filters) so each error value is an array of strings
(e.g., ["contains invalid data"] or ["invalid"]) to match the
UnprocessableEntity schema, or alternatively update the UnprocessableEntity
schema to accept nested arrays/scalars if you intentionally want those
shapes—ensure the chosen fix is applied consistently and the specs remain
OpenAPI 3.1 compliant.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
specs/templates.openapi.yml (1)
586-616: 💤 Low valueRedundant property-level
examplealongside schema-levelexampleEach new schema (
UnauthenticatedResponse,PermissionsDeniedResponse,NotFoundResponse) definesexampleboth at the individual property level and at the schema object level. The schema-level example already covers everything a renderer needs; the property-level examples are redundant.♻️ Proposed cleanup (one schema shown; same applies to the other two)
UnauthenticatedResponse: title: UnauthenticatedResponse type: object properties: error: type: string description: Error message - example: Incorrect API token example: error: Incorrect API token🤖 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 `@specs/templates.openapi.yml` around lines 586 - 616, Remove the redundant property-level "example" entries and keep only the schema-level "example" for each response schema: update UnauthenticatedResponse by deleting the example under the "error" property and keep the top-level example, update PermissionsDeniedResponse by deleting the example under the "errors" property and keep the top-level example, and update NotFoundResponse by deleting the example under the "error" property and keep the top-level example; ensure you only remove the per-property "example" keys (not the properties themselves) in UnauthenticatedResponse, PermissionsDeniedResponse, and NotFoundResponse.
🤖 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.
Nitpick comments:
In `@specs/templates.openapi.yml`:
- Around line 586-616: Remove the redundant property-level "example" entries and
keep only the schema-level "example" for each response schema: update
UnauthenticatedResponse by deleting the example under the "error" property and
keep the top-level example, update PermissionsDeniedResponse by deleting the
example under the "errors" property and keep the top-level example, and update
NotFoundResponse by deleting the example under the "error" property and keep the
top-level example; ensure you only remove the per-property "example" keys (not
the properties themselves) in UnauthenticatedResponse,
PermissionsDeniedResponse, and NotFoundResponse.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62514599-df67-4752-ac84-5f4ba3e7019d
📒 Files selected for processing (3)
specs/account-management.openapi.ymlspecs/contacts.openapi.ymlspecs/templates.openapi.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- specs/account-management.openapi.yml
- specs/contacts.openapi.yml
3 participants
https://railsware.atlassian.net/browse/MT-21122
Align OpenAPI error bodies and message/schemas with production APIs
Changes
specs/sandbox.openapi.yml
specs/account-management.openapi.yml
AccountAccess: Added organization to resources[].resource_type (observed in production).
GET .../permissions/resources: Recursive PermissionResourceNode replaces a shallow nested schema; example reflects real hierarchy.
GET / POST /api/organizations/.../sub_accounts: 403 example documents Access forbidden (org vs account messaging).
specs/contacts.openapi.yml
specs/templates.openapi.yml
specs/email-sending-bulk.openapi.yml & specs/email-sending-transactional.openapi.yml
specs/email-sending.openapi.yml (mailtrap.io account API)
specs/sandbox-sending.openapi.yml
How to test
Summary by CodeRabbit
New Features
Documentation
Refactor