Skip to content

Add Accounts API#84

Merged
DagonWat merged 6 commits intomainfrom
MT-19860-accounts
Feb 24, 2026
Merged

Add Accounts API#84
DagonWat merged 6 commits intomainfrom
MT-19860-accounts

Conversation

@DagonWat
Copy link
Contributor

@DagonWat DagonWat commented Jan 5, 2026

Motivation

Add Accounts API to Ruby SDK

Changes

  • Adding new Sandbox::Account class with AccountsAPI endpoints

Summary by CodeRabbit

  • New Features

    • Added an Accounts API to list accounts and access levels; introduced an Account data object and a usage example.
  • Documentation

    • Updated README and changelog to include the new Accounts API and usage guidance.
  • Tests

    • Added tests and recorded fixtures covering successful listing and authorization error handling.

@coderabbitai
Copy link

coderabbitai bot commented Jan 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e9a617a and 691d1f4.

📒 Files selected for processing (1)
  • lib/mailtrap.rb
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/mailtrap.rb

📝 Walkthrough

Walkthrough

Adds an Accounts API: new Account DTO and Mailtrap::AccountsAPI with a list method, VCR fixtures and RSpec tests, README and CHANGELOG updates, an examples usage file, and a new require_relative in lib/mailtrap.rb. No existing public method signatures removed.

Changes

Cohort / File(s) Summary
Changelog & README
CHANGELOG.md, README.md
Added "Accounts API" changelog entry; renamed "General API:" → "General:" and inserted examples/accounts_api.rb link under General.
Core library
lib/mailtrap.rb, lib/mailtrap/account.rb, lib/mailtrap/accounts_api.rb
Added Account Struct DTO and Mailtrap::AccountsAPI (initializer, list, response_class = Account, attr_reader :client, private base_path); added require_relative 'mailtrap/accounts_api' to lib/mailtrap.rb.
Example
examples/accounts_api.rb
New Ruby usage example showing Mailtrap client initialization and AccountsAPI#list, with env var guidance.
Tests
spec/mailtrap/account_spec.rb, spec/mailtrap/accounts_api_spec.rb
Added RSpec specs for Account and AccountsAPI covering success mapping and 401 auth error using VCR.
VCR fixtures
spec/fixtures/vcr_cassettes/Mailtrap_AccountsAPI/_list/*
Added fixtures for successful GET /api/accounts and 401 unauthorized scenario (recorded HTTP interactions).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Dev
  participant Client as Mailtrap::Client
  participant API as Mailtrap::AccountsAPI
  participant Server as Mailtrap API

  Dev->>Client: initialize(api_key)
  Dev->>API: AccountsAPI.new(client)
  Dev->>API: call list
  API->>Client: build GET /api/accounts request
  Client->>Server: GET /api/accounts (Bearer token)
  Server-->>Client: 200 OK + JSON [accounts] / 401 Unauthorized
  Client-->>API: parsed response / raised error
  API-->>Dev: Array<Account> (mapped DTOs) / AuthorizationError
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • IgorDobryn
  • mklocek

Poem

🐰 I hopped through lines to add a key,
Accounts and structs now live with glee,
Tests and VCRs caught what came to be,
README and changelog sing with me! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description includes Motivation and Changes sections but omits required sections (How to test checklist, Images and GIFs comparison table) and lacks implementation detail. Complete the description by adding the 'How to test' section with test checkboxes and clarify that the class is 'Mailtrap::Account' not 'Sandbox::Account' as mentioned in Changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add Accounts API' directly and clearly summarizes the main change in the PR, which introduces new Accounts API functionality with Account DTO, AccountsAPI class, and supporting tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch MT-19860-accounts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@i7an i7an marked this pull request as draft January 7, 2026 16:36
Base automatically changed from add-projects-api to main January 13, 2026 12:35
@DagonWat DagonWat marked this pull request as ready for review February 20, 2026 11:40
@DagonWat DagonWat requested review from IgorDobryn and i7an February 20, 2026 11:40
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (5)
lib/mailtrap/account.rb (1)

6-8: Consider using @!attribute [r] instead of @attr_reader for Struct fields.

@attr_reader is not a standard YARD tag; @!attribute [r] is the idiomatic YARD macro for documenting read-only Struct/class attributes and will be correctly picked up by YARD's parser.

♻️ Proposed refactor
-  # `@attr_reader` id [Integer] The account ID
-  # `@attr_reader` name [String] The account name
-  # `@attr_reader` access_levels [Array] The account access levels
+  # @!attribute [r] id
+  #   `@return` [Integer] The account ID
+  # @!attribute [r] name
+  #   `@return` [String] The account name
+  # @!attribute [r] access_levels
+  #   `@return` [Array] The account access levels
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/account.rb` around lines 6 - 8, Replace the nonstandard YARD
tags `@attr_reader` with the idiomatic `@!attribute [r]` doc macro for the
Struct fields (id, name, access_levels) in lib/mailtrap/account.rb so YARD
properly recognizes them; update each docline to use `@!attribute [r] id`,
`@!attribute [r] name`, and `@!attribute [r] access_levels` (preserving the
existing type and description) and remove the old `@attr_reader` entries.
spec/mailtrap/accounts_api_spec.rb (2)

12-15: Add field-level assertions to verify Account struct mapping.

The test confirms the type and count of returned objects but never verifies that the individual fields (id, name, access_levels) are actually populated from the API response. A bug in field mapping (e.g., wrong key name) would pass this test. Since the VCR cassette produces deterministic data, this is straightforward to add.

♻️ Proposed addition
 it 'maps response data to Account objects' do
   expect(list).to all(be_a(Mailtrap::Account))
   expect(list.size).to eq(1)
+
+  account = list.first
+  expect(account.id).to be_a(Integer)
+  expect(account.name).to be_a(String)
+  expect(account.access_levels).to be_an(Array)
 end

Alternatively, pin the exact values from the VCR cassette fixture to tighten the contract:

+  expect(account.id).to eq(123456)
+  expect(account.name).to eq('My Account')
+  expect(account.access_levels).to eq([1000])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/accounts_api_spec.rb` around lines 12 - 15, The test only
asserts types and count for 'list' but not field mapping; update the spec in
spec/mailtrap/accounts_api_spec.rb to assert that each Mailtrap::Account in
'list' has correctly populated fields by checking account.id, account.name and
account.access_levels against the expected values from the VCR cassette (or at
minimum that they are not nil/empty); reference the Mailtrap::Account objects
returned in 'list' and add expectations for id, name and access_levels to ensure
the API response keys are mapped properly.

22-25: Remove redundant assertion on line 24.

The error.messages array is joined into error.message (messages.join(', ')), so checking error.message.include?(...) on line 23 and error.messages.any? { |msg| msg.include?(...) } on line 24 tests the same data path twice. Keep line 23 and remove line 24.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/accounts_api_spec.rb` around lines 22 - 25, Remove the
redundant assertion that checks error.messages (the expectation using
error.messages.any? { |msg| msg.include?('Incorrect API token') }) in the spec;
keep the existing check that inspects error.message.include?('Incorrect API
token') and delete the second expectation so the spec only asserts the message
once (refer to the expectations around Mailtrap::AuthorizationError and the
lines that call error.message.include? and error.messages.any?).
examples/accounts_api.rb (2)

14-14: accounts.list return value is silently discarded when run as a script.

In an IRB/REPL session the result is printed automatically, but when the file is executed with ruby examples/accounts_api.rb the return value is thrown away and no output appears. Assign the result or use p/pp so the example is self-contained and demonstrable as a runnable script.

♻️ Proposed fix
-accounts.list
+result = accounts.list
+pp result
 # => [#<struct Mailtrap::Account id=123456, name="My Account", access_levels=[1000]>]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/accounts_api.rb` at line 14, The script calls accounts.list but
discards its return value, so running the file prints nothing; update the
example to surface the result by assigning the return to a variable (e.g.,
result = accounts.list) and then print it (e.g., p result or puts
result.inspect), or directly wrap the call with p/pp (p accounts.list) so the
script is demonstrable when executed; modify the line containing accounts.list
accordingly.

3-14: Consider restructuring the example so the env-var pattern is the active code.

The current flow is logically inverted: hardcoded credentials are used to build the client/accounts objects (lines 3–5), and only afterward does the comment tell the reader to use environment variables (lines 7–9), with the preferred no-arg form commented out (line 11). A reader scanning top-to-bottom will use the hardcoded values without realising the env-var pattern even exists.

Additionally, account_id = 3229 looks like a real account ID rather than a recognisable placeholder. For consistency with 'your-api-key' it should be something like 100_000 or 'your-account-id'.

♻️ Proposed restructure
 require 'mailtrap'

+# Option 1: Provide credentials explicitly
+client     = Mailtrap::Client.new(api_key: 'your-api-key')
+account_id = 100_000
+accounts   = Mailtrap::AccountsAPI.new(account_id, client)
+
+# Option 2: Set credentials via environment variables and use defaults
 # export MAILTRAP_API_KEY='your-api-key'
 # export MAILTRAP_ACCOUNT_ID=your-account-id
-
-client = Mailtrap::Client.new(api_key: 'your-api-key')
-account_id = 3229
-accounts = Mailtrap::AccountsAPI.new(account_id, client)
-
-# Set your API credentials as environment variables
-# export MAILTRAP_API_KEY='your-api-key'
-# export MAILTRAP_ACCOUNT_ID=your-account-id
-
-# accounts = Mailtrap::AccountsAPI.new
+# accounts = Mailtrap::AccountsAPI.new

 # Get all accounts
-accounts.list
+result = accounts.list
 # => [#<struct Mailtrap::Account id=123456, name="My Account", access_levels=[1000]>]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/accounts_api.rb` around lines 3 - 14, The example currently
constructs Mailtrap::Client.new and Mailtrap::AccountsAPI.new using hardcoded
values before showing the env-var pattern; reorder the snippet so the active
code reads credentials from ENV (use ENV['MAILTRAP_API_KEY'] and
ENV['MAILTRAP_ACCOUNT_ID'] to build Mailtrap::Client.new and
Mailtrap::AccountsAPI.new) and use a clearly placeholder account_id like 100_000
or 'your-account-id'; move the hardcoded example into a commented block below as
an alternative and keep the call to accounts.list as the final action so readers
run the env-var flow by default (refer to Mailtrap::Client.new,
Mailtrap::AccountsAPI.new, account_id, and accounts.list when making the
change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/mailtrap/account.rb`:
- Line 4: The YARD comment contains a typo where the Cyrillic string "Фссщгте"
was typed instead of "Account"; update the comment in lib/mailtrap/account.rb
(the YARD module/class comment above the Account DTO) to read "Data Transfer
Object for Account" so the documentation correctly references the Account DTO
and remove the Cyrillic text.

In
`@spec/fixtures/vcr_cassettes/Mailtrap_AccountsAPI/_list/maps_response_data_to_Account_objects.yml`:
- Line 77: Replace the real person's full name in the recorded response body by
editing the fixture entry that contains the JSON string with "name":"Yahor
Vaitsiakhouski" (the value inside the string field '[{"id":2326475,"name":"Yahor
Vaitsiakhouski","access_levels":[100]}]'). Substitute that value with an
anonymized placeholder (e.g., "name":"REDACTED_NAME" or "name":"Test User") and
save the YAML so the cassette no longer exposes PII while preserving the rest of
the record (id and access_levels).

In `@spec/mailtrap/account_spec.rb`:
- Line 17: The test description in spec/mailtrap/account_spec.rb is a copy-paste
error — change the example description in the it block that currently reads
"creates a project with all attributes" to correctly refer to an account (e.g.,
"creates an account with all attributes") so the test name matches the spec file
and the assertions in the example.

---

Nitpick comments:
In `@examples/accounts_api.rb`:
- Line 14: The script calls accounts.list but discards its return value, so
running the file prints nothing; update the example to surface the result by
assigning the return to a variable (e.g., result = accounts.list) and then print
it (e.g., p result or puts result.inspect), or directly wrap the call with p/pp
(p accounts.list) so the script is demonstrable when executed; modify the line
containing accounts.list accordingly.
- Around line 3-14: The example currently constructs Mailtrap::Client.new and
Mailtrap::AccountsAPI.new using hardcoded values before showing the env-var
pattern; reorder the snippet so the active code reads credentials from ENV (use
ENV['MAILTRAP_API_KEY'] and ENV['MAILTRAP_ACCOUNT_ID'] to build
Mailtrap::Client.new and Mailtrap::AccountsAPI.new) and use a clearly
placeholder account_id like 100_000 or 'your-account-id'; move the hardcoded
example into a commented block below as an alternative and keep the call to
accounts.list as the final action so readers run the env-var flow by default
(refer to Mailtrap::Client.new, Mailtrap::AccountsAPI.new, account_id, and
accounts.list when making the change).

In `@lib/mailtrap/account.rb`:
- Around line 6-8: Replace the nonstandard YARD tags `@attr_reader` with the
idiomatic `@!attribute [r]` doc macro for the Struct fields (id, name,
access_levels) in lib/mailtrap/account.rb so YARD properly recognizes them;
update each docline to use `@!attribute [r] id`, `@!attribute [r] name`, and
`@!attribute [r] access_levels` (preserving the existing type and description)
and remove the old `@attr_reader` entries.

In `@spec/mailtrap/accounts_api_spec.rb`:
- Around line 12-15: The test only asserts types and count for 'list' but not
field mapping; update the spec in spec/mailtrap/accounts_api_spec.rb to assert
that each Mailtrap::Account in 'list' has correctly populated fields by checking
account.id, account.name and account.access_levels against the expected values
from the VCR cassette (or at minimum that they are not nil/empty); reference the
Mailtrap::Account objects returned in 'list' and add expectations for id, name
and access_levels to ensure the API response keys are mapped properly.
- Around line 22-25: Remove the redundant assertion that checks error.messages
(the expectation using error.messages.any? { |msg| msg.include?('Incorrect API
token') }) in the spec; keep the existing check that inspects
error.message.include?('Incorrect API token') and delete the second expectation
so the spec only asserts the message once (refer to the expectations around
Mailtrap::AuthorizationError and the lines that call error.message.include? and
error.messages.any?).

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
lib/mailtrap/account.rb (2)

6-8: @attr_reader is deprecated in YARD since 0.8.0; prefer @!attribute [r].

@attr tags have been deprecated since YARD 0.8.0 in favour of the @!attribute directive. YARD's own tag docs recommend using "the more powerful @!attribute directive instead," noting that @attr_reader is "only applicable on class docstrings" and "declares a readonly attribute on a Struct or class." The Account = Struct.new(...) constant-assignment form is not a class docstring, so YARD's StructHandler may not pick up these tags correctly regardless.

📝 Proposed fix
-  # `@attr_reader` id [Integer] The account ID
-  # `@attr_reader` name [String] The account name
-  # `@attr_reader` access_levels [Array] The account access levels
+  # @!attribute [r] id
+  #   `@return` [Integer] The account ID
+  # @!attribute [r] name
+  #   `@return` [String] The account name
+  # @!attribute [r] access_levels
+  #   `@return` [Array] The account access levels
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/account.rb` around lines 6 - 8, Replace deprecated YARD
`@attr_reader` tags for the Account Struct with @!attribute directives: add
@!attribute [r] id, @!attribute [r] name and @!attribute [r] access_levels
(including their types and short descriptions) near the Account =
Struct.new(...) declaration so YARD's StructHandler will pick them up correctly;
ensure the attributes are documented as readonly and that the text mirrors the
existing comments (Integer for id, String for name, Array for access_levels).

6-8: Use @!attribute [r] instead of @attr_reader for YARD documentation.

The @attr_reader tag is not recognized by the YARD parser and will not render in API documentation. The correct YARD directive for documenting readable attributes is @!attribute [r].

📝 Proposed fix
-  # `@attr_reader` id [Integer] The account ID
-  # `@attr_reader` name [String] The account name
-  # `@attr_reader` access_levels [Array] The account access levels
+  # @!attribute [r] id
+  #   `@return` [Integer] The account ID
+  # @!attribute [r] name
+  #   `@return` [String] The account name
+  # @!attribute [r] access_levels
+  #   `@return` [Array] The account access levels
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/mailtrap/account.rb` around lines 6 - 8, Replace the non-standard YARD
tags by changing each "@attr_reader" doc line for the Account attributes to the
supported YARD directive "@!attribute [r]" and keep the type and
description—i.e., update the doc comments referencing id, name, and
access_levels in lib/mailtrap/account.rb so they become "@!attribute [r] id
[Integer] The account ID", "@!attribute [r] name [String] The account name", and
"@!attribute [r] access_levels [Array] The account access levels" (preserve
wording/types for id, name, access_levels) so YARD will render them as readable
attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@lib/mailtrap/account.rb`:
- Around line 6-8: Replace deprecated YARD `@attr_reader` tags for the Account
Struct with @!attribute directives: add @!attribute [r] id, @!attribute [r] name
and @!attribute [r] access_levels (including their types and short descriptions)
near the Account = Struct.new(...) declaration so YARD's StructHandler will pick
them up correctly; ensure the attributes are documented as readonly and that the
text mirrors the existing comments (Integer for id, String for name, Array for
access_levels).
- Around line 6-8: Replace the non-standard YARD tags by changing each
"@attr_reader" doc line for the Account attributes to the supported YARD
directive "@!attribute [r]" and keep the type and description—i.e., update the
doc comments referencing id, name, and access_levels in lib/mailtrap/account.rb
so they become "@!attribute [r] id [Integer] The account ID", "@!attribute [r]
name [String] The account name", and "@!attribute [r] access_levels [Array] The
account access levels" (preserve wording/types for id, name, access_levels) so
YARD will render them as readable attributes.


client = Mailtrap::Client.new(api_key: 'your-api-key')
account_id = 3229
accounts = Mailtrap::AccountsAPI.new(account_id, client)
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need an account id to list accounts?

lib/mailtrap.rb Outdated
require_relative 'mailtrap/projects_api'
require_relative 'mailtrap/sandbox_messages_api'
require_relative 'mailtrap/sandbox_attachments_api'
require_relative 'mailtrap/suppressions_api'
Copy link
Contributor

Choose a reason for hiding this comment

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

please fix this mess

Copy link
Contributor

Choose a reason for hiding this comment

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

@DagonWat could you please minimise changes in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad! Fixed

General:

- Templates CRUD – [`email_templates_api.rb`](examples/email_templates_api.rb)
- Action Mailer – [`action_mailer.rb`](examples/action_mailer.rb)
Copy link
Contributor

Choose a reason for hiding this comment

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

you wanted to fix "Action Mailer", didnt you?

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
examples/accounts_api.rb (1)

3-9: Credential guidance appears after the code that uses a hardcoded key.

The env-variable comment (lines 6–7) is placed after the hardcoded-key initialization on line 3, inverting the intended setup order. Readers following top-to-bottom will copy the hardcoded path and miss the env-var guidance.

♻️ Suggested reordering
 require 'mailtrap'
 
+# Set your API credentials as environment variables
+# export MAILTRAP_API_KEY='your-api-key'
+
+client = Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY'))
-client = Mailtrap::Client.new(api_key: 'your-api-key')
 accounts = Mailtrap::AccountsAPI.new(client)
 
-# Set your API credentials as environment variables
-# export MAILTRAP_API_KEY='your-api-key'
-
-# accounts = Mailtrap::AccountsAPI.new
 
 # Get all accounts
 accounts.list
 # => [#<struct Mailtrap::Account id=123456, name="My Account", access_levels=[1000]>]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/accounts_api.rb` around lines 3 - 9, Move the environment-variable
guidance before the code that constructs the client so readers see the
recommended setup before a hardcoded key; update examples/accounts_api.rb to
place the comment lines about MAILTRAP_API_KEY and the alternative
initialization (Mailtrap::Client.new using ENV['MAILTRAP_API_KEY'] or
Mailtrap::AccountsAPI.new without args) above the current
Mailtrap::Client.new(api_key: 'your-api-key') and
Mailtrap::AccountsAPI.new(client) lines, and optionally replace the hardcoded
example with the ENV-based example to encourage secure usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@spec/mailtrap/accounts_api_spec.rb`:
- Around line 20-25: The test's raise_error matcher is too permissive; change
the expectation to assert the specific error type and message by calling
raise_error with Mailtrap::AuthorizationError and a message pattern (e.g.,
/Incorrect API token/) for the main matcher, and keep only the custom assertion
about error.messages inside the block; update the spec around the call to list
so it uses raise_error(Mailtrap::AuthorizationError, /Incorrect API token/) and
then yields the error to verify error.messages contains the expected string.

---

Nitpick comments:
In `@examples/accounts_api.rb`:
- Around line 3-9: Move the environment-variable guidance before the code that
constructs the client so readers see the recommended setup before a hardcoded
key; update examples/accounts_api.rb to place the comment lines about
MAILTRAP_API_KEY and the alternative initialization (Mailtrap::Client.new using
ENV['MAILTRAP_API_KEY'] or Mailtrap::AccountsAPI.new without args) above the
current Mailtrap::Client.new(api_key: 'your-api-key') and
Mailtrap::AccountsAPI.new(client) lines, and optionally replace the hardcoded
example with the ENV-based example to encourage secure usage.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e84d004 and e9a617a.

📒 Files selected for processing (3)
  • examples/accounts_api.rb
  • lib/mailtrap/accounts_api.rb
  • spec/mailtrap/accounts_api_spec.rb
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/mailtrap/accounts_api.rb

Comment on lines +20 to +25
expect { list }.to raise_error do |error|
expect(error).to be_a(Mailtrap::AuthorizationError)
expect(error.message).to include('Incorrect API token')
expect(error.messages.any? { |msg| msg.include?('Incorrect API token') }).to be true
end
end
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for the AuthorizationError class definition
find . -name "*.rb" -type f | xargs rg -l "class AuthorizationError"

Repository: mailtrap/mailtrap-ruby

Length of output: 90


🏁 Script executed:

#!/bin/bash
# Look for error class definitions in the mailtrap directory structure
fd -e rb "error" --path "*mailtrap*" -type f

Repository: mailtrap/mailtrap-ruby

Length of output: 299


🏁 Script executed:

#!/bin/bash
# Search for AuthorizationError class and its attributes
rg -n "class AuthorizationError" -A 30 --type ruby

Repository: mailtrap/mailtrap-ruby

Length of output: 965


🏁 Script executed:

#!/bin/bash
# Also search for any definition of 'messages' or 'details' attributes in error classes
rg -n "(attr_reader|attr_accessor|def messages|def details)" --type ruby -B 3 -A 1 | head -100

Repository: mailtrap/mailtrap-ruby

Length of output: 7635


🏁 Script executed:

#!/bin/bash
# View the entire errors.rb file to see the Error base class definition
cat -n lib/mailtrap/errors.rb

Repository: mailtrap/mailtrap-ruby

Length of output: 1174


🏁 Script executed:

#!/bin/bash
# Search for Error class definition and attributes
rg -n "class Error" -A 25 --type ruby lib/mailtrap/errors.rb

Repository: mailtrap/mailtrap-ruby

Length of output: 845


Specify error type in raise_error to avoid false positives.

The raise_error matcher without a specific error type argument can match unintended exceptions raised inside the block (e.g., NoMethodError), causing tests to pass silently when they shouldn't. While error.messages is a valid attribute on Mailtrap::AuthorizationError, the bare matcher remains a code smell.

Pass the expected error type and message pattern directly to raise_error, and move only the custom assertion inside the block:

♻️ Proposed fix
-      expect { list }.to raise_error do |error|
-        expect(error).to be_a(Mailtrap::AuthorizationError)
-        expect(error.message).to include('Incorrect API token')
-        expect(error.messages.any? { |msg| msg.include?('Incorrect API token') }).to be true
-      end
+      expect { list }.to raise_error(Mailtrap::AuthorizationError, /Incorrect API token/) do |error|
+        expect(error.messages).to include(a_string_including('Incorrect API token'))
+      end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@spec/mailtrap/accounts_api_spec.rb` around lines 20 - 25, The test's
raise_error matcher is too permissive; change the expectation to assert the
specific error type and message by calling raise_error with
Mailtrap::AuthorizationError and a message pattern (e.g., /Incorrect API token/)
for the main matcher, and keep only the custom assertion about error.messages
inside the block; update the spec around the call to list so it uses
raise_error(Mailtrap::AuthorizationError, /Incorrect API token/) and then yields
the error to verify error.messages contains the expected string.

@DagonWat DagonWat requested a review from i7an February 23, 2026 13:57
@DagonWat DagonWat merged commit 5aad83b into main Feb 24, 2026
4 checks passed
@DagonWat DagonWat deleted the MT-19860-accounts branch February 24, 2026 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants