-
Notifications
You must be signed in to change notification settings - Fork 7
add projects API #67
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?
add projects API #67
Conversation
WalkthroughThis change introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ProjectsAPI
participant MailtrapAPI
Client->>ProjectsAPI: list/get/create/update/delete
ProjectsAPI->>MailtrapAPI: HTTP request (GET/POST/PATCH/DELETE)
MailtrapAPI-->>ProjectsAPI: API response (JSON)
ProjectsAPI-->>Client: Project object(s) or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 2
🧹 Nitpick comments (5)
lib/mailtrap/project.rb (3)
6-6
: Fix documentation comment.The documentation refers to "contact ID" but should reference "project ID" since this is a Project struct.
- # @attr_reader id [String] The contact ID + # @attr_reader id [String] The project ID
24-27
: Fix documentation comment and approve implementation.The documentation refers to "contact" but should reference "project". The implementation logic is correct.
- # @return [Boolean] Whether the contact was newly created + # @return [Boolean] Whether the project was newly created
29-32
: Fix documentation comment and approve implementation.The documentation refers to "contact attributes" but should reference "project attributes". The implementation using
super.compact
is correct for removing nil values.- # @return [Hash] The contact attributes as a hash + # @return [Hash] The project attributes as a hashspec/mailtrap/projects_api_spec.rb (2)
80-93
: Remove duplicate test context.The "with hash request" context duplicates the main create test with identical request data and expectations. Consider removing this redundant test case.
- context 'with hash request' do - 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' - ) - end - end
133-146
: Remove duplicate test context.Similar to the create method, this "with hash request" context duplicates the main update test. Consider removing this redundant test case.
- context 'with hash request' do - let(:request) do - { - name: 'Updated Project' - } - end - - it 'maps response data to Project object' do - expect(update).to be_a(Mailtrap::Project) - expect(update).to have_attributes( - name: 'Updated Project' - ) - end - end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/mailtrap.rb
(1 hunks)lib/mailtrap/project.rb
(1 hunks)lib/mailtrap/projects_api.rb
(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/mailtrap/project_spec.rb
(1 hunks)spec/mailtrap/projects_api_spec.rb
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
spec/mailtrap/project_spec.rb (1)
lib/mailtrap/project.rb (2)
newly_created?
(25-27)to_h
(30-32)
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)
🔇 Additional comments (14)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/maps_response_data_to_Project_objects.yml (1)
1-167
: LGTM!This VCR cassette properly captures a 401 Unauthorized response scenario for testing API error handling. The structure and content are appropriate for testing the ProjectsAPI authentication flow.
lib/mailtrap/project.rb (2)
11-18
: LGTM!The struct definition with keyword initialization is well-structured and follows Ruby conventions.
19-22
: LGTM!The custom initialize method properly extracts the action parameter before calling the parent constructor. This approach allows the struct to track creation/update state while maintaining the struct's functionality.
lib/mailtrap.rb (1)
11-11
: LGTM!The require statement is properly placed and follows the established pattern for integrating API modules into the main library.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
1-167
: No duplication detected—fixtures cover distinct scenarios
Both VCR cassettes exercise different responses (one maps project data; the other returns a 401 error with{"error":"Incorrect API token"}
), and the header differences you saw (nonces, request IDs, runtimes, etc.) are expected dynamic values. These files serve unique test purposes, so this duplication concern can be closed.spec/mailtrap/project_spec.rb (3)
3-60
: LGTM!The initialization tests provide excellent coverage of the Project struct, including complex nested data structures for share_links, inboxes, and permissions. The test data is realistic and comprehensive.
62-105
: LGTM!The
newly_created?
method tests cover all relevant scenarios:
- Action is 'created' → returns true
- Action is 'updated' → returns false
- Action is nil → returns true
This thoroughly validates the method's logic from
lib/mailtrap/project.rb
.
107-183
: LGTM!The
to_h
method tests comprehensively verify:
- Hash conversion with all attributes present
- Proper handling of nil values (compact behavior)
This validates the
super.compact
implementation in the Project struct.spec/mailtrap/projects_api_spec.rb (4)
1-8
: LGTM!Good use of frozen string literal and proper RSpec setup with VCR integration. The environment variable fallbacks provide flexibility for local testing.
9-31
: Comprehensive test coverage for the list method.Good testing of both successful response mapping and authorization error handling. The error assertions properly validate error type, message content, and the messages array.
33-62
: Well-structured get method tests.The test appropriately creates a project dependency for the positive test case and properly validates both successful retrieval and not found error scenarios.
172-197
: Proper delete method testing.Good coverage of both successful deletion (returning nil) and not found error scenarios. The test structure is consistent with other method tests.
lib/mailtrap/projects_api.rb (2)
1-13
: Proper API class setup.Good use of frozen string literal, appropriate dependencies, and correct BaseAPI integration. The supported options and response class are properly configured.
57-66
: Private methods correctly implemented.The
base_path
constructs the appropriate API endpoint andwrap_request
properly wraps options in the expectedproject
key format.
expect(list).to all(be_a(Mailtrap::Project)) | ||
expect(list.first).to have_attributes( | ||
id: be_a(Integer), | ||
name: be_a(String) | ||
) |
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.
all(be_a(Mailtrap::Project))
already ensures specific type. What is the intention for asserting specific attributes on first item?
end | ||
|
||
# @return [Boolean] Whether the project was newly created | ||
def newly_created? |
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.
it is not needed for projects api.
Motivation
add API support for projects
Changes
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation