Skip to content

Contacts api #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Contacts api #49

wants to merge 4 commits into from

Conversation

sarco3t
Copy link

@sarco3t sarco3t commented Jun 17, 2025

Motivation

Support new functionality (Contacts API)
https://help.mailtrap.io/article/147-contacts

Changes

  • updated handling Net::HTTPForbidden and HTTPClientError to work with mailtrap.io error responses
  • Added new Mailtrap::ContactsAPI entity for interactions with Contact API
    • create
    • update
    • delete
    • list
  • Mailtrap::ContactCreateRequest, Mailtrap::ContactUpdateRequest, Mailtrap::ContactUpdateResponse and Mailtrap::Contact DTOs
  • Added new Mailtrap::ContactListsAPI entity for interactions with ContactList API
    • create
    • update
    • get
    • delete
    • list
  • Mailtrap::Contact and Mailtrap::ContactListRequest DTOs
  • Added new tests
  • Added examples

How to test

rspec

or set yout api key and account id


contact_list = Mailtrap::ContactList.new(account_id, client)
list = contact_list.create(name: 'Test List')

contacts = Mailtrap::Contact.new(account_id, client)
contact = contacts.create(email: 'test@example.com', fields: { first_name: 'John Doe' }, list_ids: [list.id])
puts "Created Contact: #{contact.id}"
# other examples can be found in examples/contact.rb

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Introduced support for managing email templates, contacts, and contact lists via new API client methods.
    • Added example scripts demonstrating usage of contact, contact list, and email template APIs.
  • Bug Fixes

    • Improved error handling and messaging for API client responses.
  • Documentation

    • Updated changelog to document the addition of EmailTemplates API functionality.
  • Tests

    • Added comprehensive test suites for email templates, contacts, and contact lists.
    • Included new VCR fixtures to support API interaction testing.
  • Chores

    • Enhanced VCR configuration to normalize account IDs and filter sensitive data in recorded tests.

Copy link

coderabbitai bot commented Jun 17, 2025

Walkthrough

This change introduces comprehensive support for managing email templates, contacts, and contact lists via the Mailtrap API in the Ruby client library. It adds new API modules, data transfer objects, service classes, example scripts, and extensive test coverage, including VCR fixtures for HTTP interaction replay. The client is refactored for generic HTTP method support and improved error handling.

Changes

File(s) Change Summary
CHANGELOG.md Added changelog entry for v2.3.1 noting EmailTemplates API functionality.
lib/mailtrap.rb Added requires for new modules: api, template, contact, contact_list.
lib/mailtrap/api.rb New module with helpers for request/response normalization and entity building.
lib/mailtrap/client.rb Refactored: supports generic HTTP methods (get, post, patch, delete), improved error handling, new API_HOST const.
lib/mailtrap/template.rb New: EmailTemplateRequest/EmailTemplate DTOs and Template service class for CRUD operations on email templates.
lib/mailtrap/contact.rb New: ContactCreateRequest/ContactUpdateRequest/Contact DTOs and ContactsAPI class for managing contacts.
lib/mailtrap/contact_list.rb New: ContactListRequest/ContactList DTOs and ContactListsAPI class for managing contact lists.
examples/email_template.rb Added example usage for email template CRUD lifecycle.
examples/contact.rb New example script for managing contacts and contact lists.
spec/mailtrap/template_spec.rb New RSpec suite for Template, EmailTemplateRequest, and EmailTemplate classes, covering all CRUD and error cases.
spec/mailtrap/contact_lists_api_spec.rb New RSpec suite for ContactListsAPI, covering list, get, create, update, delete, and error scenarios.
spec/mailtrap/contacts_api_spec.rb New RSpec suite for ContactsAPI, covering get, create, update, delete, and error scenarios.
spec/mailtrap/client_spec.rb Updated client error test to expect response body in error message.
spec/spec_helper.rb VCR config: added normalization for account IDs in URIs and response bodies.
spec/fixtures/vcr_cassettes/Mailtrap_Template/... (multiple new files) Added VCR cassettes for all email template API interactions (create, get, list, update, delete, error cases).

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Client
  participant API
  participant Service as Template/Contacts/ContactLists Service

  User->>Client: Initialize with API key/account id
  User->>Service: Call CRUD method (e.g., create/update/get/delete)
  Service->>Client: Invoke HTTP method (get/post/patch/delete)
  Client->>API: Make HTTP request
  API-->>Client: Return response (success/error)
  Client-->>Service: Return parsed object or raise error
  Service-->>User: Return DTO or error
Loading

Poem

In the warren of code, new features appear,
Templates and contacts are managed with cheer.
The client now speaks HTTP with grace,
Handling errors with a rabbit’s embrace.
With specs and examples, the tests all pass—
🐇 Hopping forward, this release is first-class!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9871da8 and ed52b49.

