Skip to content

Add Stats API#96

Open
piobeny wants to merge 1 commit intomainfrom
MT-20967-ruby-sdk-add-stats-functionality
Open

Add Stats API#96
piobeny wants to merge 1 commit intomainfrom
MT-20967-ruby-sdk-add-stats-functionality

Conversation

@piobeny
Copy link

@piobeny piobeny commented Mar 4, 2026

Motivation

  • Add support for the Email Sending Stats API (/api/accounts/{account_id}/stats) to the Ruby SDK, enabling users to retrieve aggregated email sending statistics.

Changes

  • Add SendingStats DTO struct with delivery, bounce, open, click, and spam counts/rates
  • Add StatsAPI class with 5 methods: get, by_domains, by_categories, by_email_service_providers, by_date
  • Add query param handling for array filters (sending_domain_ids[], sending_streams[], categories[], email_service_providers[])
  • Add usage example in examples/stats_api.rb
  • Update README with Stats API reference
  • Update CHANGELOG with unreleased entry

How to test

  • Mailtrap::StatsAPI.new(account_id, client).get method with different parameters (start_date, end_date, sending_domain_ids[], sending_streams[], categories[], email_service_providers[])
  • Test grouped endpoints (by_domains, by_categories, by_email_service_providers, by_date) with filters

Examples

require 'mailtrap'

account_id = id
client = Mailtrap::Client.new(api_key: 'api_key')
stats = Mailtrap::StatsAPI.new(account_id, client)

# With optional filters
stats.get(
  start_date: '2026-01-01',
  end_date: '2026-01-31',
  categories: ['Welcome email']
)

#<struct Mailtrap::SendingStats
  delivery_count=11349,
  delivery_rate=0.974665063552044,
  bounce_count=295,
  bounce_rate=0.02533493644795603,
  open_count=5531,
  open_rate=0.4873557141598379,
  click_count=2052,
  click_rate=0.1808088818398097,
  spam_count=12,
  spam_rate=0.00105736188210415
>

# Get stats grouped by date 
stats.by_date(start_date: '2026-01-01', end_date: '2026-01-02')
[
  #<struct Mailtrap::SendingStatGroup
    name=:date,
    value="2026-01-01",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=2220,
      delivery_rate=0.9749670619235836,
      bounce_count=57,
      bounce_rate=0.02503293807641634,
      open_count=1066,
      open_rate=0.4801801801801802,
      click_count=416,
      click_rate=0.1873873873873874,
      spam_count=0,
      spam_rate=0.0>
      >,
  #<struct Mailtrap::SendingStatGroup
    name=:date,
    value="2026-01-02",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=1152,
      delivery_rate=0.9770992366412213,
      bounce_count=27,
      bounce_rate=0.02290076335877863,
      open_count=524,
      open_rate=0.4548611111111111,
      click_count=162,
      click_rate=0.140625,
      spam_count=0,
      spam_rate=0.0>
    >
]

# Get stats grouped by categories
stats.by_categories(start_date: '2026-01-01', end_date: '2026-01-02')
[
  #<struct Mailtrap::SendingStatGroup
    name=:category,
    value="Welcome email",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=580,
      delivery_rate=0.9682804674457429,
      bounce_count=19,
      bounce_rate=0.03171953255425709,
      open_count=278,
      open_rate=0.4793103448275862,
      click_count=105,
      click_rate=0.1810344827586207,
      spam_count=0,
      spam_rate=0.0
      >
    >,
  #<struct Mailtrap::SendingStatGroup
    name=:category,
    value="Invoice email",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=571,
      delivery_rate=0.9710884353741497,
      bounce_count=17,
      bounce_rate=0.02891156462585034,
      open_count=270,
      open_rate=0.4728546409807355,
      click_count=99,
      click_rate=0.1733800350262697,
      spam_count=0,
      spam_rate=0.0>
    >,
  ...
]

