Merge pull request #49020 from Shopify/allow-redefining-to-param-delimiter

Allow redefining `to_param` delimiter using `param_delimiter`

activemodel/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Add `ActiveModel::Conversion.param_delimiter` to configure delimiter being used in `to_param`
+
+    *Nikita Vasilevsky*
+
 *   `undefine_attribute_methods` undefines alias attribute methods along with attribute methods.
 
     *Nikita Vasilevsky*

activemodel/lib/active_model/conversion.rb

@@ -24,6 +24,14 @@ module ActiveModel
   module Conversion
     extend ActiveSupport::Concern
 
+    included do
+      ##
+      # :singleton-method:
+      #
+      # Accepts a string that will be used as a delimiter of object's key values in the `to_param` method.
+      class_attribute :param_delimiter, instance_reader: false, default: "-"
+    end
+
     # If your object is already designed to implement all of the \Active \Model
     # you can use the default <tt>:to_model</tt> implementation, which simply
     # returns +self+.
@@ -80,7 +88,7 @@ module ActiveModel
     #   person = Person.new(1)
     #   person.to_param # => "1"
     def to_param
-      (persisted? && key = to_key) ? key.join("-") : nil
+      (persisted? && key = to_key) ? key.join(self.class.param_delimiter) : nil
     end
 
     # Returns a +string+ identifying the path associated with the object.

activemodel/test/cases/conversion_test.rb

@@ -53,4 +53,25 @@ class ConversionTest < ActiveModel::TestCase
   test "to_partial_path handles namespaced models" do
     assert_equal "helicopter/comanches/comanche", Helicopter::Comanche.new.to_partial_path
   end
+
+  test "#to_param_delimiter allows redefining the delimiter used in #to_param" do
+    old_delimiter = Contact.param_delimiter
+    Contact.param_delimiter = "_"
+    assert_equal("abc_xyz", Contact.new(id: ["abc", "xyz"]).to_param)
+  ensure
+    Contact.param_delimiter = old_delimiter
+  end
+
+  test "#to_param_delimiter is defined per class" do
+    old_contact_delimiter = Contact.param_delimiter
+    custom_contract = Class.new(Contact)
+
+    Contact.param_delimiter = "_"
+    custom_contract.param_delimiter = ";"
+
+    assert_equal("abc_xyz", Contact.new(id: ["abc", "xyz"]).to_param)
+    assert_equal("abc;xyz", custom_contract.new(id: ["abc", "xyz"]).to_param)
+  ensure
+    Contact.param_delimiter = old_contact_delimiter
+  end
 end

activerecord/lib/active_record/base.rb

@@ -331,6 +331,8 @@ module ActiveRecord # :nodoc:
     include Suppressor
     include Normalization
     include Marshalling::Methods
+
+    self.param_delimiter = "_"
   end
 
   ActiveSupport.run_load_hooks(:active_record, Base)

activerecord/lib/active_record/integration.rb

@@ -55,8 +55,8 @@ module ActiveRecord
     #   user = User.find_by(name: 'Phusion')
     #   user_path(user)  # => "/users/Phusion"
     def to_param
-      # We can't use alias_method here, because method 'id' optimizes itself on the fly.
+      return unless id
-      id && id.to_s # Be sure to stringify the id for routes
+      Array(id).join(self.class.param_delimiter)
     end
 
     # Returns a stable cache key that can be used to identify this record.

activerecord/test/cases/integration_test.rb

@@ -6,6 +6,7 @@ require "models/developer"
 require "models/computer"
 require "models/owner"
 require "models/pet"
+require "models/cpk"
 
 class IntegrationTest < ActiveRecord::TestCase
   fixtures :companies, :developers, :owners, :pets
@@ -97,6 +98,32 @@ class IntegrationTest < ActiveRecord::TestCase
     assert_equal "Firm", Firm.to_param
   end
 
+  def test_to_param_for_a_composite_primary_key_model
+    assert_equal "1_123", Cpk::Order.new(id: [1, 123]).to_param
+  end
+
+  def test_param_delimiter_changes_delimiter_used_in_to_param
+    old_delimiter = Cpk::Order.param_delimiter
+    Cpk::Order.param_delimiter = ","
+    assert_equal("1,123", Cpk::Order.new(id: [1, 123]).to_param)
+  ensure
+    Cpk::Order.param_delimiter = old_delimiter
+  end
+
+  def test_param_delimiter_is_defined_per_class
+    old_order_delimiter = Cpk::Order.param_delimiter
+    old_book_delimiter = Cpk::Book.param_delimiter
+
+    Cpk::Order.param_delimiter = ","
+    Cpk::Book.param_delimiter = ";"
+
+    assert_equal("1,123", Cpk::Order.new(id: [1, 123]).to_param)
+    assert_equal("1;123", Cpk::Book.new(id: [1, 123]).to_param)
+  ensure
+    Cpk::Order.param_delimiter = old_order_delimiter
+    Cpk::Order.param_delimiter = old_book_delimiter
+  end
+
   def test_cache_key_for_existing_record_is_not_timezone_dependent
     utc_key = Developer.first.cache_key