📒 Files selected for processing (3)
  • lib/mailtrap/api.rb (1 hunks)
  • spec/mailtrap/contact_lists_api_spec.rb (1 hunks)
  • spec/mailtrap/contacts_api_spec.rb (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/mailtrap/api.rb
🧰 Additional context used
🪛 ast-grep (0.38.1)
spec/mailtrap/contact_lists_api_spec.rb

[warning] 3-3: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

spec/mailtrap/contacts_api_spec.rb

[warning] 5-5: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

🪛 RuboCop (1.75.5)
spec/mailtrap/contact_lists_api_spec.rb

[convention] 15-27: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 36-46: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 48-57: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 66-79: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 81-93: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 103-116: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 118-130: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 132-144: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 158-167: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 169-178: Example has too many lines. [7/5]

(RSpec/ExampleLength)

spec/mailtrap/contacts_api_spec.rb

[convention] 30-43: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 64-74: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 76-87: Example has too many lines. [9/5]

(RSpec/ExampleLength)


[convention] 91-100: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 102-112: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 142-155: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 157-170: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 201-222: Example has too many lines. [19/5]

(RSpec/ExampleLength)


[convention] 224-241: Example has too many lines. [15/5]

(RSpec/ExampleLength)

🔇 Additional comments (9)
spec/mailtrap/contacts_api_spec.rb (4)

1-114: LGTM! Comprehensive test coverage for contact retrieval.

The test setup and #get method tests are well-structured, covering UUID and email-based retrieval, proper URL encoding for special characters, and error scenarios. The HTTP stubbing is correctly implemented.


116-171: LGTM! Well-designed create method tests.

The #create method tests properly cover both successful contact creation and validation error scenarios. The request body structure and HTTP stubbing are correctly implemented.


173-243: LGTM! Thorough update method testing.

The #update method tests comprehensively cover both ID and email-based updates with complex data structures. The nested response validation using RSpec matchers is well-implemented.


245-262: LGTM! Appropriate delete method tests.

The #delete method tests correctly cover both ID and email-based deletion with proper HTTP method usage and response validation.

spec/mailtrap/contact_lists_api_spec.rb (5)

1-28: LGTM! Well-structured list method test.

The #list method test properly verifies array response handling and individual contact list attributes. The HTTP stubbing is correctly implemented.


30-58: LGTM! Comprehensive get method testing.

The #get method tests properly cover both successful retrieval and error scenarios (404 Not Found) with appropriate response validation.


60-94: LGTM! Excellent create method test coverage.

The #create method tests appropriately cover successful creation and rate limiting scenarios. The inclusion of 429 rate limit testing demonstrates good API testing practices.


96-145: LGTM! Thorough update method testing.

The #update method tests comprehensively cover successful updates and multiple error scenarios (404 Not Found, 422 validation errors). The validation testing with empty name is particularly valuable.


147-180: LGTM! Comprehensive delete method error handling.

The #delete method tests excellently cover successful deletion and multiple error scenarios (404, 401). The specific testing of Mailtrap::AuthorizationError for 401 status demonstrates good error type differentiation.

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (4)
spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/when_template_does_not_exist/raises_not_found_error.yml (1)

1-327: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)

1-164: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/returns_an_EmailTemplate_object.yml (1)

1-333: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)

1-168: Slim down the VCR fixture by filtering out dynamic and unnecessary headers
Duplicate of the comment for the _list cassette above.

🧹 Nitpick comments (29)
CHANGELOG.md (1)

1-3: Changelog entry omits major user-facing additions

The 2.3.1 note only mentions EmailTemplates, but this PR also delivers the new Contacts and ContactLists APIs. Omitting them makes the changelog an unreliable source for upgrade information.

-## [2.3.1] - 2025-06-17
-- Add EmailTemplates API functionality
+## [2.3.1] - 2025-06-17
+- Add EmailTemplates API functionality
+- Add Contacts API (`Mailtrap::Contact`, DTOs)
+- Add ContactLists API (`Mailtrap::ContactList`, DTOs)
lib/mailtrap.rb (1)

7-9: Consider lazy-loading to keep boot-time low

Unconditionally requiring three new files forces all Contact / ContactList / Template code (and their dependencies) to be loaded even in apps that never use them. Using autoload (or Zeitwerk if available) defers cost until first use.

-module Mailtrap; end
+module Mailtrap
+  autoload :Template,     'mailtrap/template'
+  autoload :Contact,      'mailtrap/contact'
+  autoload :ContactList,  'mailtrap/contact_list'
+end
spec/mailtrap/client_spec.rb (1)

217-219: Error message expectation contains a stray comma

The message being asserted ('client error:, 🫖') has an extra comma after the colon, which looks accidental and makes the text clunky.

-        expect { send_mail }.to raise_error(Mailtrap::Error, 'client error:, 🫖')
+        expect { send_mail }.to raise_error(Mailtrap::Error, 'client error: 🫖')
spec/spec_helper.rb (1)

17-19: Mask account_id in query-params as well

The current substitutions only touch the path and JSON body. If the account_id ever appears in a query string (e.g., /accounts/123/emails?account_id=123) it will leak into the cassette.

-    interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/')
+    interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/')
+    interaction.request.uri.gsub!(/account_id=\d+/, 'account_id=1111111')
examples/email_template.rb (2)

1-3: Prefer ENV variables over hard-coding secrets

Hard-coding an API key in example code makes it too easy for newcomers to accidentally commit real credentials.
Use an environment variable so that the snippet is copy-paste-safe:

-client = Mailtrap::Client.new(api_key: 'your-api-key')
+client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))

Likewise consider ENV['MAILTRAP_ACCOUNT_ID'] (or similar) for the numeric ID.


21-32: Inconsistent accessor style – pick obj.id over hash-style obj[:id]

created, updated, etc. are EmailTemplate objects. Switching between created[:id] and
created.id (or found[:name] vs found.name) is confusing and leaks internal implementation
details ([] most likely delegates to @data). Sticking to reader methods keeps the public
surface minimal and discoverable.