# Get stats grouped by email service providers
stats.by_email_service_providers(start_date: '2026-01-01', end_date: '2026-01-02', categories: ['Welcome email'])
[
  #<struct Mailtrap::SendingStatGroup
    name=:email_service_provider,
    value="Google",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=197,
      delivery_rate=0.9752475247524752,
      bounce_count=5,
      bounce_rate=0.02475247524752475,
      open_count=96,
      open_rate=0.4873096446700508,
      click_count=41,
      click_rate=0.2081218274111675,
      spam_count=0,
      spam_rate=0.0
      >
    >,
  #<struct Mailtrap::SendingStatGroup
    name=:email_service_provider,
    value="Google Workspace",
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=146,
      delivery_rate=0.9864864864864865,
      bounce_count=2,
      bounce_rate=0.01351351351351351,
      open_count=74,
      open_rate=0.5068493150684932,
      click_count=18,
      click_rate=0.1232876712328767,
      spam_count=0,
      spam_rate=0.0
      >
    >,
    ...
]

# Get stats grouped by domains
stats.by_domains(start_date: '2026-01-01', end_date: '2026-01-02', categories: ['Welcome email'], email_service_providers: ['Google'])
[
  #<struct Mailtrap::SendingStatGroup
    name=:sending_domain_id,
    value=75581,
    stats=
    #<struct Mailtrap::SendingStats
      delivery_count=197,
      delivery_rate=0.9752475247524752,
      bounce_count=5,
      bounce_rate=0.02475247524752475,
      open_count=96,
      open_rate=0.4873096446700508,
      click_count=41,
      click_rate=0.2081218274111675,
      spam_count=0,
      spam_rate=0.0
    >
  >
]

Summary by CodeRabbit

  • New Features

    • Added Sending Stats API to fetch aggregated and grouped sending statistics (delivery, bounce, open, click, spam counts and rates) with support for domain/category/ESP/date groupings and flexible filters.
  • Documentation

    • Updated CHANGELOG and README to document the new Sending Stats API.
  • Examples

    • Added a usage example demonstrating fetching and filtering sending stats.
  • Tests

    • Added test fixtures and specs to validate stats retrieval and grouping.

@coderabbitai
Copy link

coderabbitai bot commented Mar 4, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new Sending Stats API: DTOs for aggregated and grouped stats, a Mailtrap::StatsAPI client with aggregation and grouping endpoints (by domain/category/ESP/date), example usage, RSpec tests with VCR fixtures, and changelog/README updates.

Changes

Cohort / File(s) Summary
API Implementation
lib/mailtrap/stats_api.rb, lib/mailtrap.rb
New Mailtrap::StatsAPI class added and required via lib/mailtrap.rb. Provides get and grouped methods (by_domain, by_category, by_email_service_provider, by_date) plus private helpers for query building and date normalization.
Data Structures
lib/mailtrap/sending_stats.rb, lib/mailtrap/sending_stat_group.rb
New Struct-based DTOs: SendingStats (delivery/bounce/open/click/spam counts and rates) and SendingStatGroup (name, value, nested stats).
Examples & Docs
examples/stats_api.rb, README.md, CHANGELOG.md
New example demonstrating StatsAPI usage; README updated to list Sending Stats API example; CHANGELOG Unreleased section updated to mention Sending Stats API.
Tests
spec/mailtrap/stats_api_spec.rb, spec/spec_helper.rb
New RSpec suite covering aggregated and grouped retrievals, filters, date normalization/validation, and auth error handling. Spec helper matcher adjusted to support matcher-like expected values.
Fixtures (VCR)
spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/...
Added multiple VCR cassette YAML files for aggregated stats, filtered stats, auth error, and grouped stats (by_domain, by_category, by_email_service_provider, by_date).

Sequence Diagram(s)

sequenceDiagram
  participant Client as Client
  participant API as Mailtrap::StatsAPI
  participant Remote as Mailtrap API (HTTP)
  participant DTO as SendingStats / SendingStatGroup

  Client->>API: call get(start_date,end_date, filters)
  API->>Remote: GET /api/accounts/{id}/stats?start_date=...&end_date=...&filters...
  Remote-->>API: 200 OK + JSON payload
  API->>DTO: map JSON -> SendingStats / SendingStatGroup
  API-->>Client: return DTO or Array[SendingStatGroup]
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Suggested reviewers

  • IgorDobryn
  • i7an

Poem

🐰 A carroted client makes a call so spry,

Stats hop back grouped by domain, date, and sky,
DTOs tidy numbers in neat little rows,
VCRs keep the stories the tests love to know,
Hooray — the stats API bounces and grows! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add Stats API' directly describes the main feature being introduced in this PR—a new Stats API module for the Mailtrap Ruby SDK.
Description check ✅ Passed The PR description includes all required template sections with detailed information: Motivation explains the purpose, Changes lists five key modifications, and How to test provides specific testing instructions with example code.

✏️ 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-20967-ruby-sdk-add-stats-functionality

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.

@piobeny piobeny force-pushed the MT-20967-ruby-sdk-add-stats-functionality branch from c5a2318 to bd40917 Compare March 4, 2026 10:10
@piobeny piobeny marked this pull request as ready for review March 4, 2026 10:14
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/stats_api.rb (1)

3-5: Use ENV-based placeholders for runnable credentials.

For executable examples, prefer ENV.fetch for account_id and api_key to reduce accidental hardcoded-secret edits in local copies.

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

