Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughIntroduces a new Billing API module to the Mailtrap gem. Adds BillingAPI class with a usage method to fetch billing information, backed by a BillingUsage data transfer object. Includes documentation, examples, comprehensive tests with VCR fixtures, and library integration updates. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/mailtrap/billing_api_spec.rb (1)
27-33: Consider simplifying the error expectation block.The block form of
raise_errorworks but could be made more concise. That said, since it intentionally tests botherror.message(string) anderror.messages(array), this is reasonable if the two accessors have distinct behavior.♻️ Optional: Simplified error assertion (if `messages` check isn't needed separately)
- it 'raises authorization error' do - expect { get }.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 + it 'raises authorization error' do + expect { get }.to raise_error(Mailtrap::AuthorizationError, /Incorrect API token/) + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/mailtrap/billing_api_spec.rb` around lines 27 - 33, The current block form of expect { get }.to raise_error can be simplified: replace the multiline block in the spec with a single matcher call like expect { get }.to raise_error(Mailtrap::AuthorizationError, /Incorrect API token/) to assert both the class and the message in one line; if you still need to assert the error.messages array, keep a short two-step assertion instead—first use expect { get }.to raise_error(Mailtrap::AuthorizationError) to capture the error, then assert on error.messages to check the array contents (refer to the raise_error matcher and the error.messages accessor used in the existing spec).
🤖 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/billing_api.rb`:
- Line 13: The YARD `@return` tag uses angle brackets instead of YARD's required
square-bracket syntax; update the doc comment that currently reads "@return
<Billing>" to "@return [Billing]" so the return type is correctly recognized by
YARD (i.e., replace the angle brackets with square brackets in the `@return` tag).
---
Nitpick comments:
In `@spec/mailtrap/billing_api_spec.rb`:
- Around line 27-33: The current block form of expect { get }.to raise_error can
be simplified: replace the multiline block in the spec with a single matcher
call like expect { get }.to raise_error(Mailtrap::AuthorizationError, /Incorrect
API token/) to assert both the class and the message in one line; if you still
need to assert the error.messages array, keep a short two-step assertion
instead—first use expect { get }.to raise_error(Mailtrap::AuthorizationError) to
capture the error, then assert on error.messages to check the array contents
(refer to the raise_error matcher and the error.messages accessor used in the
existing spec).
a94d87f to
e81d19c
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new Billing API surface to the Mailtrap Ruby SDK to fetch per-account billing cycle and usage information, along with tests and a usage example.
Changes:
- Introduce
Mailtrap::BillingAPI#usageandMailtrap::BillingUsageDTO for mapping the billing usage response. - Add RSpec coverage (unit + VCR) and recorded cassettes for the new endpoint.
- Update docs/examples entry points (README, example script, changelog) and refresh a Project docs link.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/mailtrap/billing_usage_spec.rb | Unit spec for the new BillingUsage DTO struct mapping. |
| spec/mailtrap/billing_api_spec.rb | VCR-backed spec validating BillingAPI#usage behavior and auth error handling. |
| spec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/when_api_key_is_incorrect/raises_authorization_error.yml | VCR cassette for unauthorized billing usage request. |
| spec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/maps_response_data_to_Billing_object.yml | VCR cassette for successful billing usage request. |
| lib/mailtrap/project.rb | Updates Project documentation link. |
| lib/mailtrap/billing_usage.rb | Adds BillingUsage DTO struct. |
| lib/mailtrap/billing_api.rb | Adds BillingAPI with #usage endpoint call and entity mapping. |
| lib/mailtrap.rb | Wires BillingAPI into the main require tree. |
| examples/billing_api.rb | Adds example demonstrating how to call the Billing API. |
| README.md | Adds Billing example link to the examples list. |
| CHANGELOG.md | Notes Billing API addition under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
spec/mailtrap/billing_api_spec.rb
Outdated
| let(:account_id) { ENV.fetch('MAILTRAP_ACCOUNT_ID', 1_111_111) } | ||
| let(:client) { Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY', 'local-api-key')) } | ||
|
|
||
| describe '#get' do |
There was a problem hiding this comment.
The example group is labeled describe '#get', but the API method being exercised is #usage. This makes the spec/cassette naming misleading (the VCR cassette path also ends up under _get). Rename the describe block (and corresponding cassette directory) to #usage for clarity and consistency with the rest of the API specs.
| describe '#get' do | |
| describe '#usage' do |
README.md
Outdated
| - Templates CRUD – [`email_templates_api.rb`](examples/email_templates_api.rb) | ||
| - Action Mailer – [`action_mailer.rb`](examples/action_mailer.rb) | ||
| - Accounts API – [`accounts_api.rb`](examples/accounts_api.rb) | ||
| - Billing – [`billing_api.rb`](examples/billing_api.rb) |
There was a problem hiding this comment.
In the examples list, this entry is labeled just "Billing" while the other API entries are labeled with "... API" (e.g., "Accounts API"). Consider renaming it to "Billing API" to avoid ambiguity and keep the list consistent.
| - Billing – [`billing_api.rb`](examples/billing_api.rb) | |
| - Billing API – [`billing_api.rb`](examples/billing_api.rb) |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/mailtrap/billing_usage_spec.rb (1)
42-74: Consider reusingattributesin the expectation to avoid payload drift.The expected structure is duplicated verbatim, which can become noisy to maintain.
♻️ Optional simplification
- it 'creates a billing data with all attributes' do - expect(billing).to match_struct( - billing: { - cycle_start: '2024-02-15T21:11:59.624Z', - cycle_end: '2024-02-15T21:11:59.624Z' - }, - testing: { - plan: { - name: 'Individual' - }, - usage: { - sent_messages_count: { - current: 1234, - limit: 5000 - }, - forwarded_messages_count: { - current: 0, - limit: 100 - } - } - }, - sending: { - plan: { - name: 'Basic 10K' - }, - usage: { - sent_messages_count: { - current: 6789, - limit: 10_000 - } - } - } - ) - end + it 'creates billing data with all attributes' do + expect(billing).to match_struct(attributes) + end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/mailtrap/billing_usage_spec.rb` around lines 42 - 74, The test duplicates the expected billing payload inline; update the example to reuse the existing attributes helper instead of hardcoding the whole structure: locate the spec where `billing` is asserted with `match_struct(...)` and replace the duplicated literal structure with a reference to the shared `attributes` (or merge relevant keys from `attributes`) so the expectation derives values from the single source of truth (`attributes`) rather than duplicating them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/mailtrap/billing_usage_spec.rb`:
- Around line 42-74: The test duplicates the expected billing payload inline;
update the example to reuse the existing attributes helper instead of hardcoding
the whole structure: locate the spec where `billing` is asserted with
`match_struct(...)` and replace the duplicated literal structure with a
reference to the shared `attributes` (or merge relevant keys from `attributes`)
so the expectation derives values from the single source of truth (`attributes`)
rather than duplicating them.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.mdREADME.mdexamples/billing_api.rblib/mailtrap.rblib/mailtrap/billing_api.rblib/mailtrap/billing_usage.rblib/mailtrap/project.rbspec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/maps_response_data_to_Billing_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/when_api_key_is_incorrect/raises_authorization_error.ymlspec/mailtrap/billing_api_spec.rbspec/mailtrap/billing_usage_spec.rb
🚧 Files skipped from review as they are similar to previous changes (6)
- examples/billing_api.rb
- lib/mailtrap.rb
- spec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/maps_response_data_to_Billing_object.yml
- spec/mailtrap/billing_api_spec.rb
- spec/fixtures/vcr_cassettes/Mailtrap_BillingAPI/_get/when_api_key_is_incorrect/raises_authorization_error.yml
- CHANGELOG.md
3e37b1c to
cda32ec
Compare
cda32ec to
0170d27
Compare
Motivation
Changes
Summary by CodeRabbit
New Features
Documentation
Tests