Skip to content

Commit

Permalink
Merge pull request #37073 from woahdae/fix-hierarchical-route-options
Browse files Browse the repository at this point in the history
Fix route from "(:a)(:b)" when given only :a or :b
  • Loading branch information
tenderlove committed Dec 12, 2019
2 parents cad1303 + c218233 commit 285cc00
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
17 changes: 13 additions & 4 deletions actionpack/lib/action_dispatch/routing/mapper.rb
Expand Up @@ -398,12 +398,21 @@ def dispatcher(raise_on_name_error)
end
end

# Invokes Journey::Router::Utils.normalize_path and ensure that
# (:locale) becomes (/:locale) instead of /(:locale). Except
# for root cases, where the latter is the correct one.
# Invokes Journey::Router::Utils.normalize_path, then ensures that
# /(:locale) becomes (/:locale). Except for root cases, where the
# former is the correct one.
def self.normalize_path(path)
path = Journey::Router::Utils.normalize_path(path)
path.gsub!(%r{/(\(+)/?}, '\1/') unless %r{^/(\(+[^)]+\)){1,}$}.match?(path)

# the path for a root URL at this point can be something like
# "/(/:locale)(/:platform)/(:browser)", and we would want
# "/(:locale)(/:platform)(/:browser)"

# reverse "/(", "/((" etc to "(/", "((/" etc
path.gsub!(%r{/(\(+)/?}, '\1/')
# if a path is all optional segments, change the leading "(/" back to
# "/(" so it evaluates to "/" when interpreted with no options.
path.sub!(%r{^(\(+)/}, '/\1') if %r{^(\(+[^)]+\)){1,}$}.match?(path)
path
end

Expand Down
16 changes: 15 additions & 1 deletion actionpack/test/dispatch/routing_test.rb
Expand Up @@ -1382,7 +1382,7 @@ def test_scoped_root_as_name
assert_equal "projects#index", @response.body
end

def test_optionally_scoped_root_unscoped_access
def test_optional_scoped_root_hierarchy
draw do
scope "(:locale)" do
scope "(:platform)" do
Expand All @@ -1394,8 +1394,22 @@ def test_optionally_scoped_root_unscoped_access
end

assert_equal "/", root_path
assert_equal "/en", root_path(locale: "en")
assert_equal "/en/osx", root_path(locale: "en", platform: "osx")
assert_equal "/en/osx/chrome",
root_path(locale: "en", platform: "osx", browser: "chrome")

get "/"
assert_equal "projects#index", @response.body

get "/en"
assert_equal "projects#index", @response.body

get "/en/osx"
assert_equal "projects#index", @response.body

get "/en/osx/chrome"
assert_equal "projects#index", @response.body
end

def test_scope_with_format_option
Expand Down

0 comments on commit 285cc00

Please sign in to comment.