In `@examples/stats_api.rb` around lines 3 - 5, Replace hardcoded credentials with
ENV-based placeholders: use ENV.fetch to obtain the API key and account id and
convert the account id to an integer before constructing Mailtrap::StatsAPI.
Update the account_id variable (currently "account_id = 3229"), the
Mailtrap::Client.new call that passes api_key: 'your-api-key', and the
Mailtrap::StatsAPI.new(account_id, client) usage to read
ENV.fetch('MAILTRAP_ACCOUNT_ID') (to_i) and ENV.fetch('MAILTRAP_API_KEY') so the
example is runnable without hardcoded secrets.
🤖 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/stats_api_spec.rb`:
- Around line 12-159: Add a cassette-backed spec that exercises the optional
array filters and [] serialization by calling the relevant API method (e.g.,
stats_api.get or a grouping method like stats_api.by_categories) with filter
params sending_domain_ids, sending_streams, categories, and
email_service_providers set to arrays; assert the request was recorded/played
(VCR cassette) and that the response mapping still succeeds, and additionally
verify the request query string uses the [] form (e.g.,
"sending_domain_ids[]=1") or that the client sent multiple param[] entries—use
the existing examples (subject(:stats) { ... }) and test helpers (match_struct)
to validate the returned stats while exercising these filters so query param
construction/regression is covered.

---

Nitpick comments:
In `@examples/stats_api.rb`:
- Around line 3-5: Replace hardcoded credentials with ENV-based placeholders:
use ENV.fetch to obtain the API key and account id and convert the account id to
an integer before constructing Mailtrap::StatsAPI. Update the account_id
variable (currently "account_id = 3229"), the Mailtrap::Client.new call that
passes api_key: 'your-api-key', and the Mailtrap::StatsAPI.new(account_id,
client) usage to read ENV.fetch('MAILTRAP_ACCOUNT_ID') (to_i) and
ENV.fetch('MAILTRAP_API_KEY') so the example is runnable without hardcoded
secrets.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 123b0e56-2fa0-4011-b3ed-cec1ba30ca8d

📥 Commits

Reviewing files that changed from the base of the PR and between af396c8 and bd40917.

📒 Files selected for processing (15)
  • CHANGELOG.md
  • README.md
  • examples/stats_api.rb
  • lib/mailtrap.rb
  • lib/mailtrap/sending_stat_group.rb
  • lib/mailtrap/sending_stats.rb
  • lib/mailtrap/stats_api.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_categories/returns_stats_grouped_by_categories.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_date/returns_stats_grouped_by_date.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_domains/returns_stats_grouped_by_domains.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_email_service_providers/returns_stats_grouped_by_email_service_providers.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/returns_aggregated_sending_stats.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/when_api_key_is_incorrect/raises_authorization_error.yml
  • spec/mailtrap/stats_api_spec.rb
  • spec/spec_helper.rb

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.

♻️ Duplicate comments (1)
spec/mailtrap/stats_api_spec.rb (1)

27-49: ⚠️ Potential issue | 🟡 Minor

Still missing an assertion for [] query serialization.

This example exercises the filters, but it only checks response mapping. The PR’s main risk is the outgoing query shape (sending_domain_ids[], sending_streams[], etc.), and that still is not asserted here.

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

In `@spec/mailtrap/stats_api_spec.rb` around lines 27 - 49, The test exercises
stats_api.get with filter arrays but doesn't assert that array query params are
serialized with trailing brackets (e.g. sending_domain_ids[], sending_streams[],
categories[], email_service_providers[]); update the spec for stats_api.get to
also assert the outgoing request/query string includes those bracketed keys —
either by inspecting the stubbed HTTP request made by the client or by asserting
the full generated query params on the request double used in the test so the
spec ensures correct "[]"-style serialization for sending_domain_ids,
sending_streams, categories and email_service_providers.
🧹 Nitpick comments (1)
spec/mailtrap/stats_api_spec.rb (1)

68-92: Avoid coupling grouped-spec assertions to response order.

These examples all pin first/last. If the API returns the same groups in a different order, the SDK is still correct but the suite fails. Prefer sorting by value first or using order-insensitive expectations.

Also applies to: 98-123, 129-153, 159-183

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

In `@spec/mailtrap/stats_api_spec.rb` around lines 68 - 92, The test assumes a
fixed response order by using stats.first/stats.last which makes it flaky;
update the assertions to be order-insensitive by either sorting the stats array
by the grouping key (e.g., sort by element[:value]) before asserting, or locate
each group by its value and assert its contents (use stats.find { |s| s[:value]
== 1 } and stats.find { |s| s[:value] == 2 }), and keep the existing
match_struct checks (name: :sending_domain_id, value: ..., stats: ...) unchanged
so the test validates group contents regardless of order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@spec/mailtrap/stats_api_spec.rb`:
- Around line 27-49: The test exercises stats_api.get with filter arrays but
doesn't assert that array query params are serialized with trailing brackets
(e.g. sending_domain_ids[], sending_streams[], categories[],
email_service_providers[]); update the spec for stats_api.get to also assert the
outgoing request/query string includes those bracketed keys — either by
inspecting the stubbed HTTP request made by the client or by asserting the full
generated query params on the request double used in the test so the spec
ensures correct "[]"-style serialization for sending_domain_ids,
sending_streams, categories and email_service_providers.

---

Nitpick comments:
In `@spec/mailtrap/stats_api_spec.rb`:
- Around line 68-92: The test assumes a fixed response order by using
stats.first/stats.last which makes it flaky; update the assertions to be
order-insensitive by either sorting the stats array by the grouping key (e.g.,
sort by element[:value]) before asserting, or locate each group by its value and
assert its contents (use stats.find { |s| s[:value] == 1 } and stats.find { |s|
s[:value] == 2 }), and keep the existing match_struct checks (name:
:sending_domain_id, value: ..., stats: ...) unchanged so the test validates
group contents regardless of order.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2ffca3dc-28fe-4244-b8d2-5be950b6e72f

📥 Commits

Reviewing files that changed from the base of the PR and between bd40917 and 6f96204.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • CHANGELOG.md
  • lib/mailtrap/version.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/with_optional_filters/returns_filtered_sending_stats.yml
  • spec/mailtrap/stats_api_spec.rb
✅ Files skipped from review due to trivial changes (1)
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/with_optional_filters/returns_filtered_sending_stats.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

require_relative 'mailtrap/inboxes_api'
require_relative 'mailtrap/sandbox_messages_api'
require_relative 'mailtrap/sandbox_attachments_api'
require_relative 'mailtrap/stats_api'
Copy link
Contributor

