search

markets / maily

📫 Rails Engine to preview emails in the browser
https://rubygems.org/gems/maily
MIT License
707 stars 31 forks source link

Support ActionMailer::Parameterized #39

Closed krismichalski closed 5 years ago

krismichalski commented 5 years ago

Hello! :wave: This PR adds support for ActionMailer::Parameterized introduced in Rails 5.1

I'm successfully using it in my project, but let me know if you still would want to improve it somehow :)

One thing I'm not sure about if it's right is that the hook is not required even if we use params in mail, but I don't know how to test for that :/

markets commented 5 years ago

Thanks for contributing here @krismichalski!

To be honest, I'm still not using ActionMailer::Parameterized on any Rails app right now, so I'm not sure how useful is that change in Maily or what "limitations" impose its absence to users using that feature. But I'm totally positive about merging this and support new framework additions. Looks pretty good in general :ok_hand:, could you please give me more details about this point:

One thing I'm not sure about if it's right is that the hook is not required even if we use params in mail

In addition, we'll need to update the docs explaining this new feature 📚

Thanks!

krismichalski commented 5 years ago

I meant even if we are using params in mailer require_hook? will be false and I'm not sure if that's OK with you :)

And sure, I will add some info to the README soon :)

markets commented 5 years ago

Ah! I think I got it, you meant: with regular params, we can detect if you have some miss-configurations via the require_hook? (we actually display a good error message 👌 ) but this doesn't work when using ActionMailer::Parameterized.

As I said in my previous comment, still not using this feature, so right now I have no idea how to fix this or even if this is "detectable"...

Anyway, I think it's more important to ship support for this feature, and eventually improve it if possible.

markets commented 5 years ago

@krismichalski v0.10.0 released:

:rocket: :rocket:

krismichalski commented 5 years ago

Nice! :) I agree that's it's more important to support this feature for now and eventually improve it in the future :+1: Thank you :rocket: