Skip to content

Allow controller to pass params to reset password email #2207

Closed
wants to merge 4 commits into from

5 participants

@tigris
tigris commented Jan 10, 2013

These changes allow the password controller to pass parameters to the reset password email via the optional mailer arguments added in devise 2.2.0.

Unfortunately this means you shouldn't really call send_reset_password_instructions on the class method, because that method already takes a hash of arguments used to find the resource. I could have used an optional mailer key in that hash, the same way rails allows for a html key in hash options for things like form_for etc, but I thought this was a better option. Happy to discuss any alternative ideas people might have.

Also, I only implemented controller parameters for the passwords controller, but the framework is there for confirmable and lockable as well. I just wasn't as familiar with those controllers as I don't use them, so I didn't fell 100% comfortable in changing them.

@josevalim
Plataformatec member

Thanks for the PR. One small note:

Unfortunately this means you shouldn't really call send_reset_password_instructions on the class method, because that method already takes a hash of arguments used to find the resource.

It could simply accept a second argument with the delivery options. That would be my preferred approach.

@tigris
tigris commented Jan 11, 2013

Thanks Jose. I will do it that way and see how it looks. Generally people don't like methods that take 2 lots of hashes.

@tigris
tigris commented Jan 13, 2013

Branch has been updated @josevalim , thank you for your comments.

@vince
vince commented Feb 12, 2013

I currently have an open stackoverflow question on this exact issue. Is there a particular version of the gem that this feature is included in? It looks pretty straight forward but any documentation I can view or contribute?

@josevalim
Plataformatec member

This feature has not been merged yet, so the best you can do is to depend on @tigris fork.

@nashby nashby closed this Feb 12, 2013
@nashby nashby reopened this Feb 12, 2013
@vince
vince commented Feb 12, 2013

Ok perfect. Thanks @tigris and @josevalim; I've done just that and it seems to be working as expected. Let me know if there's anything I can do to help this along.

@josevalim
Plataformatec member

I merged this pull request locally and I tried adding similar feature to confirmable. However, since the confirmable is sending on creation, we can't pass the parameters to the mailer using the current structure. That said, I am dropping this approach for now because I couldn't find a way to make it work consistently across the request. Maybe the best option is to store the request information on a thread local variable.

@josevalim josevalim closed this Apr 14, 2013
@josevalim
Plataformatec member

Regardless, thanks for the pull request @tigris !

@emilevictor

Hey @josevalim, is there any movement on this? I was considering making my own pull request with this feature until I saw this one. I think this would be a useful feature if we could somehow introduce it. In the meantime, I'm going to be doing a bit of a kludge in my own codebase to get it working.

@tigris
tigris commented May 31, 2013

FWIW, I implemented this on my project by storing the params in a thread local variable. Using the request_store gem I have a before_filter in my ApplicationController setting the appropriate variables, in then in UserMailer I can pull them out as needed.

The issue of course is if you ever want to call UserMailer methods manually from a console or cron or something, and rely on any params from request_store then you are going to need to account for that with some defaults, or allow them to be passed in.

I left my branch around cos I knew it worked and hoped devise would find a way to make it happen, but I needed upgraded features from newer devise so that is why I went with the solution above rather than my own branch.

@vince
vince commented May 31, 2013

@tigris We're using your branch at the moment but I cloned a copy just in case. Thanks for your work - I'm sad this didn't make it into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.