Choose a reason for hiding this comment

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

sending_stats and sending_stat_group are required in stats_api. thus no need to require them explicitly

stats = Mailtrap::StatsAPI.new(account_id, client)

# Get aggregated sending stats
stats.get(start_date: '2026-01-01', end_date: '2026-01-31')
Copy link
Contributor

@i7an i7an Mar 6, 2026

Choose a reason for hiding this comment

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

i believe the start and end dates should be of type #to_date:

stats.get(start_date: Date.today - 10, end_date: Date.today)

In the code you can do date.to_date which should work for Date and Time. In RoR applications it should work with strings as well.

URI.encode_www_form({start_date: (Time.now - 10).to_date})
# => "start_date=2026-03-06"

Copy link
Author

Choose a reason for hiding this comment

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

In our client we are already doing
uri.query = URI.encode_www_form(query_params) if query_params.any?

so you can safely use Date in any query param

Copy link
Contributor

Choose a reason for hiding this comment

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

require 'time'

account_id = 3229
client = Mailtrap::Client.new(api_key: 'your-api-key')
stats = Mailtrap::StatsAPI.new(account_id, client)

stats.get(start_date: Time.now, end_date: '2026-01-31')
# => https://mailtrap.io/api/accounts/3229/stats?start_date=2026-03-09+11%3A25%3A22+%2B0100&end_date=2026-01-31

