Support the use of primary keys other than "id" in ActiveRecord. (#237)

Previously the diffing code made an assumption that "id" would always be
the primary field. This is not always the case, and the use of
`read_attribute(:id)` is deprecated in rails/rails@39997a0.

This changes adds support for use of custom primary key fields, and
updates the Person test model to use the "person_id" primary key.

Note that this will almost certainly still run into issues if the model
uses the newly introduced composite primary key type, so this would
require additional changes outside the scope of this fix.

Author: benk-gc

lib/super_diff/active_record/inspection_tree_builders/active_record_model.rb

@@ -6,6 +6,10 @@ def self.applies_to?(value)
           value.is_a?(::ActiveRecord::Base)
         end
 
+        def id
+          object.class.primary_key
+        end
+
         def call
           Core::InspectionTree.new do |t1|
             t1.as_lines_when_rendering_to_lines(
@@ -21,13 +25,17 @@ def call
 
             t1.nested do |t2|
               t2.insert_separated_list(
-                ["id"] + (object.attributes.keys.sort - ["id"])
+                [id] + (object.attributes.keys.sort - [id])
               ) do |t3, name|
                 t3.as_prefix_when_rendering_to_lines do |t4|
                   t4.add_text "#{name}: "
                 end
 
-                t3.add_inspection_of object.read_attribute(name)
+                if name == id
+                  t3.add_inspection_of object.id
+                else
+                  t3.add_inspection_of object.read_attribute(name)
+                end
               end
             end
 

lib/super_diff/active_record/monkey_patches.rb

@@ -2,9 +2,11 @@
 class ActiveRecord::Base
   # TODO: Remove this monkey patch if possible
   def attributes_for_super_diff
-    (attributes.keys.sort - ["id"]).reduce({ id: id }) do |hash, key|
-      hash.merge(key.to_sym => attributes[key])
-    end
+    id_attr = self.class.primary_key
+
+    (attributes.keys.sort - [id_attr]).reduce(
+      { id_attr.to_sym => id }
+    ) { |hash, key| hash.merge(key.to_sym => attributes[key]) }
   end
 end
 # rubocop:enable Style/BracesAroundHashParameters, Style/ClassAndModuleChildren

lib/super_diff/active_record/operation_tree_builders/active_record_model.rb

@@ -9,8 +9,12 @@ def self.applies_to?(expected, actual)
 
         protected
 
+        def id
+          expected.class.primary_key
+        end
+
         def attribute_names
-          ["id"] + (expected.attributes.keys.sort - ["id"])
+          [id] + (expected.attributes.keys.sort - [id])
         end
       end
     end

spec/support/integration/matchers/produce_output_when_run_matcher.rb

@@ -12,7 +12,7 @@ def initialize(expected_output)
       end
 
       def removing_object_ids
-        first_replacing(/#<([\w:]+):0x[a-f0-9]+/, '#<\1')
+        first_replacing(/#<([\w_:]+):0x[a-f0-9]+/, '#<\1')
         self
       end
 

spec/support/models/active_record/person.rb

@@ -3,6 +3,7 @@ module Test
     module Models
       module ActiveRecord
         class Person < ::ActiveRecord::Base
+          self.primary_key = "person_id"
         end
       end
     end
@@ -13,7 +14,13 @@ class Person < ::ActiveRecord::Base
   config.before do
     ActiveRecord::Base
       .connection
-      .create_table(:people, force: true) do |t|
+      .create_table(
+        :people,
+        id: false,
+        primary_key: "person_id",
+        force: true
+      ) do |t|
+        t.primary_key :person_id, null: false
         t.string :name, null: false
         t.integer :age, null: false
       end

spec/support/shared_examples/active_record.rb

@@ -86,7 +86,7 @@
                 proc do
                   line do
                     plain "Expected "
-                    actual %|#<SuperDiff::Test::Models::ActiveRecord::Person id: nil, age: 31, name: "Elliot">|
+                    actual %|#<SuperDiff::Test::Models::ActiveRecord::Person person_id: nil, age: 31, name: "Elliot">|
                   end
 
                   line do
@@ -244,7 +244,7 @@
                 proc do
                   line do
                     plain "Expected "
-                    actual %|{ name: "Marty McFly", shipping_address: #<SuperDiff::Test::Models::ActiveRecord::Person id: nil, age: 31, name: "Elliot"> }|
+                    actual %|{ name: "Marty McFly", shipping_address: #<SuperDiff::Test::Models::ActiveRecord::Person person_id: nil, age: 31, name: "Elliot"> }|
                   end
 
                   line do
@@ -265,7 +265,7 @@
                   expected_line %|-     zip: "90382"|
                   expected_line "-   }>"
                   actual_line "+   shipping_address: #<SuperDiff::Test::Models::ActiveRecord::Person {"
-                  actual_line "+     id: nil,"
+                  actual_line "+     person_id: nil,"
                   actual_line "+     age: 31,"
                   actual_line %|+     name: "Elliot"|
                   actual_line "+   }>"
@@ -390,7 +390,7 @@
                 proc do
                   line do
                     plain "Expected "
-                    actual %|[#<SuperDiff::Test::Models::ActiveRecord::Query @results=#<ActiveRecord::Relation [#<SuperDiff::Test::Models::ActiveRecord::Person id: 1, age: 20, name: "Murphy">]>>]|
+                    actual %|[#<SuperDiff::Test::Models::ActiveRecord::Query @results=#<ActiveRecord::Relation [#<SuperDiff::Test::Models::ActiveRecord::Person person_id: 1, age: 20, name: "Murphy">]>>]|
                   end
 
                   line do
@@ -404,7 +404,7 @@
                   plain_line "    #<SuperDiff::Test::Models::ActiveRecord::Query {"
                   plain_line "      @results=#<ActiveRecord::Relation ["
                   plain_line "        #<SuperDiff::Test::Models::ActiveRecord::Person {"
-                  plain_line "          id: 1,"
+                  plain_line "          person_id: 1,"
                   # expected_line %|-         age: 19,|  # TODO
                   expected_line "-         age: 19"
                   actual_line "+         age: 20,"

spec/unit/active_record/object_inspection_spec.rb

@@ -14,7 +14,7 @@
               as_lines: false
             )
           expect(string).to eq(
-            %(#<SuperDiff::Test::Models::ActiveRecord::Person id: nil, age: 31, name: "Elliot">)
+            %(#<SuperDiff::Test::Models::ActiveRecord::Person person_id: nil, age: 31, name: "Elliot">)
           )
         end
       end
@@ -42,7 +42,7 @@
               an_object_having_attributes(
                 type: :delete,
                 indentation_level: 2,
-                prefix: "id: ",
+                prefix: "person_id: ",
                 value: "nil",
                 add_comma: true
               ),
@@ -91,7 +91,7 @@
             )
 
           expect(string).to eq(
-            %(#<ActiveRecord::Relation [#<SuperDiff::Test::Models::ActiveRecord::Person id: 1, age: 19, name: "Marty">, #<SuperDiff::Test::Models::ActiveRecord::Person id: 2, age: 17, name: "Jennifer">]>)
+            %(#<ActiveRecord::Relation [#<SuperDiff::Test::Models::ActiveRecord::Person person_id: 1, age: 19, name: "Marty">, #<SuperDiff::Test::Models::ActiveRecord::Person person_id: 2, age: 17, name: "Jennifer">]>)
           )
         end
       end
@@ -132,7 +132,7 @@
               an_object_having_attributes(
                 type: :delete,
                 indentation_level: 3,
-                prefix: "id: ",
+                prefix: "person_id: ",
                 value: "1",
                 add_comma: true
               ),
@@ -165,7 +165,7 @@
               an_object_having_attributes(
                 type: :delete,
                 indentation_level: 3,
-                prefix: "id: ",
+                prefix: "person_id: ",
                 value: "2",
                 add_comma: true
               ),