Merge pull request #23636 from rab/fix-to_param-truncation

fix to_param to maximize content

3 files changed, 41 insertions, 4 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 4637847987..68772499c3 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,14 @@
+*   Fix the generated `#to_param` method to use `omission:''` so that
+    the resulting output is actually up to 20 characters, not
+    effectively 17 to leave room for the default "...".
+    Also call `#parameterize` before `#truncate` and make the
+    `separator: /-/` to maximize the information included in the
+    output.
+
+    Fixes #23635
+
+    *Rob Biedenharn*
+
 *   Ensure concurrent invocations of the connection reaper cannot allocate the
     same connection to two threads.
 
diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb
index 466c8509a4..d729deb07b 100644
--- a/activerecord/lib/active_record/integration.rb
+++ b/activerecord/lib/active_record/integration.rb
@@ -86,7 +86,7 @@ module ActiveRecord
       #
       #   user = User.find_by(name: 'David Heinemeier Hansson')
       #   user.id         # => 125
-      #   user_path(user) # => "/users/125-david"
+      #   user_path(user) # => "/users/125-david-heinemeier"
       #
       # Because the generated param begins with the record's +id+, it is
       # suitable for passing to +find+. In a controller, for example:
@@ -100,7 +100,7 @@ module ActiveRecord
           define_method :to_param do
             if (default = super()) &&
                  (result = send(method_name).to_s).present? &&
-                   (param = result.squish.truncate(20, separator: /\s/, omission: nil).parameterize).present?
+                   (param = result.squish.parameterize.truncate(20, separator: /-/, omission: '')).present?
               "#{default}-#{param}"
             else
               default
diff --git a/activerecord/test/cases/integration_test.rb b/activerecord/test/cases/integration_test.rb
index 08a186ae07..97a0cdb5db 100644
--- a/activerecord/test/cases/integration_test.rb
+++ b/activerecord/test/cases/integration_test.rb
@@ -29,10 +29,30 @@ class IntegrationTest < ActiveRecord::TestCase
     assert_equal '4-flamboyant-software', firm.to_param
   end
 
+  def test_to_param_class_method_truncates_words_properly
+    firm = Firm.find(4)
+    firm.name << ', Inc.'
+    assert_equal '4-flamboyant-software', firm.to_param
+  end
+
+  def test_to_param_class_method_truncates_after_parameterize
+    firm = Firm.find(4)
+    firm.name = "Huey, Dewey, & Louie LLC"
+    #               123456789T123456789v
+    assert_equal '4-huey-dewey-louie-llc', firm.to_param
+  end
+
+  def test_to_param_class_method_truncates_after_parameterize_with_hyphens
+    firm = Firm.find(4)
+    firm.name = "Door-to-Door Wash-n-Fold Service"
+    #               123456789T123456789v
+    assert_equal '4-door-to-door-wash-n', firm.to_param
+  end
+
   def test_to_param_class_method_truncates
     firm = Firm.find(4)
     firm.name = 'a ' * 100
-    assert_equal '4-a-a-a-a-a-a-a-a-a', firm.to_param
+    assert_equal '4-a-a-a-a-a-a-a-a-a-a', firm.to_param
   end
 
   def test_to_param_class_method_truncates_edge_case
@@ -41,10 +61,16 @@ class IntegrationTest < ActiveRecord::TestCase
     assert_equal '4-david', firm.to_param
   end
 
+  def test_to_param_class_method_truncates_case_shown_in_doc
+    firm = Firm.find(4)
+    firm.name = 'David Heinemeier Hansson'
+    assert_equal '4-david-heinemeier', firm.to_param
+  end
+
   def test_to_param_class_method_squishes
     firm = Firm.find(4)
     firm.name = "ab \n" * 100
-    assert_equal '4-ab-ab-ab-ab-ab-ab', firm.to_param
+    assert_equal '4-ab-ab-ab-ab-ab-ab-ab', firm.to_param
   end
 
   def test_to_param_class_method_multibyte_character