# @param email_service_providers [Array<String>] Filter by email service providers
# @return [Array<SendingStatGroup>] Array of SendingStatGroup structs with sending_domain_id and stats
# @!macro api_errors
def by_domains(start_date:, end_date:, sending_domain_ids: nil, sending_streams: nil, categories: nil, # rubocop:disable Metrics/ParameterLists
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it should be by_domain singular? you would normally say "group by domain"

applies to all by_ methods

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure about this. I agree that with group by it sounds better, but in our API we have same naming convention - I think you should stick with it:

/api/accounts/{account_id}/stats
/api/accounts/{account_id}/stats/domains
/api/accounts/{account_id}/stats/categories
/api/accounts/{account_id}/stats/email_service_providers
/api/accounts/{account_id}/stats/date

Copy link
Contributor

@i7an i7an Mar 9, 2026

Choose a reason for hiding this comment

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

I agree that with group by it sounds better,

its sdk. it's a wrapper around our api. you have to look at the ruby code and make it "nice". the ruby code is the interface for other ruby devs.

- h3=":443"; ma=86400
body:
encoding: ASCII-8BIT
string: '[{"email_service_provider":"Gmail","stats":{"delivery_count":80,"delivery_rate":0.97,"bounce_count":2,"bounce_rate":0.03,"open_count":70,"open_rate":0.88,"click_count":35,"click_rate":0.5,"spam_count":1,"spam_rate":0.013}},{"email_service_provider":"Yahoo","stats":{"delivery_count":70,"delivery_rate":0.93,"bounce_count":6,"bounce_rate":0.07,"open_count":50,"open_rate":0.71,"click_count":25,"click_rate":0.5,"spam_count":1,"spam_rate":0.014}}]'
Copy link
Contributor

Choose a reason for hiding this comment

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

API docs should be updated. stats are wrapped in an array there:
Image

Copy link
Author

Choose a reason for hiding this comment

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

Created ticket for that, thanks!

@piobeny piobeny force-pushed the MT-20967-ruby-sdk-add-stats-functionality branch from 6f96204 to 54c2421 Compare March 9, 2026 09:19
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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Around line 1-3: The changelog currently marks version header "## [2.9.0] -
2026-03-06" as a dated release but this entry must remain unreleased; locate the
header "## [2.9.0] - 2026-03-06" (the section containing "Add Sending Stats
API") and either remove the date or move the bullet under an "## [Unreleased]"
section so it is not listed as a published release (for example change the
header to "## [2.9.0] - Unreleased" or place the "Add Sending Stats API" entry
under an existing Unreleased header).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8c8382c7-60d3-4a0d-b1e0-b3561bd19688

📥 Commits

Reviewing files that changed from the base of the PR and between 6f96204 and 54c2421.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • CHANGELOG.md
  • lib/mailtrap.rb
  • lib/mailtrap/version.rb
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/mailtrap.rb

@piobeny piobeny force-pushed the MT-20967-ruby-sdk-add-stats-functionality branch from 54c2421 to 430864f Compare March 10, 2026 10:05
@piobeny piobeny requested a review from i7an March 10, 2026 10:06
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)
spec/mailtrap/stats_api_spec.rb (1)

68-92: Assert SendingStatGroup explicitly in the grouped examples.

match_struct also accepts hash-like objects (spec/spec_helper.rb:69-107), so these examples would still pass if by_* regressed from returning Mailtrap::SendingStatGroup instances to plain hashes. Adding a type assertion keeps the new public DTO contract covered.

💡 Suggested test hardening
     it 'returns stats grouped by domain' do
+      expect(stats).to all(be_a(Mailtrap::SendingStatGroup))
       expect(stats.size).to eq(2)
@@
     it 'returns stats grouped by category' do
+      expect(stats).to all(be_a(Mailtrap::SendingStatGroup))
       expect(stats.size).to eq(2)
@@
     it 'returns stats grouped by email service provider' do
+      expect(stats).to all(be_a(Mailtrap::SendingStatGroup))
       expect(stats.size).to eq(2)
@@
     it 'returns stats grouped by date' do
+      expect(stats).to all(be_a(Mailtrap::SendingStatGroup))
       expect(stats.size).to eq(2)

Also applies to: 98-123, 129-153, 159-183

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

In `@spec/mailtrap/stats_api_spec.rb` around lines 68 - 92, The grouped examples
currently only use match_struct and can pass if the group objects degrade to
plain hashes; add an explicit type assertion that the returned group objects are
Mailtrap::SendingStatGroup (e.g. expect(stats.first).to
be_a(Mailtrap::SendingStatGroup>) and same for stats.last and the other grouped
examples referenced) so the spec verifies the DTO class as well as its shape;
update the examples at the shown block and the other ranges (98-123, 129-153,
159-183) to include this type check alongside the existing match_struct
assertions.
🤖 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/stats_api.rb`:
- Around line 3-5: Add a top-level require for Ruby's stdlib date by inserting
require 'date' alongside the existing requires, and update the normalize_date
method to use strict parsing instead of Date.parse: replace Date.parse(...) with
Date.strptime(date_string, '%Y-%m-%d') and still rescue Date::Error (and handle
nil/empty input the same way) so only exact YYYY-MM-DD strings are accepted;
reference the normalize_date method and Date::Error in your change to tighten
validation.

---

Nitpick comments:
In `@spec/mailtrap/stats_api_spec.rb`:
- Around line 68-92: The grouped examples currently only use match_struct and
can pass if the group objects degrade to plain hashes; add an explicit type
assertion that the returned group objects are Mailtrap::SendingStatGroup (e.g.
expect(stats.first).to be_a(Mailtrap::SendingStatGroup>) and same for stats.last
and the other grouped examples referenced) so the spec verifies the DTO class as
well as its shape; update the examples at the shown block and the other ranges
(98-123, 129-153, 159-183) to include this type check alongside the existing
match_struct assertions.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 17886a7c-083d-47a4-bbd0-99fb7b2850ce

📥 Commits

Reviewing files that changed from the base of the PR and between 54c2421 and 430864f.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • README.md
  • examples/stats_api.rb
  • lib/mailtrap.rb
  • lib/mailtrap/sending_stat_group.rb
  • lib/mailtrap/sending_stats.rb
  • lib/mailtrap/stats_api.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_category/returns_stats_grouped_by_category.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_date/returns_stats_grouped_by_date.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_domain/returns_stats_grouped_by_domain.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_email_service_provider/returns_stats_grouped_by_email_service_provider.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/returns_aggregated_sending_stats.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/when_api_key_is_incorrect/raises_authorization_error.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/with_optional_filters/returns_filtered_sending_stats.yml
  • spec/mailtrap/stats_api_spec.rb
  • spec/spec_helper.rb
✅ Files skipped from review due to trivial changes (2)
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_category/returns_stats_grouped_by_category.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_domain/returns_stats_grouped_by_domain.yml
🚧 Files skipped from review as they are similar to previous changes (8)
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_date/returns_stats_grouped_by_date.yml
  • spec/spec_helper.rb
  • examples/stats_api.rb
  • CHANGELOG.md
  • README.md
  • lib/mailtrap.rb
  • lib/mailtrap/sending_stats.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/returns_aggregated_sending_stats.yml

@piobeny piobeny force-pushed the MT-20967-ruby-sdk-add-stats-functionality branch from 430864f to 3933b2d Compare March 10, 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.

🧹 Nitpick comments (1)
lib/mailtrap/stats_api.rb (1)

130-131: Consider documenting timezone behavior for Time inputs.

strftime('%F') uses the local timezone of the Time object. If a user passes a Time near midnight in a non-UTC timezone, the resulting date string may differ from their expectation. This is fine behavior, but a brief note in the YARD docs for start_date/end_date parameters could help users understand this.

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

In `@lib/mailtrap/stats_api.rb` around lines 130 - 131, The code path that handles
Time inputs uses value.strftime('%F') which formats the date using the Time
object's zone (local time), so update the YARD docs for the start_date and
end_date parameters (and any related method in StatsApi) to state that Time
inputs are interpreted in their own timezone and may produce a different ISO
date near midnight in non-UTC zones; suggest callers pass a Date, a UTC Time
(time.getutc), or convert via time.to_date if they want timezone-agnostic
behavior, and mention the Time branch that calls strftime('%F') so readers can
find the logic.
🤖 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/stats_api.rb`:
- Around line 130-131: The code path that handles Time inputs uses
value.strftime('%F') which formats the date using the Time object's zone (local
time), so update the YARD docs for the start_date and end_date parameters (and
any related method in StatsApi) to state that Time inputs are interpreted in
their own timezone and may produce a different ISO date near midnight in non-UTC
zones; suggest callers pass a Date, a UTC Time (time.getutc), or convert via
time.to_date if they want timezone-agnostic behavior, and mention the Time
branch that calls strftime('%F') so readers can find the logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 20586c3e-6ef5-4028-a4d6-48f20204414e

