Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cause rails to correctly place optional path parameter booleans #42283

Merged
merged 1 commit into from Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions actionpack/lib/action_dispatch/journey/formatter.rb
Expand Up @@ -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, _|
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/journey/visitors.rb
Expand Up @@ -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

Expand Down
29 changes: 29 additions & 0 deletions actionpack/test/dispatch/url_generation_test.rb
Expand Up @@ -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

Expand Down Expand Up @@ -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

HParker marked this conversation as resolved.
Show resolved Hide resolved
test "generating URLs with trailing slashes" do
assert_equal "/bars.json", bars_path(
trailing_slash: true,
Expand Down