Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Update Mailable.php #40868

Merged
merged 1 commit into from
Feb 8, 2022
Merged

[9.x] Update Mailable.php #40868

merged 1 commit into from
Feb 8, 2022

Conversation

rentalhost
Copy link
Contributor

Allow ->from(new Address($address, $name)) support again (was ->from($address, $name) in Laravel 8).

Allow `->from(new Address($address, $name))` support again (was `->from($address, $name)` in Laravel 8).
@rentalhost
Copy link
Contributor Author

Just to be clear, with the Laravel 9, the call to ->from() and similar methods stopped supporting email address and name and started to support only the address itself (without the name). However, it still allows receiving an Address object from Symfony Mailer, which contains both information. However, the normalization process throws an exception when using Address, even though the method says accept.

In this case, I still suggest an update to the guide:

Instead of using ->from($address, $name) use ->from(new Address($address, $name)).

@taylorotwell
Copy link
Member

Are we unable to support ->from($address, $name) in Laravel 9? Why do we need to force people to create an Address instance?

@rentalhost
Copy link
Contributor Author

@taylorotwell I do not see any problem to keep support to old way (that I personally prefer).

But the current code is:

    public function from(Address|string ...$addresses): static
    {
        return $this->setListAddressHeaderBody('From', $addresses);
    }

To support old way it need be modified.

The PR is only a fix to supports also Address, as this method parameter indicates.

@rentalhost
Copy link
Contributor Author

Ah! It seems these methods are provided directly by Symfony Mailer. Anyway, the Laravel Message will create an Address instance and the exception will be throw anyway:

/**
* Add a "from" address to the message.
*
* @param string|array $address
* @param string|null $name
* @return $this
*/
public function from($address, $name = null)
{
is_array($address)
? $this->message->from(...$address)
: $this->message->from(new Address($address, (string) $name));
return $this;
}

@taylorotwell
Copy link
Member

Hmm, so does ->from($address, $name) work? I got a little lost in the last message. What do we need to fix?

@rentalhost
Copy link
Contributor Author

Not in fact, because it will throw an exception when you set the $name. As the normalizeRecipient() will not handle the instanceof Address, it will just return the own instance of Address that does not supports $email property (or $name). Then the PR fixes that.

image

@taylorotwell taylorotwell merged commit ca33630 into laravel:9.x Feb 8, 2022
@taylorotwell
Copy link
Member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants