Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 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
Comment on lines
2790
to
2804
| 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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