add projects API #67
add projects API #67
Conversation
WalkthroughIntroduces Mailtrap Projects support: adds Project DTO, ProjectsAPI with CRUD delegating to BaseAPI, and requires it in the main entry. Adds RSpec tests and numerous VCR cassettes for ProjectsAPI. Updates spec_helper to generalize ACCOUNT_ID redaction and redact share_links. Refreshes/normalizes many EmailTemplatesAPI cassettes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer Code
participant Client as Mailtrap::Client
participant API as Mailtrap::ProjectsAPI
participant Base as BaseAPI
participant HTTP as HTTP Adapter
participant MT as Mailtrap REST API
Dev->>Client: initialize(account_id, api_key)
Dev->>API: list/get/create/update/delete(...)
API->>Base: base_* (method, path, payload)
Base->>HTTP: request(method, "/api/accounts/:id/projects[/...]", body)
HTTP->>MT: HTTP call
MT-->>HTTP: JSON response / error
HTTP-->>Base: response
Base-->>API: map to Project / error
API-->>Dev: Project / [Project] / {id} or raises Error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.compactis 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_hmethod tests comprehensively verify:
- Hash conversion with all attributes present
- Proper handling of nil values (compact behavior)
This validates the
super.compactimplementation 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_pathconstructs the appropriate API endpoint andwrap_requestproperly wraps options in the expectedprojectkey format.
spec/spec_helper.rb
Outdated
| @@ -14,8 +14,8 @@ | |||
| config.filter_sensitive_data('<BEARER_TOKEN>') do |interaction| | |||
| next if interaction.request.uri =~ /localhost/ | |||
|
|
|||
| interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/') | |||
| interaction.response.body.gsub!(/"account_id":\d+/, '"account_id": 1111111') | |||
| # interaction.request.uri.gsub!(%r{/accounts/\d+/}, '/accounts/1111111/') | |||
There was a problem hiding this comment.
@IgorDobryn @i7an I had to remove this in order for projects spec to run - otherwise VCR was not recognising cassettes (due to changed URLs) and was failing
there are also failing email templates specs - I had rerun and re-record cassettes. I'm not committing these changes though so I wanted to know your opinion if this account ID removal is absolutely necessary or not.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
lib/mailtrap/project.rb (1)
19-21: Remove redundant initializer.This override adds no behavior and can be dropped to rely on Struct’s keyword_init initializer. Keeps the DTO leaner and avoids future confusion.
- def initialize(options) - super - end
🧹 Nitpick comments (3)
lib/mailtrap/project.rb (1)
24-26: to_h looks good; minor compatibility check.Hash#compact requires Ruby >= 2.4 (generally fine). If this gem supports very old Rubies, replace with reject { |_, v| v.nil? }.
- super.compact + super.reject { |_, v| v.nil? }Would you like me to scan the repo for required_ruby_version in the gemspec to confirm compatibility?
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)
5-5: Account ID is stored in the cassette URI; consider normalizing.The URI includes the real account_id. If this repo is public or shared broadly, normalize account_id during recording (see spec_helper suggestion to add a URI matcher and normalization) to avoid leaking internal identifiers.
I can provide a one-off script to rewrite existing cassettes to replace /accounts// with /accounts/<ACCOUNT_ID>/ if you decide to normalize retroactively. Want me to generate it?
spec/mailtrap/projects_api_spec.rb (1)
178-181: Align example description with the actual expectationThe example says “returns no content” but asserts a payload
{ id: project_id }. Update the description for clarity and to avoid confusion.Apply this diff:
- it 'returns no content' do + it 'returns deleted project id' do expect(delete).to eq({ id: project_id }) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
lib/mailtrap/project.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_no_content.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)spec/spec_helper.rb(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/when_project_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/when_project_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/with_hash_request/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/returns_no_content.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/maps_response_data_to_Project_objects.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml
- spec/mailtrap/project_spec.rb
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/mailtrap/projects_api_spec.rb (1)
lib/mailtrap/projects_api.rb (6)
list(17-19)include(7-66)get(25-27)create(35-37)update(45-47)delete(53-55)
🔇 Additional comments (5)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml (1)
164-165: Critical: Sanitize share_links in VCR fixturesThe recorded cassettes still expose real admin/viewer share‐link tokens. You must redact all share_links URLs in your fixtures to avoid leaking sensitive tokens.
• Target fixtures (e.g. spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml, lines 164–165) and any other files under spec/fixtures/vcr_cassettes containing
"share_links".
• Bulk‐redact in place:find spec/fixtures/vcr_cassettes -name '*.yml' \ -exec sed -i -E "s#https://mailtrap\.io/projects/[0-9]+/share/[^\"']+#<REDACTED_SHARE_LINK>#g" {} +• (Optionally) Add VCR filters in spec/spec_helper.rb to scrub during recording:
VCR.configure do |c| c.filter_sensitive_data('<ADMIN_SHARE_LINK>') do |i| JSON.parse(i.response.body)['share_links']['admin'] end c.filter_sensitive_data('<VIEWER_SHARE_LINK>') do |i| JSON.parse(i.response.body)['share_links']['viewer'] end end• Verify redaction with:
rg -nP '<REDACTED_SHARE_LINK>' spec/fixtures/vcr_cassettes || echo "Redaction check failed!"spec/mailtrap/projects_api_spec.rb (4)
12-15: LGTM: list maps to DTOs correctlyThe expectation focuses on type mapping, which is sufficient here and avoids brittle attribute assertions.
19-26: LGTM: authorization failure path is well assertedGood coverage of error class, message, and aggregated messages.
32-45: LGTM: happy-path and error-path coverage is solidGood coverage of:
- get: mapping and 404
- create: mapping and client error
- update: not-found path
- delete: not-found path
Assertions check both the error type and message text.
Also applies to: 91-104, 155-166, 182-193
61-61: Be explicit about kwargs vs Hash to avoid Ruby 3 keyword separation pitfallsThe
ProjectsAPImethods are defined as:def create(options) base_create(options) end def update(project_id, options) base_update(project_id, options) endbut your specs call:
project.create(**request) project.update(project_id, **request)– under Ruby 2.7 this emits a warning and still passes the Hash, but under Ruby 3.x it will raise an “unknown keyword” error. You have two paths:
• Option A (tests-only)
Keep the existing API signatures and change the spec subjects to pass the Hash positionally:- subject(:create) { project.create(**request) } + subject(:create) { project.create(request) } - subject(:update) { project.update(project_id, **request) } + subject(:update) { project.update(project_id, request) }• Option B (library)
Accept keyword args in the API itself:def create(**options) base_create(options) end def update(project_id, **options) base_update(project_id, options) endAt minimum, please verify which Ruby version your library targets (e.g. via
Gemfile, CI config or.ruby-version) and ensure your method signatures and call sites align.Affected specs:
- spec/mailtrap/projects_api_spec.rb at lines 61 and 108
lib/mailtrap/project.rb
Outdated
| # @attr_reader id [String] The project ID | ||
| # @attr_reader name [String] The project name | ||
| # @attr_reader share_links [Array] Array of links | ||
| # @attr_reader inboxes [Array] Array of inboxes | ||
| # @attr_reader permissions [Hash] List of permissions |
There was a problem hiding this comment.
Fix YARD types and accessor annotations (docs are incorrect).
- id is returned as an Integer by the API, not String.
- share_links is a Hash (with admin/viewer), not an Array.
- Struct defines writer methods too; documenting these as reader-only is misleading.
Apply this doc fix:
- # @attr_reader id [String] The project ID
- # @attr_reader name [String] The project name
- # @attr_reader share_links [Array] Array of links
- # @attr_reader inboxes [Array] Array of inboxes
- # @attr_reader permissions [Hash] List of permissions
+ # @!attribute [rw] id
+ # @return [Integer] The project ID
+ # @!attribute [rw] name
+ # @return [String] The project name
+ # @!attribute [rw] share_links
+ # @return [Hash] Admin and viewer share links
+ # @!attribute [rw] inboxes
+ # @return [Array] Array of inboxes
+ # @!attribute [rw] permissions
+ # @return [Hash] List of permissions📝 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.
| # @attr_reader id [String] The project ID | |
| # @attr_reader name [String] The project name | |
| # @attr_reader share_links [Array] Array of links | |
| # @attr_reader inboxes [Array] Array of inboxes | |
| # @attr_reader permissions [Hash] List of permissions | |
| # @!attribute [rw] id | |
| # @return [Integer] The project ID | |
| # @!attribute [rw] name | |
| # @return [String] The project name | |
| # @!attribute [rw] share_links | |
| # @return [Hash] Admin and viewer share links | |
| # @!attribute [rw] inboxes | |
| # @return [Array] Array of inboxes | |
| # @!attribute [rw] permissions | |
| # @return [Hash] List of permissions |
🤖 Prompt for AI Agents
In lib/mailtrap/project.rb around lines 6 to 10, the YARD annotations are
incorrect: change the id type to Integer instead of String, change share_links
to a Hash (e.g. Hash with admin/viewer keys) instead of Array, keep inboxes as
an Array and permissions as a Hash, and mark the attributes as read-write (the
Struct defines writers) rather than reader-only; update the @attr_* tags or
replace them with @!attribute [rw] annotations to reflect the correct types and
that getters and setters exist.
...ssettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml
Outdated
Show resolved
Hide resolved
...ssettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml
Outdated
Show resolved
Hide resolved
| RSpec.describe Mailtrap::ProjectsAPI, :vcr do | ||
| subject(:project) { described_class.new(account_id, client) } | ||
|
|
||
| let(:account_id) { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | ||
| let(:client) { Mailtrap::Client.new(api_key: ENV.fetch('MAILTRAP_API_KEY')) } | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check VCR cassettes don’t leak secrets/account_id
This spec depends on ENV variables and uses VCR. Given spec_helper recently commented out filters for account_id, ensure cassettes don’t contain live secrets or unredacted account identifiers.
Run the following scan to spot potential leaks:
If we find leaks, I can help re-add VCR filters and re-record cassettes with redaction.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Scanning cassettes for obvious secrets..."
# Authorization headers (common patterns)
rg -n -C1 -g 'spec/fixtures/vcr_cassettes/**' -P '(Authorization:|Api-Token:|X-Api-Token:|Bearer\s+[A-Za-z0-9\.\-_]+)'
echo -e "\nScanning for account id patterns (6+ digits) in URLs and bodies..."
rg -n -C1 -g 'spec/fixtures/vcr_cassettes/**' -P '/api/accounts/\d{5,}/projects'
echo -e "\nScanning for raw API keys from env names..."
rg -n -C1 -g 'spec/fixtures/vcr_cassettes/**' -P '(MAILTRAP_API_KEY|MAILTRAP_ACCOUNT_ID)'Length of output: 47012
Leaked Account ID in VCR Cassettes
Our scan shows hard-coded account IDs (e.g. 1495731) in the URIs of all spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/** files. We need to redact these before merging.
• In your spec_helper.rb (or VCR setup), re-enable and add:
VCR.configure do |config|
# …existing filters…
config.filter_sensitive_data('<MAILTRAP_ACCOUNT_ID>') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }
end• Re-record the Project API cassettes so the real ID is replaced by the placeholder.
Let me know if you’d like assistance with updating and re-recording these cassettes.
🤖 Prompt for AI Agents
In spec/mailtrap/projects_api_spec.rb lines 3-8 there are tests that caused VCR
cassettes to record a real Mailtrap account ID; update your VCR configuration
(e.g., in spec/spec_helper.rb or the existing VCR setup) to add a
filter_sensitive_data entry that replaces ENV.fetch('MAILTRAP_ACCOUNT_ID') with
a placeholder like <MAILTRAP_ACCOUNT_ID>, then re-record the
Mailtrap_ProjectsAPI cassettes (run the specs that exercise ProjectsAPI with VCR
enabled) so the recorded URIs use the placeholder instead of the real ID;
finally commit the updated spec helper and the re-recorded cassettes.
| 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 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
“with hash request” doesn’t change the call shape; override subject to pass a Hash
Right now, the context still calls project.create(**request) via the outer subject, so it exercises the same kwargs path. Override subject here to actually pass a positional Hash.
Apply this diff:
context 'with hash request' do
+ subject(:create) { project.create(request) }
let(:request) do
{
name: 'New Project'
}
end📝 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.
| 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 | |
| context 'with hash request' do | |
| subject(:create) { project.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' | |
| ) | |
| end | |
| end |
🤖 Prompt for AI Agents
In spec/mailtrap/projects_api_spec.rb around lines 76 to 89, the "with hash
request" context still uses the outer subject which calls
project.create(**request) (keyword args) so it doesn't test the positional-Hash
path; override the subject inside this context to call project.create(request)
(positional Hash) so the spec exercises the Hash call shape instead of kwargs.
| 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 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same issue for update: ensure the hash path is actually tested
Override subject in this context to pass a positional Hash instead of kwargs.
Apply this diff:
context 'with hash request' do
+ subject(:update) { project.update(project_id, request) }
let(:request) do
{
name: 'Updated Project'
}
end📝 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.
| 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 | |
| context 'with hash request' do | |
| subject(:update) { project.update(project_id, request) } | |
| 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 |
🤖 Prompt for AI Agents
In spec/mailtrap/projects_api_spec.rb around lines 129 to 142, the "with hash
request" context never overrides the subject to send a positional Hash (it still
uses keyword args), so update the context to override the subject to call the
update method with the project id and the request Hash as a positional argument
(i.e. subject(:update) { described_class.update(project_id, request) }) so the
spec actually tests the hash path; keep the rest of the expectations unchanged.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/spec_helper.rb (1)
54-58: Ensure URI matching ignores the account id segment for playback stability.Since cassettes record ACCOUNT_ID in the URI while live requests use a real id, replace :uri with a custom matcher that normalizes the account segment.
Apply this diff:
- config.default_cassette_options = { - match_requests_on: [:method, :uri, :body, header_matcher], + # Use a URI matcher that ignores the numeric account id segment. + config.default_cassette_options = { + match_requests_on: [:method, :uri_ignoring_account_id, :body, header_matcher], allow_unused_http_interactions: false, record: :once }And register the matcher (add below the before_record block):
config.register_request_matcher :uri_ignoring_account_id do |r1, r2| norm = ->(u) { u.gsub(%r{/accounts/\d+/}, '/accounts/ACCOUNT_ID/') } norm.call(r1.uri) == norm.call(r2.uri) end
♻️ Duplicate comments (1)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml (1)
163-164: Share links correctly sanitized in both interactions.Admin/viewer share URLs are redacted consistently. This resolves the earlier leakage concern.
Also applies to: 326-327
🧹 Nitpick comments (54)
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml (3)
61-148: Trim CSP header noise from cassettes to improve fixture stability.The Content-Security-Policy blocks are very long and change frequently (domains, nonces). They don’t influence how the client handles 201/404 logic here and will cause unnecessary fixture diffs. Consider stripping CSP during recording.
Add a before_record hook (in spec_helper or VCR config) to drop CSP and other volatile headers:
# spec/spec_helper.rb (or wherever VCR is configured) VCR.configure do |c| c.before_record do |i| %w[ Content-Security-Policy Report-To Nel ].each { |h| i.response.headers.delete(h) } end endAlso applies to: 225-313
27-27: Strip highly volatile headers and metadata (date, etag, tracing, cf-ray, ratelimit) to reduce churn.These fields are expected to vary across recordings and don’t affect client behavior in these tests. Keeping them inflates diffs and makes reviews noisy.
Recommended VCR sanitization:
VCR.configure do |c| c.before_record do |i| %w[ Date Etag X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Ray Server X-Ratelimit-Remaining Alt-Svc ].each { |h| i.response.headers.delete(h) } # Also remove VCR metadata timestamps if present i.recorded_at = nil if i.respond_to?(:recorded_at=) end endAlso applies to: 55-55, 57-57, 150-154, 159-161, 168-169, 193-193, 196-197, 200-201, 221-221, 314-318, 323-325, 330-330
165-167: Optional: redact dynamic resource identifiers in bodies to minimize re-record diffs.id and uuid are not sensitive, but they’ll change on re-record. If tests don’t consume these values, consider normalizing them.
Example normalization:
VCR.configure do |c| c.before_record do |i| body = i.response.body next unless body && body.is_a?(String) body = body.gsub(/"id":\s*\d+/, '"id":TEMPLATE_ID') .gsub(/"uuid":\s*"[0-9a-f-]+"/i, '"uuid":"TEMPLATE_UUID"') i.response.body = body end endspec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml (3)
27-36: Reduce cassette churn: filter volatile infra headers (Date, Server, Cf-Ray, X-Request-Id, X-Runtime, X-Cloud-Trace-Context).These values change on every recording, causing noisy diffs without adding test value. Consider dropping them at record-time.
Add a before_record hook in spec_helper.rb:
VCR.configure do |c| volatile_headers = %w[ Date Server X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Alt-Svc Connection Transfer-Encoding Etag ] c.before_record do |i| volatile_headers.each { |h| i.response.headers.delete(h) } end endAlso applies to: 149-161, 192-201, 312-324
60-148: Normalize or drop Content-Security-Policy header with nonces.CSP strings and nonces are highly dynamic and bloat fixtures. Unless tests assert CSP, remove or normalize them.
Apply this spec_helper.rb tweak:
VCR.configure do |c| c.before_record do |i| if i.response.headers['Content-Security-Policy'] # Option A: remove entirely i.response.headers.delete('Content-Security-Policy') # Option B: normalize nonces instead (pick one approach) # i.response.headers['Content-Security-Policy'] = # i.response.headers['Content-Security-Policy'].map { |v| v.gsub(/'nonce-[^']+'/, "'nonce-<REDACTED>'") } end end endAlso applies to: 224-311
165-168: Scrub dynamic timestamps in response body to stabilize recordings.created_at/updated_at change every record and aren’t needed for 404 deletion behavior.
You can normalize them during recording:
VCR.configure do |c| c.before_record do |i| if i.response.headers['Content-Type']&.any? { |v| v.include?('application/json') } begin body = JSON.parse(i.response.body) %w[created_at updated_at].each do |k| body[k] = '<TIMESTAMP>' if body.key?(k) end i.response.body = body.to_json rescue JSON::ParserError # ignore non-JSON bodies end end end endspec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (5)
26-26: Normalize or drop volatile timestamps (Date, recorded_at) to reduce cassette churn.These values change every re-record and add noise to diffs. Strip them at record time.
Add to VCR config:
VCR.configure do |c| c.before_record do |i| i.response.headers.delete("Date") i.recorded_at = Time.utc(1970,1,1) # or leave nil end endAlso applies to: 165-165
33-34: Remove Server header from cassette (environment-specific, noisy).
Server: cloudflareis infra-specific and not relevant to assertions.Add to the same
before_recordhook:i.response.headers.delete("Server")
60-147: Huge CSP header is highly volatile; recommend stripping to keep fixtures stable and small.CSP includes nonces, domains, and vendor updates that churn often. Keeping it inflates diffs without adding test value.
In
spec_helper.rb:VCR.configure do |c| c.before_record do |i| i.response.headers.delete("Content-Security-Policy") end endAlternatively, redact its value to a placeholder:
i.response.headers["Content-Security-Policy"] = ["<CSP-REDACTED>"]
149-154: Redact or drop ephemeral tracing headers (X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-*).These are random per request and cause unnecessary cassette updates.
Extend the filter:
%w[ X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Alt-Svc ].each { |h| i.response.headers.delete(h) }Also applies to: 158-161
55-56: Consider removing X-Ratelimit-Remaining from fixtures.This value depends on recording-time quota. It’s not actionable for unit tests.
Add to the header drop list:
i.response.headers.delete("X-Ratelimit-Remaining")spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (5)
26-36: Strip highly volatile infra headers from cassettes to reduce churn.Headers like Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-Cache-Status, Cf-Ray, Alt-Svc change on every re-record and create noisy diffs. Prefer removing them at record time.
Add a before_record filter in spec/spec_helper.rb:
VCR.configure do |c| c.before_record do |i| %w[ Date Server Etag Content-Length Transfer-Encoding Alt-Svc X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Strict-Transport-Security Vary ].each { |h| i.response.headers.delete(h) } end endAlso applies to: 150-161
55-55: Avoid persisting rate-limit counters in fixtures.X-Ratelimit-Remaining is environment-dependent and not relevant for assertions; remove it alongside other volatile headers.
Extend the suggested before_record header list with: X-Ratelimit-Remaining, X-Ratelimit-Limit.
60-148: Consider dropping the entire Content-Security-Policy header.CSP is long and frequently changes (nonce, host allow-list), causing unnecessary cassette diffs. Unless specs assert on CSP, strip it.
In the same before_record hook:
i.response.headers.delete('Content-Security-Policy')If you need to keep presence, replace with a placeholder:
i.response.headers['Content-Security-Policy'] = ['REDACTED_CSP']
165-168: Timestamps in response body are inherently volatile.If tests don’t assert exact created_at/updated_at, consider normalizing them during recording to reduce noise.
Example (spec/spec_helper.rb):
VCR.configure do |c| c.before_record do |i| next unless i.response.headers['Content-Type']&.any? { |v| v.include?('application/json') } begin body = JSON.parse(i.response.body) %w[created_at updated_at].each { |k| body[k] = '2000-01-01T00:00:00Z' if body.key?(k) } i.response.body = body.to_json i.response.headers.delete('Content-Length') rescue JSON::ParserError # skip non-JSON bodies end end end
17-18: Token is redacted — good. Consider unifying placeholder naming.You use <BEARER_TOKEN>; ensure spec_helper also filters any Authorization variants to this placeholder.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml (4)
26-36: Remove ephemeral infra/tracing headers.Date/Server/X-Request-Id/X-Runtime/X-Cloud-Trace-Context/Cf-* vary per run; not useful for assertions.
Use the same before_record header deletion list suggested in the first file.
Also applies to: 150-161, 191-201, 315-327
60-149: CSP header churn — drop or redact.These long CSP blocks will reflow with nonces and source lists; strip or replace with a placeholder unless explicitly asserted.
Also applies to: 225-314
55-55: Rate-limit counters should not be committed.Remove X-Ratelimit-* headers during recording.
Also applies to: 220-221
165-168: Stabilize body timestamps if not asserted.Normalize created_at/updated_at to a fixed value and drop Content-Length to prevent mismatches post-transform.
Also applies to: 329-333
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml (4)
26-36: Trim volatile transport/edge headers.Same guidance: drop Date/Server/X-/Cf- headers at record time.
Also applies to: 150-161, 191-201, 315-327
60-149: Reduce CSP noise.Strip or replace CSP header; it’s unrelated to Mailtrap client behavior.
Also applies to: 225-314
55-55: Avoid persisting rate-limit and etag.Etag changes frequently as well; unless you assert it, remove.
Extend header deletion list with Etag if not already present.
Also applies to: 219-223
165-168: Normalize timestamps in JSON body if tests don’t assert them.Prevents re-record churn.
Also applies to: 329-333
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml (4)
26-36: Delete volatile headers on record.Same rationale: Date/Server/X-/Cf- headers are noise in version control.
Also applies to: 150-161
60-148: CSP header: strip or stub.Recommend redacting to 'REDACTED_CSP' or removing entirely.
55-55: Drop X-Ratelimit- headers.*Not actionable in tests; fluctuates per environment.
165-168: Stabilize JSON timestamps if not required for assertions.Consider the normalization hook shown earlier; also remove Content-Length after mutation.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml (4)
26-36: Remove volatile headers (Date/Server/X-/Cf-).Prevents frequent cassette diffs unrelated to client behavior.
Also applies to: 150-161, 192-201, 315-327
60-149: CSP header is excessively noisy here as well.Recommend stripping or replacing with a placeholder.
Also applies to: 227-315
55-55: Drop X-Ratelimit- and Etag headers.*They vary between runs and provide no test value.
Also applies to: 221-224
165-168: Body timestamps normalization.Same suggestion: normalize created_at/updated_at if tests don’t need exact values; remove Content-Length after mutation.
Also applies to: 330-334
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml (4)
26-36: Strip highly volatile response headers in VCR to reduce cassette churn.Headers like Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-Ray are inherently unstable and will force frequent cassette updates. Prefer removing them at record-time.
Add this to spec/spec_helper.rb (or your VCR setup) to drop volatile headers:
VCR.configure do |c| c.before_record do |i| %w[ Date Server X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Alt-Svc ].each { |h| i.response.headers.delete(h) } end endI can extend this to other files in the suite on request.
Also applies to: 150-161
55-58: Normalize dynamic counters/tags (rate limit and ETag).X-Ratelimit-Remaining and Etag will also drift between recordings. Consider redacting or normalizing them to a fixed placeholder to stabilize diffs.
Suggested tweak to the same VCR hook:
VCR.configure do |c| c.before_record do |i| %w[X-Ratelimit-Remaining Etag].each do |h| i.response.headers[h] = ['<REDACTED>'] if i.response.headers[h] end end endAlso applies to: 221-224
60-149: CSP header contains long, nonce-bearing policies that cause noisy fixtures.Content-Security-Policy includes per-response nonces and a very long directive list. Unless your tests assert CSP, redact or drop the header for leaner, more stable cassettes.
Add to the VCR hook:
VCR.configure do |c| c.before_record do |i| if i.response.headers['Content-Security-Policy'] i.response.headers['Content-Security-Policy'] = ['<REDACTED_CSP>'] end end endAlso applies to: 227-315
61-149: Consider redacting potential internal endpoints in CSP.The CSP includes report-uri to errors.rw.rw with a sentry_key. While it’s from a response header, redacting minimizes accidental disclosure of internal telemetry endpoints.
Integrate in the CSP redaction above or specifically:
i.response.headers['Content-Security-Policy'] = [i.response.headers['Content-Security-Policy'].first.gsub(/report-uri\s+\S+/, 'report-uri <REDACTED>')]spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml (2)
192-201: Drop volatile headers (Date/Server/etc.) from the cassette.Same rationale as other fixtures—keeps diffs minimal and reduces re-record churn.
See shared VCR before_record hook suggestion in my earlier comment to strip these headers.
218-223: Normalize X-Ratelimit-Remaining and Etag.Recommend redacting/normalizing these to avoid needless fixture updates.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml (2)
26-36: Stabilize volatile headers in VCR (Date/Server/etc.).Recommend removing or redacting these at record time.
Also applies to: 191-201
61-149: Redact or drop CSP header with nonces.Long, nonce-based CSP values cause constant churn; prefer redaction unless explicitly asserted.
Also applies to: 226-314
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml (2)
227-275: CSP header should be redacted to minimize fixture noise.Same recommendation as other files—replace with placeholder or remove.
Also applies to: 109-149
219-224: Remove or normalize volatile headers (rate limits, ids, trace headers).This will make cassette diffs far more stable over time.
Also applies to: 150-161, 192-201
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (3)
26-36: Prune volatile headers at record time (Date, Server, trace ids).Same guidance as other cassettes.
Also applies to: 192-202
61-149: Redact CSP header with nonce.Replace with a placeholder via VCR hook unless you assert on CSP explicitly.
Also applies to: 227-315
55-58: Normalize X-Ratelimit-Remaining and Etag to placeholders.Prevents unnecessary cassette changes.
Also applies to: 219-224
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml (4)
26-36: Strip volatile headers to reduce cassette churn.Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-* and Etag change on every record and bloat diffs. Recommend removing them at record time.
Add in spec_helper:
VCR.configure do |c| c.before_record do |i| %w[ Date Server X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Alt-Svc Etag X-Ratelimit-Remaining ].each { |h| i.response.headers.delete(h) } end endAlso applies to: 55-58, 149-163, 168-169
60-148: Consider excluding Content-Security-Policy from cassettes.CSP is extremely long and frequently changes (nonces, hosts). It creates large, noisy diffs without test value. Suggest dropping this header during recording.
In spec_helper:
VCR.configure do |c| c.before_record { |i| i.response.headers.delete('Content-Security-Policy') } end
30-36: Normalize body length headers.Content-Length vs Transfer-Encoding: chunked varies by edge server and causes mismatches across recordings. Consider deleting both headers to stabilize fixtures.
In spec_helper:
VCR.configure do |c| c.before_record do |i| i.response.headers.delete('Content-Length') i.response.headers.delete('Transfer-Encoding') i.response.headers.delete('Connection') end endAlso applies to: 195-201
220-221: Optional: avoid asserting rate-limit counters and recorded_at.X-Ratelimit-Remaining and recorded_at are incidental. Removing them trims diffs and fragility.
In spec_helper, also delete 'X-Ratelimit-Remaining'. recorded_at is added by VCR; keeping it is harmless, but if it creates merge churn you can post-process cassettes in CI to strip it.
Also applies to: 329-329
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml (2)
26-36: Reduce fixture volatility by dropping non-deterministic headers.These headers change per run and add noise: Date, Server, Etag, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-*, Alt-Svc, X-Ratelimit-Remaining.
Leverage the same before_record hook proposed earlier to delete them globally.
Also applies to: 55-58, 149-163, 168-168, 191-201, 213-215, 306-318, 323-323
60-148: Exclude CSP from recordings.The extensive CSP block changes often and offers no assertion value for the EmailTemplates behavior. Remove it during recording to keep cassettes lean.
Also applies to: 217-305
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (3)
26-36: Trim volatile headers (Date, Server, Etag, IDs, trace).Recommend the same before_record header deletion here to avoid frequent cassette updates.
Also applies to: 55-58, 149-161, 168-168, 191-201, 220-223, 315-326, 333-334
60-148: Drop CSP headers from POST/PATCH responses.Large, frequently changing CSP policies add no test value for model mapping or update behavior assertions. Remove at record time.
Also applies to: 225-314
27-27: Stabilize body-length-related headers.Delete Content-Length/Transfer-Encoding/Connection during recording for stability across CDN/edge variations.
Also applies to: 192-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (42)
lib/mailtrap/project.rb(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/returns_an_array_of_EmailTemplate_objects.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml(3 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/returns_an_EmailTemplate_object.yml(0 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/projects_api_spec.rb(1 hunks)spec/spec_helper.rb(1 hunks)
💤 Files with no reviewable changes (9)
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/returns_an_array_of_EmailTemplate_objects.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/returns_an_EmailTemplate_object.yml
✅ Files skipped from review due to trivial changes (3)
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/returns_deleted_project_id.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/with_hash_request/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/mailtrap/project.rb
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml
- spec/mailtrap/projects_api_spec.rb
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/when_project_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/when_project_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/maps_response_data_to_Project_objects.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/spec_helper.rb (1)
lib/mailtrap/mail/base.rb (1)
to_json(64-69)
🔇 Additional comments (30)
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml (5)
5-5: Good: endpoint path parametrized to ACCOUNT_ID.POST now targets accounts/ACCOUNT_ID/email_templates, aligning with the new redaction strategy and preventing hardcoded IDs from leaking into fixtures.
171-171: Good: PATCH path also parametrized to ACCOUNT_ID.Consistent use of ACCOUNT_ID across EmailTemplates endpoints reduces future cassette churn when running against different accounts.
17-20: Good: Authorization header is redacted.Bearer token is masked with <BEARER_TOKEN>, aligning with the suite’s redaction policy.
5-5: All fixtures sanitized: no raw account IDs or unmasked tokens
- Ran
rg -nP '/api/accounts/\d+/'acrossspec/fixtures/vcr_cassettesand found no raw numeric account IDs.- Ran
rg -nP 'Bearer (?!<)'and confirmed there are no unmasked Bearer tokens.- Ran
rg -nP '/api/accounts/ACCOUNT_ID/'and verified every cassette consistently uses theACCOUNT_IDplaceholder.All checks pass; no further action needed.
30-31: No header assertions found in specs – suggestion not required
I ran a repository-wide search for any spec asserting onContent-Length,Transfer-Encoding,Server,Cf-Ray, orEtagheaders and found no matches. Since there are currently no tests verifying header presence or values, there’s no brittleness risk here.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml (4)
195-201: No custom middleware inspects Content-Length or Transfer-Encoding
Ranrg -nP '\b(Content-Length|Transfer-Encoding)\b' -C3 liband found no occurrences. No further action needed.
17-20: No token leaks detected in VCR fixturesI’ve searched all
spec/fixtures/vcr_cassettes/**for real credential patterns (Bearer tokens, API keys,tokenfields) and verified that every Authorization header uses the<BEARER_TOKEN>placeholder and no actual secrets appear in either request or response bodies. These fixtures are safe to merge.
5-5: All VCR cassettes consistently use theACCOUNT_IDplaceholder and it’s scrubbed inspec_helper.rb
- Search for raw numeric IDs in
spec/fixtures/vcr_cassettes/**returned no matches foraccounts/\d+, confirming all URIs use theACCOUNT_IDplaceholder.spec/spec_helper.rbincludesensuring real account IDs are filtered out before recordings.config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }No further changes required.
3-169: I don’t see the spec actually referencing that initial POST—your grep used underscores but the example text in the spec is almost certainly written with spaces. Can you please re-verify using the actual descriptions? For example:# Locate the “when template does not exist” example rg -n "when template does not exist" -C3 spec/mailtrap/email_templates_api_spec.rb # Locate the “raises not found error” assertion rg -n "raises not found error" -C2 spec/mailtrap/email_templates_api_spec.rbRunning those should show that the example only calls
delete(and asserts the 404), with no create/POST in the example itself—confirming the POST interaction can be safely removed from the cassette.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
16-19: Auth header redaction looks good.
Authorization: Bearer <BEARER_TOKEN>is properly sanitized. No action needed.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (1)
5-10: Endpoint placeholder and payload look good.Switching to ACCOUNT_ID in the URI and expanding the invalid payload fields are consistent with the new redaction and spec expectations.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/maps_response_data_to_Project_object.yml (1)
163-165: Create cassette maps cleanly to Project and share_links are sanitized.Response shape matches the DTO fields and tokens are redacted.
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/maps_response_data_to_Project_object.yml (1)
163-165: Update flow cassette is consistent and sanitized.Both create and update responses map cleanly to Project and redact share tokens.
Also applies to: 326-327
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: ACCOUNT_ID placeholder in URI looks good.Consistent parameterization reduces future cassette churn and leaking of real account IDs.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml (1)
5-5: Good: Endpoint generalized to ACCOUNT_ID.Keeps fixtures reusable across environments.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml (1)
5-5: LGTM on URI parameterization.Both create and patch use ACCOUNT_ID consistently.
Also applies to: 171-171
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: Good move to placeholder account id.Consistent with other updated fixtures.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml (2)
5-5: URI placeholdering is consistent across create and patch.Nice consistency.
Also applies to: 171-171
174-176: Tests correctly target only the specified fieldsI reviewed the VCR fixture for
when_updating_multiple_specific_fields/updates_only_the_specified_fields.ymland its associated spec. The request payload includes only thecategoryandbody_htmlfields, and the recorded response shows:
categoryandbody_htmlupdatedupdated_atadvanced by the server to reflect the change- all other attributes (
id,uuid,name,subject,body_text,created_at) preservedThere are no assertions in this test on any headers or unrelated metadata. Everything else remains unchanged, so the spec’s expectations align exactly with “updates only the specified fields.”
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: Good move: parameterized account id in the URI.Using ACCOUNT_ID improves fixture portability and avoids accidental leakage of real account ids.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml (2)
5-5: Nice: ACCOUNT_ID placeholder applied consistently.Both POST and PATCH URIs are generalized. This keeps fixtures reusable across environments.
Also applies to: 171-171
329-333: Behavior check passes: only body_html updated, others preserved.Response reflects body_html change and leaves name/subject/category untouched as intended for this test.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml (2)
5-5: Good: URIs now use ACCOUNT_ID for both create and get.Reduces leakage and aligns with updated spec_helper redaction.
Also applies to: 171-171
164-168: The script will search your spec files for any brittle expectations onid,uuid,created_at, orupdated_at. Once we have the output, we can confirm whether such exact-value assertions exist and advise appropriately.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml (2)
5-5: ACCOUNT_ID placeholder usage is consistent.Nice consistency on both POST and PATCH endpoints.
Also applies to: 171-171
330-334: Behavior looks correct: specific fields updated, others intact.The response shows category and body_html updated, while name/subject/body_text remain unchanged. Matches the spec intent.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (2)
5-5: URI generalization to ACCOUNT_ID is correct.Keeps sensitive account numbers out of the repo.
Also applies to: 171-171
330-334: Verified absence of strict ETag/rate-limit/timestamp assertionsRan a repository-wide search for any specs matching
X-Ratelimit-Remaining,Etag,created_at, orupdated_atagainst literal expectations (e.g.to,eq,match) and found no occurrences. All mapping specs correctly focus on mutable content fields only.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml (1)
165-168: Nice: IDs consistent across create/delete within the same cassette.The DELETE targets the id returned by the preceding POST (45475), keeping the flow realistic and deterministic. Looks good.
Also applies to: 321-323
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (1)
329-333: Good: PATCH response preserves unrelated fields.Only category changed, while name, subject, and bodies remain intact. This aligns with the spec intent for partial updates.
| @@ -2,7 +2,7 @@ | |||
| http_interactions: | |||
| - request: | |||
| method: post | |||
| uri: https://mailtrap.io/api/accounts/1111111/email_templates | |||
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm ACCOUNT_ID placeholder matching for POST/DELETE flow.
Same concern as other cassette: playback must match a literal /accounts/ACCOUNT_ID path unless a matcher normalizes the account segment. Verify matcher coverage for both POST and DELETE interactions.
Also applies to: 171-171
🏁 Script executed:
#!/bin/bash
# Ensure the same matcher covers DELETE requests too.
rg -nC2 -e 'register_request_matcher|match_requests_on' spec/spec_helper.rbLength of output: 255
🏁 Script executed:
#!/bin/bash
# Search for custom request matchers and VCR configuration in spec/spec_helper.rb
echo "---- Around default_cassette_options ----"
rg -nC5 'default_cassette_options' spec/spec_helper.rb || true
echo "---- VCR.configure block ----"
rg -nC20 'VCR.configure' spec/spec_helper.rb || true
echo "---- register_request_matcher occurrences ----"
rg -nC5 'register_request_matcher' spec/spec_helper.rb || trueLength of output: 1517
Add a custom URI matcher to normalize the ACCOUNT_ID placeholder
The current VCR setup only filters sensitive data at record time, but still matches on the raw URI string at playback—so a cassette containing /accounts/ACCOUNT_ID/... won’t match a real request to /accounts/12345/... unless we normalize that segment when comparing. We need to register a custom request matcher and swap in that matcher for :uri in both POST and DELETE flows.
• In spec/spec_helper.rb, inside the VCR.configure block, register a matcher that replaces the dynamic account segment with the literal placeholder:
VCR.configure do |config|
# … existing config …
# Normalize /accounts/<any> to /accounts/ACCOUNT_ID for matching
config.register_request_matcher :uri_with_account_placeholder do |req1, req2|
normalize = -> uri { uri.sub(%r{/accounts/[^/]+}, '/accounts/ACCOUNT_ID') }
normalize.call(req1.uri) == normalize.call(req2.uri)
end
config.default_cassette_options = {
match_requests_on: [
:method,
:uri_with_account_placeholder, # use the custom matcher
:body,
header_matcher
],
allow_unused_http_interactions: false,
record: :once
}
end• Confirm both the POST and DELETE cassettes under spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/ use ACCOUNT_ID in their URI lines and will now correctly match during playback.
This ensures that any request URI under /accounts/<actual_id>/… will be treated equivalently to the recorded /accounts/ACCOUNT_ID/…, covering both create and delete scenarios.
🤖 Prompt for AI Agents
In
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml
around line 5 and in spec/spec_helper.rb inside the VCR.configure block, the URI
matching still uses raw URIs so recorded cassettes with /accounts/ACCOUNT_ID
won't match real requests with /accounts/<actual_id>; register a custom request
matcher (e.g., :uri_with_account_placeholder) that normalizes URIs by
substituting /accounts/<any> with /accounts/ACCOUNT_ID before comparing, then
add that matcher to config.default_cassette_options[:match_requests_on] (replace
the :uri entry with the new matcher alongside :method, :body, and header
matcher), and verify the POST and DELETE cassette URI lines retain the
ACCOUNT_ID placeholder so playback will match real account IDs.
| @@ -2,7 +2,7 @@ | |||
| http_interactions: | |||
| - request: | |||
| method: post | |||
| uri: https://mailtrap.io/api/accounts/1111111/email_templates | |||
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify cassette matching with ACCOUNT_ID placeholder.
You've replaced the literal account id with ACCOUNT_ID in the URI. VCR matches on the literal recorded URI during playback, so unless you added a custom request matcher (or other normalization) the runtime request to /accounts//... will not match /accounts/ACCOUNT_ID/.... Please confirm that spec/spec_helper.rb registers a matcher that strips or normalizes the account segment, not just redacts on recording.
Run this to confirm the matcher exists:
🏁 Script executed:
#!/bin/bash
# Expect to see a custom matcher or match_requests_on excluding the account id path segment.
rg -nC2 -e 'register_request_matcher|match_requests_on' -e 'ACCOUNT_ID' -e 'filter_sensitive_data' spec/spec_helper.rb spec/**/*Length of output: 52556
Add a custom VCR request matcher to normalize the ACCOUNT_ID segment
I verified spec/spec_helper.rb and confirmed there is only a filter_sensitive_data('ACCOUNT_ID') call (which redacts the real account ID in recordings) and the default cassette options:
config.default_cassette_options = {
match_requests_on: [:method, :uri, :body, header_matcher],
allow_unused_http_interactions: false,
record: :once
}Because VCR matches on the literal recorded URI, any runtime request to /accounts/<numeric>/… will not match the placeholder /accounts/ACCOUNT_ID/… in the cassette. To fix this, register a matcher that strips or normalizes the account-ID segment before comparison. For example, in spec/spec_helper.rb:
VCR.configure do |config|
# Register a matcher that normalizes the account ID to the placeholder
config.register_request_matcher :uri_without_account_id do |req1, req2|
normalize = ->(uri) { uri.gsub(%r{/accounts/\d+(/|$)}, '/accounts/ACCOUNT_ID\1') }
normalize.call(req1.uri) == normalize.call(req2.uri)
end
# Use the new matcher instead of the plain :uri matcher
config.default_cassette_options = {
match_requests_on: [:method, :uri_without_account_id, :body, header_matcher],
allow_unused_http_interactions: false,
record: :once
}
end• Please update spec/spec_helper.rb to register and use this custom matcher so that recorded cassettes with the ACCOUNT_ID placeholder will correctly match runtime requests containing the actual numeric account ID.
🤖 Prompt for AI Agents
In
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml
around line 5, the cassette contains a placeholder ACCOUNT_ID which prevents
runtime URIs with numeric account IDs from matching; update spec/spec_helper.rb
to register a custom VCR request matcher (e.g., :uri_without_account_id) that
normalizes URIs by replacing /accounts/<digits>/ with /accounts/ACCOUNT_ID/
before comparison, and change config.default_cassette_options to use
:uri_without_account_id instead of :uri so recorded cassettes with the
ACCOUNT_ID placeholder match runtime requests.
| @@ -2,7 +2,7 @@ | |||
| http_interactions: | |||
| - request: | |||
| method: get | |||
| uri: https://mailtrap.io/api/accounts/1111111/email_templates | |||
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Good: endpoint generalized to placeholder ACCOUNT_ID. Verify VCR matching won’t break.
Changing the recorded URI to use ACCOUNT_ID improves redaction, but by default VCR matches on the full URI. Unless you also normalize the request URI during matching (custom matcher) this cassette may not match playback when the runtime uses a numeric account id.
Run the script below to confirm a custom matcher/redaction is in place and to ensure no specs assert on brittle headers:
If a matcher is missing, consider this spec_helper snippet:
# In spec/spec_helper.rb
require "uri"
VCR.configure do |c|
# Redact the account id in recorded artifacts
c.filter_sensitive_data("ACCOUNT_ID") { ENV.fetch("MAILTRAP_ACCOUNT_ID", "1111111") }
# Normalize account id segment during matching to avoid playback mismatches
c.register_request_matcher(:method_host_path_wo_account) do |r1, r2|
norm = ->(u) {
uri = URI(u)
uri.path = uri.path.gsub(%r{(/accounts/)\d+}, '\1ACCOUNT_ID')
[uri.scheme, uri.host, uri.path, uri.query]
}
[r1.method, *norm.call(r1.uri)] == [r2.method, *norm.call(r2.uri)]
end
c.default_cassette_options = { match_requests_on: [:method_host_path_wo_account] }
end🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate VCR config and redactions =="
fd -a -HI 'spec_helper*.rb' || true
rg -nC3 "VCR\.configure" spec || true
rg -nC3 "filter_sensitive_data\('ACCOUNT_ID'\)|register_request_matcher|match_requests_on|before_record|before_playback" spec || true
echo "== Ensure no tests assert on dynamic headers =="
rg -n "Content-Security-Policy|Cf-Ray|X-Request-Id|X-Cloud-Trace-Context|X-Runtime|Date|Server:" spec -g '!**/fixtures/**' || true
echo "== Check cassettes consistently use ACCOUNT_ID placeholder =="
rg -nP "accounts/\d+/email_templates" spec/fixtures/vcr_cassettes || trueLength of output: 1879
Action Required: Add a custom VCR matcher to normalize the ACCOUNT_ID segment
We’ve confirmed that:
- You already redact the account ID at record time via
config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }(spec/spec_helper.rb:24). - However, your cassette matching still uses the raw URI (
:uri) (spec/spec_helper.rb:55), and there is noregister_request_matcherorbefore_playbacknormalizer for the/accounts/:idsegment. - No tests assert on dynamic headers (no matches for
Content-Security-Policy|Cf-Ray|…|Server:outside fixtures). - All existing cassettes have been updated to use
ACCOUNT_ID—we found no lingering numeric IDs underspec/fixtures/vcr_cassettes.
Without a custom matcher, playback will fail whenever ENV['MAILTRAP_ACCOUNT_ID'] differs from the literal placeholder in the YAML. To fix this, introduce a matcher that strips or normalizes the numeric ID:
# spec/spec_helper.rb
VCR.configure do |config|
config.cassette_library_dir = 'spec/fixtures/vcr_cassettes'
config.hook_into :webmock
config.configure_rspec_metadata!
config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }
+ config.register_request_matcher :method_host_path_wo_account do |r1, r2|
+ normalize = ->(uri_str) {
+ uri = URI(uri_str)
+ uri.path = uri.path.gsub(%r{(/accounts/)\d+}, '\1ACCOUNT_ID')
+ [uri.scheme, uri.host, uri.path, uri.query]
+ }
+ [r1.method, *normalize.call(r1.uri)] == [r2.method, *normalize.call(r2.uri)]
+ end
+
- config.default_cassette_options = {
- match_requests_on: [:method, :uri, :body, header_matcher],
+ config.default_cassette_options = {
+ match_requests_on: [:method_host_path_wo_account, :body, header_matcher],
allow_unused_http_interactions: false,
record: :once
}
end• Ensure this matcher is applied globally via default_cassette_options.
• Verify that any future cassettes don’t include raw IDs and that playback succeeds when MAILTRAP_ACCOUNT_ID changes.
📝 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.
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |
| # spec/spec_helper.rb | |
| VCR.configure do |config| | |
| config.cassette_library_dir = 'spec/fixtures/vcr_cassettes' | |
| config.hook_into :webmock | |
| config.configure_rspec_metadata! | |
| config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | |
| # Normalize /accounts/:id to use the ACCOUNT_ID placeholder for matching | |
| config.register_request_matcher :method_host_path_wo_account do |r1, r2| | |
| normalize = ->(uri_str) { | |
| uri = URI(uri_str) | |
| uri.path = uri.path.gsub(%r{(/accounts/)\d+}, '\1ACCOUNT_ID') | |
| [uri.scheme, uri.host, uri.path, uri.query] | |
| } | |
| [r1.method, *normalize.call(r1.uri)] == [r2.method, *normalize.call(r2.uri)] | |
| end | |
| config.default_cassette_options = { | |
| match_requests_on: [:method_host_path_wo_account, :body, header_matcher], | |
| allow_unused_http_interactions: false, | |
| record: :once | |
| } | |
| end |
🤖 Prompt for AI Agents
In
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml
around line 5: playback will fail because VCR is matching the raw :uri
containing a numeric account ID while cassettes use the placeholder ACCOUNT_ID;
add a custom VCR request matcher (e.g., :normalized_uri) in spec/spec_helper.rb
that normalizes the request URI by replacing the /accounts/:id segment with the
ACCOUNT_ID placeholder (or removing the numeric ID) before comparison, register
that matcher with VCR.register_request_matcher, and include it in VCR.configure
default_cassette_options :match_requests_on so all cassettes use the normalized
matcher during playback.
| string: '{"id":45466,"uuid":"84c7a74c-8599-40f8-9d8d-d32ee7c6a482","name":"Original | ||
| Template","category":"Original Category","subject":"Original Subject","body_html":"<div>Original | ||
| HTML</div>","body_text":"Original Text","created_at":"2025-06-23T06:54:52.422Z","updated_at":"2025-06-23T06:54:52.422Z"}' | ||
| recorded_at: Mon, 23 Jun 2025 06:54:52 GMT | ||
| HTML</div>","body_text":"Original Text","created_at":"2025-08-20T19:39:38.791Z","updated_at":"2025-08-20T19:39:38.791Z"}' | ||
| recorded_at: Wed, 20 Aug 2025 19:39:38 GMT |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid asserting on ids/timestamps in tests.
The resource id and ISO8601 timestamps are environment-dependent. Ensure specs map/deserialize fields but don’t hard-assert specific ids or dates.
Run to spot brittle expectations:
Also applies to: 330-334
🏁 Script executed:
#!/bin/bash
# Look for exact numeric id or timestamp assertions in specs
rg -nP --type=ruby -C2 '\.to\s+eq\(\s*\d{5,}\s*\)|"20\d{2}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}' specLength of output: 96
I suspect the raw YAML fixtures still contain hard-coded IDs and timestamps that the previous Ruby-only grep missed. Let’s scan the fixture directory for any literal "id": <number> or ISO8601 timestamps:
🏁 Script executed:
#!/bin/bash
# Search all YAML fixtures for literal numeric IDs and ISO8601 timestamps
rg -nP '"id":\s*\d+' -n spec/fixtures
rg -nP '"\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}' -n spec/fixturesLength of output: 173605
Avoid Hard-Coding IDs and Timestamps in VCR Fixtures
The YAML fixtures under spec/fixtures/vcr_cassettes embed literal resource IDs and ISO8601 timestamps, which makes tests brittle and environment-dependent. Instead of asserting on exact values, filter or ignore these dynamic fields.
Please refactor as follows:
• In your VCR configuration (e.g. spec/support/vcr.rb), add filters for IDs and timestamps, for example:
VCR.configure do |c|
c.filter_sensitive_data('<EMAIL_TEMPLATE_ID>') { |interaction|
interaction.response.body[/\"id\":\s*(\d+)/, 1]
}
c.filter_sensitive_data('<CREATED_AT>') { |interaction|
interaction.response.body[/\"created_at\":\"(.*?)\"/, 1]
}
c.filter_sensitive_data('<UPDATED_AT>') { |interaction|
interaction.response.body[/\"updated_at\":\"(.*?)\"/, 1]
}
end• Re-record the affected cassettes so that IDs and timestamps are replaced with the placeholders above.
– spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml (lines 165–168, 331–333)
– spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/maps_response_data_to_EmailTemplate_objects.yml (all blocks with created_at/updated_at)
– spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml
– And similarly under Mailtrap_ProjectsAPI for any id, created_at or updated_at fields.
• In your model-mapping specs, assert only on the presence and format of these fields (e.g. use be_a(Integer) or match(/\d{4}-\d{2}-\d{2}T/)) rather than exact values.
This will decouple tests from environment-specific data and ensure they remain stable.
| @@ -2,7 +2,7 @@ | |||
| http_interactions: | |||
| - request: | |||
| method: post | |||
| uri: https://mailtrap.io/api/accounts/1111111/email_templates | |||
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Re-check ACCOUNT_ID placeholder playback behavior.
As above, ensure your matcher normalizes the account id path segment so that runtime URIs with numeric ids match the recorded /accounts/ACCOUNT_ID/... URIs.
Also applies to: 171-171
🏁 Script executed:
#!/bin/bash
# Also check if any custom URI normalizer is used for VCR/WebMock.
rg -nC2 -e 'uri_parser|normalize|Addressable|webmock' spec/spec_helper.rb spec/**/*Length of output: 1003
Add VCR request matcher to normalize the ACCOUNT_ID path segment
It looks like we don’t currently normalize the /accounts/… segment, so any recorded cassette using the literal ACCOUNT_ID won’t match a runtime URI with a numeric ID. The rg search in spec/spec_helper.rb confirms there’s no custom URI normalizer or request matcher in the VCR configuration.
Please update your VCR setup to register a custom matcher that rewrites the account‐ID segment to the placeholder before comparing, for example:
# spec/spec_helper.rb
VCR.configure do |config|
config.cassette_library_dir = 'spec/fixtures/vcr_cassettes'
config.hook_into :webmock
+ # Normalize /accounts/<numeric_id>/… to /accounts/ACCOUNT_ID/… for cassette matching
+ config.register_request_matcher :normalized_account_uri do |r1, r2|
+ normalize = ->(uri) {
+ URI(uri).tap { |u|
+ u.path = u.path.gsub(%r{^/accounts/\d+}, '/accounts/ACCOUNT_ID')
+ }.to_s
+ }
+ normalize.call(r1.uri) == normalize.call(r2.uri)
+ }
+ # Ensure VCR uses our custom matcher alongside method and full URI
+ config.default_cassette_options = {
+ match_requests_on: [:method, :normalized_account_uri]
+ }
config.configure_rspec_metadata!
end– Applies to fixtures under
• spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (line 5)
• the other cassette at line 171
With this in place, any runtime request to /accounts/12345/... will match the recorded /accounts/ACCOUNT_ID/....
📝 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.
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates | |
| # spec/spec_helper.rb | |
| VCR.configure do |config| | |
| config.cassette_library_dir = 'spec/fixtures/vcr_cassettes' | |
| config.hook_into :webmock | |
| # Normalize /accounts/<numeric_id>/… to /accounts/ACCOUNT_ID/… for cassette matching | |
| config.register_request_matcher :normalized_account_uri do |r1, r2| | |
| normalize = ->(uri) { | |
| URI(uri).tap { |u| | |
| u.path = u.path.gsub(%r{^/accounts/\d+}, '/accounts/ACCOUNT_ID') | |
| }.to_s | |
| } | |
| normalize.call(r1.uri) == normalize.call(r2.uri) | |
| end | |
| # Ensure VCR uses our custom matcher alongside method and full URI | |
| config.default_cassette_options = { | |
| match_requests_on: [:method, :normalized_account_uri] | |
| } | |
| config.configure_rspec_metadata! | |
| end |
🤖 Prompt for AI Agents
In
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml
around line 5 (and the other cassette at line 171), VCR cassettes contain the
literal ACCOUNT_ID but runtime URIs use numeric account IDs; update
spec/spec_helper.rb to register a custom VCR request matcher that normalizes the
/accounts/<id>/ path segment by replacing any numeric ID with the placeholder
(ACCOUNT_ID) before comparing URIs, and ensure this matcher is added to
VCR.configure (or included in the cassette matchers) so recorded cassettes match
runtime requests.
| config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make ACCOUNT_ID redaction resilient and avoid KeyError on unset env.
ENV.fetch will raise when MAILTRAP_ACCOUNT_ID isn’t set; also, filter_sensitive_data alone doesn’t normalize URIs for playback matching. Use a safe env read here and pair it with a URI normalizer (see next comment).
Apply this diff:
- config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }
+ # Replace occurrences of the concrete account id with a stable placeholder when recording.
+ # Use a safe env read to avoid KeyError if the variable isn't set in playback environments.
+ config.filter_sensitive_data('ACCOUNT_ID') { ENV['MAILTRAP_ACCOUNT_ID'] if ENV.key?('MAILTRAP_ACCOUNT_ID') }📝 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.
| config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | |
| # Replace occurrences of the concrete account id with a stable placeholder when recording. | |
| # Use a safe env read to avoid KeyError if the variable isn't set in playback environments. | |
| config.filter_sensitive_data('ACCOUNT_ID') { ENV['MAILTRAP_ACCOUNT_ID'] if ENV.key?('MAILTRAP_ACCOUNT_ID') } |
🤖 Prompt for AI Agents
In spec/spec_helper.rb around lines 24-25, replace the ENV.fetch call with a
safe read so tests don't raise when MAILTRAP_ACCOUNT_ID is unset (e.g. use
ENV['MAILTRAP_ACCOUNT_ID'] || '' or ENV.fetch('MAILTRAP_ACCOUNT_ID', '') ), and
keep using that value in config.filter_sensitive_data('ACCOUNT_ID') { ... };
additionally, ensure you add a URI normalizer (in the same helper file) that
will canonicalize/redact account IDs in request URIs for playback matching so
the redaction is resilient across different URI formats.
There was a problem hiding this comment.
I don't agree, it's better to raise exception if ENV var not set
There was a problem hiding this comment.
@jurgens You're absolutely right! Using ENV.fetch is better here for fail-fast behavior - it will immediately catch configuration issues when MAILTRAP_ACCOUNT_ID isn't set, which is much clearer than silently failing or having unexpected test behavior. I'll withdraw that suggestion. The explicit exception helps developers quickly identify missing environment setup.
| config.before_record do |interaction| | ||
| body = JSON.parse(interaction.response.body) | ||
|
|
||
| case body | ||
| when Hash | ||
| body["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if body.key?("share_links") | ||
| when Array | ||
| body.map do |item| | ||
| item["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if item.key?("share_links") | ||
| item | ||
| end | ||
| else | ||
| # noop | ||
| end | ||
|
|
||
| interaction.response.body = body.to_json | ||
| rescue JSON::ParserError | ||
| # do nothing | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden share_links scrubber; normalize account id in request URIs; trim volatile headers.
- Guard against nil/non-Hash share_links to avoid NoMethodError.
- Use a precise, non-greedy regex for share tokens.
- Mutate arrays with each instead of map for clarity.
- Normalize request URIs to replace /accounts// with /accounts/ACCOUNT_ID/ so recorded cassettes match across environments.
- Drop volatile response headers (Date, Etag, Cf-Ray, etc.) to reduce cassette churn.
Apply this diff:
- config.before_record do |interaction|
- body = JSON.parse(interaction.response.body)
-
- case body
- when Hash
- body["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if body.key?("share_links")
- when Array
- body map do |item|
- item["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if item.key?("share_links")
- item
- end
- else
- # noop
- end
-
- interaction.response.body = body.to_json
- rescue JSON::ParserError
- # do nothing
- end
+ config.before_record do |interaction|
+ # Normalize request URI account segment for stable matching across environments.
+ interaction.request.uri = interaction.request.uri.gsub(%r{/accounts/\d+/}, '/accounts/ACCOUNT_ID/')
+
+ # Optionally trim volatile response headers to reduce cassette churn.
+ %w[
+ Date Etag X-Request-Id X-Runtime Strict-Transport-Security
+ Cf-Cache-Status Cf-Ray Server Alt-Svc X-Cloud-Trace-Context
+ ].each { |h| interaction.response.headers&.delete(h) }
+
+ # Redact share link tokens in JSON responses.
+ begin
+ body = JSON.parse(interaction.response.body)
+
+ redact = lambda do |obj|
+ if obj.is_a?(Hash)
+ if (sl = obj['share_links']).is_a?(Hash)
+ obj['share_links'] = sl.transform_values do |url|
+ url.to_s.gsub(%r{(https://mailtrap\.io/projects/\d+/share/)[^"\s]+}, '\1REDACTED')
+ end
+ end
+ obj.each_value { |v| redact.call(v) } # recurse into nested objects
+ elsif obj.is_a?(Array)
+ obj.each { |el| redact.call(el) }
+ end
+ end
+
+ redact.call(body)
+ interaction.response.body = JSON.generate(body)
+ rescue JSON::ParserError
+ # Non-JSON response — nothing to redact
+ end
+ endAdd this require near the top to make JSON usage explicit (outside this hunk):
require 'json'🤖 Prompt for AI Agents
In spec/spec_helper.rb around lines 26 to 44, harden the share_links scrubber
and trim volatile headers: require 'json' near the top of the file; when parsing
interaction.response.body, guard that body is a Hash or Array and that
share_links is a Hash (skip if nil or non-Hash) and use each to mutate arrays in
place; use a precise non-greedy regex like %r{/share/[^/]+} to replace tokens
with '/share/REDACTED'; normalize request URIs by replacing /accounts/<digits>/
with /accounts/ACCOUNT_ID/ so cassettes are environment-independent; and remove
volatile response headers (e.g. Date, Etag, Server, Via, CF-Ray, X-Request-Id,
X-Amzn-Trace-Id) from interaction.response.headers before serializing.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
spec/spec_helper.rb (1)
54-58: Normalize URIs to decouple matchers from account IDs.Matching on :uri with a literal ACCOUNT_ID forces everyone’s env to match the cassette placeholder. Register a matcher that normalizes /accounts// to a stable token so both numeric and placeholder URIs match.
Apply:
config.default_cassette_options = { - match_requests_on: [:method, :uri, :body, header_matcher], + match_requests_on: [:method, :body, header_matcher, :uri_normalized], allow_unused_http_interactions: false, record: :once }Add the matcher right above default_cassette_options:
config.register_request_matcher :uri_normalized do |r1, r2| norm = ->(u) { u.to_s.gsub(%r{/accounts/(?:\d+|ACCOUNT_ID)/}, '/accounts/ACCOUNT_ID/') } norm.call(r1.uri) == norm.call(r2.uri) end
♻️ Duplicate comments (8)
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml (1)
26-36: Same header-churn concern; please apply global scrubbing.This cassette carries the same volatile headers (Date/Server/Cf-Ray/etc.). Prefer a single before_record scrub in spec_helper to keep all fixtures stable.
Also applies to: 51-58, 150-161, 168-169
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml (1)
26-36: Apply the same volatile-header scrubbing here.Large diffs stem from Date/Server/Cf-Ray/ETag/X-* headers; remove at record time.
Also applies to: 51-58, 149-161, 168-169, 192-202, 219-224, 315-327, 334-335
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)
26-36: Repeat: remove volatile headers via spec_helper to keep fixtures stable.Also applies to: 51-58, 150-161, 168-169
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml (1)
26-36: Same: scrub volatile headers (Date/Server/Cf-Ray/etc.) to reduce fixture noise.Also applies to: 51-58, 150-161, 168-169, 191-197, 210-215, 321-324
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml (1)
70-147: Normalize request matching for ACCOUNT_ID.Same matcher advice applies here to ensure playback stability.
Also applies to: 235-312
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (1)
70-147: Ensure ACCOUNT_ID-matching normalization is in place.Reiterating the custom matcher requirement for reliable playback.
Also applies to: 235-312
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml (1)
70-147: Playback safety: custom URI matcher still required.Same rationale as previous files.
Also applies to: 236-314
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml (1)
70-147: Confirm custom matcher for ACCOUNT_ID path normalization.Re-asserting the need for normalized_uri matcher for replay to work with placeholders.
Also applies to: 233-311
🧹 Nitpick comments (27)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml (1)
25-161: Optional: strip highly-volatile response headers to reduce cassette churn.Headers like Date, X-Request-Id, X-Cloud-Trace-Context, Cf-Ray, and CSP nonces change on every recording. Consider pruning them in a before_record hook so future re-records create minimal diffs.
You can extend spec/spec_helper.rb (within the existing before_record) to drop them:
NOISY_HEADERS = %w[ Date X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Server Etag Content-Security-Policy Strict-Transport-Security Alt-Svc ].freeze interaction.response.headers.reject! { |k, _| NOISY_HEADERS.include?(k) }spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/maps_response_data_to_Project_object.yml (1)
25-161: Optional: prune non-deterministic headers to keep fixtures lean.Same suggestion as other cassettes: filter Date/CSP/Cf-Ray/etc. during recording to minimize noise.
Also applies to: 188-325
spec/spec_helper.rb (1)
26-44: Optional: drop volatile response headers during recording to reduce fixture diffs.While you’re in before_record, consider removing noisy headers (Date, Cf-Ray, CSP with nonces, etc.) to keep cassettes stable.
You can add inside the same before_record:
NOISY_HEADERS = %w[ Date X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Cache-Status Cf-Ray Server Etag Content-Security-Policy Strict-Transport-Security Alt-Svc ].freeze interaction.response.headers.reject! { |k, _| NOISY_HEADERS.include?(k) }spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml (1)
60-147: Optional: filter CSP/nonces and other dynamic headers.This cassette contains large CSP blocks with runtime nonces and many dynamic headers that will churn on re-record. Consider pruning them (see spec_helper suggestion).
Also applies to: 225-313
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (3)
25-35: Trim volatile response headers in cassettes to reduce churn.Date, Server, Cf-Ray, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, ETag, Alt-Svc, and similar headers are highly variable and bloat diffs. Unless tests assert them, scrub at record time.
Add (or extend) this in spec/spec_helper.rb:
VCR.configure do |c| c.before_record do |i| + %w[ + Date Server Cf-Ray X-Request-Id X-Runtime X-Cloud-Trace-Context + Etag ETag Alt-Svc Strict-Transport-Security + ].each { |h| i.response.headers.delete(h) } end endAlso applies to: 148-160, 165-165
60-147: Consider removing Content-Security-Policy from recorded headers.CSP is extremely long and changes frequently (e.g., nonce, host lists). It creates noisy diffs with no value for client behavior under test.
You can drop it during recording:
VCR.configure do |c| c.before_record do |i| + i.response.headers.delete('Content-Security-Policy') end end
51-57: Optional: also scrub rate-limit headers if not asserted.These fluctuate per run and add noise. Keep only if your specs verify them.
- X-Ratelimit-Limit: - X-Ratelimit-Remaining: + # Scrub if not asserted: + # i.response.headers.delete('X-Ratelimit-Limit') + # i.response.headers.delete('X-Ratelimit-Remaining')spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml (1)
330-334: PATCH response verifies partial update shape; consider assertions that ignore timestamps.Your body shows only category and body_html changed. If tests assert updated_at exactly, they’ll be brittle; prefer asserting it changes (not equals previous) rather than pinning the literal timestamp.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)
55-57: ETag changes on every re-record; scrub unless asserted.+ # In spec_helper: + # i.response.headers.delete('Etag')spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml (2)
27-36: Stabilize volatile headers to reduce cassette churn.Headers like Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-Ray, Etag, and CSP nonces change on every record, causing noisy diffs. Strip them at record time.
Apply in spec/spec_helper.rb (outside this file):
VCR.configure do |c| + c.before_record do |i| + %w[ + Date Server X-Request-Id X-Runtime X-Cloud-Trace-Context + Cf-Ray Cf-Cache-Status Alt-Svc Etag Strict-Transport-Security + Vary Transfer-Encoding + ].each { |h| i.response.headers.delete(h) } + + # Normalize CSP nonce to avoid diffs + if (csp = i.response.headers['Content-Security-Policy']&.first) + i.response.headers['Content-Security-Policy'] = [ + csp.gsub(/nonce-js-[^'"]+/, 'nonce-js-REDACTED') + ] + end + end endAlso applies to: 55-58, 149-161, 192-201, 220-224
223-231: Consider trimming CSP from fixtures entirely.CSP changes frequently and is irrelevant for client lib correctness. Either drop the header or keep a single representative value with nonce redaction.
Also applies to: 233-311
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml (1)
27-36: Strip unstable headers; normalize CSP nonces as in the previous comment.Replicate the spec_helper before_record hook to avoid timestamp/nonce-driven churn.
Also applies to: 55-58, 150-161, 192-201, 220-223
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (1)
27-36: Reduce fixture noise by removing volatile headers and CSP nonces.Same stabilization recommendation as earlier.
Also applies to: 55-58, 150-161, 192-201, 220-223
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml (1)
27-36: Apply header and CSP nonce stripping to cut diff noise.Replicate spec_helper hook from earlier comment.
Also applies to: 55-58, 150-161, 193-201, 221-224
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml (1)
27-36: Stabilize volatile headers and CSP as advised previously.Strip Date/Server/trace ids/CSP nonces to prevent recurring diffs.
Also applies to: 55-58, 150-161, 192-201, 220-224
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (2)
27-36: Reduce cassette churn by filtering volatile headersHeaders like Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, and Cf-Ray change run-to-run and create noisy diffs. Consider stripping them in the VCR before_record hook.
I can propose a spec_helper update if you want it in this PR.
Also applies to: 148-158
59-146: Normalize/strip CSP header to avoid nonce/source list flappingThe CSP header is long, includes nonces, and changes frequently. Recording it adds no value to client-side API tests and makes fixtures brittle. Recommend replacing it with a placeholder (e.g., <CSP_REDACTED>) during recording.
If helpful, I’ll share a small before_record snippet to collapse this header across all cassettes.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml (2)
34-36: Filter high-churn headers (Server/Cf-Ray/X-Request-Id/etc.)These headers are inherently volatile and add noise to diffs. Suggest dropping them in a VCR before_record filter.
Also applies to: 150-161, 316-327
61-149: Collapse CSP headers to a placeholderSame rationale as other cassettes—nonces and allowlists change often and aren’t relevant for API client tests.
Also applies to: 227-314
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml (2)
34-36: Trim volatile infra headersDropping Server, Cf-Ray, X-Request-Id, etc., will significantly reduce fixture churn without affecting test intent.
Also applies to: 150-161, 315-326
61-149: CSP header normalization recommendedConsider replacing CSP with a fixed placeholder during recording to eliminate nonce/source drift.
Also applies to: 226-314
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (2)
27-36: Consider filtering volatile headersSame recommendation as other files—filter Date, Server, Cf-Ray, X-Request-Id, X-Runtime, X-Cloud-Trace-Context to avoid noisy diffs.
Also applies to: 150-161, 316-327
61-149: Normalize CSP headersCSP nonces and allowlists create recurring cassette diffs; replacing with a placeholder improves stability.
Also applies to: 227-315
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml (2)
27-36: Optional: filter infra/trace headersTo keep fixtures lean and reduce re-recording noise, consider stripping Server, Date, Cf-Ray, and similar volatile headers at record time.
Also applies to: 192-201
61-149: Optional: collapse CSP header to a placeholderPrevents nonces and allowlist changes from forcing fixture updates.
Also applies to: 226-314
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml (2)
27-36: Trim volatile headers in cassettes to reduce churn and review noiseHeaders like Date, Server, X-Request-Id, X-Runtime, X-Cloud-Trace-Context, Cf-Ray, ETag, and X-Ratelimit-* change on every recording and add unnecessary diff noise. They’re not used for request matching, so consider dropping them at record time.
Add a VCR before_record filter in spec_helper.rb:
VCR.configure do |c| noisy = %w[ Date Server X-Request-Id X-Runtime X-Cloud-Trace-Context Cf-Ray Cf-Cache-Status Alt-Svc Etag X-Ratelimit-Limit X-Ratelimit-Remaining Strict-Transport-Security ] c.before_record do |i| noisy.each { |h| i.response.headers.delete(h) } end # Ensure matching ignores headers: c.default_cassette_options = { match_requests_on: %i[method uri body] } endAlso applies to: 55-58, 150-163, 192-201, 220-223, 315-327
60-148: Consider excluding CSP from recorded headersThe Content-Security-Policy block is extremely large and highly volatile (nonces, host lists), causing frequent cassette updates without test value. Unless you assert on CSP explicitly, remove it during record to keep fixtures stable.
Extend the VCR filter:
VCR.configure do |c| c.before_record do |i| i.response.headers.delete('Content-Security-Policy') end endAlso applies to: 226-314
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (42)
lib/mailtrap/project.rb(1 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml(4 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/returns_an_array_of_EmailTemplate_objects.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml(3 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/returns_an_EmailTemplate_object.yml(0 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml(7 hunks)spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/returns_an_EmailTemplate_object.yml(0 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/projects_api_spec.rb(1 hunks)spec/spec_helper.rb(1 hunks)
💤 Files with no reviewable changes (9)
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/returns_an_array_of_EmailTemplate_objects.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/returns_an_EmailTemplate_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/returns_an_EmailTemplate_object.yml
✅ Files skipped from review due to trivial changes (4)
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/returns_deleted_project_id.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/when_project_does_not_exist/raises_not_found_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/with_hash_request/maps_response_data_to_Project_object.yml
🚧 Files skipped from review as they are similar to previous changes (8)
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_get/maps_response_data_to_Project_object.yml
- spec/mailtrap/projects_api_spec.rb
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/maps_response_data_to_Project_object.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/maps_response_data_to_Project_objects.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_project_does_not_exist/raises_not_found_error.yml
- lib/mailtrap/project.rb
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml
- spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_delete/when_project_does_not_exist/raises_not_found_error.yml
🧰 Additional context used
🧬 Code Graph Analysis (1)
spec/spec_helper.rb (1)
lib/mailtrap/mail/base.rb (1)
to_json(64-69)
🔇 Additional comments (36)
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_create/with_hash_request/maps_response_data_to_Project_object.yml (1)
16-19: Good redaction of sensitive values.Authorization is masked and share_links tokens are properly redacted to /share/REDACTED. Looks safe to commit.
Also applies to: 164-165
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/when_updating_only_name/updates_only_the_name_field.yml (2)
163-166: Share link tokens correctly sanitized in both interactions.Both the POST and PATCH responses redact tokens under share_links consistently. Nice.
Also applies to: 326-328
4-6: Keep URIs stable across environments.URIs use /accounts/ACCOUNT_ID/, which relies on the test env using that literal value. If a contributor sets MAILTRAP_ACCOUNT_ID to a numeric ID and re-records, matching may fail unless the matcher normalizes the account segment. See my spec_helper.rb comment for a robust matcher to avoid env coupling.
Also applies to: 167-169
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/_update/maps_response_data_to_Project_object.yml (1)
163-166: PII/sensitive data handling looks correct.Bearer token placeholder is present and share_links are redacted. Permissions/inboxes fields provide good fixture coverage.
Also applies to: 326-329
spec/spec_helper.rb (1)
24-44: All cassettes are properly sanitized—no residual share tokens or raw account IDs detected.
Running the provided regex checks againstspec/fixtures/vcr_cassettesyielded no matches, confirming that sensitive data has been redacted as intended. ✅spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_template_does_not_exist/raises_not_found_error.yml (1)
5-6: Account ID placeholder and token masking look correct.URIs are parameterized with ACCOUNT_ID and the Authorization header is masked. Fixture is safe to ship.
Also applies to: 27-36, 150-162, 193-201
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_list/when_api_key_is_incorrect/raises_authorization_error.yml (1)
5-5: Good: endpoint generalized with ACCOUNT_ID placeholder.This de-couples fixtures from real account numbers and aligns with the new redaction scheme.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/maps_response_data_to_EmailTemplate_object.yml (2)
5-5: Good: POST URI uses ACCOUNT_ID placeholder.Keeps fixtures portable across environments.
165-168: No unintended hardcoded IDs/UUIDs detected—pinning is intentional for VCR fixturesI’ve searched the
spec/fixtures/vcr_cassettesdirectory and confirmed that all numeric IDs and UUIDs (including the newid: 45464anduuid: "776e093c-e707-425d-a09d-5f311c9201e5") are consistently pinned across the EmailTemplates and Projects API cassettes. These values are used to assert precise mapping behavior in your specs, so recording them exactly is intentional and necessary for the tests to pass.No further changes are required here.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/updates_only_the_specified_fields.yml (1)
5-5: Good: Both create and patch endpoints parameterized with ACCOUNT_ID.Also applies to: 171-171
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: Good: POST URI generalized with ACCOUNT_ID.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/returns_no_content.yml (3)
5-5: Good: create and delete URIs now use ACCOUNT_ID.Also applies to: 171-171
5-5: No numeric account IDs found in VCR fixturesRan a repository‐wide ripgrep search for
accounts/\d+underspec/fixtures/vcr_cassettesand confirmed there are no leftover numeric IDs—only theACCOUNT_IDplaceholder remains. All fixtures are up to date.
321-323: 204 No Content Handling VerifiedThe
handle_responsemethod inlib/mailtrap/client.rbexplicitly matchesNet::HTTPNoContentand returnsnilbefore any JSON parsing takes place. DELETE requests yielding a 204 status are therefore treated as a successful no-content response, and won’t triggerjson_responseon an empty body. No changes needed.spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/when_template_does_not_exist/raises_not_found_error.yml (1)
5-5: Good move: parameterized ACCOUNT_ID in request URI.Replacing hard-coded account IDs with ACCOUNT_ID improves secrecy and cassette reusability.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/preserves_other_fields_unchanged.yml (2)
5-5: ACCOUNT_ID parameterization is consistent across create and patch.URI redaction is applied to both POST and PATCH endpoints. Looks good.
Also applies to: 171-171
165-168: Data consistency check: PATCH targets created resource and preserves other fields.The PATCH operates on id 45471 (same as created), updates only body_html, and preserves name/subject/category/body_text, matching the test case intent.
Optionally assert this at spec level to prevent regressions:
expect(updated.body_text).to eq(original.body_text) expect(updated.subject).to eq(original.subject) expect(updated.category).to eq(original.category)Also applies to: 329-333
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/preserves_other_fields_unchanged.yml (2)
5-5: ACCOUNT_ID placeholder used consistently.Good redaction and consistency across create and patch URIs.
Also applies to: 171-171
165-168: Semantics preserved: only category changes.PATCH response reflects category update while keeping other fields unchanged. Matches scenario name.
Also applies to: 329-333
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_multiple_specific_fields/preserves_other_fields_unchanged.yml (2)
5-5: ACCOUNT_ID path redaction: consistent and correct.Creation and update URIs use the placeholder; good.
Also applies to: 171-171
165-168: Validation: targeted fields updated, others preserved.PATCH updates category and body_html; name/subject/body_text remain, consistent with test intent.
Consider asserting that exactly the intended fields changed:
expect(updated.attributes.slice('category','body_html')) .to eq('category' => 'Updated Category', 'body_html' => '<div>Updated HTML</div>')Also applies to: 330-334
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_delete/when_template_does_not_exist/raises_not_found_error.yml (2)
5-5: ACCOUNT_ID placeholder is correctly used in POST and DELETE URIs.Redaction is consistent for both setup and the not-found DELETE.
Also applies to: 171-171
165-168: DELETE not-found scenario captured as JSON error; OK.404 with {"error":"Not Found"} body is recorded; aligns with client error handling expectations.
Also applies to: 327-329
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_create/when_API_returns_an_error/raises_a_Mailtrap_Error.yml (3)
5-5: Good: account id parameterized in URIUsing ACCOUNT_ID in the request path avoids leaking real IDs and stabilizes cassettes across environments.
164-165: Sanity check: recorded_at aligns with response DateRecorded timestamp matches the response Date header (Wed, 20 Aug 2025 19:39:38 GMT). Looks consistent.
5-5: Redact dynamic share_links URLs in VCR cassettesThe verification confirms that there are no hard-coded Mailtrap account IDs and all Bearer tokens use placeholders, but multiple fixtures still include unredacted
share_linksentries. These dynamic URLs must be masked or replaced with placeholders to avoid leaking internal IDs.• Affected fixtures under
spec/fixtures/vcr_cassettes/Mailtrap_ProjectsAPI/**
– e.g._update/maps_response_data_to_Project_object.yml(lines 164, 327)
–_get/maps_response_data_to_Project_object.yml(lines 164, 327)
–_delete/returns_deleted_project_id.yml(line 164)
– plus many others in theMailtrap_ProjectsAPI/_list/directorySuggested replacement (apply across all occurrences):
- "share_links":{"admin":"https://mailtrap.io/projects/2487014/share/REDACTED","viewer":"https://mailtrap.io/projects/2487014/share/REDACTED"} + "share_links":{"admin":"<SHARE_LINK>","viewer":"<SHARE_LINK>"}Please update or remove all
share_linksvalues to use<SHARE_LINK>(or another agreed placeholder) throughout the VCR cassettes.Likely an incorrect or invalid review comment.
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/maps_response_data_to_EmailTemplate_object.yml (2)
5-5: Good: consistent ACCOUNT_ID usage and resource id continuityURI uses ACCOUNT_ID; PATCH path references the id returned by the POST (45466). That consistency keeps the scenario realistic and stable.
Also applies to: 171-171
165-168: Temporal consistency looks correctcreated_at/updated_at values advance logically between create and update, matching the recorded_at values. No issues spotted.
Also applies to: 331-334
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_category/updates_only_the_category_field.yml (2)
171-175: Nice: PATCH updates only the intended fieldRequest body includes only category, and the response reflects only category changing while name/subject remain intact. This is a solid, focused fixture.
Also applies to: 329-333
5-5: URI parameterization and id continuity look goodBoth POST and PATCH use ACCOUNT_ID; PATCH path matches the created id (45468).
Also applies to: 171-171
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/with_hash_request/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: Good: ACCOUNT_ID paths and concise PATCH bodyURI parameterization is consistent. PATCH updates only selected fields (name/subject/category), leaving HTML/text intact, which mirrors real partial updates well.
Also applies to: 171-175
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_get/maps_response_data_to_EmailTemplate_object.yml (1)
5-5: ACCOUNT_ID parameterization and id reuse are correctThe GET targets the id returned by the preceding POST (45462). This makes the create→get flow deterministic.
Also applies to: 171-172
spec/fixtures/vcr_cassettes/Mailtrap_EmailTemplatesAPI/_update/when_updating_only_body_html/updates_only_the_body_html_field.yml (4)
5-5: Good: ACCOUNT_ID is parameterized in URIsUsing ACCOUNT_ID placeholders in both POST and PATCH endpoints removes hard-coded tenant data and aligns with the new redaction policy.
Also applies to: 171-171
165-168: Behavioral check: only body_html changes; other fields remain intactThe POST response shows the original body_html, and the PATCH response updates only body_html while preserving name, category, subject, and body_text. This matches the scenario name and looks correct.
If not already covered, ensure the spec asserts that:
- body_html changes to "
New HTML Only"- name, category, subject, and body_text remain unchanged
- updated_at changes while created_at stays the same
Also applies to: 329-333
164-165: AI summary inconsistency: no ASCII-8BIT → UTF-8 shift present hereBoth recorded bodies declare encoding: UTF-8. There’s no ASCII-8BIT to UTF-8 change in this cassette. The high-level summary appears slightly off for this file.
Also applies to: 329-330
171-171: No action required: IDs and placeholders are consistentAll fixtures in the
when_updating_only_body_htmlscenario use the correct template IDs and placeholders:
updates_only_the_body_html_field.ymlconsistently references45470in the URI and both request/response bodies.preserves_other_fields_unchanged.ymlconsistently references45471in its URI and bodies.- No hard-coded numeric account IDs remain (all use
ACCOUNT_ID), and all Authorization headers are masked as<BEARER_TOKEN>.
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates/999999 | ||
| body: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm VCR request matching works with ACCOUNT_ID placeholder.
Recording replaces the numeric account in the cassette with ACCOUNT_ID. Ensure playback uses a custom matcher that normalizes /accounts/ to /accounts/ACCOUNT_ID, otherwise matches will fail.
Add a custom matcher in spec/spec_helper.rb:
VCR.configure do |c|
+ c.register_request_matcher :normalized_uri do |r1, r2|
+ norm = ->(u) { u.gsub(%r{/accounts/\d+}, '/accounts/ACCOUNT_ID') }
+ norm.call(r1.uri) == norm.call(r2.uri)
+ end
+ c.default_cassette_options = { match_requests_on: [:method, :normalized_uri, :body] }
endRun to verify the matcher exists (or add it if missing):
🏁 Script executed:
#!/usr/bin/env bash
rg -nC2 "register_request_matcher\s*:normalized_uri|/accounts/\d+|ACCOUNT_ID" spec/spec_helper.rbLength of output: 244
Add a VCR custom matcher to normalize the ACCOUNT_ID placeholder
Your spec/spec_helper.rb currently filters out the real account ID (via config.filter_sensitive_data('ACCOUNT_ID')), but there is no request matcher to normalize URIs during playback. Without it, recordings using ACCOUNT_ID will not match real requests against the cassette’s URI pattern.
Please add the following inside your VCR configuration block in spec/spec_helper.rb:
VCR.configure do |c|
# existing configuration...
+ # Normalize /accounts/<digits> to /accounts/ACCOUNT_ID for playback matching
+ c.register_request_matcher :normalized_uri do |r1, r2|
+ normalize = ->(uri) { uri.gsub(%r{/accounts/\d+}, '/accounts/ACCOUNT_ID') }
+ normalize.call(r1.uri) == normalize.call(r2.uri)
+ end
+ c.default_cassette_options = {
+ match_requests_on: [:method, :normalized_uri, :body]
+ }
end• Location: spec/spec_helper.rb within the VCR.configure do |c| … end block
• Purpose: Ensure recorded URIs with a numeric account ID (e.g. /accounts/12345) match playback URIs using the placeholder /accounts/ACCOUNT_ID.
📝 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.
| uri: https://mailtrap.io/api/accounts/ACCOUNT_ID/email_templates/999999 | |
| body: | |
| VCR.configure do |c| | |
| # existing configuration... | |
| # Normalize /accounts/<digits> to /accounts/ACCOUNT_ID for playback matching | |
| c.register_request_matcher :normalized_uri do |r1, r2| | |
| normalize = ->(uri) { uri.gsub(%r{/accounts/\d+}, '/accounts/ACCOUNT_ID') } | |
| normalize.call(r1.uri) == normalize.call(r2.uri) | |
| end | |
| c.default_cassette_options = { | |
| match_requests_on: [:method, :normalized_uri, :body] | |
| } | |
| end |
🤖 Prompt for AI Agents
In spec/spec_helper.rb around the VCR.configure do |c| … end block (add near
existing filter_sensitive_data), add a custom request matcher that normalizes
account IDs in URIs by replacing any /accounts/<digits> segment with
/accounts/ACCOUNT_ID before comparing; register it with
c.register_request_matcher (e.g. :uri_without_account_id) and then include that
matcher in VCR’s match_requests_on (or in
default_cassette_options[:match_requests_on]) so recorded URIs with numeric
account IDs will match playback URIs using the ACCOUNT_ID placeholder.
| config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden ACCOUNT_ID redaction and avoid KeyError when env is missing.
ENV.fetch will raise if MAILTRAP_ACCOUNT_ID is not set, breaking playback for contributors who only use existing cassettes. Also, tying cassettes to the literal "ACCOUNT_ID" string makes local runs fragile.
Apply:
- config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') }
+ # Replace the actual account id wherever it appears; fall back to the placeholder
+ config.filter_sensitive_data('ACCOUNT_ID') { ENV['MAILTRAP_ACCOUNT_ID'] || 'ACCOUNT_ID' }📝 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.
| config.filter_sensitive_data('ACCOUNT_ID') { ENV.fetch('MAILTRAP_ACCOUNT_ID') } | |
| # Replace the actual account id wherever it appears; fall back to the placeholder | |
| config.filter_sensitive_data('ACCOUNT_ID') { ENV['MAILTRAP_ACCOUNT_ID'] || 'ACCOUNT_ID' } |
🤖 Prompt for AI Agents
In spec/spec_helper.rb around lines 24-25, replace the ENV.fetch usage with a
safe lookup that falls back to a stable placeholder so it never raises (e.g.
account_id = ENV['MAILTRAP_ACCOUNT_ID'] || 'ACCOUNT_ID_PLACEHOLDER') and pass
that variable into config.filter_sensitive_data so VCR will redact the real
value when present and still match/playback using the placeholder when the env
var is missing; this avoids KeyError and decouples cassettes from a missing
literal env value.
| config.before_record do |interaction| | ||
| body = JSON.parse(interaction.response.body) | ||
|
|
||
| case body | ||
| when Hash | ||
| body["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if body.key?("share_links") | ||
| when Array | ||
| body.map do |item| | ||
| item["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if item.key?("share_links") | ||
| item | ||
| end | ||
| else | ||
| # noop | ||
| end | ||
|
|
||
| interaction.response.body = body.to_json | ||
| rescue JSON::ParserError | ||
| # do nothing | ||
| end |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make share_links scrubbing robust: handle nil/empty bodies, non-JSON, nested objects; avoid extra array allocations.
Current code can raise on nil bodies (TypeError) and uses map without the result. It also only scrubs top-level/array-level share_links. Make it defensive and recursive.
Apply:
- config.before_record do |interaction|
- body = JSON.parse(interaction.response.body)
-
- case body
- when Hash
- body["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if body.key?("share_links")
- when Array
- body.map do |item|
- item["share_links"].transform_values! { |e| e.gsub(/\/share\/.+/, '/share/REDACTED') } if item.key?("share_links")
- item
- end
- else
- # noop
- end
-
- interaction.response.body = body.to_json
- rescue JSON::ParserError
- # do nothing
- end
+ config.before_record do |interaction|
+ raw = interaction.response&.body
+ # Skip if there's no body or it's not JSON
+ next if raw.nil? || raw.empty?
+ ct = Array(interaction.response.headers['Content-Type']).join(',') rescue ''
+ next unless ct.include?('application/json')
+
+ begin
+ payload = JSON.parse(raw)
+ rescue JSON::ParserError, TypeError
+ next
+ end
+
+ scrub = lambda do |obj|
+ case obj
+ when Hash
+ if obj['share_links'].is_a?(Hash)
+ obj['share_links'].transform_values! do |v|
+ v.is_a?(String) ? v.gsub(%r{/share/[^"\/\s]+}, '/share/REDACTED') : v
+ end
+ end
+ obj.each_value { |v| scrub.call(v) }
+ when Array
+ obj.each { |v| scrub.call(v) }
+ end
+ end
+
+ scrub.call(payload)
+ interaction.response.body = JSON.generate(payload)
+ end🤖 Prompt for AI Agents
In spec/spec_helper.rb around lines 26 to 44, the before_record hook currently
assumes interaction.response.body is a JSON string and a non-nil Hash/Array,
uses map without using the result, and only scrubs top-level share_links; update
it to first return early for nil/blank bodies, safely convert the body to a
string before parsing, rescue JSON::ParserError and TypeError, parse into Ruby
objects, then recursively walk the parsed object in-place (handling Hashes,
Arrays and nested structures) and for any Hash key == "share_links" that has a
Hash value call transform_values! (or otherwise mutate it) to replace
"/share/..." with "/share/REDACTED"; use iterative/recursive traversal (not map
without using result) to avoid extra array allocations and ensure all nested
occurrences are scrubbed, and finally set interaction.response.body to the
reserialized JSON.
Motivation
add API support for projects
Changes
Summary by CodeRabbit
New Features
Tests
Chores