Contacts api #49
Contacts api #49
Conversation
WalkthroughThe update introduces new API client classes and data transfer objects for managing contacts and contact lists within the Mailtrap library. It adds comprehensive RSpec test suites for these APIs, along with an example script demonstrating usage. The main Mailtrap library file is updated to require these new modules. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContactsAPI
participant ContactListsAPI
participant Client
participant MailtrapAPI
User->>ContactsAPI: create(options)
ContactsAPI->>Client: POST /accounts/:id/contacts
Client->>MailtrapAPI: HTTP POST
MailtrapAPI-->>Client: JSON response
Client-->>ContactsAPI: Response data
ContactsAPI-->>User: ContactUpdateResponse
User->>ContactListsAPI: create(options)
ContactListsAPI->>Client: POST /accounts/:id/contact_lists
Client->>MailtrapAPI: HTTP POST
MailtrapAPI-->>Client: JSON response
Client-->>ContactListsAPI: Response data
ContactListsAPI-->>User: ContactList
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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_listcassette 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_listcassette 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_listcassette 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_listcassette 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.idover hash-styleobj[:id]
created,updated, etc. areEmailTemplateobjects. 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: Largeputsdumps harm readability of example outputDumping whole arrays / objects via
putsproduces 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
TheGETresponse 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
POSTandPATCHinteractions 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_datainspec_helper.rbto 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_failuresRuboCop flags these as exceeding the 5-line guideline. Either:
- Split assertions into separate
itblocks, or- Add
aggregate_failuresto 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 responseleaks noisy output to the test run and CI logs.- pp responseDelete 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_responsehash (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_payloadin 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_LENGTHandMAX_BODY_LENGTHare dead codeThe constants are declared but never referenced.
Either implement input validation (raiseArgumentErrorwhen 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_hostfor 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.buildwould supply). Useuri.portto 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-TypeandX-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 replacing1111111with<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/999999correctly 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 thelistendpoint. 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
ThePOSTto/email_templatesincludes the expected JSON body and<BEARER_TOKEN>placeholder. This aligns with yourMailtrap::Template.createimplementation.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 thecreateinteraction 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 initialPOSTto set up the fixture is correct and matches yourMailtrap::Template.createcall.
167-182: Review delete request path for non-existent ID
TheDELETEis 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 firstPOSTfor creation and subsequentGETfor retrieval are correctly ordered and reflect yourTemplate.findworkflow.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
POSTcreation 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:vcrmetadata, 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#createexpects{ data: … }whileTemplate#createexpects the object itself.
If the real Mailtrap API wraps template responses similarly (docs sometimes use"data"), this will raiseNoMethodErrorwhenresponseis aHashwith: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
Contactuses{ contact: … }, butContactList#createposts 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::Patchconstant
Net::HTTP::Patchis 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.
examples/contact.rb
Outdated
| contact = contacts.create(email: '[email protected]', 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: '[email protected]', fields: { first_name: 'Jane Doe' }, list_ids: [2]) | ||
| puts "Updated Contact: #{contact.data.email}" | ||
|
|
There was a problem hiding this comment.
🛠️ 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: '[email protected]', 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: '[email protected]',
+ 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: '[email protected]', 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: '[email protected]', fields: { first_name: 'Jane Doe' }, list_ids: [2]) | |
| puts "Updated Contact: #{contact.data.email}" | |
| contact = contacts.create( | |
| email: '[email protected]', | |
| 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: '[email protected]', | |
| 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.
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.
🛠️ 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.
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/[email protected]→ 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.
🛠️ 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.
lib/mailtrap/client.rb
Outdated
| API_HOST = 'mailtrap.io' | ||
|
|
There was a problem hiding this comment.
🛠️ 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.
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.
Actionable comments posted: 2
♻️ Duplicate comments (3)
lib/mailtrap/contact.rb (3)
87-89: URL encoding missing for contact identifier.The
contact_idparameter 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 responsestatement appears to be leftover debug code and should be removed.response = client.update(contact_id, update_data) - pp responsespec/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_requestmethod 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_entitymethod 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.escapefor 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_himplementations usingcompactto 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_himplementations.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_idis an integer, no URL encoding is needed.Also applies to: 52-55, 65-70, 79-81
85-88: LGTM! Efficient list method implementation.The
listmethod efficiently maps the response array to ContactList objects using the sharedbuild_entityhelper.
| # @attr_reader status [String] The contact status (subscribed/unsubscribed) | ||
| # @attr_reader created_at [Integer] The creation timestamp | ||
| # @attr_reader updated_at [Integer] The last update timestamp | ||
| Contact = Struct.new( |
There was a problem hiding this comment.
@IgorDobryn if the only motivation for resource classes it to achieve strictness, then we can do this:
contact_hash = {id: 1}
contact_hash.default_proc = proc { |_, k| raise "unknown key '#{k}'" }Thoughts?
There was a problem hiding this comment.
It can work, but rather unexpected. So, I'd prefer to keep separate class
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/mailtrap/contacts_api.rb (1)
1-72: Good implementation with minor inconsistency in response handling.The
ContactsAPIclass is well-implemented with:
- Proper inheritance from
BaseAPI- Comprehensive CRUD operations
- Good parameter validation using predefined constants
- Excellent documentation with YARD syntax
- Support for both UUID and email identifiers
However, there's an inconsistency in response handling:
def get(contact_id) response = client.get("#{base_path}/#{contact_id}") - build_entity(response[:data], Contact) + build_entity(response, Contact) endThe
getmethod accessesresponse[:data]whilecreateandupdatemethods expect the response structure to match the entity. Consider standardizing the response handling pattern across all methods or document why different approaches are needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
examples/contacts_api.rb(1 hunks)lib/mailtrap.rb(1 hunks)lib/mailtrap/base_api.rb(1 hunks)lib/mailtrap/contact.rb(1 hunks)lib/mailtrap/contact_list.rb(1 hunks)lib/mailtrap/contact_lists_api.rb(1 hunks)lib/mailtrap/contacts_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 due to trivial changes (1)
- examples/contacts_api.rb
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/mailtrap.rb
- lib/mailtrap/contact_list.rb
- lib/mailtrap/contact.rb
🧰 Additional context used
🧬 Code Graph Analysis (4)
lib/mailtrap/contacts_api.rb (4)
lib/mailtrap/contact_lists_api.rb (6)
freeze(6-69)get(14-17)base_path(66-68)create(25-30)update(39-46)delete(52-54)lib/mailtrap/action_mailer/delivery_method.rb (1)
client(22-24)lib/mailtrap/base_api.rb (2)
build_entity(26-28)validate_options!(19-24)lib/mailtrap/client.rb (2)
post(81-83)patch(90-92)
lib/mailtrap/contact_lists_api.rb (4)
lib/mailtrap/contacts_api.rb (4)
get(16-19)create(29-34)update(47-55)delete(61-63)lib/mailtrap/action_mailer/delivery_method.rb (1)
client(22-24)lib/mailtrap/base_api.rb (2)
build_entity(26-28)validate_options!(19-24)lib/mailtrap/client.rb (2)
post(81-83)patch(90-92)
lib/mailtrap/base_api.rb (2)
lib/mailtrap/contact.rb (1)
initialize(34-37)lib/mailtrap/action_mailer/delivery_method.rb (1)
client(22-24)
spec/mailtrap/contact_lists_api_spec.rb (2)
lib/mailtrap/contact_lists_api.rb (5)
list(59-62)get(14-17)create(25-30)update(39-46)delete(52-54)lib/mailtrap/contacts_api.rb (4)
get(16-19)create(29-34)update(47-55)delete(61-63)
🪛 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)
🔇 Additional comments (4)
lib/mailtrap/base_api.rb (1)
1-31: LGTM! Well-designed base class with proper error handling.The
BaseAPIclass provides a solid foundation for API clients with good practices:
- Proper initialization with default values and validation
- Clear error messages for missing required parameters
- Reusable validation and entity building methods
- Clean separation of concerns
The implementation follows Ruby conventions and provides a good abstraction layer for the specific API classes.
lib/mailtrap/contact_lists_api.rb (1)
1-71: Excellent implementation following consistent patterns.The
ContactListsAPIclass demonstrates:
- Consistent inheritance from
BaseAPI- Complete CRUD operations plus listing functionality
- Proper validation using
SUPPORTED_OPTIONSconstant- Good documentation with YARD syntax
- Clean method implementations
The code follows the same patterns established in
ContactsAPIand maintains consistency across the API client library. Thelistmethod properly transforms the response array into entity objects.spec/mailtrap/contacts_api_spec.rb (1)
1-263: Comprehensive test suite with excellent coverage.The test suite demonstrates:
- Complete coverage of all CRUD operations
- Proper testing of both UUID and email identifiers
- Edge case handling (special characters in emails)
- Comprehensive error scenarios
- Clean test structure with proper mocking
The static analysis warning about hardcoded credentials is a false positive - these are test values used for mocking API responses, not production credentials. This is standard practice in test suites.
The tests properly validate response attributes and error handling, ensuring the API client behaves correctly under various conditions.
spec/mailtrap/contact_lists_api_spec.rb (1)
1-181: Well-structured test suite with comprehensive error handling.The test suite provides:
- Complete coverage of all API methods (
list,get,create,update,delete)- Comprehensive error scenario testing (404, 429, 422, 401)
- Proper distinction between different error types (
Mailtrap::ErrorvsMailtrap::AuthorizationError)- Clean test organization with appropriate mocking
The static analysis warning about hardcoded credentials is a false positive - these are test values for mocking, which is standard practice. The test suite effectively validates both successful operations and error conditions, ensuring robust API client behavior.
Motivation
Support new functionality (Contacts API)
https://help.mailtrap.io/article/147-contacts
Changes
Net::HTTPForbiddenandHTTPClientErrorto work withmailtrap.ioerror responsesMailtrap::ContactsAPIentity for interactions withContactAPIMailtrap::ContactCreateRequest,Mailtrap::ContactUpdateRequest,Mailtrap::ContactUpdateResponseandMailtrap::ContactDTOsMailtrap::ContactListsAPIentity for interactions withContactListAPIMailtrap::ContactandMailtrap::ContactListRequestDTOsHow to test
or set yout api key and account id
Summary by CodeRabbit
New Features
Documentation
Tests