Batch Sending Api #53
Conversation
WalkthroughA new batch email sending feature is introduced in the Mailtrap Ruby client. This includes a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailtrapClient
participant MailtrapAPI
User->>MailtrapClient: call send_batch(base, requests)
MailtrapClient->>MailtrapClient: validate arguments
MailtrapClient->>MailtrapAPI: POST /api/batch (base + requests)
MailtrapAPI-->>MailtrapClient: API response (success or errors)
MailtrapClient-->>User: return response
Possibly related issues
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
examples/batch.rb (2)
10-15: PreferBase64.strict_encode64to avoid soft line-breaks
Base64.encode64inserts\nevery 60 chars, which can mangle attachments or exceed Mailtrap’s size limits.- content: Base64.encode64('Attachment content'), # base64 encoded content or IO string + content: Base64.strict_encode64('Attachment content'), # single-line base64
24-24: Use ENV for secrets in examplesHard-coding even a placeholder key encourages bad habits.
-client = Mailtrap::Client.new(api_key: 'your-api-key') +client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))lib/mailtrap/client.rb (2)
67-70: Consider optimizing the validation logic.The
all?method call could be more efficient by usingis_a?with the method argument.- unless requests.all?(Mail::Base) - raise ArgumentError, - 'requests should be an array of Mailtrap::Mail::Base objects' - end + unless requests.all? { |request| request.is_a?(Mail::Base) } + raise ArgumentError, + 'requests should be an array of Mailtrap::Mail::Base objects' + end
31-55: Note: Cyclomatic complexity increase due to additional validation.The pipeline failure indicates that the
initializemethod's cyclomatic complexity has exceeded the threshold (8/7). While the new validation on line 42 is necessary and follows the existing pattern, consider refactoring the initialization logic to reduce complexity.Consider extracting validation logic into a separate private method:
private def validate_configuration!(sandbox, bulk, inbox_id) raise ArgumentError, 'bulk mode is not applicable for sandbox API' if sandbox && bulk raise ArgumentError, 'inbox_id is required for sandbox API' if sandbox && inbox_id.nil? endWould you like me to generate a refactored version of the
initializemethod to address the complexity issue?spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/in_sandbox_mode/successfully_sends_a_batch_of_emails.yml (1)
8-10: Use reserved example domains to avoid accidental trafficThe cassette payload embeds
[email protected]and[email protected]. While harmless, RFC 2606 recommendsexample.com,example.org, orexample.netfor documentation & fixtures to guarantee they never resolve in production.
Switching keeps us 100 % safe from stray e-mails if the cassette is ever replayed against live endpoints.spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_template/successfully_sends_a_batch_of_emails_with_template.yml (1)
8-9: Consider RFC-2606 example domains in fixture addressesSame note as the sandbox cassette—replace real-looking domains (
[email protected],[email protected]) withexample.comvariants to remove any chance of misrouting.spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/in_bulk_mode/successfully_sends_a_batch_of_emails.yml (1)
8-9: Prefer placeholder domains reserved for docsFor consistency and zero-risk, substitute
[email protected]/[email protected]withexample.comaddresses in the recorded JSON string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
examples/batch.rb(1 hunks)lib/mailtrap/client.rb(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/in_bulk_mode/successfully_sends_a_batch_of_emails.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/in_sandbox_mode/successfully_sends_a_batch_of_emails.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_API_errors/handles_API_errors.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_template/successfully_sends_a_batch_of_emails_with_template.yml(1 hunks)spec/mailtrap/client_spec.rb(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
examples/batch.rb (2)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(22-24)lib/mailtrap/client.rb (1)
send_batch(64-76)
spec/mailtrap/client_spec.rb (2)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(22-24)lib/mailtrap/client.rb (1)
send_batch(64-76)
lib/mailtrap/client.rb (3)
lib/mailtrap/mail/from_template.rb (1)
as_json(34-41)lib/mailtrap/mail/base.rb (1)
as_json(39-55)lib/mailtrap/attachment.rb (1)
as_json(19-27)
🪛 RuboCop (1.75.5)
spec/mailtrap/client_spec.rb
[convention] 303-303: Context description should match /^when\b/, /^with\b/, or /^without\b/.
(RSpec/ContextWording)
[convention] 320-320: Context description should match /^when\b/, /^with\b/, or /^without\b/.
(RSpec/ContextWording)
🪛 GitHub Actions: Ruby
spec/mailtrap/client_spec.rb
[error] 303-303: Rubocop RSpec/ContextWording: Context description should match /^when\b/, /^with\b/, or /^without\b/.
[error] 320-320: Rubocop RSpec/ContextWording: Context description should match /^when\b/, /^with\b/, or /^without\b/.
lib/mailtrap/client.rb
[error] 31-31: Rubocop Metrics/CyclomaticComplexity: Cyclomatic complexity for initialize is too high. [8/7]
🔇 Additional comments (6)
examples/batch.rb (1)
35-46: Confirm per-recipient minimal payload is acceptedEach
Mailtrap::Mail::Baseinrequestsonly setsto:. If the API does not auto-inheritfromandsubjectfrombase, the call will reproduce the 422 validation error captured in the cassette ('from' is required,'subject' is required). Please verify this with a live call or spec.spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_API_errors/handles_API_errors.yml (1)
45-47: Check double-quoted field names in error stringsThe response shows
''from'' is required(double single-quotes). If the client surfaces these messages directly, they’ll look odd to users. Consider normalising or stripping surplus quotes when presenting validation errors.spec/mailtrap/client_spec.rb (1)
254-390: Comprehensive test coverage for the batch sending feature.The test suite thoroughly covers various scenarios including bulk mode, sandbox mode, template usage, and error handling. The use of VCR cassettes for HTTP interaction recording is appropriate for integration testing.
lib/mailtrap/client.rb (3)
57-76: Well-implemented batch sending method with proper validation.The implementation follows established patterns from the existing
sendmethod and includes comprehensive parameter validation. The use ofcompact_with_empty_arraysfor data cleaning is appropriate for API payloads.
146-148: Consistent path generation following established patterns.The
batch_request_pathmethod properly mirrors the structure ofsend_pathwith appropriate sandbox inbox ID handling.
215-217: Useful utility method for API payload cleaning.The
compact_with_empty_arraysmethod provides more thorough data cleaning than the standardcompactmethod by also removing empty arrays, which is appropriate for API payloads.
lib/mailtrap/client.rb
Outdated
| perform_request(:post, api_host, batch_request_path, { | ||
| base: compact_with_empty_arrays(base.as_json), | ||
| requests: | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the requests array transformation in the API payload.
The requests array is passed directly without applying compact_with_empty_arrays transformation. Ensure this is intentional and that individual request objects don't require the same cleaning as the base object.
🏁 Script executed:
#!/bin/bash
# Description: Check how requests array is handled in the codebase and verify the API expects raw request objects
# Search for batch API usage patterns and documentation
rg -A 10 -B 5 "batch.*api|api.*batch" --type md
# Look for any examples or documentation about batch request format
fd -e rb -e md | xargs rg -l "send_batch|batch.*request" | head -5 | xargs catLength of output: 20855
🏁 Script executed:
#!/bin/bash
# Inspect Mailtrap::Mail::Base class and its JSON serialization methods
rg -n "class Base" -A10 -B2 --type rbLength of output: 66
🏁 Script executed:
#!/bin/bash
# Locate and inspect Mailtrap::Mail::Base#as_json implementation
files=$(fd -e rb base.rb)
for f in $files; do
echo "=== Inspecting $f ==="
rg -n "def as_json" -A20 -B2 "$f"
doneLength of output: 887
🏁 Script executed:
#!/bin/bash
# Display Mailtrap::Mail::Base to inspect defaults, initialize, and to_json
sed -n '1,200p' lib/mailtrap/mail/base.rbLength of output: 2019
Clean request objects in batch payload
In send_batch (lib/mailtrap/client.rb), we apply compact_with_empty_arrays to the base JSON but pass each request raw. Since Mailtrap::Mail::Base#as_json only uses Hash#compact (removing nils but not empty arrays), any empty‐array fields (e.g. cc, bcc, attachments, headers, custom_variables) will still be sent. To ensure consistency and avoid sending unwanted empty arrays, wrap each request in the same cleanup:
• File: lib/mailtrap/client.rb
Method: send_batch
Proposed change:
- perform_request(:post, api_host, batch_request_path, {
- base: compact_with_empty_arrays(base.as_json),
- requests:
- })
+ perform_request(:post, api_host, batch_request_path, {
+ base: compact_with_empty_arrays(base.as_json),
+ requests: requests.map { |req|
+ compact_with_empty_arrays(req.as_json)
+ }
+ })If the API is fine with empty arrays in individual requests, please confirm this is intentional.
📝 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.
| perform_request(:post, api_host, batch_request_path, { | |
| base: compact_with_empty_arrays(base.as_json), | |
| requests: | |
| }) | |
| perform_request(:post, api_host, batch_request_path, { | |
| base: compact_with_empty_arrays(base.as_json), | |
| requests: requests.map { |req| | |
| compact_with_empty_arrays(req.as_json) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 72 to 75, the send_batch method applies
compact_with_empty_arrays to the base JSON but sends each request without
cleaning empty arrays, causing unwanted empty array fields to be included. To
fix this, apply compact_with_empty_arrays to each request's as_json output
before including it in the requests array, ensuring all empty arrays are removed
consistently from both base and individual requests.
lib/mailtrap/client.rb
Outdated
| @@ -53,6 +51,27 @@ def initialize( # rubocop:disable Metrics/ParameterLists | |||
| @http_clients = {} | |||
| end | |||
|
|
|||
| # Sends a batch of emails | |||
| # @param base [Mailtrap::Mail::Base] The base email configuration | |||
There was a problem hiding this comment.
According to API, base parameter has less fields than Mailtrap::Mail::Base. Please consider those options:
- Dedicated class for
basee.g.Mailtrap::Batch::Base - Raise
ArgumentErrorifbasehasto,cc,bcc
@IgorDobryn what do you think?
There was a problem hiding this comment.
I would go with dedicated class, so in batch_send method we won't need to do base.as_json.except('to', 'cc', 'bcc') and having dedicated class would be more descriptive and straitforward
There was a problem hiding this comment.
The problem with the dedicated class is that you'd need two of them: one for regular emails, another for template emails. This creates potentially confusing hierarchy. Having said that, I agree with your point.
There was a problem hiding this comment.
Valid point. In that case, what about having dedicated serializer for Mailtrap::Mail::Base handling batch case and raise errors there, like
perform_request(:post, api_host, batch_request_path, {
base: BaseBatchSerializer(base).to_h,
...,
})
class BaseBatchSerializer
def initialize(base)
raise ArgumentError unless base.to.empty?
end
def to_h
@base.as_json(%w[to cc, bcc])
end
ans as result we will have dedicated batch logic encapsulated somewhere
There was a problem hiding this comment.
Let's use separate class. Otherwise it will create misexpectations. I.e. when somebody constructs mail object with to/cc/bcc
examples/batch.rb
Outdated
There was a problem hiding this comment.
- please remove unnecessary requires
- update formatting making it similar to the existing example files
- make sure the example works
- the the example more unique by removing the comments that present in other files.
| Mailtrap::Mail::Base.new( | ||
| to: [ | ||
| { | ||
| email: ENV.fetch('MAILTRAP_TO_EMAIL', '[email protected]'), |
There was a problem hiding this comment.
Why do you need ENV.fetch here? Could you please explain the use case?
There was a problem hiding this comment.
Only to not expose own email address, as sender [email protected] allows you to send only to account owner.
lib/mailtrap/client.rb
Outdated
| end | ||
|
|
||
| perform_request(:post, api_host, batch_request_path, { | ||
| base: compact_with_empty_arrays(base.as_json), |
There was a problem hiding this comment.
- how about
base.as_json.except('to', 'cc', 'bcc')? - why do you want to
compactbase json?
There was a problem hiding this comment.
why do you want to compact base json?
only to remove 'to', 'cc', 'bcc', which in case of base has to be empty
There was a problem hiding this comment.
It wont be necessary once you add classes for base.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
lib/mailtrap/client.rb (1)
72-75: Apply consistent payload cleaning to request objects.Based on previous review feedback, the
requestsarray should have the same empty array cleaning applied as the base object to ensure consistency and avoid sending unwanted empty fields.Apply the same
compact_with_empty_arraystreatment to each request:perform_request(:post, api_host, batch_request_path, { base: base.as_json.except('to', 'cc', 'bcc'), - requests: + requests: requests.map { |req| compact_with_empty_arrays(req.as_json) } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
examples/batch.rb(1 hunks)lib/mailtrap.rb(1 hunks)lib/mailtrap/client.rb(4 hunks)lib/mailtrap/mail/base.rb(1 hunks)lib/mailtrap/mail/batch/base.rb(1 hunks)lib/mailtrap/mail/batch/from_template.rb(1 hunks)lib/mailtrap/mail/from_template.rb(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/when_in_bulk_mode/successfully_sends_a_batch_of_emails.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/when_in_sandbox_mode/successfully_sends_a_batch_of_emails.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_API_errors/handles_API_errors.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_template/successfully_sends_a_batch_of_emails_with_template.yml(1 hunks)spec/mailtrap/client_spec.rb(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/mailtrap.rb
🚧 Files skipped from review as they are similar to previous changes (6)
- spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_template/successfully_sends_a_batch_of_emails_with_template.yml
- spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/with_API_errors/handles_API_errors.yml
- spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/when_in_bulk_mode/successfully_sends_a_batch_of_emails.yml
- spec/mailtrap/client_spec.rb
- examples/batch.rb
- spec/fixtures/vcr_cassettes/Mailtrap_Client/_send_batch/when_in_sandbox_mode/successfully_sends_a_batch_of_emails.yml
🧰 Additional context used
🧬 Code Graph Analysis (3)
lib/mailtrap/mail/from_template.rb (1)
lib/mailtrap/mail/base.rb (1)
to_batch(84-96)
lib/mailtrap/mail/base.rb (1)
lib/mailtrap/mail/from_template.rb (1)
to_batch(46-56)
lib/mailtrap/mail/batch/base.rb (3)
lib/mailtrap/mail/batch/from_template.rb (3)
attr_accessor(13-51)initialize(21-39)as_json(43-50)lib/mailtrap/mail/base.rb (4)
attr_accessor(7-97)initialize(11-37)attachments(64-66)as_json(39-55)lib/mailtrap/client.rb (2)
attr_reader(8-221)initialize(31-52)
🔇 Additional comments (7)
lib/mailtrap/mail/base.rb (1)
81-96: Well-implemented conversion method.The
to_batchmethod correctly converts a single mail object to a batch mail object, preserving all relevant attributes while excluding recipient-specific fields that are handled separately in batch operations.lib/mailtrap/mail/from_template.rb (1)
43-56: Consistent template-to-batch conversion.The method follows the same pattern as the base class
to_batchmethod while correctly handling template-specific attributes. Good consistency across the codebase.lib/mailtrap/mail/batch/from_template.rb (1)
1-54: Well-structured template batch class.The
FromTemplateclass properly extends the batch base functionality with template-specific attributes and maintains clean inheritance patterns. The implementation correctly handles template UUID and variables while preserving base batch behavior.lib/mailtrap/client.rb (3)
40-40: Good refactoring of validation logic.Extracting argument validation into a dedicated method improves code organization and maintainability.
146-148: Correct batch endpoint path construction.The batch request path correctly mirrors the send path pattern, including sandbox inbox ID when needed.
215-220: Well-organized validation method.The extracted validation method consolidates all argument checks in a logical sequence and provides clear error messages.
lib/mailtrap/mail/batch/base.rb (1)
71-73: Correct JSON serialization for batch base.The
as_jsonmethod correctly excludes recipient fields (to,cc,bcc) that are not relevant for batch base objects, as these are handled in individual requests.
| def send_batch(base, requests) | ||
| unless base.is_a?(Mail::Batch::Base) | ||
| raise ArgumentError, | ||
| 'base should be Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate object' | ||
| end | ||
|
|
||
| unless requests.all?(Mail::Base) | ||
| raise ArgumentError, | ||
| 'requests should be an array of Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate objects' | ||
| end |
There was a problem hiding this comment.
Fix request validation logic.
The validation on line 68 checks requests.all?(Mail::Base) but should validate batch mail objects instead. Batch requests should be instances of Mail::Batch::Base or its subclasses.
- unless requests.all?(Mail::Base)
+ unless requests.all?(Mail::Batch::Base)
raise ArgumentError,
'requests should be an array of Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate objects'
end📝 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.
| def send_batch(base, requests) | |
| unless base.is_a?(Mail::Batch::Base) | |
| raise ArgumentError, | |
| 'base should be Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate object' | |
| end | |
| unless requests.all?(Mail::Base) | |
| raise ArgumentError, | |
| 'requests should be an array of Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate objects' | |
| end | |
| def send_batch(base, requests) | |
| unless base.is_a?(Mail::Batch::Base) | |
| raise ArgumentError, | |
| 'base should be Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate object' | |
| end | |
| unless requests.all?(Mail::Batch::Base) | |
| raise ArgumentError, | |
| 'requests should be an array of Mailtrap::Mail::Batch::Base or Mailtrap::Mail::FromTemplate objects' | |
| end | |
| # ... rest of method ... | |
| end |
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 62 to 71, the validation for the requests
array incorrectly checks if all elements are instances of Mail::Base, but it
should verify that each element is an instance of Mail::Batch::Base or its
subclasses. Update the validation condition to check
requests.all?(Mail::Batch::Base) to correctly enforce the expected type for
batch mail objects.
| def initialize( # rubocop:disable Metrics/ParameterLists,Lint/MissingSuper | ||
| from: nil, | ||
| reply_to: nil, | ||
| subject: nil, | ||
| text: nil, | ||
| html: nil, | ||
| attachments: [], | ||
| headers: {}, | ||
| custom_variables: {}, | ||
| category: nil | ||
| ) | ||
| @from = from | ||
| @reply_to = reply_to | ||
| @subject = subject | ||
| @text = text | ||
| @html = html | ||
| self.attachments = attachments | ||
| @headers = headers | ||
| @custom_variables = custom_variables | ||
| @category = category | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix initialization to properly inherit from parent class.
The initializer disables Lint/MissingSuper but doesn't call the parent's initialize method. This could cause issues with inherited functionality, particularly attachment handling which relies on the parent's setter method.
- def initialize( # rubocop:disable Metrics/ParameterLists,Lint/MissingSuper
+ def initialize( # rubocop:disable Metrics/ParameterLists
from: nil,
reply_to: nil,
subject: nil,
text: nil,
html: nil,
attachments: [],
headers: {},
custom_variables: {},
category: nil
)
+ super(
+ from: from,
+ reply_to: reply_to,
+ subject: subject,
+ text: text,
+ html: html,
+ attachments: attachments,
+ headers: headers,
+ custom_variables: custom_variables,
+ category: category,
+ to: [], # Batch emails don't have direct recipients
+ cc: [],
+ bcc: []
+ )
- @from = from
- @reply_to = reply_to
- @subject = subject
- @text = text
- @html = html
- self.attachments = attachments
- @headers = headers
- @custom_variables = custom_variables
- @category = category
end🤖 Prompt for AI Agents
In lib/mailtrap/mail/batch/base.rb lines 47 to 67, the initialize method
disables the Lint/MissingSuper cop but does not call super, which can break
inherited behavior. Fix this by adding a call to super with the appropriate
arguments at the start of the initialize method to ensure the parent class's
initialization logic, especially for attachments, is executed properly.
| # @param attachments [Array<Hash>] The attachments for the email. | ||
| # @param headers [Hash] The custom headers for the email. | ||
| # @param custom_variables [Hash] The custom variables for the email. | ||
| class Base < Mailtrap::Mail::Base |
There was a problem hiding this comment.
The hierarchy should be reworked. I'll have to prepare a separate PR with the refactoring. Until that, this PR is on hold.
Motivation
Integrate Batch Sending API to support sending multiple emails in a single request, improving efficiency and reducing API calls.
Changes
send_batchtoMailtrap::ClientHow to test
or set yout api key
Summary by CodeRabbit
New Features
Documentation
Tests