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

Support preset parameter in email action #257

Merged
merged 4 commits into from
Feb 21, 2024
Merged

Conversation

bezin
Copy link
Contributor

@bezin bezin commented Feb 16, 2024

Hi all,

thanks for this wonderful plugin. I have a minor thing that bugs me for a long time already:

We have a default email preset in all our projects, that removes the X-Mailer header to not expose the PHPMailer version.

We use your plugin to handle all our forms. Everytime we call the emailAction, we have to repeat the default email preset settings:

->emailAction([
    // […] All other email action related settings removed for brevity
    'beforeSend' => function ($mailer) {
        $mailer->XMailer = ' ';
    },
])

This PR adds the possibility to just pass along a preset parameter:

->emailAction([
    // […] All other email action related settings removed for brevity
    'preset' => 'default'
])

This way, the desired preset is just passed along to the Kirby email method and handled accordingly.

Would love to see that merged, because you can also globally define even more parameters like default sender and reply-to addresses and what not.

Thanks and cheers

@mzur
Copy link
Owner

mzur commented Feb 19, 2024

Thanks! Do you see any way to add a test case for this?

@bezin
Copy link
Contributor Author

bezin commented Feb 20, 2024

Yes, of course 👍 I added two test cases and actually caught a bug that I had not anticipated in my application code I tested the changes with.

I had to refactor in order to allow loading required parameters from and to from the preset. We now load the options from the presets ourselves, rather than letting \Kirby\Cms\Email do it. I also throw an exception if you want to use an undefined preset. I reused \Kirby\Exception\NotFoundException as in the original Email class. Let me know if you would prefer a custom Uniform exception.

Copy link
Owner

@mzur mzur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you! Now if you could also extend the documentation with a description of the new option it would be perfect 👌 You should also mention that a preset can define "to" and "from", making these no longer required options in that case.

src/Actions/EmailAction.php Outdated Show resolved Hide resolved
Copy link
Owner

@mzur mzur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, thank you!

@mzur mzur merged commit 305dd97 into mzur:master Feb 21, 2024
6 checks passed
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