-puts "Created Template: #{created[:id]}"
+puts "Created Template: #{created.id}"-found = templates.get(created[:id])
+found = templates.get(created.id)-puts "Updated Template Name: #{updated[:name]}"
+puts "Updated Template Name: #{updated.name}"
examples/contact.rb (2)

4-5: Same secret-handling remark as above

-client = Mailtrap::Client.new(api_key: 'your-api-key')
+client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))

22-24: Large puts dumps harm readability of example output

Dumping whole arrays / objects via puts produces noisy, often unreadable JSON. Consider pretty
printing or at least outputting counts / ids to keep the example concise.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/maps_response_data_to_EmailTemplate_objects.yml (1)

21-60: Filter out dynamic response headers
This cassette includes many volatile headers (Date, Cf-Ray, X-Request-Id, ETag, X-Runtime, etc.) that can break tests on reruns. Consider configuring VCR to strip or stub these headers to improve fixture stability.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/maps_response_data_to_EmailTemplate_object.yml (1)

25-63: Remove or filter noisy response headers
The response contains a long list of transient security and tracing headers (e.g., Cache-Control, Content-Security-Policy, Cf-Ray, X-Request-Id). Trimming these in fixtures or filtering via VCR will make your tests more maintainable.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/returns_an_EmailTemplate_object.yml (1)

56-63: Strip volatile values from headers
Response headers like Etag, Strict-Transport-Security, and dynamic CSP strings add noise. Filter or remove them for cleaner fixtures.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/when_template_does_not_exist/raises_not_found_error.yml (1)

185-193: Simplify error response headers
The 404 response body is concise, but you still include dynamic headers (Cf-Ray, Cache-Control, etc.). Consider filtering these out to avoid brittle tests.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/returns_an_EmailTemplate_object.yml (1)

167-175: Filter dynamic values in GET response
The GET response includes transient headers (ETag, Cache-Control, Cf-Ray, etc.). Removing or filtering them will help keep the fixture lean and stable.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)

1-332: Consider filtering dynamic response headers to reduce fixture noise.

The cassette correctly captures the POST and PATCH interactions and uses placeholders for sensitive values. However, repeated dynamic headers (e.g., Date, Etag, Cf-Ray, X-Request-Id) inflate the fixture and may change frequently, leading to brittle tests.
Recommend adding VCR header filtering or using filter_sensitive_data in spec_helper.rb to strip or normalize these headers.

spec/mailtrap/contact_list_spec.rb (2)

1-1: Add the frozen string literal magic comment

The rest of the spec suite uses the Ruby 2.3+ immutable string convention. Adding the directive keeps style-checks green and avoids needless diff-noise.

+# frozen_string_literal: true
 RSpec.describe Mailtrap::ContactList do

17-29: Trim long examples or enable aggregate_failures

RuboCop flags these as exceeding the 5-line guideline. Either:

  1. Split assertions into separate it blocks, or
  2. Add aggregate_failures to keep them concise while silencing the cop.

Low-priority, but keeping specs terse improves readability.

spec/mailtrap/contact_spec.rb (3)

215-217: Remove stray debugging output

pp response leaks noisy output to the test run and CI logs.

-      pp response

Delete the line or replace it with a purposeful assertion if you need to inspect the object.


17-31: Avoid large duplicated JSON payloads – extract to a helper

The same expected_response hash (or very similar) is repeated in multiple contexts.
Extract to a helper/factory to DRY the spec and make future payload changes cheaper:

def contact_payload(overrides = {})
  {
    'data' => {
      'id' => contact_id,
      'email' => email,
      'created_at' => 1_748_163_401_202,
      'updated_at' => 1_748_163_401_202,
      'list_ids' => [1, 2],
      'status' => 'subscribed',
      'fields' => { 'first_name' => 'John', 'last_name' => nil }
    }
  }.deep_merge(overrides)
end

Then use contact_payload in every stub.

Also applies to: 51-66, 129-144


6-7: Hard-coded API key/account ID in specs – prefer ENV placeholders

Even in test code, static secrets trigger scanners and may be copy-pasted elsewhere.

-let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) }
+let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_TEST_API_KEY', 'dummy'))) }

Keeps tooling quiet and avoids accidental leakage.

lib/mailtrap/template.rb (1)

46-48: MAX_LENGTH and MAX_BODY_LENGTH are dead code

The constants are declared but never referenced.
Either implement input validation (raise ArgumentError when exceeded) or drop the constants to avoid false “intention” signals.

-    MAX_LENGTH = 255
-    MAX_BODY_LENGTH = 10_000_000
spec/mailtrap/template_spec.rb (1)

40-50: Creating real templates inside let! couples tests and slows suites

let! executes before every example in the group, creating N identical templates (and recording N cassettes).
Prefer before(:all) / around(:all) or a VCR cassette recorded once, then reuse the data to keep the suite fast and deterministic.

before(:all) do
  @created_template = template.create(...)
end
let(:template_id) { @created_template.id }

Remember to clean up in an after(:all) block if the endpoint has side-effects.

lib/mailtrap/client.rb (3)

34-36: Parameter naming drift

The rest of the client uses api_host/api_port; introducing a second host field (general_api_host) complicates configuration and increases the chance of mismatching hosts. Consider re-using the existing api_host for all non-send traffic or clearly documenting when each one is used.

