[Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread richard schneeman
Here's a change in behavior I would love on the mailers. ## Backstory When you send an email, you'll likely need links. Those links must be the full path i.e. 'http://example.com/foo' instead of relative (just '/foo'). Unfortunately most devs are so used to using the *_path helpers, they use the

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread Caleb Thompson
Totally in support of this as well. I've probably made this mistake more often than I've done it correctly, and it's so easy to miss. I've seen email templates go months without this being caught. I'd probably prefer the exception route myself, just by undefining the methods for ActionMailer. Ca

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread Vipul A M
I have been bitten by this too, and agree with it. I think resolving a full URL would be better. People sometimes reuse normal views from Mailers, and would be better off using *_path helpers there. Exceptions would cause an issue. (A warning maybe?) Vipul A.M. +91-8149-204995 On Thu, Jun 12, 2

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread pathé Sene
Agree with the feature request. I made this error too. 2014-06-12 18:04 GMT+00:00 Vipul A M : > I have been bitten by this too, and agree with it. > > I think resolving a full URL would be better. People sometimes reuse > normal views from Mailers, and would be better off using *_path > helpers

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread Florian Thomas
I'm in support of this as well. As a solution i would prefer to resolve the full URL by default. Raising an exception seems to me a bit like rails telling the programmer "I know what you're intending, and we both know the solution but you have to fix it on your own." > Here's a change in behav

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-12 Thread Xavier Noria
On Thu, Jun 12, 2014 at 8:07 PM, Florian Thomas wrote: I’m in support of this as well. > As a solution i would prefer to resolve the full URL by default. Raising > an exception seems to me a bit like rails telling the programmer „I know > what you’re intending, and we both know the solution but y

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-17 Thread richard schneeman
Are there any valid uses for *_path in an email? Can you do reference links with ID elements in an email? We could make it configurable. Default it to non-hair-pulling behavior, but allow anyone with a legitimate `*_path` use to preserve functionality. ``` config.action_mailer.path_helper_behavior

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-17 Thread Andrew Kaspick
Slight aside, but wouldn't a simple test case catch such an issue? Whether that be an actual mailer test or reviewing a generated email? On Jun 17, 2014 6:14 PM, "richard schneeman" wrote: > Are there any valid uses for *_path in an email? Can you do reference > links with ID elements in an emai

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-17 Thread richard schneeman
Nope it wouldn't. Rendering the emails don't currently raise errors, so your tests will still pass. The only way to find out is to actually get the email and click on it. Even if you look at it in mail_view you get the right URL generated as the relative path will work. Right now it's all just :(

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Xavier Noria
I cannot think of a valid use case for *_path in mailer views, so after thinking about it I personally would be inclined to remove them. People cannot upgrade and have their mailers crashing in production, so I think we have no option but to go through a deprecation cycle. If someone had a rare n

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Prem Sichanugrist
I like that. Let's deprecate *_path in Mailer in 4.2 and remove in 5.0 -Prem > On Jun 18, 2014, at 3:33 AM, Xavier Noria wrote: > > I cannot think of a valid use case for *_path in mailer views, so after > thinking about it I personally would be inclined to remove them. > > People cannot upgr

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Adam Piotrowski
Nice idea in my opinion. 2014-06-18 11:08 GMT+02:00 Prem Sichanugrist : > I like that. Let's deprecate *_path in Mailer in 4.2 and remove in 5.0 > > -Prem > > On Jun 18, 2014, at 3:33 AM, Xavier Noria wrote: > > I cannot think of a valid use case for *_path in mailer views, so after > thinking

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread richard schneeman
If we go down this route, the error message should be crystal clear: "The method `user_path` cannot be used in a mailer as relative links in emails do not work. Use `user_url` instead" Also need to make sure that it raises when rendered with mail_view. On Wed, Jun 18, 2014 at 4:11 AM, Adam Piot

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Xavier Noria
On Wed, Jun 18, 2014 at 5:45 PM, richard schneeman < richard.schnee...@gmail.com> wrote: If we go down this route, the error message should be crystal clear: > > "The method `user_path` cannot be used in a mailer as relative links in > emails do not work. Use `user_url` instead" > > Also need to m

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Carlos Antonio da Silva
+1 from my side, never seen a use case for *_path on mailers. On Wed, Jun 18, 2014 at 12:47 PM, Xavier Noria wrote: > On Wed, Jun 18, 2014 at 5:45 PM, richard schneeman < > richard.schnee...@gmail.com> wrote: > > If we go down this route, the error message should be crystal clear: >> >> "The me

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Jeremy Evans
On Wed, Jun 18, 2014 at 12:33 AM, Xavier Noria wrote: > I cannot think of a valid use case for *_path in mailer views, so after > thinking about it I personally would be inclined to remove them. > > People cannot upgrade and have their mailers crashing in production, so I > think we have no optio

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-06-18 Thread Xavier Noria
I have never checked it myself (could be cargo culting), but my understanding has always been that using the BASE tag is not a good practice for portable HTML emails. See for example: http://stackoverflow.com/questions/14611225/html-base-tag-in-email I personally believe preventing *_path cal

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-07-08 Thread richard schneeman
Here's the working PR for those interested in following along: https://github.com/rails/rails/pull/15840 On Wed, Jun 18, 2014 at 1:40 PM, Xavier Noria wrote: > I have never checked it myself (could be cargo culting), but my > understanding has always been that using the BASE tag is not a good >

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-07-30 Thread Xavier Noria
Merged. Thanks a lot Richard! <3 -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, sen

Re: [Rails-core] Feature Request: Remove or replace *_path helpers for mailers

2014-07-30 Thread richard schneeman
Thanks for the review and thanks to Sean, he did most of the driving on the initial pair. Happy to have this in :D -- Richard Schneeman @schneems On Wed, Jul 30, 2014 at 5:27 PM, Xavier Noria wrote: > Merged. Thanks a lot Richard! <3 > > -- > You received this m