diff options
Diffstat (limited to 'actionpack')
| -rw-r--r-- | actionpack/CHANGELOG.md | 11 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/formatter.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/route.rb | 9 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 13 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 41 |
5 files changed, 65 insertions, 11 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index a84f72eca9..4bcf313ecc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,14 @@ +* Keep part when scope option has value + + When a route was defined within an optional scope, if that route didn't + take parameters the scope was lost when using path helpers. This commit + ensures scope is kept both when the route takes parameters or when it + doesn't. + + Fixes #33219 + + *Alberto Almagro* + * Added `deep_transform_keys` and `deep_transform_keys!` methods to ActionController::Parameters. *Gustavo Gutierrez* diff --git a/actionpack/lib/action_dispatch/journey/formatter.rb b/actionpack/lib/action_dispatch/journey/formatter.rb index 52396ec901..53c404ee7c 100644 --- a/actionpack/lib/action_dispatch/journey/formatter.rb +++ b/actionpack/lib/action_dispatch/journey/formatter.rb @@ -67,7 +67,7 @@ module ActionDispatch parameterized_parts = recall.merge(options) keys_to_keep = route.parts.reverse_each.drop_while { |part| - !options.key?(part) || (options[part] || recall[part]).nil? + !(options.key?(part) || route.scope_options.key?(part)) || (options[part] || recall[part]).nil? } | route.required_parts parameterized_parts.delete_if do |bad_key, _| diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 8165709a3d..587ebcb873 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -4,9 +4,9 @@ module ActionDispatch # :stopdoc: module Journey class Route - attr_reader :app, :path, :defaults, :name, :precedence + attr_reader :app, :path, :defaults, :name, :precedence, :constraints, + :internal, :scope_options - attr_reader :constraints, :internal alias :conditions :constraints module VerbMatchers @@ -51,13 +51,13 @@ module ActionDispatch def self.build(name, app, path, constraints, required_defaults, defaults) request_method_match = verb_matcher(constraints.delete(:request_method)) - new name, app, path, constraints, required_defaults, defaults, request_method_match, 0 + new name, app, path, constraints, required_defaults, defaults, request_method_match, 0, {} end ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, internal = false) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match, precedence, scope_options, internal = false) @name = name @app = app @path = path @@ -72,6 +72,7 @@ module ActionDispatch @decorated_ast = nil @precedence = precedence @path_formatter = @path.build_formatter + @scope_options = scope_options @internal = internal end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f29f66990d..695eed3adc 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -70,17 +70,17 @@ module ActionDispatch ANCHOR_CHARACTERS_REGEX = %r{\A(\\A|\^)|(\\Z|\\z|\$)\Z} OPTIONAL_FORMAT_REGEX = %r{(?:\(\.:format\)+|\.:format|/)\Z} - attr_reader :requirements, :defaults - attr_reader :to, :default_controller, :default_action - attr_reader :required_defaults, :ast + attr_reader :requirements, :defaults, :to, :default_controller, + :default_action, :required_defaults, :ast, :scope_options def self.build(scope, set, ast, controller, default_action, to, via, formatted, options_constraints, anchor, options) options = scope[:options].merge(options) if scope[:options] defaults = (scope[:defaults] || {}).dup scope_constraints = scope[:constraints] || {} + scope_options = scope[:options] || {} - new set, ast, defaults, controller, default_action, scope[:module], to, formatted, scope_constraints, scope[:blocks] || [], via, options_constraints, anchor, options + new set, ast, defaults, controller, default_action, scope[:module], to, formatted, scope_constraints, scope_options, scope[:blocks] || [], via, options_constraints, anchor, options end def self.check_via(via) @@ -111,7 +111,7 @@ module ActionDispatch format != false && path !~ OPTIONAL_FORMAT_REGEX end - def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, blocks, via, options_constraints, anchor, options) + def initialize(set, ast, defaults, controller, default_action, modyoule, to, formatted, scope_constraints, scope_options, blocks, via, options_constraints, anchor, options) @defaults = defaults @set = set @@ -122,6 +122,7 @@ module ActionDispatch @anchor = anchor @via = via @internal = options.delete(:internal) + @scope_options = scope_options path_params = ast.find_all(&:symbol?).map(&:to_sym) @@ -161,7 +162,7 @@ module ActionDispatch def make_route(name, precedence) Journey::Route.new(name, application, path, conditions, required_defaults, - defaults, request_method, precedence, @internal) + defaults, request_method, precedence, scope_options, @internal) end def application 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 |