Also applies to: 49-50


55-57: Build URIs with HTTPS, not HTTP

All requests are forced to use_ssl = true, but the URI objects are created with URI::HTTP.build. While Ruby will still perform TLS, logging, redirects, and HSTS logic will report the wrong scheme. Switch to URI::HTTPS.build (or URI("https://…")) for accuracy and clarity.

Also applies to: 59-66, 67-75, 76-83, 85-91


111-117: Use the port from the URI

Net::HTTP.new(uri.host, @api_port) ignores the port embedded in the URI (which URI::HTTPS.build would supply). Use uri.port to stay consistent with the constructed URI.

-http_client = Net::HTTP.new(uri.host, @api_port)
+http_client = Net::HTTP.new(uri.host, uri.port)
spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/returns_an_array_of_EmailTemplate_objects.yml (1)

1-319: Slim down the VCR fixture by filtering out dynamic and unnecessary headers

This cassette includes dozens of dynamic or security-related headers (e.g., Cf-Ray, ETag, Cache-Control, Content-Security-Policy) that bloat its size and make tests brittle. Please configure VCR to strip all but essential headers (for instance only Content-Type and X-Mailtrap-Version) and match requests on just method and URI. Consider adding to spec/spec_helper.rb:

VCR.configure do |config|
  config.before_record do |interaction|
    # Keep only essential response headers
    interaction.response.headers.select! { |key, _| ['Content-Type', 'X-Mailtrap-Version'] }
  end
  config.default_cassette_options = { match_requests_on: [:method, :uri] }
end
spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/maps_response_data_to_EmailTemplate_object.yml (2)

1-167: Consolidate and trim verbose headers for the create interaction
The http_interactions[0] entry includes a large set of low-value headers (e.g., Content-Security-Policy, X-Frame-Options, Cf-Ray, etc.) that bloat the fixture and make it brittle. Please filter out or remove non-essential headers and rely on VCR’s validation of core fields (method, URI, status code) to stabilize tests. Also ensure that the account ID (1111111) and the bearer token are replaced with VCR placeholders (<ACCOUNT_ID>, <BEARER_TOKEN>) to avoid leaking sensitive data.


168-331: Apply same header cleanup and placeholder normalization for the update interaction
Repeat the trimming of unnecessary headers in the second interaction (PATCH /accounts/1111111/email_templates/39715). Confirm that dynamic values—especially the account ID—are filtered to <ACCOUNT_ID> and that only stable headers are retained in the cassette.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/when_template_does_not_exist/raises_not_found_error.yml (2)

1-167: Normalize sensitive data and remove noise from the create step
The initial POST interaction carries the same verbose headers and a hard-coded account ID. Please consolidate the fixture by stripping non-critical headers and replacing 1111111 with <ACCOUNT_ID>, ensuring the bearer token remains a placeholder (<BEARER_TOKEN>).


168-328: Validate 404 scenario and clean up headers
The error case for PATCH /accounts/1111111/email_templates/999999 correctly returns a 404 payload. To improve readability and maintainability, trim unnecessary headers, normalize the account ID to <ACCOUNT_ID>, and verify that your client’s test suite expects this exact {"error":"Not Found"} body.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fee2a44 and 923642c.

📒 Files selected for processing (31)
  • CHANGELOG.md (1 hunks)
  • examples/contact.rb (1 hunks)
  • examples/email_template.rb (2 hunks)
  • lib/mailtrap.rb (1 hunks)
  • lib/mailtrap/client.rb (5 hunks)
  • lib/mailtrap/contact.rb (1 hunks)
  • lib/mailtrap/contact_list.rb (1 hunks)
  • lib/mailtrap/template.rb (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/maps_response_data_to_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/returns_an_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/with_hash_request/returns_an_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/returns_true.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/when_template_does_not_exist/raises_not_found_error.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/maps_response_data_to_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/returns_an_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/when_template_does_not_exist/raises_not_found_error.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/maps_response_data_to_EmailTemplate_objects.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/returns_an_array_of_EmailTemplate_objects.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/maps_response_data_to_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/returns_an_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/when_template_does_not_exist/raises_not_found_error.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1 hunks)
  • spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/with_hash_request/returns_an_EmailTemplate_object.yml (1 hunks)
  • spec/mailtrap/client_spec.rb (1 hunks)
  • spec/mailtrap/contact_list_spec.rb (1 hunks)
  • spec/mailtrap/contact_spec.rb (1 hunks)
  • spec/mailtrap/template_spec.rb (1 hunks)
  • spec/spec_helper.rb (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
examples/email_template.rb (3)
lib/mailtrap/action_mailer/delivery_method.rb (1)
  • client (22-24)
lib/mailtrap/template.rb (5)
  • create (75-82)
  • list (58-61)
  • get (66-69)
  • update (89-96)
  • delete (101-103)
lib/mailtrap/client.rb (2)
  • get (62-65)
  • delete (88-91)
lib/mailtrap/template.rb (2)
lib/mailtrap/client.rb (5)
  • initialize (28-50)
  • get (62-65)
  • post (71-74)
  • patch (80-83)
  • delete (88-91)
lib/mailtrap/action_mailer/delivery_method.rb (1)
  • client (22-24)
lib/mailtrap/client.rb (4)
lib/mailtrap/contact_list.rb (2)
  • get (33-36)
  • delete (60-62)
lib/mailtrap/contact.rb (2)
  • get (82-85)
  • delete (109-111)
lib/mailtrap/template.rb (2)
  • get (66-69)
  • delete (101-103)
lib/mailtrap/mail/base.rb (1)
  • to_json (57-62)
🪛 RuboCop (1.75.5)
spec/mailtrap/template_spec.rb

[convention] 3-3: Do not use multiple top-level example groups - try to nest them.

(RSpec/MultipleDescribes)


[convention] 55-62: Example has too many lines. [6/5]

(RSpec/ExampleLength)


[convention] 258-266: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 283-291: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 331-343: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 363-375: Example has too many lines. [11/5]

(RSpec/ExampleLength)

spec/mailtrap/contact_spec.rb

[convention] 34-47: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 68-78: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 80-91: Example has too many lines. [9/5]

(RSpec/ExampleLength)


[convention] 95-104: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 106-116: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 146-159: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 161-174: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 205-227: Example has too many lines. [20/5]

(RSpec/ExampleLength)


[convention] 229-246: Example has too many lines. [15/5]

(RSpec/ExampleLength)

spec/mailtrap/contact_list_spec.rb

[convention] 1-1: Missing frozen string literal comment.

(Style/FrozenStringLiteralComment)


[convention] 17-29: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 38-48: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 50-59: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 68-81: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 83-95: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 105-118: Example has too many lines. [11/5]

(RSpec/ExampleLength)

🪛 ast-grep (0.38.1)
spec/mailtrap/contact_spec.rb

[warning] 5-5: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

spec/mailtrap/contact_list_spec.rb

[warning] 1-1: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

🔇 Additional comments (15)
spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)

