Pass params to url_for that ONLY get used for named segments; otherwise discard them

actionpack/CHANGELOG.md

@@ -1,3 +1,40 @@
+*   The url_for helpers now support a new option called `path_params`.
+    This is very useful in situations where you only want to add a required param that is part of the route's URL but for other route not append an extraneous query param.
+
+    Given the following router...
+    ```ruby
+    Rails.application.routes.draw do
+      scope ":account_id" do
+        get "dashboard" => "pages#dashboard", as: :dashboard
+        get "search/:term" => "search#search", as: :search
+      end
+      delete "signout" => "sessions#destroy", as: :signout
+    end
+    ```
+
+    And given the following `ApplicationController`
+    ```ruby
+      class ApplicationController < ActionController::Base
+        def default_url_options
+          { path_params: { account_id: "foo" } }
+        end
+      end
+    ```
+
+    The standard url_for helper and friends will now behave as follows:
+
+    ```ruby
+    dashboard_path # => /foo/dashboard
+    dashboard_path(account_id: "bar") # => /bar/dashboard
+
+    signout_path # => /signout
+    signout_path(account_id: "bar") # => /signout?account_id=bar
+    signout_path(account_id: "bar", path_params: { account_id: "baz" }) # => /signout?account_id=bar
+    search_path("quin") # => /foo/search/quin
+    ```
+
+    *Jason Meller, Jeremy Beker*
+
 *   Change `action_dispatch.show_exceptions` to one of `:all`, `:rescuable`, or
     `:none`. `:all` and `:none` behave the same as the previous `true` and
     `false` respectively. The new `:rescuable` option will only show exceptions

actionpack/lib/action_dispatch/journey/formatter.rb

@@ -57,6 +57,9 @@ def message
       end
 
       def generate(name, options, path_parameters)
+        original_options = options.dup
+        path_params = options.delete(:path_params) || {}
+        options = path_params.merge(options)
         constraints = path_parameters.merge(options)
         missing_keys = nil
 
@@ -70,8 +73,11 @@ def generate(name, options, path_parameters)
 
           missing_keys = missing_keys(route, parameterized_parts)
           next if missing_keys && !missing_keys.empty?
-          params = options.dup.delete_if do |key, _|
-            parameterized_parts.key?(key) || route.defaults.key?(key)
+          params = options.delete_if do |key, _|
+            # top-level params' normal behavior of generating query_params
+            # should be preserved even if the same key is also a bind_param
+            parameterized_parts.key?(key) || route.defaults.key?(key) ||
+              (path_params.key?(key) && !original_options.key?(key))
           end
 
           defaults       = route.defaults

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -282,7 +282,7 @@ def handle_positional_args(controller_options, inner_options, args, result, path
 
               if args.size < path_params_size
                 path_params -= controller_options.keys
-                path_params -= result.keys
+                path_params -= (result[:path_params] || {}).merge(result).keys
               else
                 path_params = path_params.dup
               end

actionpack/lib/action_dispatch/routing/url_for.rb

@@ -136,6 +136,8 @@ def url_options
       # * <tt>:port</tt> - Optionally specify the port to connect to.
       # * <tt>:anchor</tt> - An anchor name to be appended to the path.
       # * <tt>:params</tt> - The query parameters to be appended to the path.
+      # * <tt>:path_params</tt> - The query parameters that will only be used
+      #   for the named dynamic segments of path. If unused, they will be discarded.
       # * <tt>:trailing_slash</tt> - If true, adds a trailing slash, as in <tt>"/archive/2009/"</tt>.
       # * <tt>:script_name</tt> - Specifies application path relative to domain root. If provided, prepends application path.
       #

actionpack/test/controller/url_for_test.rb

@@ -321,6 +321,58 @@ def test_relative_url_root_is_respected_for_named_routes
         end
       end
 
+      def test_path_params_with_default_url_options
+        with_routing do |set|
+          set.draw do
+            scope ":account_id" do
+              get "dashboard" => "pages#dashboard", as: :dashboard
+              get "search(/:term)" => "search#search", as: :search
+            end
+            delete "signout" => "sessions#destroy", as: :signout
+          end
+
+          kls = Class.new do
+            include set.url_helpers
+            def default_url_options
+              { path_params: { account_id: "foo" } }
+            end
+          end
+
+          controller = kls.new
+
+          assert_equal("/foo/dashboard", controller.dashboard_path)
+          assert_equal("/bar/dashboard", controller.dashboard_path(account_id: "bar"))
+          assert_equal("/signout", controller.signout_path)
+          assert_equal("/signout?account_id=bar", controller.signout_path(account_id: "bar"))
+          assert_equal("/signout?account_id=bar", controller.signout_path(account_id: "bar", path_params: { account_id: "baz" }))
+          assert_equal("/foo/search/quin", controller.search_path("quin"))
+        end
+      end
+
+      def test_path_params_without_default_url_options
+        with_routing do |set|
+          set.draw do
+            scope ":account_id" do
+              get "dashboard" => "pages#dashboard", as: :dashboard
+              get "search(/:term)" => "search#search", as: :search
+            end
+            delete "signout" => "sessions#destroy", as: :signout
+          end
+
+          kls = Class.new { include set.url_helpers }
+          controller = kls.new
+
+          assert_raise(ActionController::UrlGenerationError) do
+            controller.dashboard_path # missing required keys :account_id
+          end
+
+          assert_equal("/bar/dashboard", controller.dashboard_path(path_params: { account_id: "bar" }))
+          assert_equal("/signout", controller.signout_path(path_params: { account_id: "bar" }))
+          assert_equal("/signout?account_id=bar", controller.signout_path(account_id: "bar", path_params: { account_id: "baz" }))
+          assert_equal("/foo/search/quin", controller.search_path("foo", "quin"))
+        end
+      end
+
       def test_using_nil_script_name_properly_concats_with_original_script_name
         add_host!
         assert_equal("https://www.basecamphq.com/subdir/c/a/i",