📥 Commits

Reviewing files that changed from the base of the PR and between 430864f and 3933b2d.

📒 Files selected for processing (16)
  • CHANGELOG.md
  • README.md
  • examples/stats_api.rb
  • lib/mailtrap.rb
  • lib/mailtrap/sending_stat_group.rb
  • lib/mailtrap/sending_stats.rb
  • lib/mailtrap/stats_api.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_category/returns_stats_grouped_by_category.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_date/returns_stats_grouped_by_date.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_domain/returns_stats_grouped_by_domain.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_email_service_provider/returns_stats_grouped_by_email_service_provider.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/returns_aggregated_sending_stats.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/when_api_key_is_incorrect/raises_authorization_error.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/with_optional_filters/returns_filtered_sending_stats.yml
  • spec/mailtrap/stats_api_spec.rb
  • spec/spec_helper.rb
🚧 Files skipped from review as they are similar to previous changes (8)
  • README.md
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_get/with_optional_filters/returns_filtered_sending_stats.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_domain/returns_stats_grouped_by_domain.yml
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_category/returns_stats_grouped_by_category.yml
  • CHANGELOG.md
  • lib/mailtrap/sending_stat_group.rb
  • lib/mailtrap.rb
  • spec/fixtures/vcr_cassettes/Mailtrap_StatsAPI/_by_email_service_provider/returns_stats_grouped_by_email_service_provider.yml

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.

4 participants