1-165: Cassette looks good – nothing to flag

Recorded interaction is properly anonymised and aligned with the new filtering rules.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_list/maps_response_data_to_EmailTemplate_objects.yml (1)

3-19: Verify placeholder and request structure
The request correctly uses a <BEARER_TOKEN> placeholder, and the HTTP method and URI match the list endpoint. Ensure your VCR setup filters the real token when recording.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/maps_response_data_to_EmailTemplate_object.yml (1)

3-12: Validate create request payload
The POST to /email_templates includes the expected JSON body and <BEARER_TOKEN> placeholder. This aligns with your Mailtrap::Template.create implementation.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/returns_an_EmailTemplate_object.yml (1)

3-12: Ensure consistency in create-and-return fixture
This cassette mirrors the create interaction and returns the new template object. The structure and data align with your domain DTO.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/when_template_does_not_exist/raises_not_found_error.yml (2)

3-24: Validate setup request for delete scenario
The initial POST to set up the fixture is correct and matches your Mailtrap::Template.create call.


167-182: Review delete request path for non-existent ID
The DELETE is correctly pointed at /email_templates/999999, simulating a missing resource. This matches the intended error case.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/returns_an_EmailTemplate_object.yml (1)

3-24: Confirm create-and-fetch sequence
The first POST for creation and subsequent GET for retrieval are correctly ordered and reflect your Template.find workflow.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_get/maps_response_data_to_EmailTemplate_object.yml (1)

1-331: VCR get-recording looks comprehensive and accurate.

The fixture cleanly captures the create (POST) and retrieval (GET) of an email template, with correct placeholders and normalized account ID. No issues found.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_delete/returns_true.yml (1)

1-321: Delete-recording fixture is properly structured.

The cassette covers the full lifecycle of creating and deleting a template, using placeholders for tokens and IDs. Everything aligns with expectations.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_create/with_hash_request/returns_an_EmailTemplate_object.yml (1)

1-168: Create-recording fixture is well-formed.

This fixture accurately captures the POST creation response and uses consistent placeholders for dynamic values. No concerns.

spec/fixtures/vcr_cassettes/Mailtrap_Template/_update/returns_an_EmailTemplate_object.yml (1)

1-333: Update-recording fixture is correct and complete.

The two-step interactions (create then patch) are clearly recorded, with placeholders in place. It matches the client’s expected behavior.

spec/mailtrap/contact_spec.rb (1)

11-13: around { VCR.turned_off … } disables all VCR recording in this file

Every example in the spec relies solely on WebMock stubs, so disabling VCR is fine.
However, if another contributor later adds :vcr metadata, this block will silently bypass cassette recording and the spec will behave differently from others.

Consider scoping VCR off only where needed:

around(:example, :without_vcr) { |ex| VCR.turned_off { ex.run } }

…and tag the relevant examples with :without_vcr.

lib/mailtrap/template.rb (1)

75-82: Inconsistent response wrapping vs. contacts API

Contact#create expects { data: … } while Template#create expects the object itself.
If the real Mailtrap API wraps template responses similarly (docs sometimes use "data"), this will raise NoMethodError when response is a Hash with :data.

Double-check the endpoint and, if wrapped, adjust:

-      EmailTemplate.new(response)
+      EmailTemplate.new(response[:data])

Aligning DTO handling across resources makes the client predictable.

lib/mailtrap/contact_list.rb (1)

41-45: Body wrapper may be required by the API

Contact uses { contact: … }, but ContactList#create posts raw attributes.
If the Mailtrap endpoint expects { contact_list: {...} }, the request will get 422.

Please confirm and wrap similarly for consistency:

-      response = @client.post(base_path, normalised.to_h)
+      response = @client.post(base_path, { contact_list: normalised.to_h })
lib/mailtrap/client.rb (1)

