-
Notifications
You must be signed in to change notification settings - Fork 7
Add Sandbox Projects API #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds Projects API support: new ProjectsAPI REST wrapper, Project and Inbox DTOs, a require in the library entrypoint, comprehensive RSpec coverage, and many VCR fixtures for list/get/create/update/delete scenarios. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (2)
spec/mailtrap/projects_api_spec.rb (1)
60-89: "with hash request" contexts don't exercise hash-argument callsBoth
#createand#updateexamples useproject_api.create(**request)andproject_api.update(project_id, **request)in their parent subjects (lines 61 and 108). The nested"with hash request"contexts (lines 76–89 and 129–142) only redefinelet(:request)but inherit the parent subject, so they still unpack the hash with**and hit the same code path.To verify both calling conventions, redefine
subjectin the nested contexts to pass the raw Hash argument:context 'with hash request' do + subject(:create) { project_api.create(request) } + let(:request) do { name: 'New Project' } end it 'maps response data to Project object' do expect(create).to be_a(Mailtrap::Project) expect(create).to have_attributes( - name: 'New Project' + name: 'New Project' ) end endApply the same fix to the
#updatecontext (lines 129–142), substitutingsubject(:update) { project_api.update(project_id, request) }.Also applies to: 129-142
lib/mailtrap/project.rb (1)
25-32: Use safe navigation (&.) for inbox mappingRubocop flags this code with Style/SafeNavigation. Simplify the initialization of
@inboxesusing safe navigation:- raw_inboxes = args[:inboxes] - @inboxes = - if raw_inboxes - raw_inboxes.map do |inbox| - inbox.is_a?(Mailtrap::Inbox) ? inbox : Mailtrap::Inbox.new(**inbox) - end - end + raw_inboxes = args[:inboxes] + @inboxes = raw_inboxes&.map do |inbox| + inbox.is_a?(Mailtrap::Inbox) ? inbox : Mailtrap::Inbox.new(**inbox) + endSemantics remain unchanged (
@inboxesisnilwhen no inboxes are provided), but this reduces nesting and aligns with idiomatic Ruby style.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
lib/mailtrap.rb(1 hunks)lib/mailtrap/inbox.rb(1 hunks)lib/mailtrap/project.rb(1 hunks)lib/mailtrap/projects_api.rb(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/maps_response_data_to_Project_object.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/returns_deleted_project_id.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/when_project_does_not_exist/raises_not_found_error.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/maps_response_data_to_Project_object.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/when_project_does_not_exist/raises_not_found_error.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/maps_response_data_to_Project_objects.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/maps_response_data_to_Project_object.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/with_hash_request/maps_response_data_to_Project_object.yml(1 hunks)spec/mailtrap/project_spec.rb(1 hunks)spec/mailtrap/projects_api_spec.rb(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
lib/mailtrap/inbox.rb (1)
lib/mailtrap/project.rb (1)
to_h(35-43)
spec/mailtrap/project_spec.rb (2)
lib/mailtrap/inbox.rb (1)
to_h(58-60)lib/mailtrap/project.rb (1)
to_h(35-43)
spec/mailtrap/projects_api_spec.rb (1)
lib/mailtrap/projects_api.rb (6)
list(18-20)include(8-67)get(26-28)create(36-38)update(46-48)delete(54-56)
lib/mailtrap/project.rb (1)
lib/mailtrap/inbox.rb (1)
to_h(58-60)
lib/mailtrap/projects_api.rb (1)
lib/mailtrap/base_api.rb (7)
supported_options(27-29)response_class(31-33)base_list(67-70)base_get(46-49)base_create(51-55)base_update(57-61)base_delete(63-65)
🪛 GitHub Actions: Ruby
spec/mailtrap/project_spec.rb
[error] 54-54: Rubocop: Layout/FirstArrayElementIndentation: Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis. ({ ...
[error] 59-59: Rubocop: Layout/FirstArrayElementIndentation: Indent the right bracket the same as the first position after the preceding left parenthesis. })
spec/mailtrap/projects_api_spec.rb
[error] 42-42: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (id: project_id, ...)
[error] 72-72: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (name: 'New Project')
[error] 86-86: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (name: 'New Project')
[error] 125-125: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (name: 'Updated Project')
[error] 139-139: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (name: 'Updated Project')
[error] 150-150: Rubocop: Layout/FirstArgumentIndentation: Indent the first argument one step more than the start of the previous line. (name: 'New Name Only')
lib/mailtrap/project.rb
[error] 15-15: Rubocop: Style/SymbolArray: Use %i or %I for an array of symbols. [:id, :name, :share_links, :inboxes, :permissions]
[error] 27-27: Rubocop: Style/SafeNavigation: Use safe navigation (&.) instead of checking if an object exists before calling the method. (if raw_inboxes ...)
🔇 Additional comments (9)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/when_project_does_not_exist/raises_not_found_error.yml (1)
1-331: LGTM! Test fixture properly captures the not-found error scenario.The VCR cassette correctly records the project creation setup and the subsequent 404 response when attempting to retrieve a non-existent project, with sensitive data appropriately redacted.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml (1)
1-333: LGTM! Fixture correctly validates partial update behavior.This cassette appropriately tests updating only the name field, demonstrating that the API supports partial updates without requiring all fields.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/with_hash_request/maps_response_data_to_Project_object.yml (1)
1-333: LGTM! Test fixture correctly captures the update flow.The cassette appropriately records the project creation and update lifecycle, verifying the hash request format and response mapping.
lib/mailtrap/inbox.rb (1)
30-61: LGTM! Clean DTO implementation with appropriate serialization.The Struct-based approach with
keyword_init: trueand the compactedto_hoverride provide a clean, idiomatic Ruby DTO implementation.spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml (1)
1-331: LGTM! Fixture properly tests update error handling.Correctly captures the 404 response when attempting to update a non-existent project.
lib/mailtrap.rb (1)
13-13: LGTM! Projects API properly wired into the module.The require statement is correctly positioned and follows the existing pattern for other API modules.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)
1-166: LGTM! Fixture correctly captures validation error response.Appropriately tests the API's validation behavior when creating a project with an invalid (empty) name, returning a 422 status with detailed error messages.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml (1)
1-168: LGTM! Fixture properly records successful project creation.Correctly captures the hash request format and the successful 200 response with full project details.
spec/mailtrap/projects_api_spec.rb (1)
39-45: Fixhave_attributesindentation to satisfy Rubocop Layout/FirstArgumentIndentationRubocop is flagging the first arguments to
have_attributesas over‑indented. Align them one indentation step past the previous line, e.g.:- expect(get).to have_attributes( - id: project_id, - name: 'Test Project' - ) + expect(get).to have_attributes( + id: project_id, + name: 'Test Project' + ) @@ - expect(create).to have_attributes( - name: 'New Project' - ) + expect(create).to have_attributes( + name: 'New Project' + ) @@ - expect(create).to have_attributes( - name: 'New Project' - ) + expect(create).to have_attributes( + name: 'New Project' + ) @@ - expect(update).to have_attributes( - name: 'Updated Project' - ) + expect(update).to have_attributes( + name: 'Updated Project' + ) @@ - expect(update).to have_attributes( - name: 'Updated Project' - ) + expect(update).to have_attributes( + name: 'Updated Project' + ) @@ - expect(update).to have_attributes( - name: 'New Name Only' - ) + expect(update).to have_attributes( + name: 'New Name Only' + )This should clear the Layout/FirstArgumentIndentation offenses reported in CI. Based on pipeline failures, this is currently blocking.
Also applies to: 69-74, 83-88, 122-127, 136-141, 147-151
| # Deletes a project | ||
| # @param project_id [Integer] The project ID | ||
| # @return nil | ||
| # @!macro api_errors | ||
| def delete(project_id) | ||
| base_delete(project_id) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find all delete methods in the lib directory
rg -n "def delete\(" lib/mailtrap --type rbRepository: mailtrap/mailtrap-ruby
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Check the spec file to verify delete expectations
find . -name "*projects_api*spec*" -type fRepository: mailtrap/mailtrap-ruby
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Look at the BaseAPI to understand base_delete behavior
find . -name "*base*api*" -type f | grep -E "\.(rb|py)$"Repository: mailtrap/mailtrap-ruby
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Search for delete methods without type flag
rg -n "def delete\(" lib/mailtrap/Repository: mailtrap/mailtrap-ruby
Length of output: 489
🏁 Script executed:
#!/bin/bash
# Read the projects_api_spec.rb to verify delete expectations
cat ./spec/mailtrap/projects_api_spec.rbRepository: mailtrap/mailtrap-ruby
Length of output: 5345
🏁 Script executed:
#!/bin/bash
# Read the base_api.rb to understand base_delete
cat ./lib/mailtrap/base_api.rbRepository: mailtrap/mailtrap-ruby
Length of output: 2361
🏁 Script executed:
#!/bin/bash
# Read the Client class to see what delete returns
cat ./lib/mailtrap/client.rb | head -250Repository: mailtrap/mailtrap-ruby
Length of output: 7799
🏁 Script executed:
#!/bin/bash
# Also check projects_api.rb to see the full context
cat ./lib/mailtrap/projects_api.rbRepository: mailtrap/mailtrap-ruby
Length of output: 1692
🏁 Script executed:
#!/bin/bash
# Check other delete method YARD docs for consistency
grep -A 3 "def delete(" lib/mailtrap/*.rb | grep -E "(def delete|@return)"Repository: mailtrap/mailtrap-ruby
Length of output: 467
🏁 Script executed:
#!/bin/bash
# Check a couple of other spec files to see if they also expect hash return
grep -A 2 "it 'returns" spec/mailtrap/*_spec.rb | grep -E "(it 'returns|expect.*delete)"Repository: mailtrap/mailtrap-ruby
Length of output: 1662
Update #delete YARD documentation to reflect actual return value
The YARD comment for delete states # @return nil, but the spec in spec/mailtrap/projects_api_spec.rb expects it to return { id: project_id }. The implementation delegates to base_delete, which returns the raw response from client.delete.
Update the documentation to match the actual behavior:
- # @return nil
+ # @return [Hash] Deleted project identifier, e.g. { id: project_id }🤖 Prompt for AI Agents
In lib/mailtrap/projects_api.rb around lines 50 to 56, the YARD comment for
#delete incorrectly states `@return nil` while the method actually returns the
raw client response (expected by specs to be `{ id: project_id }`); update the
YARD block to reflect the actual return value (for example `# @return [Hash] the
deleted project response, e.g. { id: project_id }`) and ensure any other related
tags/comments describe that it delegates to base_delete/client.delete.
| expect(project.inboxes.map(&:to_h)).to eq([ | ||
| { | ||
| id: 456, | ||
| name: 'Test Inbox', | ||
| username: '[email protected]' | ||
| } | ||
| ]) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix array indentation to resolve Rubocop violations.
The array starting at line 54 and closing at line 59 has incorrect indentation, causing CI failures.
Apply this diff to fix the indentation:
- expect(project.inboxes.map(&:to_h)).to eq([
- {
- id: 456,
- name: 'Test Inbox',
- username: '[email protected]'
- }
- ])
+ expect(project.inboxes.map(&:to_h)).to eq(
+ [{
+ id: 456,
+ name: 'Test Inbox',
+ username: '[email protected]'
+ }]
+ )📝 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.
| expect(project.inboxes.map(&:to_h)).to eq([ | |
| { | |
| id: 456, | |
| name: 'Test Inbox', | |
| username: '[email protected]' | |
| } | |
| ]) | |
| end | |
| expect(project.inboxes.map(&:to_h)).to eq( | |
| [{ | |
| id: 456, | |
| name: 'Test Inbox', | |
| username: '[email protected]' | |
| }] | |
| ) | |
| end |
🧰 Tools
🪛 GitHub Actions: Ruby
[error] 54-54: Rubocop: Layout/FirstArrayElementIndentation: Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis. ({ ...
[error] 59-59: Rubocop: Layout/FirstArrayElementIndentation: Indent the right bracket the same as the first position after the preceding left parenthesis. })
🤖 Prompt for AI Agents
In spec/mailtrap/project_spec.rb around lines 53 to 60, the multi-line array
used in the expectation is mis-indented causing RuboCop failures; fix it by
aligning the array elements and closing bracket with the expect call indent
level (indent the inner hash one additional level so each key/value and the
closing brace line up consistently and the final closing ]) aligns with the
expect(...).to eq([ line).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/mailtrap/inbox.rb(1 hunks)lib/mailtrap/project.rb(1 hunks)spec/mailtrap/project_spec.rb(1 hunks)spec/mailtrap/projects_api_spec.rb(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/mailtrap/projects_api_spec.rb
- spec/mailtrap/project_spec.rb
🧰 Additional context used
🧬 Code graph analysis (1)
lib/mailtrap/inbox.rb (1)
lib/mailtrap/project.rb (1)
to_h(33-41)
🔇 Additional comments (1)
lib/mailtrap/project.rb (1)
4-41: Project DTO and inbox normalization are correctly implemented.The
Projectclass properly handles inbox initialization, supporting bothMailtrap::Inboxinstances and raw hashes. Theto_hmethod correctly serializes inboxes via safe navigation and mapping. Tests confirm the implementation works as expected with symbol-keyed inbox hashes, which is the standard pattern throughout the codebase.
| ### Email Templates API | ||
|
|
||
| ```ruby | ||
| require 'mailtrap' | ||
|
|
||
| client = Mailtrap::Client.new(api_key: 'your-api-key') | ||
| templates = Mailtrap::EmailTemplatesAPI.new 3229, client | ||
|
|
||
| templates.create( | ||
| name: 'Welcome Email', | ||
| subject: 'Welcome to Mailtrap!', | ||
| body_html: '<h1>Hello</h1>', | ||
| body_text: 'Hello', | ||
| category: 'welcome' | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Email templates are already a big chunk of examples, I see no reason to have it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/mailtrap/inbox.rb (1)
4-29: Double‑check the@seelink points to inbox, not project, docs.The DTO and comments clearly describe an inbox, but the
@seeURL slug ends withcreate-project. If there is an inbox‑specific endpoint doc, consider linking that instead so consumers don’t confuse project vs inbox semantics. If this URL is intentionally shared between project and inbox, a brief note in the comment could clarify that.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)examples/projects_api.rb(1 hunks)lib/mailtrap/inbox.rb(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- README.md
- examples/projects_api.rb
🧰 Additional context used
🧬 Code graph analysis (1)
lib/mailtrap/inbox.rb (1)
lib/mailtrap/project.rb (1)
to_h(33-41)
🔇 Additional comments (1)
lib/mailtrap/inbox.rb (1)
30-61: Inbox struct definition andto_hoverride look correct and consistent.Member list matches the documented attributes,
keyword_init: truegives a clean API for callers, andsuper.compactaligns with the existing DTO pattern of dropping nils before serialization. No functional issues spotted here; just ensure the gem’s minimum Ruby version supportsStruct.new(..., keyword_init: true).
Motivation
Add Sandbox Projects API support for Rybu SDK
Changes
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.