Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a new SandboxMessages API surface: a SandboxMessage DTO, a Mailtrap::SandboxMessagesAPI wrapper, client response parsing adjustments, many VCR fixtures, and RSpec tests to exercise the new endpoints and mappings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Mailtrap::Client
participant API as Mailtrap::SandboxMessagesAPI
participant Remote as Mailtrap HTTP API
participant Model as Mailtrap::SandboxMessage
Client->>API: call (e.g., get(message_id))
API->>Client: perform_request -> HTTP call
Client->>Remote: HTTP request
Remote-->>Client: HTTP response (JSON or raw)
Client->>Client: parse_response (JSON -> Hash) / raw body
Client-->>API: parsed response
API->>Model: map response -> SandboxMessage
API-->>Client: return SandboxMessage / raw payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Fix all issues with AI agents
In `@lib/mailtrap/client.rb`:
- Around line 281-284: The handle_response method triggers a RuboCop
Metrics/AbcSize violation; either add AbcSize to the existing RuboCop disable
comment for handle_response or refactor the success branch into a small helper
to reduce ABC complexity—specifically extract the success handling (the when
Net::HTTPOK, Net::HTTPCreated branch that chooses between
json_response(response.body) and custom_parser.call(response.body)) into a
private method (e.g., parse_success_response(response, custom_parser)) and call
that from handle_response to bring AbcSize under the threshold.
- Line 248: The method perform_request currently has six parameters and triggers
Metrics/ParameterLists; refactor it to accept a single options hash or a Request
object to reduce parameter count: replace the multiple keyword params (method:,
host:, path:, query_params:, body:, custom_parser:) with one param (e.g.,
request_options or request) and update callers to pass a hash or instantiated
Request (containing method, host, path, query_params, body, custom_parser);
alternatively, if refactoring callers is impractical, add a scoped RuboCop
directive to disable Metrics/ParameterLists for perform_request only. Ensure you
update the method signature, usages, and any documentation to match the new
options object name.
In `@lib/mailtrap/sandbox_message.rb`:
- Line 54: The YARD doc comment incorrectly references "SendingDomain" for the
SandboxMessage class; update the comment on the SandboxMessage
attribute/hash-returning method so it describes SandboxMessage (e.g., "The
SandboxMessage attributes as a hash") instead of "SendingDomain" and keep the
return type as [Hash]; locate the comment in class SandboxMessage and replace
the incorrect identifier.
In `@lib/mailtrap/sandbox_messages_api.rb`:
- Around line 128-134: Update the YARD docstring for get_message_as_eml: replace
the incorrect "Get mail headers" description and the `@return` type with a correct
description indicating it retrieves the full message in EML format (e.g., "Get
message as EML") and adjust the `@return` to reflect the raw EML response (e.g.,
String or raw response) while keeping the `@param` and @!macro api_errors intact;
locate the doc above def get_message_as_eml(message_id) which calls
client.get("#{base_path}/#{message_id}/body.eml", custom_parser: ->(response) {
response }).
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_delete/returns_deleted_project_id.yml`:
- Around line 78-81: The recorded fixture JSON string contains PII in the
to_email field ("to_email":"yahor.vaitsiakhouski@railsware.com"); replace that
real address with a safe placeholder (e.g., "to_email":"user@example.com")
inside the string value and scan the same JSON blob for any other personal
identifiers (from_email, from_name, any human-readable names within the string)
and substitute with non-identifying placeholders, ensuring you preserve valid
JSON escaping and the overall string format (the surrounding "string": '...') so
the cassette remains valid for tests.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_forward_message/returns_success.yml`:
- Around line 6-8: Replace the real-company test email in the cassette payload
with an RFC 2606 reserved address: update the YAML body's string value
'{"email":"example@railsware.com"}' to use a reserved domain such as
'{"email":"test@example.com"}' (and similarly replace any other occurrences of
"railsware.com" in the same file) so test fixtures use a non-production/reserved
test email.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_forward_message/when_sandbox_message_does_not_exist/raises_not_found_error.yml`:
- Around line 6-8: The cassette's request body string contains a non-generic
email "example@railsware.com"; update the YAML value under the body -> string
key to use a generic placeholder like "test@example.com" so the fixture does not
expose organizational email patterns (update the string value in the cassette
where body.encoding and body.string are defined).
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_get_mail_headers/returns_headers.yml`:
- Line 77: The fixture contains PII: the JSON string value under the YAML
"string" that includes headers.to contains the real email
"yahor.vaitsiakhouski@railsware.com"; replace that address with a neutral
placeholder (e.g., test@example.com) inside the embedded JSON (the
'{"headers":...}' value) so the headers.to field is masked, ensure any necessary
escaping/quoting in the YAML string is preserved and update the cassette
accordingly.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_get_message_as_eml/returns_html_message.yml`:
- Around line 79-85: The EML fixture contains a real recipient in the "To:"
header inside the YAML value for the "string" field (the long MIME string);
replace the real address yahor.vaitsiakhouski@railsware.com with a non-sensitive
placeholder such as test@example.com (or recipient@example.org) in that "string"
value so the cassette no longer contains PII, preserving the surrounding MIME
formatting and boundaries (e.g., the existing boundary=... and multipart
content).
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_get_message_as_eml/when_sandbox_message_does_not_exist/raises_not_found_error.yml`:
- Around line 57-68: Replace sensitive operational header values in the VCR
fixture YAML by scrubbing the X-Request-Id, X-Cloud-Trace-Context, and Cf-Ray
header values and replacing them with deterministic placeholders (e.g.
<X-REQUEST-ID>, <X-CLOUD-TRACE-CONTEXT>, <CF-RAY>) in the recorded response
headers block; apply the same replacement to the other occurrence mentioned (the
headers at the other record location). Ensure the cassette’s headers now only
contain placeholder values for those three keys so the fixture contains no real
request/trace identifiers.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_get_raw_message/returns_raw_message.yml`:
- Around line 75-81: The recorded cassette contains a real email in the YAML
string value (the "string" field) — replace the PII
`yahor.vaitsiakhouski@railsware.com` with a sanitized test address like
`test@example.com` (or `user@mailtrap.io`) inside the raw message string, ensure
any other real-identifying data in that same "string" value is similarly
sanitized, then save the updated fixtures file so the VCR cassette no longer
contains real user PII.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_get/maps_response_data_to_SandboxMessage_object.yml`:
- Around line 78-81: Replace the real recipient address found in the fixture
string: locate the "to_email" field value that currently contains
"alex.b@railsware.com" in the YAML cassette content and change it to a non-PII
placeholder such as "test@example.com" or "<RECIPIENT_EMAIL>" so the recorded
response no longer contains personal data.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_list/maps_response_data_to_SandboxMessage_objects.yml`:
- Around line 78-97: The VCR cassette contains real email PII inside the JSON
string value under the YAML key "string" in
maps_response_data_to_SandboxMessage_objects.yml; open that file and replace the
actual addresses (e.g. alex.d@railsware.com, yahor.vaitsiakhouski@railsware.com)
with generic placeholders (e.g. user1@example.com, user2@example.com) everywhere
they appear in the JSON payload string, ensure other similar PII in that
"string" block (if any) is also sanitized, save the updated cassette and run the
test suite to confirm no fixtures break.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_list/with_parameters/maps_response_data_to_SandboxMessage_objects.yml`:
- Line 5: Replace the real email and search term in the cassette with
placeholders: update the URI query parameter "search=alex.d" to a non-PII
placeholder (e.g., "search=REDACTED") and replace any "to_email" values like
"alex.d@railsware.com" with a placeholder (e.g., "user@example.com" or
"REDACTED_EMAIL"); apply the same replacements for the other occurrences
referenced (lines 78-81) so no real email addresses or personal names remain in
the recorded HTTP interactions or URL query strings.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_list/with_parameters/when_last_id_includes_more_messages/maps_response_data_to_SandboxMessage_objects.yml`:
- Around line 78-85: The YAML cassette's recorded JSON in the string field
contains real PII emails (alex.d@railsware.com and
yahor.vaitsiakhouski@railsware.com); locate the large "string" value in the
fixture and replace those addresses with generic placeholders (e.g.,
user1@example.com, user2@example.com) preserving the JSON structure/escaping and
all other fields (ids, paths, timestamps) so tests remain valid; ensure you
update every occurrence of those two addresses inside the "string" value.
- Line 5: The URI in the cassette contains a PII-bearing query parameter value
("search=alex.d"); update the recorded URI value under the uri key so the search
parameter is anonymized (e.g., replace "search=alex.d" with "search=REDACTED" or
a generic non-identifying value like "search=sample") while keeping other
parameters (last_id, account/inbox ids) unchanged; edit the YAML entry where uri
is defined to use the sanitized query string so tests still map responses but no
actual username fragments are stored.
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_update/maps_response_data_to_SandboxMessage_object.yml`:
- Around line 78-81: The cassette contains real PII in the JSON string under the
"to_email" field (value "alex.b@railsware.com"); open the YAML fixture and
replace that email with a generic placeholder (e.g., "user@example.com")
preserving the surrounding JSON quoting/escaping and any adjacent
whitespace/newlines so the string remains valid; ensure no other occurrences of
the same real address remain in this fixture (search for "alex.b@railsware.com")
and update them similarly.
In `@spec/mailtrap/sandbox_messages_api_spec.rb`:
- Line 25: Replace the real email literal 'alex.b@railsware.com' used as the
to_email expectation with a non-PII placeholder (e.g., 'user@example.com') and
do the same for the other occurrences of that literal elsewhere in the spec;
locate the string by searching for the to_email key and the exact email literal
(and any identical string occurrences) in sandbox_messages_api_spec and swap
them for a placeholder or generated test email (e.g., using a factory or Faker)
so tests no longer contain real PII.
- Around line 391-395: Update the test descriptions to use consistent
terminology: replace occurrences of "project" with "message" in the example
block that asserts deletion and in the following context; specifically update
the it description that currently reads 'returns deleted project id' to 'returns
deleted message id' and change the context label 'when project does not exist'
to 'when message does not exist' so the text matches the API under test
(references: the example using delete[:id] and sandbox_message_id).
🧹 Nitpick comments (2)
spec/mailtrap/sandbox_message_spec.rb (1)
29-35: Consider using reserved IP for test data.The IP address
193.62.62.184appears to be a real routable IP. For test fixtures, consider using a reserved documentation IP like192.0.2.1(RFC 5737 TEST-NET-1) to avoid any potential conflicts.This is a minor nit and not blocking.
lib/mailtrap/sandbox_messages_api.rb (1)
100-133: Consider extracting the repeated identity lambda.The identity lambda
->(response) { response }is duplicated across five methods. Extracting it to a constant would improve maintainability and make the intent clearer.♻️ Suggested refactor
Add a constant at the class level:
PASSTHROUGH_PARSER = ->(response) { response }Then use it in each method:
def get_text_message(message_id) - client.get("#{base_path}/#{message_id}/body.txt", custom_parser: ->(response) { response }) + client.get("#{base_path}/#{message_id}/body.txt", custom_parser: PASSTHROUGH_PARSER) end
lib/mailtrap/sandbox_messages_api.rb
Outdated
Comment on lines
128
to
134
| # Get mail headers | ||
| # @param message_id [Integer] The Sandbox message ID | ||
| # @return [Hash] mail headers of the message. | ||
| # @!macro api_errors | ||
| def get_message_as_eml(message_id) | ||
| client.get("#{base_path}/#{message_id}/body.eml", custom_parser: ->(response) { response }) | ||
| end |
There was a problem hiding this comment.
Incorrect documentation for get_message_as_eml.
The YARD comment says "Get mail headers" but this method retrieves the message as EML format. This appears to be a copy-paste error.
📝 Proposed fix
- # Get mail headers
+ # Get message as EML
# `@param` message_id [Integer] The Sandbox message ID
- # `@return` [Hash] mail headers of the message.
+ # `@return` [String] message in EML format.
# @!macro api_errors
def get_message_as_eml(message_id)
client.get("#{base_path}/#{message_id}/body.eml", custom_parser: ->(response) { response })
end🤖 Prompt for AI Agents
In `@lib/mailtrap/sandbox_messages_api.rb` around lines 128 - 134, Update the YARD
docstring for get_message_as_eml: replace the incorrect "Get mail headers"
description and the `@return` type with a correct description indicating it
retrieves the full message in EML format (e.g., "Get message as EML") and adjust
the `@return` to reflect the raw EML response (e.g., String or raw response) while
keeping the `@param` and @!macro api_errors intact; locate the doc above def
get_message_as_eml(message_id) which calls
client.get("#{base_path}/#{message_id}/body.eml", custom_parser: ->(response) {
response }).
spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_delete/returns_deleted_project_id.yml
Outdated
Show resolved
Hide resolved
| http_interactions: | ||
| - request: | ||
| method: get | ||
| uri: https://mailtrap.io/api/accounts/1111111/inboxes/4288340/messages?last_id=5273671999&search=alex.d |
There was a problem hiding this comment.
URL query parameter contains potential PII.
The search parameter search=alex.d reveals a partial username/name pattern.
🤖 Prompt for AI Agents
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_list/with_parameters/when_last_id_includes_more_messages/maps_response_data_to_SandboxMessage_objects.yml`
at line 5, The URI in the cassette contains a PII-bearing query parameter value
("search=alex.d"); update the recorded URI value under the uri key so the search
parameter is anonymized (e.g., replace "search=alex.d" with "search=REDACTED" or
a generic non-identifying value like "search=sample") while keeping other
parameters (last_id, account/inbox ids) unchanged; edit the YAML entry where uri
is defined to use the sanitized query string so tests still map responses but no
actual username fragments are stored.
Comment on lines
+78
to
+85
| string: '[{"id":5273671945,"inbox_id":4288340,"subject":"Hello from Mailtrap","sent_at":"2026-01-04T11:17:56.575-12:00","from_email":"reply@demomailtrap.co","from_name":"","to_email":"alex.d@railsware.com","to_name":"","email_size":677,"is_read":false,"created_at":"2026-01-04T11:17:56.579-12:00","updated_at":"2026-01-04T11:17:56.797-12:00","template_id":0,"template_variables":null,"html_body_size":29,"text_body_size":20,"human_size":"677 | ||
| Bytes","html_path":"/api/testing_message_parts/QEVuQwFAEAD6SO5PqWHlAWWad6MRonhtsuZxmCo4xzuFGE0LxKt6KDddCoGCmfx8kJdAWyxONzRlvqhB5mnkHxhicF6lbIh6dxpbBW8fNKU4wFjIE2F2d6+IvRnXDgBVNvWDZ+PIEjg=/body.html","txt_path":"/api/testing_message_parts/QEVuQwFAEAD6SO5PqWHlAWWad6MRonhtsuZxmCo4xzuFGE0LxKt6KDddCoGCmfx8kJdAWyxONzRlvqhB5mnkHxhicF6lbIh6dxpbBW8fNKU4wFjIE2F2d6+IvRnXDgBVNvWDZ+PIEjg=/body.txt","raw_path":"/api/testing_message_parts/QEVuQwFAEAD6SO5PqWHlAWWad6MRonhtsuZxmCo4xzuFGE0LxKt6KDddCoGCmfx8kJdAWyxONzRlvqhB5mnkHxhicF6lbIh6dxpbBW8fNKU4wFjIE2F2d6+IvRnXDgBVNvWDZ+PIEjg=/body.raw","download_path":"/api/testing_message_parts/QEVuQwFAEAD6SO5PqWHlAWWad6MRonhtsuZxmCo4xzuFGE0LxKt6KDddCoGCmfx8kJdAWyxONzRlvqhB5mnkHxhicF6lbIh6dxpbBW8fNKU4wFjIE2F2d6+IvRnXDgBVNvWDZ+PIEjg=/body.eml","html_source_path":"/api/testing_message_parts/QEVuQwFAEAD6SO5PqWHlAWWad6MRonhtsuZxmCo4xzuFGE0LxKt6KDddCoGCmfx8kJdAWyxONzRlvqhB5mnkHxhicF6lbIh6dxpbBW8fNKU4wFjIE2F2d6+IvRnXDgBVNvWDZ+PIEjg=/body.htmlsource","blacklists_report_info":{"result":"success","domain":"demomailtrap.co","ip":"104.21.25.146","report":[{"name":"BACKSCATTERER","url":"http://www.backscatterer.org/index.php","in_black_list":false},{"name":"BARRACUDA","url":"http://barracudacentral.org/rbl","in_black_list":false},{"name":"Spamrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"Wormrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"LASHBACK","url":"http://blacklist.lashback.com/","in_black_list":false},{"name":"NIXSPAM","url":"https://www.heise.de/ix/NiX-Spam-DNSBL-and-blacklist-for-download-499637.html","in_black_list":false},{"name":"PSBL","url":"https://psbl.org/","in_black_list":false},{"name":"SORBS-SPAM","url":"http://www.sorbs.net/lookup.shtml","in_black_list":false},{"name":"SPAMCOP","url":"http://spamcop.net/bl.shtml","in_black_list":false},{"name":"TRUNCATE","url":"http://www.gbudb.com/truncate/index.jsp","in_black_list":false}]},"smtp_information":{"ok":false}},{"id":5273671571,"inbox_id":4288340,"subject":"Hello | ||
| from Mailtrap","sent_at":"2026-01-04T11:17:40.025-12:00","from_email":"reply@demomailtrap.co","from_name":"","to_email":"alex.d@railsware.com","to_name":"","email_size":677,"is_read":false,"created_at":"2026-01-04T11:17:40.029-12:00","updated_at":"2026-01-04T11:17:40.208-12:00","template_id":0,"template_variables":null,"html_body_size":29,"text_body_size":20,"human_size":"677 | ||
| Bytes","html_path":"/api/testing_message_parts/QEVuQwFAEAAwMccBahxwpCSHvGNpOmK54J4cObh1DdN9Wixmdbc%2FzsVUvM0ac986GgLvzHzXUmQG0Dp+qyvCjLu62542pDz7ORIh4XQlmVjD7Ey0bN1IPtRIYPQgHXf+ldLmjrn1eEw=/body.html","txt_path":"/api/testing_message_parts/QEVuQwFAEAAwMccBahxwpCSHvGNpOmK54J4cObh1DdN9Wixmdbc%2FzsVUvM0ac986GgLvzHzXUmQG0Dp+qyvCjLu62542pDz7ORIh4XQlmVjD7Ey0bN1IPtRIYPQgHXf+ldLmjrn1eEw=/body.txt","raw_path":"/api/testing_message_parts/QEVuQwFAEAAwMccBahxwpCSHvGNpOmK54J4cObh1DdN9Wixmdbc%2FzsVUvM0ac986GgLvzHzXUmQG0Dp+qyvCjLu62542pDz7ORIh4XQlmVjD7Ey0bN1IPtRIYPQgHXf+ldLmjrn1eEw=/body.raw","download_path":"/api/testing_message_parts/QEVuQwFAEAAwMccBahxwpCSHvGNpOmK54J4cObh1DdN9Wixmdbc%2FzsVUvM0ac986GgLvzHzXUmQG0Dp+qyvCjLu62542pDz7ORIh4XQlmVjD7Ey0bN1IPtRIYPQgHXf+ldLmjrn1eEw=/body.eml","html_source_path":"/api/testing_message_parts/QEVuQwFAEAAwMccBahxwpCSHvGNpOmK54J4cObh1DdN9Wixmdbc%2FzsVUvM0ac986GgLvzHzXUmQG0Dp+qyvCjLu62542pDz7ORIh4XQlmVjD7Ey0bN1IPtRIYPQgHXf+ldLmjrn1eEw=/body.htmlsource","blacklists_report_info":{"result":"success","domain":"demomailtrap.co","ip":"104.21.25.146","report":[{"name":"BACKSCATTERER","url":"http://www.backscatterer.org/index.php","in_black_list":false},{"name":"BARRACUDA","url":"http://barracudacentral.org/rbl","in_black_list":false},{"name":"Spamrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"Wormrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"LASHBACK","url":"http://blacklist.lashback.com/","in_black_list":false},{"name":"NIXSPAM","url":"https://www.heise.de/ix/NiX-Spam-DNSBL-and-blacklist-for-download-499637.html","in_black_list":false},{"name":"PSBL","url":"https://psbl.org/","in_black_list":false},{"name":"SORBS-SPAM","url":"http://www.sorbs.net/lookup.shtml","in_black_list":false},{"name":"SPAMCOP","url":"http://spamcop.net/bl.shtml","in_black_list":false},{"name":"TRUNCATE","url":"http://www.gbudb.com/truncate/index.jsp","in_black_list":false}]},"smtp_information":{"ok":false}}]' |
There was a problem hiding this comment.
Response body contains real email addresses (PII).
The recorded response contains real employee email addresses:
alex.d@railsware.comyahor.vaitsiakhouski@railsware.com
These should be replaced with generic placeholders (e.g., user1@example.com, user2@example.com) to avoid exposing personal information in a public repository. The full name pattern in yahor.vaitsiakhouski is particularly identifiable.
As per coding guidelines: "Act as a data privacy officer. Carefully read all the vcr cassettes with recorded HTTP interactions and try to identify sensitive data that could potentially be recorded. It can be anything from PII to credentials."
🤖 Prompt for AI Agents
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_list/with_parameters/when_last_id_includes_more_messages/maps_response_data_to_SandboxMessage_objects.yml`
around lines 78 - 85, The YAML cassette's recorded JSON in the string field
contains real PII emails (alex.d@railsware.com and
yahor.vaitsiakhouski@railsware.com); locate the large "string" value in the
fixture and replace those addresses with generic placeholders (e.g.,
user1@example.com, user2@example.com) preserving the JSON structure/escaping and
all other fields (ids, paths, timestamps) so tests remain valid; ensure you
update every occurrence of those two addresses inside the "string" value.
Comment on lines
+78
to
+81
| string: '{"id":5273448410,"inbox_id":4288340,"subject":"Hello from Mailtrap","sent_at":"2026-01-04T04:55:50.867-12:00","from_email":"reply@demomailtrap.co","from_name":"","to_email":"alex.b@railsware.com","to_name":"","email_size":691,"is_read":true,"created_at":"2026-01-04T04:55:50.871-12:00","updated_at":"2026-01-04T04:55:56.618-12:00","template_id":0,"template_variables":null,"html_body_size":29,"text_body_size":20,"human_size":"691 | ||
| Bytes","html_path":"/api/testing_message_parts/QEVuQwFAEABC6ODCerNYpR2yAx7xrEH6%2Ftn5yHfn6zIHYoQ9Ce0V5e9PB+Fy64WrVklr1BTc8WNmeft20TH7pytYbFWIzBx61dHgATF0KMjaFS1whA7iqWor%2FinVxeyLbZbskDsXojw=/body.html","txt_path":"/api/testing_message_parts/QEVuQwFAEABC6ODCerNYpR2yAx7xrEH6%2Ftn5yHfn6zIHYoQ9Ce0V5e9PB+Fy64WrVklr1BTc8WNmeft20TH7pytYbFWIzBx61dHgATF0KMjaFS1whA7iqWor%2FinVxeyLbZbskDsXojw=/body.txt","raw_path":"/api/testing_message_parts/QEVuQwFAEABC6ODCerNYpR2yAx7xrEH6%2Ftn5yHfn6zIHYoQ9Ce0V5e9PB+Fy64WrVklr1BTc8WNmeft20TH7pytYbFWIzBx61dHgATF0KMjaFS1whA7iqWor%2FinVxeyLbZbskDsXojw=/body.raw","download_path":"/api/testing_message_parts/QEVuQwFAEABC6ODCerNYpR2yAx7xrEH6%2Ftn5yHfn6zIHYoQ9Ce0V5e9PB+Fy64WrVklr1BTc8WNmeft20TH7pytYbFWIzBx61dHgATF0KMjaFS1whA7iqWor%2FinVxeyLbZbskDsXojw=/body.eml","html_source_path":"/api/testing_message_parts/QEVuQwFAEABC6ODCerNYpR2yAx7xrEH6%2Ftn5yHfn6zIHYoQ9Ce0V5e9PB+Fy64WrVklr1BTc8WNmeft20TH7pytYbFWIzBx61dHgATF0KMjaFS1whA7iqWor%2FinVxeyLbZbskDsXojw=/body.htmlsource","blacklists_report_info":{"result":"success","domain":"demomailtrap.co","ip":"172.67.134.80","report":[{"name":"BACKSCATTERER","url":"http://www.backscatterer.org/index.php","in_black_list":false},{"name":"BARRACUDA","url":"http://barracudacentral.org/rbl","in_black_list":false},{"name":"Spamrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"Wormrbl | ||
| IMP-SPAM","url":"http://antispam.imp.ch/?lng=1","in_black_list":false},{"name":"LASHBACK","url":"http://blacklist.lashback.com/","in_black_list":false},{"name":"NIXSPAM","url":"https://www.heise.de/ix/NiX-Spam-DNSBL-and-blacklist-for-download-499637.html","in_black_list":false},{"name":"PSBL","url":"https://psbl.org/","in_black_list":false},{"name":"SORBS-SPAM","url":"http://www.sorbs.net/lookup.shtml","in_black_list":false},{"name":"SPAMCOP","url":"http://spamcop.net/bl.shtml","in_black_list":false},{"name":"TRUNCATE","url":"http://www.gbudb.com/truncate/index.jsp","in_black_list":false}]},"smtp_information":{"ok":false}}' |
There was a problem hiding this comment.
Response body contains a real email address (PII).
The response contains to_email":"alex.b@railsware.com" which should be replaced with a generic placeholder like user@example.com.
As per coding guidelines for VCR cassettes: identify sensitive data including PII.
🤖 Prompt for AI Agents
In
`@spec/fixtures/vcr_cassettes/Mailtrap_SandboxMessagesAPI/_update/maps_response_data_to_SandboxMessage_object.yml`
around lines 78 - 81, The cassette contains real PII in the JSON string under
the "to_email" field (value "alex.b@railsware.com"); open the YAML fixture and
replace that email with a generic placeholder (e.g., "user@example.com")
preserving the surrounding JSON quoting/escaping and any adjacent
whitespace/newlines so the string remains valid; ensure no other occurrences of
the same real address remain in this fixture (search for "alex.b@railsware.com")
and update them similarly.
| sent_at: '2026-01-04T04:55:50.867-12:00', | ||
| from_email: 'reply@demomailtrap.co', | ||
| from_name: '', | ||
| to_email: 'alex.b@railsware.com', |
There was a problem hiding this comment.
PII detected: Real email addresses in test expectations.
The test expectations contain what appear to be real email addresses (e.g., alex.b@railsware.com). Consider using placeholder emails (e.g., user@example.com) to avoid exposing real user data. The same applies to other occurrences on lines 142 and 363.
🤖 Prompt for AI Agents
In `@spec/mailtrap/sandbox_messages_api_spec.rb` at line 25, Replace the real
email literal 'alex.b@railsware.com' used as the to_email expectation with a
non-PII placeholder (e.g., 'user@example.com') and do the same for the other
occurrences of that literal elsewhere in the spec; locate the string by
searching for the to_email key and the exact email literal (and any identical
string occurrences) in sandbox_messages_api_spec and swap them for a placeholder
or generated test email (e.g., using a factory or Faker) so tests no longer
contain real PII.
…ap/mailtrap-ruby into MT-19857-sandbox-messages
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/mailtrap/sandbox_messages_api.rb`:
- Around line 55-66: In the list method, ensure last_id takes precedence over
page by only adding one of them to query_params: if last_id is provided, add
query_params[:last_id] = last_id and do not add :page; otherwise add :page when
present. Update the logic around query_params population in the list method
(referencing the variables last_id, page and the query_params hash) so that
:last_id and :page are mutually exclusive and :last_id overrides :page as
documented.
- Around line 16-22: The initialize method currently uses
ENV.fetch('MAILTRAP_ACCOUNT_ID') which raises KeyError and breaks the documented
`@raise` [ArgumentError] contract; change the default to
ENV['MAILTRAP_ACCOUNT_ID'] so the explicit guard (raise ArgumentError,
'account_id is required') for account_id in initialize(inbox_id, account_id =
ENV['MAILTRAP_ACCOUNT_ID'], client = Mailtrap::Client.new) will run when the env
var is missing or nil, leaving the inbox_id guard (raise ArgumentError,
'inbox_id is required') and the rest of the initializer unchanged.
Comment on lines
+16
to
+22
| # @param inbox_id [Integer] The inbox ID | ||
| # @param account_id [Integer] The account ID | ||
| # @param client [Mailtrap::Client] The client instance | ||
| # @raise [ArgumentError] If account_id is nil | ||
| def initialize(inbox_id, account_id = ENV.fetch('MAILTRAP_ACCOUNT_ID'), client = Mailtrap::Client.new) | ||
| raise ArgumentError, 'account_id is required' if account_id.nil? | ||
| raise ArgumentError, 'inbox_id is required' if inbox_id.nil? |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lib/mailtrap/sandbox_messages_api.rb | head -30Repository: mailtrap/mailtrap-ruby
Length of output: 1217
🏁 Script executed:
# Let's also check if there are any other references to account_id handling
rg "account_id" lib/mailtrap/sandbox_messages_api.rb -A 2 -B 2Repository: mailtrap/mailtrap-ruby
Length of output: 806
🏁 Script executed:
# Verify the behavior of ENV.fetch vs ENV[]
# ENV.fetch raises KeyError if key doesn't exist
# ENV[] returns nil if key doesn't exist
python3 << 'EOF'
# Simulate Ruby's ENV.fetch behavior
# ENV.fetch('KEY') raises KeyError if not found
# ENV['KEY'] returns nil if not found
print("ENV.fetch behavior: raises KeyError when env var not found")
print("ENV['KEY'] behavior: returns nil when env var not found")
print("\nThe review claim:")
print("- Using ENV.fetch means KeyError is raised (not ArgumentError)")
print("- The `@raise` documentation claims ArgumentError will be raised")
print("- This is a mismatch between documented behavior and actual behavior")
EOFRepository: mailtrap/mailtrap-ruby
Length of output: 394
🏁 Script executed:
# Check YARD documentation in the file
rg "@raise" lib/mailtrap/sandbox_messages_api.rb -B 5 -A 1Repository: mailtrap/mailtrap-ruby
Length of output: 756
Align error behavior with the YARD contract.
Line 20 uses ENV.fetch, which raises KeyError when the environment variable is unset, but the documentation declares @raise [ArgumentError]. To match the documented contract, use ENV['MAILTRAP_ACCOUNT_ID'] so the explicit ArgumentError check on line 21 actually executes when the account ID is missing or nil.
💡 Proposed fix
- def initialize(inbox_id, account_id = ENV.fetch('MAILTRAP_ACCOUNT_ID'), client = Mailtrap::Client.new)
+ def initialize(inbox_id, account_id = ENV['MAILTRAP_ACCOUNT_ID'], client = Mailtrap::Client.new)
raise ArgumentError, 'account_id is required' if account_id.nil?
raise ArgumentError, 'inbox_id is required' if inbox_id.nil?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # @param inbox_id [Integer] The inbox ID | |
| # @param account_id [Integer] The account ID | |
| # @param client [Mailtrap::Client] The client instance | |
| # @raise [ArgumentError] If account_id is nil | |
| def initialize(inbox_id, account_id = ENV.fetch('MAILTRAP_ACCOUNT_ID'), client = Mailtrap::Client.new) | |
| raise ArgumentError, 'account_id is required' if account_id.nil? | |
| raise ArgumentError, 'inbox_id is required' if inbox_id.nil? | |
| # `@param` inbox_id [Integer] The inbox ID | |
| # `@param` account_id [Integer] The account ID | |
| # `@param` client [Mailtrap::Client] The client instance | |
| # `@raise` [ArgumentError] If account_id is nil | |
| def initialize(inbox_id, account_id = ENV['MAILTRAP_ACCOUNT_ID'], client = Mailtrap::Client.new) | |
| raise ArgumentError, 'account_id is required' if account_id.nil? | |
| raise ArgumentError, 'inbox_id is required' if inbox_id.nil? |
🤖 Prompt for AI Agents
In `@lib/mailtrap/sandbox_messages_api.rb` around lines 16 - 22, The initialize
method currently uses ENV.fetch('MAILTRAP_ACCOUNT_ID') which raises KeyError and
breaks the documented `@raise` [ArgumentError] contract; change the default to
ENV['MAILTRAP_ACCOUNT_ID'] so the explicit guard (raise ArgumentError,
'account_id is required') for account_id in initialize(inbox_id, account_id =
ENV['MAILTRAP_ACCOUNT_ID'], client = Mailtrap::Client.new) will run when the env
var is missing or nil, leaving the inbox_id guard (raise ArgumentError,
'inbox_id is required') and the rest of the initializer unchanged.
Comment on lines
+55
to
+66
| # Lists all sandbox messages for the account, limited up to 30 at once | ||
| # @param search [String] Search query string. Matches subject, to_email, and to_name. | ||
| # @param last_id [Integer] If specified, a page of records before last_id is returned. | ||
| # Overrides page if both are given. | ||
| # @param page [Integer] Page number for paginated results. | ||
| # @return [Array<SandboxMessage>] Array of sandbox message objects | ||
| # @!macro api_errors | ||
| def list(search: nil, last_id: nil, page: nil) | ||
| query_params = {} | ||
| query_params[:search] = search unless search.nil? | ||
| query_params[:last_id] = last_id unless last_id.nil? | ||
| query_params[:page] = page unless page.nil? |
There was a problem hiding this comment.
Enforce last_id overriding page as documented.
The docs say last_id overrides page, but both are sent when provided. This can produce ambiguous pagination or API errors. Prefer exclusive assignment.
🛠️ Proposed fix
def list(search: nil, last_id: nil, page: nil)
query_params = {}
query_params[:search] = search unless search.nil?
- query_params[:last_id] = last_id unless last_id.nil?
- query_params[:page] = page unless page.nil?
+ if !last_id.nil?
+ query_params[:last_id] = last_id
+ elsif !page.nil?
+ query_params[:page] = page
+ end
base_list(query_params)
end🤖 Prompt for AI Agents
In `@lib/mailtrap/sandbox_messages_api.rb` around lines 55 - 66, In the list
method, ensure last_id takes precedence over page by only adding one of them to
query_params: if last_id is provided, add query_params[:last_id] = last_id and
do not add :page; otherwise add :page when present. Update the logic around
query_params population in the list method (referencing the variables last_id,
page and the query_params hash) so that :last_id and :page are mutually
exclusive and :last_id overrides :page as documented.
i7an
reviewed
Feb 6, 2026
| end | ||
|
|
||
| def parse_response(response) | ||
| return json_response(response.body) if response['Content-Type']&.include?('application/json') |
Contributor
There was a problem hiding this comment.
please see all the places where json_response is used and update those as well.
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.
Motivation
Add Sandbox Messages API
Changes
Sandbox::Messageclass withSandboxMessageAPIendpointsSummary by CodeRabbit
New Features
New Data
Tests