119-133: Risk of missing Net::HTTP::Patch constant

Net::HTTP::Patch is only available on Ruby ≥ 2.6. If you support older Rubies, this will raise NameError. Either enforce a minimum Ruby version or fall back to Net::HTTPGenericRequest.new('PATCH', …) when the constant is absent.

Comment on lines +10 to +17
contact = contacts.create(email: 'test@example.com', fields: { first_name: 'John Doe' }, list_ids: [list.id])
puts "Created Contact: #{contact.id}"

contact = contacts.get(contact.id)
puts "Contact: #{contact.email}"

contact = contacts.update(contact.id, email: 'test2@example.com', fields: { first_name: 'Jane Doe' }, list_ids: [2])
puts "Updated Contact: #{contact.data.email}"

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Mis-matched attribute access hides real-world bugs

contact is first treated as having top-level readers (contact.id, contact.email) and then as
a wrapper exposing contact.data.email. Pick one. If the DTO exposes direct readers there is no
need to traverse data.

Also, the update uses a hard-coded list id of 2, ignoring the freshly created list – causing the
call to fail in real usage.

-contact = contacts.update(contact.id, email: 'test2@example.com', fields: { first_name: 'Jane Doe' }, list_ids: [2])
-puts "Updated Contact: #{contact.data.email}"
-
-contacts.delete(contact.data.id)
+contact = contacts.update(contact.id,
+                          email: 'test2@example.com',
+                          fields: { first_name: 'Jane Doe' },
+                          list_ids: [list.id])
+puts "Updated Contact: #{contact.email}"
+
+contacts.delete(contact.id)
📝 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
contact = contacts.create(email: 'test@example.com', fields: { first_name: 'John Doe' }, list_ids: [list.id])
puts "Created Contact: #{contact.id}"
contact = contacts.get(contact.id)
puts "Contact: #{contact.email}"
contact = contacts.update(contact.id, email: 'test2@example.com', fields: { first_name: 'Jane Doe' }, list_ids: [2])
puts "Updated Contact: #{contact.data.email}"
contact = contacts.create(
email: 'test@example.com',
fields: { first_name: 'John Doe' },
list_ids: [list.id]
)
puts "Created Contact: #{contact.id}"
contact = contacts.get(contact.id)
puts "Contact: #{contact.email}"
contact = contacts.update(contact.id,
email: 'test2@example.com',
fields: { first_name: 'Jane Doe' },
list_ids: [list.id])
puts "Updated Contact: #{contact.email}"
contacts.delete(contact.id)
🤖 Prompt for AI Agents
In examples/contact.rb around lines 10 to 18, the contact object is
inconsistently accessed with direct attributes (contact.id, contact.email) and
nested attributes (contact.data.email). Choose one consistent access pattern,
preferably direct attribute readers, and update all references accordingly.
Also, replace the hard-coded list ID '2' in the update call with the dynamically
created list's ID variable to ensure the update uses the correct list and does
not fail.

Comment on lines 9 to 15
describe '#' do
let(:expected_response) do
[
{ 'id' => 1, 'name' => 'List 1' },
{ 'id' => 2, 'name' => 'List 2' }
]
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

describe '#' loses the method context

RSpec outputs “Mailtrap::ContactList #” which conveys nothing.
You probably meant describe '#list' do.

-describe '#' do
+describe '#list' do
📝 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
describe '#' do
let(:expected_response) do
[
{ 'id' => 1, 'name' => 'List 1' },
{ 'id' => 2, 'name' => 'List 2' }
]
end
describe '#list' do
let(:expected_response) do
[
{ 'id' => 1, 'name' => 'List 1' },
{ 'id' => 2, 'name' => 'List 2' }
]
end
🤖 Prompt for AI Agents
In spec/mailtrap/contact_list_spec.rb around lines 9 to 15, the describe block
uses a generic '#' which loses the method context in RSpec output. Change the
describe string from '#' to the actual instance method name being tested, for
example '#list', to clearly indicate which method the tests are for.

@@ -0,0 +1,132 @@
RSpec.describe Mailtrap::ContactList do
let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid hard-coded credentials inside the repository

Even in unit specs, shipping real-looking secrets ('correct-api-key') violates CWE-798 and triggers secret-scanners.
Inject via ENV or let VCR/wire-mocking short-circuit the request instead:

-let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) }
+let(:client) do
+  described_class.new(
+    ENV.fetch('MAILTRAP_ACCOUNT_ID', '1111111'),
+    Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY', 'dummy-key'))
+  )
+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.

Suggested change
let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) }
let(:client) do
described_class.new(
ENV.fetch('MAILTRAP_ACCOUNT_ID', '1111111'),
Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY', 'dummy-key'))
)
end
🤖 Prompt for AI Agents
In spec/mailtrap/contact_list_spec.rb at line 2, the API key is hard-coded as
'correct-api-key', which risks exposing sensitive information and triggers
secret scanners. Replace the hard-coded string with an environment variable
reference, such as ENV['MAILTRAP_API_KEY'], to inject the key securely.
Alternatively, configure VCR or a mocking library to intercept requests and
avoid using real credentials in tests.

Comment on lines 82 to 90
def get(contact_id)
response = @client.get("#{base_path}/#{contact_id}")
ContactDTO.new(response[:data])
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

URL-encoding is missing – tests will fail and real requests with @, +, etc. break

