-
Notifications
You must be signed in to change notification settings - Fork 7
Add Inboxes API #80
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: add-projects-api
Are you sure you want to change the base?
Add Inboxes API #80
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughIntroduces the InboxesAPI class enabling CRUD operations and inbox management for Mailtrap, including methods to list, retrieve, create, update, delete, clean, mark-as-read, and reset credentials for inboxes. Accompanied by comprehensive RSpec test suite with VCR fixtures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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: 3
🧹 Nitpick comments (3)
spec/mailtrap/inbox_spec.rb (1)
16-27: Consider using realistic data types for test attributes.Based on the VCR cassettes, several attributes have different types in the actual API:
usedshould be a boolean (e.g.,false), not an integersmtp_portsshould be an array (e.g.,[25, 465, 587, 2525])pop3_portsshould be an array (e.g.,[1100, 9950])Using representative data types would make the tests more realistic and could catch type-related issues in the Inbox model.
lib/mailtrap/inboxes_api.rb (2)
28-39: Documentation missingproject_idoption.The
createmethod requiresproject_idto construct the API path, but it's not documented in the YARD@optiontags.🔎 Proposed fix
# Creates a new inbox # @param [Hash] options The parameters to create # @option options [String] :name The inbox name + # @option options [Integer] :project_id The project identifier (required) # @return [Inbox] Created inbox object
57-59: Restrict updatable options to excludeproject_id.The
updatemethod currently uses the class-levelsupported_optionswhich includes:project_id, but according to the Mailtrap API documentation, the update endpoint only acceptsnameandemail_username. Theproject_idis only valid during inbox creation (to specify the target project) and cannot be updated. Thebase_updatemethod supports passing a restricted options list, so you can enforce this at the validation layer:+ UPDATE_OPTIONS = %i[name email_username].freeze + # Updates an existing Inbox # @param inbox_id [Integer] The Inbox identifier # @param [Hash] options The parameters to update # @option options [String] :name The inbox name # @option options [String] :email_username The inbox email username # @return [Inbox] Updated Inbox object # @!macro api_errors # @raise [ArgumentError] If invalid options are provided def update(inbox_id, options) - base_update(inbox_id, options) + base_update(inbox_id, options, UPDATE_OPTIONS) end
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
lib/mailtrap.rblib/mailtrap/inboxes_api.rbspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxe_objects.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.ymlspec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.ymlspec/mailtrap/inbox_spec.rbspec/mailtrap/inboxes_api_spec.rb
🧰 Additional context used
🧬 Code graph analysis (2)
spec/mailtrap/inboxes_api_spec.rb (2)
lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)lib/mailtrap/inboxes_api.rb (9)
list(16-18)include(7-97)get(24-26)create(34-39)update(57-59)clean(65-68)mark_as_read(74-77)reset_credentials(83-86)delete(45-47)
lib/mailtrap/inboxes_api.rb (3)
lib/mailtrap/base_api.rb (7)
supported_options(27-29)response_class(31-33)base_list(67-70)base_get(46-49)validate_options!(35-40)base_delete(63-65)base_update(57-61)lib/mailtrap/action_mailer/delivery_method.rb (1)
client(24-26)lib/mailtrap/client.rb (2)
post(187-194)patch(201-208)
🪛 ast-grep (0.40.3)
lib/mailtrap/inboxes_api.rb
[warning] 35-36: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: client.post("/api/accounts/#{account_id}/projects/#{options[:project_id]}/inboxes",
wrap_request(options))
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (21)
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This VCR cassette properly records a 404 Not Found scenario for updating a non-existent inbox. The Bearer token is appropriately masked, and the fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/returns_deleted_inbox_data.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful delete operation returning the deleted inbox data. The fixture structure is standard, and sensitive data is appropriately masked.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_delete/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This cassette properly records a 404 Not Found scenario for deleting a non-existent inbox. The fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: VCR fixture looks correct.This cassette properly records a 404 Not Found scenario for resetting credentials on a non-existent inbox. The fixture structure is standard.
lib/mailtrap.rb (1)
14-14: LGTM!The require statement follows the established pattern for loading API modules and is correctly placed in the sequence.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/maps_response_data_to_Inboxe_objects.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful list operation returning an array of inbox objects. The fixture structure is standard.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_reset_credentials/returns_nil.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a successful reset_credentials operation. The fixture structure is standard, and the response includes the updated inbox object with new credentials.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
1-168: VCR fixture looks correct.This cassette properly records a 401 Unauthorized scenario for an incorrect API token. The fixture structure is standard and includes the expected
WWW-Authenticateheader.spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/returns_nil.yml (1)
1-168: LGTM! Standard VCR cassette for the clean endpoint.This fixture appropriately records the PATCH request to clean an inbox and the 200 OK response. Bearer tokens are properly masked, and the response structure aligns with the Mailtrap Inboxes API.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Appropriate error case fixture.This cassette correctly captures the 404 Not Found response when attempting to mark a non-existent inbox as read. The error handling test coverage is valuable.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_clean/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Consistent error handling fixture.This cassette appropriately tests the 404 Not Found scenario for the clean endpoint when the inbox doesn't exist. Error handling coverage is consistent with other operations.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)
1-167: LGTM! Validation error fixture for create endpoint.This cassette captures a 422 Unprocessable Entity response when the sandbox limit is reached. The fixture appropriately tests API error handling with a meaningful error message.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_mark_as_read/returns_nil.yml (1)
1-168: LGTM! Standard success case for mark_as_read.This cassette correctly records the PATCH request to mark all inbox messages as read and the 200 OK response. The response structure is consistent with other successful operations.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/maps_response_data_to_Inbox_object.yml (1)
1-168: LGTM! Comprehensive fixture for inbox retrieval.This cassette appropriately captures the GET request to retrieve an inbox and includes a complete inbox payload for testing object mapping. The fixture supports validation of the Mailtrap::Inbox object initialization.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_get/when_inbox_does_not_exist/raises_not_found_error.yml (1)
1-166: LGTM! Error handling for non-existent inbox.This cassette correctly captures the 404 Not Found response when attempting to retrieve a non-existent inbox. The error handling test coverage is complete.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_create/maps_response_data_to_Inbox_object.yml (1)
1-168: LGTM! Success case for inbox creation.This cassette appropriately captures the POST request to create an inbox and the response with complete inbox data. The fixture supports testing object mapping for newly created inboxes.
Note: The API returns 200 OK for creation instead of the more conventional 201 Created, but this accurately reflects the actual API behavior.
spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/with_hash_request/maps_response_data_to_Inbox_object.yml (1)
1-168: VCR cassette looks good.The fixture properly records the PATCH request for updating an inbox with a hash request containing only the
namefield. The authorization token is correctly sanitized with a placeholder<BEARER_TOKEN>.spec/fixtures/vcr_cassettes/Mailtrap_InboxesAPI/_update/maps_response_data_to_Inbox_object.yml (1)
1-168: VCR cassette is correctly structured.The fixture properly captures the PATCH request for updating an inbox with both
nameandemail_usernamefields. Authorization token is appropriately sanitized.spec/mailtrap/inboxes_api_spec.rb (1)
1-236: Comprehensive test coverage for the InboxesAPI.The test suite covers all CRUD operations, inbox management methods (clean, mark_as_read, reset_credentials), and error handling scenarios including authorization errors and not found cases. Good use of VCR for HTTP interaction recording.
lib/mailtrap/inboxes_api.rb (2)
35-38: Static analysis false positive - no hardcoded credentials here.The ast-grep warning about hardcoded credentials is a false positive. This is simply an API endpoint URL constructed from dynamic parameters (
account_id,project_id). No actual secrets or passphrases are present in this code.
6-97: Well-structured API implementation.The
InboxesAPIclass follows the established patterns fromBaseAPI, with clean method implementations for CRUD and inbox management operations. The private helpers (wrap_request,base_path) are appropriately scoped, and the YARD documentation provides good API reference material.
Motivation
Cover Inboxes public API with Ruby SDK module.
Changes
Summary by CodeRabbit
New Features
✏️ Tip: You can customize this high-level summary in your review settings.