Fix monkey patch of ActiveRecord::Base for records without primary key (#282)

This fixes inspection and diffing for ActiveRecord objects without a primary key.

Author: atranson-electra

Diff for: CHANGELOG.md

@@ -12,6 +12,11 @@
 - Add official Rails 7.2 support. [#279](https://github.com./splitwise/super_diff/pull/279)
 - Add official Rails 8.0 support. [#281](https://github.com./splitwise/super_diff/pull/281)
 
+### Bug fixes
+
+- Fix ActiveRecord's `attributes_for_super_diff` and tree builders related to Active Records to handle models that do not have a primary key.
+  ([#282](https://github.com./splitwise/super_diff/pull/282))
+
 ### Other changes
 
 - Fix `logger` dependency issues in CI. [#277](https://github.com./splitwise/super_diff/pull/277)

Diff for: lib/super_diff/active_record/inspection_tree_builders/active_record_model.rb

@@ -27,7 +27,11 @@ def call
 
             t1.nested do |t2|
               t2.insert_separated_list(
-                [id] + (object.attributes.keys.sort - [id])
+                if id.nil?
+                  object.attributes.keys.sort
+                else
+                  [id] + (object.attributes.keys.sort - [id])
+                end
               ) do |t3, name|
                 t3.as_prefix_when_rendering_to_lines do |t4|
                   t4.add_text "#{name}: "

Diff for: lib/super_diff/active_record/monkey_patches.rb

@@ -7,7 +7,7 @@ def attributes_for_super_diff
     id_attr = self.class.primary_key
 
     (attributes.keys.sort - [id_attr]).reduce(
-      { id_attr.to_sym => id }
+      id_attr.nil? ? {} : { id_attr.to_sym => id }
     ) { |hash, key| hash.merge(key.to_sym => attributes[key]) }
   end
 end

Diff for: lib/super_diff/active_record/operation_tree_builders/active_record_model.rb

@@ -16,6 +16,8 @@ def id
         end
 
         def attribute_names
+          return expected.attributes.keys.sort if id.nil?
+
           [id] + (expected.attributes.keys.sort - [id])
         end
       end

Diff for: spec/support/models/active_record.rb

@@ -7,6 +7,7 @@ module ActiveRecord
         def self.define_tables
           SuperDiff::Test::Models::ActiveRecord::Person.define_table
           SuperDiff::Test::Models::ActiveRecord::ShippingAddress.define_table
+          SuperDiff::Test::Models::ActiveRecord::TimeSeriesData.define_table
         end
       end
     end

Diff for: spec/support/models/active_record/timeseries_data.rb

@@ -0,0 +1,24 @@
+# frozen_string_literal: true
+
+module SuperDiff
+  module Test
+    module Models
+      module ActiveRecord
+        class TimeSeriesData < ::ActiveRecord::Base
+          def self.define_table
+            ::ActiveRecord::Base
+              .connection
+              .create_table(
+                :time_series_data,
+                force: true,
+                id: false
+              ) do |t|
+              t.integer :value, null: false
+              t.datetime :at, null: false
+            end
+          end
+        end
+      end
+    end
+  end
+end

Diff for: spec/support/shared_examples/active_record.rb

@@ -61,6 +61,48 @@
       end
     end
 
+    context 'when comparing two instances of an ActiveRecord model that does not have a primary key' do
+      it 'produces the correct output' do
+        as_both_colored_and_uncolored do |color_enabled|
+          snippet = <<~TEST.strip
+            actual = SuperDiff::Test::Models::ActiveRecord::TimeSeriesData.new(
+              value: 456,
+              at: Time.parse('2015-07-04T17:05:37Z'),
+            )
+            expected = SuperDiff::Test::Models::ActiveRecord::TimeSeriesData.new(
+              value: 123,
+              at: Time.parse('2015-07-04T17:05:37Z'),
+            )
+            expect(actual).to eq(expected)
+          TEST
+          program = make_program(snippet, color_enabled: color_enabled)
+
+          expected_output =
+            build_expected_output(
+              color_enabled: color_enabled,
+              snippet: 'expect(actual).to eq(expected)',
+              newline_before_expectation: true,
+              expectation:
+                proc do
+                  line do
+                    plain 'Expected '
+                    actual %(#<SuperDiff::Test::Models::ActiveRecord::TimeSeriesData at: #<Time 2015-07-04 17:05:37 +00:00 (UTC)>, value: 456>)
+                  end
+
+                  line do
+                    plain '   to eq '
+                    expected %(#<SuperDiff::Test::Models::ActiveRecord::TimeSeriesData at: #<Time 2015-07-04 17:05:37 +00:00 (UTC)>, value: 123>)
+                  end
+                end
+            )
+
+          expect(program).to produce_output_when_run(expected_output).in_color(
+            color_enabled
+          )
+        end
+      end
+    end
+
     context 'when comparing instances of two different ActiveRecord models' do
       it 'produces the correct output' do
         as_both_colored_and_uncolored do |color_enabled|
@@ -105,6 +147,48 @@
       end
     end
 
+    context 'when comparing instances of two different ActiveRecord models with one not having a primary key' do
+      it 'produces the correct output' do
+        as_both_colored_and_uncolored do |color_enabled|
+          snippet = <<~TEST.strip
+            actual = SuperDiff::Test::Models::ActiveRecord::Person.new(
+              name: "Elliot",
+              age: 31,
+            )
+            expected = SuperDiff::Test::Models::ActiveRecord::TimeSeriesData.new(
+              value: 123,
+              at: Time.parse('2015-07-04T17:05:37Z'),
+            )
+            expect(actual).to eq(expected)
+          TEST
+          program = make_program(snippet, color_enabled: color_enabled)
+
+          expected_output =
+            build_expected_output(
+              color_enabled: color_enabled,
+              snippet: 'expect(actual).to eq(expected)',
+              newline_before_expectation: true,
+              expectation:
+                proc do
+                  line do
+                    plain 'Expected '
+                    actual %(#<SuperDiff::Test::Models::ActiveRecord::Person person_id: nil, age: 31, name: "Elliot">)
+                  end
+
+                  line do
+                    plain '   to eq '
+                    expected %(#<SuperDiff::Test::Models::ActiveRecord::TimeSeriesData at: #<Time 2015-07-04 17:05:37 +00:00 (UTC)>, value: 123>)
+                  end
+                end
+            )
+
+          expect(program).to produce_output_when_run(expected_output).in_color(
+            color_enabled
+          )
+        end
+      end
+    end
+
     context 'when comparing an ActiveRecord object with nothing' do
       it 'produces the correct output' do
         as_both_colored_and_uncolored do |color_enabled|

Diff for: spec/unit/active_record/object_inspection_spec.rb

@@ -79,6 +79,24 @@
       end
     end
 
+    context 'given an ActiveRecord object without a primary key' do
+      context 'given as_lines: false' do
+        it 'returns an inspected version of the object' do
+          string =
+            described_class.inspect_object(
+              SuperDiff::Test::Models::ActiveRecord::TimeSeriesData.new(
+                value: 123,
+                at: Time.parse('2015-07-04T17:05:37Z')
+              ),
+              as_lines: false
+            )
+          expect(string).to eq(
+            %(#<SuperDiff::Test::Models::ActiveRecord::TimeSeriesData at: #<Time 2015-07-04 17:05:37 +00:00 (UTC)>, value: 123>)
+          )
+        end
+      end
+    end
+
     context 'given an ActiveRecord::Relation object' do
       context 'given as_lines: false' do
         it 'returns an inspected version of the Relation' do