contact_id can be an e-mail. Passing it raw into the path leaves @, +, ?, … unescaped, so:

  1. Specs that stub .../contacts/#{CGI.escape(email)} won’t match the real call (/contacts/test@example.com → mismatch ⇒ test failure).
  2. Real API calls with special characters will be rejected (HTTP 400) because the server expects the component to be URL-encoded.
-      response = @client.get("#{base_path}/#{contact_id}")
+      encoded_id = CGI.escape(contact_id.to_s)
+      response = @client.get("#{base_path}/#{encoded_id}")

Apply the same CGI.escape (or ERB::Util.url_encode) in #update and #delete to stay consistent.

🤖 Prompt for AI Agents
In lib/mailtrap/contact.rb around lines 82 to 85, the contact_id used in the URL
path is not URL-encoded, causing test failures and API errors with special
characters like '@' or '+'. Fix this by applying CGI.escape (or
ERB::Util.url_encode) to contact_id before including it in the URL string in the
get method. Also, update the update and delete methods similarly to encode
contact_id consistently across all requests.

Comment on lines 100 to 118
def update(contact_id, request)
normalised = request.is_a?(ContactUpdateRequest) ? request : ContactUpdateRequest.new(**request)
response = @client.patch("#{base_path}/#{contact_id}", { contact: normalised.to_h })
ContactUpdateResponse.new(response)
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consistent encoding for update

Follow-up to the issue above – make sure contact_id is encoded before interpolating.

-      response = @client.patch("#{base_path}/#{contact_id}", { contact: normalised.to_h })
+      encoded_id = CGI.escape(contact_id.to_s)
+      response = @client.patch("#{base_path}/#{encoded_id}", { contact: normalised.to_h })

Without this, stubs in specs (lines 230-234) and real API calls diverge.

📝 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
def update(contact_id, request)
normalised = request.is_a?(ContactUpdateRequest) ? request : ContactUpdateRequest.new(**request)
response = @client.patch("#{base_path}/#{contact_id}", { contact: normalised.to_h })
ContactUpdateResponse.new(response)
end
def update(contact_id, request)
normalised = request.is_a?(ContactUpdateRequest) ? request : ContactUpdateRequest.new(**request)
encoded_id = CGI.escape(contact_id.to_s)
response = @client.patch("#{base_path}/#{encoded_id}", { contact: normalised.to_h })
ContactUpdateResponse.new(response)
end
🤖 Prompt for AI Agents
In lib/mailtrap/contact.rb around lines 100 to 104, the contact_id parameter is
used directly in the URL string interpolation without encoding, which causes
inconsistencies between test stubs and real API calls. To fix this, encode
contact_id using an appropriate URI encoding method before interpolating it into
the URL string to ensure consistent and safe URL construction.

Comment on lines +13 to 14
API_HOST = 'mailtrap.io'

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Default host likely incorrect

Documentation shows the general REST endpoints are served from api.mailtrap.io, not the naked domain. Using mailtrap.io may 301/404 and break every Contacts/Lists call.

-API_HOST = 'mailtrap.io'
+API_HOST = 'api.mailtrap.io'
📝 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
API_HOST = 'mailtrap.io'
API_HOST = 'api.mailtrap.io'
🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 13 to 14, the API_HOST constant is set to
'mailtrap.io', which is incorrect as the API endpoints are served from
'api.mailtrap.io'. Update the API_HOST value to 'api.mailtrap.io' to ensure the
client correctly targets the API server and avoids potential 301 redirects or
404 errors.

Comment on lines 150 to 154
body = json_response(response.body)
raise Mailtrap::AuthorizationError, [body[:errors] || body[:error]].flatten
when Net::HTTPForbidden
raise Mailtrap::RejectionError, json_response(response.body)[:errors]
body = json_response(response.body)
raise Mailtrap::RejectionError, [body[:errors] || body[:error]].flatten
when Net::HTTPPayloadTooLarge
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

JSON parse can fail on empty body

Some 401/403 responses come with an empty body. Calling json_response unguarded will raise JSON::ParserError, masking the real HTTP error.

- body = json_response(response.body)
- raise Mailtrap::AuthorizationError, [body[:errors] || body[:error]].flatten
+ body = response.body.to_s.empty? ? {} : json_response(response.body)
+ raise Mailtrap::AuthorizationError, Array(body[:errors] || body[:error])

Same applies to the 403 branch.

🤖 Prompt for AI Agents
In lib/mailtrap/client.rb around lines 150 to 155, the calls to json_response on
the response body for 401 and 403 errors can raise JSON::ParserError if the body
is empty, masking the real HTTP error. To fix this, add a guard to check if the
response body is present and non-empty before calling json_response; if empty,
handle the error by raising the appropriate exception with a default or empty
error message instead of parsing JSON.

@sarco3t sarco3t marked this pull request as draft June 19, 2025 06:43
@sarco3t sarco3t marked this pull request as ready for review June 20, 2025 08:38
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
lib/mailtrap/contact.rb (3)

87-89: URL encoding missing for contact identifier.

