From d83a3189d5248479bbf8f123ff8dd0c770e38421 Mon Sep 17 00:00:00 2001 From: "Eileen M. Uchitelle" Date: Tue, 1 Jun 2021 15:47:28 -0400 Subject: [PATCH] Merge pull request #42283 from HParker/named-routes-identifies-false cause rails to correctly place optional path parameter booleans --- .../lib/action_dispatch/journey/formatter.rb | 4 +-- .../lib/action_dispatch/journey/visitors.rb | 2 +- .../test/dispatch/url_generation_test.rb | 29 +++++++++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) 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,