framework
framework copied to clipboard
Duplicate replyTo when running render() prior to send
- Laravel Version: 9.40.1
- PHP Version: 8.1.10
- Database Driver & Version: not relevant
Description:
If you run render() prior to sending the email, it will double replyTo.
A bit related to #30857
Steps To Reproduce:
The below example will add [email protected] 2 times to the reply_to field.
use App\Mail\Question;
$mailTemplate = new Question( 'Hello, I have a question' );
$mail = $mailTemplate->render();
// ...store $mail in the database
Mail::to( config( 'mail.from.address' ) )
->locale( 'nl' )
->send( $mailTemplate );
<?php
namespace App\Mail\Patient;
use App\Models\Patient;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Mail\Mailable;
use Illuminate\Queue\SerializesModels;
class Question extends Mailable implements shouldQueue
{
use Queueable, SerializesModels;
/**
* Create a new message instance.
*
* @param Patient $patient
* @param string $question
*/
public function __construct( public string $question )
{
$this->queue = config( 'config.queue-email' );
}
/**
* Build the message.
*
* @return $this
*/
public function build()
{
return $this->from( [
'address' => config( 'mail.from.address' ),
'name' => config( 'mail.from.name' ),
] )
->replyTo( '[email protected]' )
->subject( 'someone has a question' )
->view( 'mail.question' );
}
}
Thank you for reporting this issue!
As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub. If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.
Thank you!
Hey everyone, this is my first attempt at contributing to the Laravel Framework so I wanted to check things with you before I submit anything.
I worked on a change that's updating the setAddress method like this:
protected function setAddress($address, $name = null, $property = 'to')
{
if (empty($address)) {
return $this;
}
foreach ($this->addressesToArray($address, $name) as $recipient) {
$recipient = $this->normalizeRecipient($recipient);
$this->{$property}[] = [
'name' => $recipient->name ?? null,
'address' => $recipient->email,
];
}
$this->{$property} = collect($this->{$property})
->unique('address')
->values()
->all();
return $this;
}
This would prevent any recipient from being duplicated, but I'm not sure if this is the expected behaviour because after this change some tests are failing and this is the asserts failing:
$mailable = new WelcomeMailableStub;
$mailable->replyTo(collect([new MailableTestUserStub, new MailableTestUserStub]));
$this->assertEquals([
['name' => 'Taylor Otwell', 'address' => '[email protected]'],
['name' => 'Taylor Otwell', 'address' => '[email protected]'],
], $mailable->replyTo);
$this->assertTrue($mailable->hasReplyTo(new MailableTestUserStub));
$this->assertTrue($mailable->hasReplyTo('[email protected]'));
$mailable->assertHasReplyTo('[email protected]');
Since with this change, the expected result would be a single item in the array.
Hello @JBtje @driesvints I pushed the change I mentioned above to my fork since IDK if this is the expected behaviour because it's making some test cases fail. I'm not going to create the PR but can you help me check if these changes are ok? Because if so I can work on updating the test cases and create the PR after that.