The contact_id parameter can be an email address containing special characters like @ and +. Without URL encoding, this will cause test failures (stubs won't match) and API errors.

   def get(contact_id)
-    response = @client.get("#{base_path}/#{contact_id}")
+    encoded_id = CGI.escape(contact_id.to_s)
+    response = @client.get("#{base_path}/#{encoded_id}")
     build_entity(response[:data], Contact)
   end

Don't forget to add require 'cgi' at the top of the file.


112-118: Apply consistent URL encoding in update method.

Same URL encoding issue applies to the update method.

   def update(contact_id, request)
+    encoded_id = CGI.escape(contact_id.to_s)
     response = @client.patch(
-      "#{base_path}/#{contact_id}",
+      "#{base_path}/#{encoded_id}",
       { contact: prepare_request(request, ContactUpdateRequest) }
     )
     build_entity(response, ContactUpdateResponse)
   end

127-129: Apply consistent URL encoding in delete method.

Same URL encoding issue applies to the delete method.

   def delete(contact_id)
-    @client.delete("#{base_path}/#{contact_id}")
+    encoded_id = CGI.escape(contact_id.to_s)
+    @client.delete("#{base_path}/#{encoded_id}")
   end
🧹 Nitpick comments (3)
lib/mailtrap/api.rb (1)

1-1: Add frozen string literal comment for performance.

The static analysis correctly identifies the missing frozen string literal comment.

+# frozen_string_literal: true
+
 module Mailtrap
spec/mailtrap/contacts_api_spec.rb (1)

212-212: Remove debug statement.

The pp response statement appears to be leftover debug code and should be removed.

       response = client.update(contact_id, update_data)
-      pp response
spec/mailtrap/contact_lists_api_spec.rb (1)

1-1: Add frozen string literal comment.

Missing frozen string literal comment for consistency with other test files.

+# frozen_string_literal: true
+
 RSpec.describe Mailtrap::ContactListsAPI do
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 923642c and 9871da8.

📒 Files selected for processing (7)
  • examples/contact.rb (1 hunks)
  • lib/mailtrap.rb (1 hunks)
  • lib/mailtrap/api.rb (1 hunks)
  • lib/mailtrap/contact.rb (1 hunks)
  • lib/mailtrap/contact_list.rb (1 hunks)
  • spec/mailtrap/contact_lists_api_spec.rb (1 hunks)
  • spec/mailtrap/contacts_api_spec.rb (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • examples/contact.rb
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/mailtrap.rb
🧰 Additional context used
🪛 RuboCop (1.75.5)
lib/mailtrap/api.rb

[convention] 1-1: Missing frozen string literal comment.

(Style/FrozenStringLiteralComment)

spec/mailtrap/contacts_api_spec.rb

[convention] 30-43: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 64-74: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 76-87: Example has too many lines. [9/5]

(RSpec/ExampleLength)


[convention] 91-100: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 102-112: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 142-155: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 157-170: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 201-223: Example has too many lines. [20/5]

(RSpec/ExampleLength)


[convention] 225-242: Example has too many lines. [15/5]

(RSpec/ExampleLength)

spec/mailtrap/contact_lists_api_spec.rb

[convention] 1-1: Missing frozen string literal comment.

(Style/FrozenStringLiteralComment)


[convention] 13-25: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 34-44: Example has too many lines. [8/5]

(RSpec/ExampleLength)


[convention] 46-55: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 64-77: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 79-91: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 101-114: Example has too many lines. [11/5]

(RSpec/ExampleLength)


[convention] 116-128: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 130-142: Example has too many lines. [10/5]

(RSpec/ExampleLength)


[convention] 156-165: Example has too many lines. [7/5]

(RSpec/ExampleLength)


[convention] 167-176: Example has too many lines. [7/5]

(RSpec/ExampleLength)

🪛 ast-grep (0.38.1)
spec/mailtrap/contacts_api_spec.rb

[warning] 5-5: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

spec/mailtrap/contact_lists_api_spec.rb

[warning] 1-1: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key'))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html

(hardcoded-secret-rsa-passphrase-ruby)

🔇 Additional comments (8)
lib/mailtrap/api.rb (2)

5-8: LGTM! Clean request normalization logic.

The prepare_request method provides a consistent way to handle both object and hash inputs by normalizing to the expected class before converting to hash.


10-12: LGTM! Efficient entity building with attribute filtering.

The build_entity method efficiently creates response objects by filtering only the relevant attributes using slice(*response_class.members).

spec/mailtrap/contacts_api_spec.rb (2)

64-87: LGTM! Proper handling of special characters in email.

The tests correctly use CGI.escape for email addresses and include a dedicated test for special characters like + in email addresses.


142-170: LGTM! Comprehensive error handling coverage.

The tests cover both successful creation and proper error handling for invalid data with appropriate HTTP status codes.

lib/mailtrap/contact.rb (1)

9-14: LGTM! Well-structured DTOs with proper documentation.

The DTO structs are well-documented with clear attribute descriptions and proper to_h implementations using compact to remove nil values.

Also applies to: 23-29

lib/mailtrap/contact_list.rb (3)

7-12: LGTM! Clean DTO implementation.

The DTO structs are well-designed with proper documentation and efficient to_h implementations.

Also applies to: 18-23


40-43: LGTM! Consistent CRUD operations.

The CRUD methods follow a consistent pattern with proper error handling documentation and clean implementation. Since list_id is an integer, no URL encoding is needed.

Also applies to: 52-55, 65-70, 79-81


85-88: LGTM! Efficient list method implementation.

The list method efficiently maps the response array to ContactList objects using the shared build_entity helper.

@sarco3t sarco3t marked this pull request as draft June 20, 2025 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant