Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded an email-logs capability to the OpenAPI spec: new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API Server
participant Store as EmailLog Store
participant Mailer as External Mailer
Client->>API: GET /api/accounts/{account_id}/email_logs?filters...
API->>Store: Query logs with filters & pagination
Store-->>API: Return EmailLogsListResponse
API-->>Client: 200 OK (EmailLogsListResponse)
Client->>API: GET /api/accounts/{account_id}/email_logs/{sending_message_id}
API->>Store: Fetch SendingMessage by id
Store-->>API: Return SendingMessage (with MessageEvent list)
API-->>Client: 200 OK (SendingMessage)
Note right of Mailer: Mailer may emit events (delivery/open/click/bounce/...) that populate Store asynchronously
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
mklocek
force-pushed
the
MT-20639-documentation-add-email-logs-api
branch
from
524839c to
2655700
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
mklocek
force-pushed
the
MT-20639-documentation-add-email-logs-api
branch
from
2655700 to
b3e2d21
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
specs/email-sending.openapi.yml (1)
1640-1731: Consider using$refto avoid schema duplication.The
messagesarray items (lines 1650-1689) duplicate theSendingMessageListItemschema definition. Using a reference would improve maintainability:Proposed refactor
properties: messages: type: array items: - type: object - properties: - message_id: - type: string - format: uuid - # ... all other properties ... + $ref: '#/components/schemas/SendingMessageListItem'This would also resolve the Spectral warning about the unused
SendingMessageListItemcomponent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/email-sending.openapi.yml` around lines 1640 - 1731, The EmailLogsListResponse schema currently inlines the message item schema under properties.messages and duplicates the existing SendingMessageListItem component; replace the inline items definition with a $ref to the existing SendingMessageListItem component (e.g. items: { $ref: '#/components/schemas/SendingMessageListItem' }) inside the EmailLogsListResponse.messages property, remove the duplicated inline structure, and keep the example values consistent with the referenced schema so the example still validates; this will also address the Spectral warning about the unused SendingMessageListItem.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/email-sending.openapi.yml`:
- Around line 1733-1789: The schema SendingMessageListItem is unused; either
remove it or reference it from EmailLogsListResponse.messages.items — update
EmailLogsListResponse so its messages.items uses a $ref to
'#/components/schemas/SendingMessageListItem' (or delete the
SendingMessageListItem definition if truly unused) to resolve the Spectral
warning and keep schema usage consistent.
- Around line 1203-1208: The x-codeSamples block currently only contains a cURL
(shell) example for the email_logs GET endpoint; add additional language
examples in the required priority order (Node.js/JavaScript, PHP, Python, Ruby,
.NET/C#, Java) under x-codeSamples to match coding guidelines, using equivalent
request examples that mirror the existing curl query string and Authorization
header; if any official Mailtrap SDK lacks this endpoint, add a short comment
inside that language sample indicating the SDK limitation and provide either a
raw HTTP example or a GitBook-generation note instead.
- Around line 1247-1252: The x-codeSamples block for the GET
/accounts/{account_id}/email_logs/{sending_message_id} endpoint currently only
includes a cURL sample; add code samples for Node.js, PHP, Python, Ruby, .NET,
and Java in that priority order alongside the existing cURL entry. For the same
x-codeSamples array (the block under the GET
'https://mailtrap.io/api/accounts/{account_id}/email_logs/{sending_message_id}'
endpoint), add entries with lang and label matching existing examples (e.g.,
lang: node, php, python, ruby, csharp, java) and a source string showing an
authenticated request using the endpoint and placeholder API key, following the
style used in the list endpoint samples. Ensure each new sample uses the same
path variable placeholders ({account_id}, {sending_message_id}) and consistent
header Authorization: Bearer YOUR_API_KEY.
- Around line 1227-1239: Add a 401 Unauthorized response to the responses block
for this endpoint to match other authenticated endpoints: beneath the existing
'400' entry add a '401' response referencing the shared component (e.g. $ref:
'#/components/responses/UNAUTHORIZED' or the spec's unauthorized response name)
so the EmailLogsListResponse endpoint includes 401 alongside 200/400/404/429 for
consistent authentication docs.
- Around line 1857-1867: The events example fails oneOf validation because each
EventDetails schema is ambiguous; fix by adding a required inline discriminator
property to each EventDetails variant (e.g., add a required string property
"type" with enum values like "delivery", "open" in EventDetailsDelivery,
EventDetailsOpen, etc.), update the MessageEvent.details oneOf to use a
discriminator on the "type" property, and ensure the example objects in the
events array include the corresponding "type" values to match their schemas;
alternatively, if you prefer permissive responses, replace the oneOf for
MessageEvent.details with a single generic object schema and document the
variants instead.
- Around line 1256-1266: The responses object for this endpoint is missing the
401 Unauthorized entry; update the responses block (where '200', '404', '429'
are defined) to include a '401' response that references the shared Unauthorized
response (e.g. add '401': $ref: '#/components/responses/Unauthorized') so the
endpoint matches other authenticated endpoints.
---
Nitpick comments:
In `@specs/email-sending.openapi.yml`:
- Around line 1640-1731: The EmailLogsListResponse schema currently inlines the
message item schema under properties.messages and duplicates the existing
SendingMessageListItem component; replace the inline items definition with a
$ref to the existing SendingMessageListItem component (e.g. items: { $ref:
'#/components/schemas/SendingMessageListItem' }) inside the
EmailLogsListResponse.messages property, remove the duplicated inline structure,
and keep the example values consistent with the referenced schema so the example
still validates; this will also address the Spectral warning about the unused
SendingMessageListItem.
42cf217 to
59b5a43
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/email-sending.openapi.yml`:
- Around line 2039-2052: The FilterSubject schema currently lists value as
required but its description allows omitting it for operator values
empty/not_empty; update the schema so value is optional for those operators and
required for the others. Modify FilterSubject (the operator enum and value
property) to either remove value from the top-level required array and add a
conditional requirement (oneOf/anyOf or if/then) that enforces value is present
when operator is not "empty"/"not_empty", or replace the single schema with two
variants: one for operator in ["empty","not_empty"] without value and one for
operators like ci_contain/ci_equal that require value. Ensure examples and
description reflect the change.
- Around line 2081-2094: The FilterNumber schema currently declares
properties.value as type: number but the description and usage (for clicks_count
and opens_count) require whole numbers; update the FilterNumber schema by
changing the value property's type from number to integer, adjust the
description to say "Numeric value (integer)" if needed, and verify any examples
or references (FilterNumber, properties.value) still use integer values (e.g.,
0) to prevent allowing decimal values.
59b5a43 to
3949a66
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
specs/email-sending.openapi.yml (1)
43-47: Consider a more descriptivex-page-description.Other tags have more descriptive page descriptions (e.g., "Manage verified sending domains"). For consistency:
Suggested improvement
- name: email-logs x-page-title: Email Logs - x-page-description: Email logs + x-page-description: View and filter sent email logs description: List and retrieve email sending logs for the account.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/email-sending.openapi.yml` around lines 43 - 47, Update the tag metadata for the "email-logs" tag to use a more descriptive x-page-description (instead of the generic "Email logs") so it matches the detail level of other tags; locate the tag block with name: email-logs and x-page-title: Email Logs and change x-page-description to a clearer phrase such as "List and manage email sending logs and delivery events for the account" (or similar) so the UI page description provides actionable context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@specs/email-sending.openapi.yml`:
- Around line 1814-1841: The MessageEvent.details oneOf is ambiguous because
several EventDetails* schemas share fields; add an OpenAPI discriminator on
MessageEvent.details with propertyName "event_type" and a mapping that links
each event_type value (delivery, open, click, soft_bounce, bounce, spam,
unsubscribe, suspension, reject) to the corresponding components/schemas
(EventDetailsDelivery, EventDetailsOpen, EventDetailsClick, EventDetailsBounce,
EventDetailsSpam, EventDetailsUnsubscribe, EventDetailsReject), and then add an
event_type property (with the correct fixed enum/value) to each EventDetails*
schema so the discriminator can unambiguously resolve which schema to validate.
---
Nitpick comments:
In `@specs/email-sending.openapi.yml`:
- Around line 43-47: Update the tag metadata for the "email-logs" tag to use a
more descriptive x-page-description (instead of the generic "Email logs") so it
matches the detail level of other tags; locate the tag block with name:
email-logs and x-page-title: Email Logs and change x-page-description to a
clearer phrase such as "List and manage email sending logs and delivery events
for the account" (or similar) so the UI page description provides actionable
context.
mklocek
force-pushed
the
MT-20639-documentation-add-email-logs-api
branch
from
3949a66 to
88893e6
Compare
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
GannaRW
approved these changes
Feb 26, 2026
piobeny
approved these changes
Feb 26, 2026
| - lang: shell | ||
| label: 'cURL' | ||
| source: | | ||
| curl -X GET 'https://mailtrap.io/api/accounts/{account_id}/email_logs?filters[sent_after]=2025-01-01T00:00:00Z&filters[sent_before]=2025-01-31T23:59:59Z&filters[to][operator]=ci_equal&filters[to][value]=recipient@example.com&filters[sending_domain_id][operator]=equal&filters[sending_domain_id][value]=3938' \ |
Contributor
There was a problem hiding this comment.
for this curl to work I needed to add turn -g option, should we add it to example?
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
3 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.
Motivation
Changes
How to test
Images and GIFs
None.
Summary by CodeRabbit