From 9685080a7677abfa5d288a81c3e078368c6bb67c Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Sun, 23 Nov 2014 13:53:01 -0800 Subject: let mailer templates generate URLs by default [Xavier Noria, Richard Schneeman] --- actionmailer/CHANGELOG.md | 7 +++ .../fixtures/url_test_mailer/exercise_url_for.erb | 1 + actionmailer/test/url_test.rb | 60 ++++++++++++++++++++++ .../lib/action_dispatch/routing/route_set.rb | 8 ++- actionpack/lib/action_dispatch/routing/url_for.rb | 6 +++ actionview/lib/action_view/rendering.rb | 4 +- actionview/lib/action_view/routing_url_for.rb | 33 +++++++++--- 7 files changed, 107 insertions(+), 12 deletions(-) create mode 100644 actionmailer/test/fixtures/url_test_mailer/exercise_url_for.erb diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 5685871ac9..1b3e802cb2 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -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. diff --git a/actionmailer/test/fixtures/url_test_mailer/exercise_url_for.erb b/actionmailer/test/fixtures/url_test_mailer/exercise_url_for.erb new file mode 100644 index 0000000000..0322c1191e --- /dev/null +++ b/actionmailer/test/fixtures/url_test_mailer/exercise_url_for.erb @@ -0,0 +1 @@ +<%= url_for(@options) %> <%= @url %> diff --git a/actionmailer/test/url_test.rb b/actionmailer/test/url_test.rb index be7532d42f..8fce7c3827 100644 --- a/actionmailer/test/url_test.rb +++ b/actionmailer/test/url_test.rb @@ -23,9 +23,32 @@ class UrlTestMailer < ActionMailer::Base 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 @@ class ActionMailerUrlTest < ActionMailer::TestCase 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 diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index a641ea3ea9..d2ae2a496f 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -457,7 +457,7 @@ module ActionDispatch RUBY end - def url_helpers(include_path_helpers = true) + def url_helpers(supports_path = true) routes = self Module.new do @@ -484,7 +484,7 @@ module ActionDispatch # 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 @@ module ActionDispatch # UrlFor (included in this module) add extra # conveniences for working with @_routes. define_method(:_routes) { @_routes || routes } + + define_method(:_generate_paths_by_default) do + supports_path + end end end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index eb554ec383..dca86858cc 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -184,6 +184,12 @@ module ActionDispatch def _routes_context self end + + private + + def _generate_paths_by_default + true + end end end end diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 5cbdfdf6c0..abd3b77c67 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -35,13 +35,13 @@ module ActionView 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 diff --git a/actionview/lib/action_view/routing_url_for.rb b/actionview/lib/action_view/routing_url_for.rb index 75febb8652..f281333a41 100644 --- a/actionview/lib/action_view/routing_url_for.rb +++ b/actionview/lib/action_view/routing_url_for.rb @@ -80,21 +80,38 @@ module ActionView 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 -- cgit v1.2.3