diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index c5c60a4c560b3..61d6d5c184c46 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -103,7 +103,7 @@ def extract_parameterized_parts(route, options, recall) parameterized_parts = recall.merge(options) keys_to_keep = route.parts.reverse_each.drop_while { |part| - !(options.key?(part) || route.scope_options.key?(part)) || (options[part] || recall[part]).nil? + !(options.key?(part) || route.scope_options.key?(part)) || (options[part].nil? && recall[part].nil?) } | route.required_parts parameterized_parts.delete_if do |bad_key, _| @@ -118,7 +118,7 @@ def extract_parameterized_parts(route, options, recall) end end - parameterized_parts.keep_if { |_, v| v } + parameterized_parts.compact! parameterized_parts end diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index ff26c9a3b0333..18b81be8acc1f 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -40,7 +40,7 @@ def evaluate(hash) @parameters.each do |index| param = parts[index] value = hash[param.name] - return "" unless value + return "" if value.nil? parts[index] = param.escape value end diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index 2cd91adc5fe5f..e69762ea11a9a 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -16,6 +16,7 @@ def index Routes.draw do get "/foo", to: "my_route_generating#index", as: :foo + get "(/optional/:optional_id)/baz", to: "my_route_generating#index", as: :baz resources :bars @@ -127,6 +128,34 @@ def app assert_equal "http://example.com/foo", foo_url(subdomain: "") end + test "keep optional path parameter when given" do + assert_equal "http://www.example.com/optional/123/baz", baz_url(optional_id: 123) + end + + test "keep optional path parameter when true" do + assert_equal "http://www.example.com/optional/true/baz", baz_url(optional_id: true) + end + + test "omit optional path parameter when false" do + assert_equal "http://www.example.com/optional/false/baz", baz_url(optional_id: false) + end + + test "omit optional path parameter when blank" do + assert_equal "http://www.example.com/baz", baz_url(optional_id: "") + end + + test "keep positional path parameter when true" do + assert_equal "http://www.example.com/optional/true/baz", baz_url(true) + end + + test "omit positional path parameter when false" do + assert_equal "http://www.example.com/optional/false/baz", baz_url(false) + end + + test "omit positional path parameter when blank" do + assert_equal "http://www.example.com/baz", baz_url("") + end + test "generating URLs with trailing slashes" do assert_equal "/bars.json", bars_path( trailing_slash: true,