-
Notifications
You must be signed in to change notification settings - Fork 4
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
base: main
Are you sure you want to change the base?
Contacts api #49
Conversation
…d response error handling Add Email Templates API functionality
WalkthroughThis 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
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
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 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. (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. (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)
✨ 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 (
|
There was a problem hiding this 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 additionsThe 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 lowUnconditionally 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' +endspec/mailtrap/client_spec.rb (1)
217-219
: Error message expectation contains a stray commaThe 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 wellThe 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 secretsHard-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 – pickobj.id
over hash-styleobj[:id]
created
,updated
, etc. areEmailTemplate
objects. Switching betweencreated[:id]
and
created.id
(orfound[:name]
vsfound.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
: Largeputs
dumps harm readability of example outputDumping 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 likeEtag
,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
TheGET
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
andPATCH
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 usingfilter_sensitive_data
inspec_helper.rb
to strip or normalize these headers.spec/mailtrap/contact_list_spec.rb (2)
1-1
: Add the frozen string literal magic commentThe 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 enableaggregate_failures
RuboCop flags these as exceeding the 5-line guideline. Either:
- Split assertions into separate
it
blocks, or- 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 helperThe 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) endThen 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 placeholdersEven 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
andMAX_BODY_LENGTH
are dead codeThe constants are declared but never referenced.
Either implement input validation (raiseArgumentError
when exceeded) or drop the constants to avoid false “intention” signals.- MAX_LENGTH = 255 - MAX_BODY_LENGTH = 10_000_000spec/mailtrap/template_spec.rb (1)
40-50
: Creating real templates insidelet!
couples tests and slows suites
let!
executes before every example in the group, creating N identical templates (and recording N cassettes).
Preferbefore(: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 driftThe 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 existingapi_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 HTTPAll requests are forced to
use_ssl = true
, but the URI objects are created withURI::HTTP.build
. While Ruby will still perform TLS, logging, redirects, and HSTS logic will report the wrong scheme. Switch toURI::HTTPS.build
(orURI("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 (whichURI::HTTPS.build
would supply). Useuri.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 headersThis 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
andX-Mailtrap-Version
) and match requests on just method and URI. Consider adding tospec/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] } endspec/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
Thehttp_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 replacing1111111
with<ACCOUNT_ID>
, ensuring the bearer token remains a placeholder (<BEARER_TOKEN>
).
168-328
: Validate 404 scenario and clean up headers
The error case forPATCH /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
📒 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 flagRecorded 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 thelist
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
ThePOST
to/email_templates
includes the expected JSON body and<BEARER_TOKEN>
placeholder. This aligns with yourMailtrap::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 thecreate
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 initialPOST
to set up the fixture is correct and matches yourMailtrap::Template.create
call.
167-182
: Review delete request path for non-existent ID
TheDELETE
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 firstPOST
for creation and subsequentGET
for retrieval are correctly ordered and reflect yourTemplate.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 fileEvery 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: … }
whileTemplate#create
expects the object itself.
If the real Mailtrap API wraps template responses similarly (docs sometimes use"data"
), this will raiseNoMethodError
whenresponse
is aHash
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: … }
, butContactList#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 missingNet::HTTP::Patch
constant
Net::HTTP::Patch
is only available on Ruby ≥ 2.6. If you support older Rubies, this will raiseNameError
. Either enforce a minimum Ruby version or fall back toNet::HTTPGenericRequest.new('PATCH', …)
when the constant is absent.
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}" | ||
|
There was a problem hiding this comment.
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.
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.
spec/mailtrap/contact_list_spec.rb
Outdated
describe '#' do | ||
let(:expected_response) do | ||
[ | ||
{ 'id' => 1, 'name' => 'List 1' }, | ||
{ 'id' => 2, 'name' => 'List 2' } | ||
] | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
spec/mailtrap/contact_list_spec.rb
Outdated
@@ -0,0 +1,132 @@ | |||
RSpec.describe Mailtrap::ContactList do | |||
let(:client) { described_class.new('1111111', Mailtrap::Client.new(api_key: 'correct-api-key')) } |
There was a problem hiding this comment.
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.
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.
lib/mailtrap/contact.rb
Outdated
def get(contact_id) | ||
response = @client.get("#{base_path}/#{contact_id}") | ||
ContactDTO.new(response[:data]) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- Specs that stub
.../contacts/#{CGI.escape(email)}
won’t match the real call (/contacts/test@example.com
→ mismatch ⇒ test failure). - 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.
lib/mailtrap/contact.rb
Outdated
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 |
There was a problem hiding this comment.
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.
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.
API_HOST = 'mailtrap.io' | ||
|
There was a problem hiding this comment.
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.
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.
lib/mailtrap/client.rb
Outdated
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this 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) endDon'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 Mailtrapspec/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
📒 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 usingslice(*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 usingcompact
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 sharedbuild_entity
helper.
Motivation
Support new functionality (Contacts API)
https://help.mailtrap.io/article/147-contacts
Changes
Net::HTTPForbidden
andHTTPClientError
to work withmailtrap.io
error responsesMailtrap::ContactsAPI
entity for interactions withContact
APIMailtrap::ContactCreateRequest
,Mailtrap::ContactUpdateRequest
,Mailtrap::ContactUpdateResponse
andMailtrap::Contact
DTOsMailtrap::ContactListsAPI
entity for interactions withContactList
APIMailtrap::Contact
andMailtrap::ContactListRequest
DTOsHow to test
or set yout api key and account id
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores