Permalink
Browse files

let mailer templates generate URLs by default [Xavier Noria, Richard …

…Schneeman]
  • Loading branch information...
fxn committed Nov 23, 2014
1 parent cdd90f3 commit 9685080a7677abfa5d288a81c3e078368c6bb67c
@@ -1,3 +1,10 @@
+* `link_to` and `url_for` generate URLs by default in templates, it is no
+ longer needed to pass `only_path: false`.
+
+ Fixes #16497 and #16589.
+
+ *Xavier Noria*, *Richard Schneeman*
+
* Attachments can be added while rendering the mail template.
Fixes #16974.
@@ -0,0 +1 @@
+<%= url_for(@options) %> <%= @url %>
@@ -23,9 +23,32 @@ def signed_up_with_url(recipient)
mail(to: recipient, subject: "[Signed up] Welcome #{recipient}",
from: "system@loudthinking.com", date: Time.local(2004, 12, 12))
end
+
+ def exercise_url_for(options)
+ @options = options
+ @url = url_for(@options)
+ mail(from: 'from@example.com', to: 'to@example.com', subject: 'subject')
+ end
end
class ActionMailerUrlTest < ActionMailer::TestCase
+ class DummyModel
+ def self.model_name
+ OpenStruct.new(route_key: 'dummy_model')
+ end
+
+ def persisted?
+ false
+ end
+
+ def model_name
+ self.class.model_name
+ end
+
+ def to_model
+ self
+ end
+ end
def encode( text, charset="UTF-8" )
quoted_printable( text, charset )
@@ -40,10 +63,47 @@ def new_mail( charset="UTF-8" )
mail
end
+ def assert_url_for(expected, options, relative: false)
+ expected = "http://www.basecamphq.com#{expected}" if expected.start_with?('/') && !relative
+ urls = UrlTestMailer.exercise_url_for(options).body.to_s.chomp.split
+
+ assert_equal expected, urls.first
+ assert_equal expected, urls.second
+ end
+
def setup
@recipient = 'test@localhost'
end
+ def test_url_for
+ UrlTestMailer.delivery_method = :test
+
+ AppRoutes.draw do
+ get ':controller(/:action(/:id))'
+ get '/welcome' => 'foo#bar', as: 'welcome'
+ get '/dummy_model' => 'foo#baz', as: 'dummy_model'
+ end
+
+ # string
+ assert_url_for 'http://foo/', 'http://foo/'
+
+ # symbol
+ assert_url_for '/welcome', :welcome
+
+ # hash
+ assert_url_for '/a/b/c', controller: 'a', action: 'b', id: 'c'
+ assert_url_for '/a/b/c', {controller: 'a', action: 'b', id: 'c', only_path: true}, relative: true
+
+ # model
+ assert_url_for '/dummy_model', DummyModel.new
+
+ # class
+ assert_url_for '/dummy_model', DummyModel
+
+ # array
+ assert_url_for '/dummy_model' , [DummyModel]
+ end
+
def test_signed_up_with_url
UrlTestMailer.delivery_method = :test
@@ -457,7 +457,7 @@ def #{name}
RUBY
end
- def url_helpers(include_path_helpers = true)
+ def url_helpers(supports_path = true)
routes = self
Module.new do
@@ -484,7 +484,7 @@ def url_options; {}; end
# named routes...
include url_helpers
- if include_path_helpers
+ if supports_path
path_helpers = routes.named_routes.path_helpers_module
else
path_helpers = routes.named_routes.path_helpers_module(true)
@@ -502,6 +502,10 @@ def url_options; {}; end
# UrlFor (included in this module) add extra
# conveniences for working with @_routes.
define_method(:_routes) { @_routes || routes }
+
+ define_method(:_generate_paths_by_default) do

This comment has been minimized.

Show comment
Hide comment
@lucasmazza

lucasmazza Dec 1, 2014

Member

This method is being returned in the MyController.action_methods set, as it is being defined as a public method - just noticed this through the Devise test suite.

Should we just add a private : _generate_paths_by_default right after this defined_method call?

@lucasmazza

lucasmazza Dec 1, 2014

Member

This method is being returned in the MyController.action_methods set, as it is being defined as a public method - just noticed this through the Devise test suite.

Should we just add a private : _generate_paths_by_default right after this defined_method call?

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Dec 1, 2014

Member
@rafaelfranca

rafaelfranca via email Dec 1, 2014

Member

This comment has been minimized.

Show comment
Hide comment
@fxn

fxn Dec 1, 2014

Member

👍

@fxn

fxn Dec 1, 2014

Member

👍

+ supports_path
+ end
end
end
@@ -184,6 +184,12 @@ def _with_routes(routes)
def _routes_context
self
end
+
+ private
+
+ def _generate_paths_by_default
+ true
+ end
end
end
end
@@ -35,13 +35,13 @@ def process(*) #:nodoc:
module ClassMethods
def view_context_class
@view_context_class ||= begin
- include_path_helpers = supports_path?
+ supports_path = supports_path?
routes = respond_to?(:_routes) && _routes
helpers = respond_to?(:_helpers) && _helpers
Class.new(ActionView::Base) do
if routes
- include routes.url_helpers(include_path_helpers)
+ include routes.url_helpers(supports_path)
include routes.mounted_helpers
end
@@ -80,21 +80,38 @@ def url_for(options = nil)
when String
options
when nil
- super({:only_path => true})
+ super(only_path: _generate_paths_by_default)
when Hash
options = options.symbolize_keys
- options[:only_path] = options[:host].nil? unless options.key?(:only_path)
+ unless options.key?(:only_path)
+ if options[:host].nil?
+ options[:only_path] = _generate_paths_by_default
+ else
+ options[:only_path] = false
+ end
+ end
+
super(options)
when :back
_back_url
- when Symbol
- ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_string_call self, options
when Array
- polymorphic_path(options, options.extract_options!)
- when Class
- ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_class_call self, options
+ if _generate_paths_by_default
+ polymorphic_path(options, options.extract_options!)
+ else
+ polymorphic_url(options, options.extract_options!)
+ end
else
- ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call self, options
+ method = _generate_paths_by_default ? :path : :url
+ builder = ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.send(method)
+
+ case options
+ when Symbol
+ builder.handle_string_call(self, options)
+ when Class
+ builder.handle_class_call(self, options)
+ else
+ builder.handle_model_call(self, options)
+ end
end
end

1 comment on commit 9685080

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Nov 24, 2014

Member

❤️ ❤️ ❤️ thanks for your work on this.

Member

schneems commented on 9685080 Nov 24, 2014

❤️ ❤️ ❤️ thanks for your work on this.

Please sign in to comment.