Merge pull request #34656 from albertoalmagro/path_helper_without_params_does_not_loose_scope

Keep part when scope option has value

2 files changed, 61 insertions, 21 deletions

diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 362488d585..0070d7af72 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -4959,6 +4959,47 @@ class TestPartialDynamicPathSegments < ActionDispatch::IntegrationTest
     end
 end
 
+class TestOptionalScopesWithOrWithoutParams < ActionDispatch::IntegrationTest
+  Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
+    app.draw do
+      scope module: "test_optional_scopes_with_or_without_params" do
+        scope "(:locale)", locale: /en|es/ do
+          get "home", to: "home#index"
+          get "with_param/:foo", to: "home#with_param", as: "with_param"
+          get "without_param", to: "home#without_param"
+        end
+      end
+    end
+  end
+
+  class HomeController < ActionController::Base
+    include Routes.url_helpers
+
+    def index
+      render inline: "<%= with_param_path(foo: 'bar') %> | <%= without_param_path %>"
+    end
+
+    def with_param; end
+    def without_param; end
+  end
+
+  APP = build_app Routes
+
+  def app
+    APP
+  end
+
+  def test_stays_unscoped_with_or_without_params
+    get "/home"
+    assert_equal "/with_param/bar | /without_param", response.body
+  end
+
+  def test_preserves_scope_with_or_without_params
+    get "/es/home"
+    assert_equal "/es/with_param/bar | /es/without_param", response.body
+  end
+end
+
 class TestPathParameters < ActionDispatch::IntegrationTest
   Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
     app.draw do
diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb
index a8bf4a11e2..8828201e4f 100644
--- a/actionpack/test/journey/route_test.rb
+++ b/actionpack/test/journey/route_test.rb
@@ -9,7 +9,7 @@ module ActionDispatch
         app      = Object.new
         path     = Path::Pattern.from_string "/:controller(/:action(/:id(.:format)))"
         defaults = {}
-        route    = Route.build("name", app, path, {}, [], defaults)
+        route    = Route.new(name: "name", app: app, path: path, defaults: defaults)
 
         assert_equal app, route.app
         assert_equal path, route.path
@@ -17,10 +17,9 @@ module ActionDispatch
       end
 
       def test_route_adds_itself_as_memo
-        app      = Object.new
-        path     = Path::Pattern.from_string "/:controller(/:action(/:id(.:format)))"
-        defaults = {}
-        route    = Route.build("name", app, path, {}, [], defaults)
+        app   = Object.new
+        path  = Path::Pattern.from_string "/:controller(/:action(/:id(.:format)))"
+        route = Route.new(name: "name", app: app, path: path)
 
         route.ast.grep(Nodes::Terminal).each do |node|
           assert_equal route, node.memo
@@ -28,30 +27,30 @@ module ActionDispatch
       end
 
       def test_path_requirements_override_defaults
-        path = Path::Pattern.build(":name", { name: /love/ }, "/", true)
-        defaults  = { name: "tender" }
-        route     = Route.build("name", nil, path, {}, [], defaults)
+        path     = Path::Pattern.build(":name", { name: /love/ }, "/", true)
+        defaults = { name: "tender" }
+        route    = Route.new(name: "name", path: path, defaults: defaults)
         assert_equal(/love/, route.requirements[:name])
       end
 
       def test_ip_address
         path  = Path::Pattern.from_string "/messages/:id(.:format)"
-        route = Route.build("name", nil, path, { ip: "192.168.1.1" }, [],
-                          controller: "foo", action: "bar")
+        route = Route.new(name: "name", path: path, constraints: { ip: "192.168.1.1" },
+                          defaults: { controller: "foo", action: "bar" })
         assert_equal "192.168.1.1", route.ip
       end
 
       def test_default_ip
         path  = Path::Pattern.from_string "/messages/:id(.:format)"
-        route = Route.build("name", nil, path, {}, [],
-                          controller: "foo", action: "bar")
+        route = Route.new(name: "name", path: path,
+                          defaults: { controller: "foo", action: "bar" })
         assert_equal(//, route.ip)
       end
 
       def test_format_with_star
         path  = Path::Pattern.from_string "/:controller/*extra"
-        route = Route.build("name", nil, path, {}, [],
-                          controller: "foo", action: "bar")
+        route = Route.new(name: "name", path: path,
+                          defaults: { controller: "foo", action: "bar" })
         assert_equal "/foo/himom", route.format(
           controller: "foo",
           extra: "himom")
@@ -59,7 +58,8 @@ module ActionDispatch
 
       def test_connects_all_match
         path  = Path::Pattern.from_string "/:controller(/:action(/:id(.:format)))"
-        route = Route.build("name", nil, path, { action: "bar" }, [], controller: "foo")
+        route = Route.new(name: "name", path: path, constraints: { action: "bar" },
+                          defaults: { controller: "foo" })
 
         assert_equal "/foo/bar/10", route.format(
           controller: "foo",
@@ -69,34 +69,33 @@ module ActionDispatch
 
       def test_extras_are_not_included_if_optional
         path  = Path::Pattern.from_string "/page/:id(/:action)"
-        route = Route.build("name", nil, path, {}, [], action: "show")
+        route = Route.new(name: "name", path: path, defaults: { action: "show" })
 
         assert_equal "/page/10", route.format(id: 10)
       end
 
       def test_extras_are_not_included_if_optional_with_parameter
         path  = Path::Pattern.from_string "(/sections/:section)/pages/:id"
-        route = Route.build("name", nil, path, {}, [], action: "show")
+        route = Route.new(name: "name", path: path, defaults: { action: "show" })
 
         assert_equal "/pages/10", route.format(id: 10)
       end
 
       def test_extras_are_not_included_if_optional_parameter_is_nil
         path  = Path::Pattern.from_string "(/sections/:section)/pages/:id"
-        route = Route.build("name", nil, path, {}, [], action: "show")
+        route = Route.new(name: "name", path: path, defaults: { action: "show" })
 
         assert_equal "/pages/10", route.format(id: 10, section: nil)
       end
 
       def test_score
-        constraints = {}
         defaults = { controller: "pages", action: "show" }
 
         path = Path::Pattern.from_string "/page/:id(/:action)(.:format)"
-        specific = Route.build "name", nil, path, constraints, [:controller, :action], defaults
+        specific = Route.new name: "name", path: path, required_defaults: [:controller, :action], defaults: defaults
 
         path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)"
-        generic = Route.build "name", nil, path, constraints, [], {}
+        generic = Route.new name: "name", path: path
 
         knowledge = { "id" => true, "controller" => true, "action" => true }