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

NotificationMailSenderConfig - There should be no from key in $data #1866

Open
mho22 opened this issue Jan 6, 2025 · 1 comment
Open

NotificationMailSenderConfig - There should be no from key in $data #1866

mho22 opened this issue Jan 6, 2025 · 1 comment

Comments

@mho22
Copy link
Contributor

mho22 commented Jan 6, 2025

Trying out the newly released version 9.2.0 I encountered an unexpected issue when running composer install on a Laravel project where I don't have a MAIL_FROM_ADDRESS environment variable, even if I have a mail key in my config [ based on previous merged pull requests ].

Image

I found out that in the NotificationMailSenderConfig file on line 18 :

public static function fromArray(array $data): self
{
    $address = $data['from']['address'] ?? config('mail.from.address');

   ...
}

There is a $data['from']['address'] instead of probably $data['address']. Basically because this is the MailSenderConfig so it doesn't have to go through the from key.

Furthermore, when I was looking for the location where NotificationMailSenderConfig::fromArray is called, in NotificationMailConfig.php on line 33:

public static function fromArray(array $data): self
{
    ...

    return new self(
        to: $data['to'],
        from: NotificationMailSenderConfig::fromArray($data['from'] ?? []),
    );
}

it was already passing the $data['from'] array as first parameter.

Should I suggest a PR to simply remove the ['from'] characters from NotificationMailSenderConfig ?

The reason why this has not been identified before is probably because every Laravel project has a MAIL_FROM_ADDRESS variable within its environment file.

@mho22
Copy link
Contributor Author

mho22 commented Jan 6, 2025

@Nielsvanpach I hope this will be the last issue I post.

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

No branches or pull requests

1 participant