Conversation
📝 WalkthroughWalkthroughAdds a new Resend mailer adapter: Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Adapter as ResendAdapter
participant Client as HttpClient
participant API as Resend API
App->>Adapter: setFrom(), setAddress(), setSubject(), setBody()
App->>Adapter: send()
activate Adapter
Adapter->>Adapter: prepare() -- build payload (from, to[], subject?, html)
Adapter->>Client: post(url: "https://api.resend.com/emails", jsonPayload, headers: Authorization: Bearer ...)
activate Client
Client->>API: POST /emails
API-->>Client: 200 OK (or error)
Client-->>Adapter: response / exception
deactivate Client
Adapter-->>App: true / false
deactivate Adapter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/Mailer/Factories/MailerFactory.php (1)
27-27: Minor: Import placement inconsistency.The
ResendAdapterimport is placed afterSmtpAdapterand beforeDiException, breaking the alphabetical grouping of adapter imports. Consider moving it to maintain consistency with other adapter imports.Suggested reordering
use Quantum\Mailer\Adapters\SendinblueAdapter; use Quantum\Mailer\Exceptions\MailerException; use Quantum\Config\Exceptions\ConfigException; use Quantum\Mailer\Adapters\MandrillAdapter; +use Quantum\Mailer\Adapters\ResendAdapter; use Quantum\Mailer\Adapters\SendgridAdapter; use Quantum\Mailer\Adapters\MailgunAdapter; use Quantum\App\Exceptions\BaseException; use Quantum\Mailer\Adapters\SmtpAdapter; -use Quantum\Mailer\Adapters\ResendAdapter; use Quantum\Di\Exceptions\DiException;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Mailer/Factories/MailerFactory.php` at line 27, The import for ResendAdapter is out of alphabetical order among adapter imports; in MailerFactory (class MailerFactory) move the line "use Quantum\Mailer\Adapters\ResendAdapter;" so it sits with the other adapter imports in correct alphabetical order (e.g., alongside SmtpAdapter, SendmailAdapter, etc.) to restore consistent import grouping and keep unrelated imports like DiException separate.tests/Unit/Mailer/Adapters/ResendAdapterTest.php (1)
25-36: Test coverage note:sendEmail()is not exercised.Since
mail_trapistruein the test configuration,send()routes tosaveEmail()rather thansendEmail(). This test validates the trait integration but does not cover the actual HTTP call logic inResendAdapter::sendEmail().This is consistent with other adapter tests in the codebase, but consider adding integration tests with mocked HTTP responses to ensure the Resend API payload and error handling are correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Mailer/Adapters/ResendAdapterTest.php` around lines 25 - 36, The test only exercises the trait path because mail_trap=true so ResendAdapter::send() calls saveEmail() and never exercises ResendAdapter::sendEmail(); add a new unit/integration test that configures the adapter to avoid mail trapping (or mocks the mail_trap check) and mocks the HTTP client used by ResendAdapter to assert the POST payload and response handling of sendEmail(), verifying successful responses and error paths (use the adapter's sendEmail(), mock the HTTP client used inside ResendAdapter, and assert proper request body, headers and exception handling).src/Mailer/Adapters/ResendAdapter.php (2)
94-94: Potential issue:json_encodecan returnfalse.
json_encode($this->data)can returnfalseon encoding failure (e.g., malformed UTF-8). PassingfalsetosetData()may cause unexpected behavior.Suggested fix with error handling
- ->setData(json_encode($this->data)) + ->setData(json_encode($this->data, JSON_THROW_ON_ERROR))This will throw a
JsonExceptionon encoding failure, which will be caught by the existingcatch (Exception $e)block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Mailer/Adapters/ResendAdapter.php` at line 94, Replace the raw json_encode call used in the ResendAdapter when setting payload data (the ->setData(json_encode($this->data)) call) with a call that fails loudly on encoding errors: either use json_encode($this->data, JSON_THROW_ON_ERROR) so a JsonException is thrown into the existing catch (Exception $e) block, or explicitly check for a false return and throw a JsonException before calling setData; ensure you reference the same setData() invocation and $this->data so encoding failures aren’t passed silently.
35-37: Use native type declaration for$apiKey.The property uses a docblock
@var string|nullbut lacks a native PHP type declaration. For consistency with PHP 7.4+ style used elsewhere in the codebase, use the native type.Suggested fix
- /** - * `@var` string|null - */ - private $apiKey; + private ?string $apiKey;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Mailer/Adapters/ResendAdapter.php` around lines 35 - 37, The $apiKey property in the ResendAdapter lacks a native type declaration; change its declaration to use the nullable string type (e.g., declare the property as ?string $apiKey) so it matches the `@var` string|null docblock and project's PHP 7.4+ style; update any constructor parameter or assignments in ResendAdapter that set $apiKey to ensure types remain compatible with the new ?string property.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Mailer/Adapters/ResendAdapter.php`:
- Around line 61-65: Remove the trailing whitespace (or tabs) on the line that
assigns $fromName and ensure indentation uses spaces consistently; specifically
update the block that sets $fromName and $this->data['from'] (the lines
referencing $fromName and $this->data['from'] in ResendAdapter) to eliminate any
trailing spaces/tabs so PHP CS Fixer no longer flags the file.
- Around line 84-101: sendEmail() currently returns true immediately after
$this->httpClient->start() without checking the HTTP response; change it to call
$this->httpClient->info(CURLINFO_HTTP_CODE) after start(), treat only 2xx codes
as success, and return false (and/or log the error) for 4xx/5xx statuses; update
the same pattern in other adapters (SendgridAdapter, MailgunAdapter,
MandrillAdapter) so each method uses the HTTP code from info(CURLINFO_HTTP_CODE)
to decide success/failure instead of assuming start() succeeded.
In `@tests/Unit/Mailer/Adapters/ResendAdapterTest.php`:
- Around line 37-38: Remove the trailing blank line at the end of the file to
satisfy PHP CS Fixer: open the tests/Unit/Mailer/Adapters/ResendAdapterTest.php
file (containing the ResendAdapterTest class) and delete the extra empty newline
after the closing brace so the file ends immediately after the final "}"
character with a single newline or no extra blank line.
---
Nitpick comments:
In `@src/Mailer/Adapters/ResendAdapter.php`:
- Line 94: Replace the raw json_encode call used in the ResendAdapter when
setting payload data (the ->setData(json_encode($this->data)) call) with a call
that fails loudly on encoding errors: either use json_encode($this->data,
JSON_THROW_ON_ERROR) so a JsonException is thrown into the existing catch
(Exception $e) block, or explicitly check for a false return and throw a
JsonException before calling setData; ensure you reference the same setData()
invocation and $this->data so encoding failures aren’t passed silently.
- Around line 35-37: The $apiKey property in the ResendAdapter lacks a native
type declaration; change its declaration to use the nullable string type (e.g.,
declare the property as ?string $apiKey) so it matches the `@var` string|null
docblock and project's PHP 7.4+ style; update any constructor parameter or
assignments in ResendAdapter that set $apiKey to ensure types remain compatible
with the new ?string property.
In `@src/Mailer/Factories/MailerFactory.php`:
- Line 27: The import for ResendAdapter is out of alphabetical order among
adapter imports; in MailerFactory (class MailerFactory) move the line "use
Quantum\Mailer\Adapters\ResendAdapter;" so it sits with the other adapter
imports in correct alphabetical order (e.g., alongside SmtpAdapter,
SendmailAdapter, etc.) to restore consistent import grouping and keep unrelated
imports like DiException separate.
In `@tests/Unit/Mailer/Adapters/ResendAdapterTest.php`:
- Around line 25-36: The test only exercises the trait path because
mail_trap=true so ResendAdapter::send() calls saveEmail() and never exercises
ResendAdapter::sendEmail(); add a new unit/integration test that configures the
adapter to avoid mail trapping (or mocks the mail_trap check) and mocks the HTTP
client used by ResendAdapter to assert the POST payload and response handling of
sendEmail(), verifying successful responses and error paths (use the adapter's
sendEmail(), mock the HTTP client used inside ResendAdapter, and assert proper
request body, headers and exception handling).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 580c86e6-bf1d-4f64-abd8-d80b38191815
📒 Files selected for processing (7)
src/Mailer/Adapters/ResendAdapter.phpsrc/Mailer/Enums/MailerType.phpsrc/Mailer/Factories/MailerFactory.phptests/Unit/Mailer/Adapters/ResendAdapterTest.phptests/Unit/Mailer/Factories/MailerFactoryTest.phptests/Unit/Mailer/Helpers/MailerHelperFunctionsTest.phptests/_root/shared/config/mailer.php
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #440 +/- ##
============================================
+ Coverage 82.64% 82.65% +0.01%
- Complexity 2721 2730 +9
============================================
Files 241 242 +1
Lines 7305 7334 +29
============================================
+ Hits 6037 6062 +25
- Misses 1268 1272 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Mailer/Adapters/ResendAdapter.php (1)
50-54: Consider adding defensive validation forapi_keyparameter.If
api_keyis missing from$params, this will produce an undefined array key warning. Consider using null coalescing with a fallback or explicit validation.♻️ Suggested defensive approach
public function __construct(array $params) { $this->httpClient = new HttpClient(); - $this->apiKey = $params['api_key']; + $this->apiKey = $params['api_key'] ?? null; }Alternatively, throw an explicit exception if the key is required:
if (!isset($params['api_key'])) { throw new \InvalidArgumentException('Resend adapter requires api_key parameter'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Mailer/Adapters/ResendAdapter.php` around lines 50 - 54, The constructor (__construct) currently assigns $this->apiKey = $params['api_key'] which can raise an undefined index warning; add defensive validation in the ResendAdapter constructor to either (a) check isset($params['api_key']) and throw an \InvalidArgumentException with a clear message if missing, or (b) use the null-coalescing fallback ($params['api_key'] ?? null) and handle a null apiKey later; ensure the chosen approach sets $this->apiKey deterministically and documents the requirement in the constructor.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Mailer/Adapters/ResendAdapter.php`:
- Around line 50-54: The constructor (__construct) currently assigns
$this->apiKey = $params['api_key'] which can raise an undefined index warning;
add defensive validation in the ResendAdapter constructor to either (a) check
isset($params['api_key']) and throw an \InvalidArgumentException with a clear
message if missing, or (b) use the null-coalescing fallback ($params['api_key']
?? null) and handle a null apiKey later; ensure the chosen approach sets
$this->apiKey deterministically and documents the requirement in the
constructor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9e7d976-1d23-4f67-90f8-e41013752268
📒 Files selected for processing (2)
src/Mailer/Adapters/ResendAdapter.phptests/Unit/Mailer/Adapters/ResendAdapterTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/Mailer/Adapters/ResendAdapterTest.php
Closes #439
Summary by CodeRabbit
New Features
Chores
Tests