diff --git a/CHANGELOG.md b/CHANGELOG.md index a65b4767..397d4dcc 100644 --- a/CHANGELOG.md +++ b/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 --git a/lib/super_diff/active_record/inspection_tree_builders/active_record_model.rb b/lib/super_diff/active_record/inspection_tree_builders/active_record_model.rb index a3ebab66..519de84a 100644 --- a/lib/super_diff/active_record/inspection_tree_builders/active_record_model.rb +++ b/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 --git a/lib/super_diff/active_record/monkey_patches.rb b/lib/super_diff/active_record/monkey_patches.rb index 7b7adccc..00c2c796 100644 --- a/lib/super_diff/active_record/monkey_patches.rb +++ b/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 --git a/lib/super_diff/active_record/operation_tree_builders/active_record_model.rb b/lib/super_diff/active_record/operation_tree_builders/active_record_model.rb index 17965746..95be65bf 100644 --- a/lib/super_diff/active_record/operation_tree_builders/active_record_model.rb +++ b/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 --git a/spec/support/models/active_record.rb b/spec/support/models/active_record.rb index 9e793042..c48f8cee 100644 --- a/spec/support/models/active_record.rb +++ b/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 --git a/spec/support/models/active_record/timeseries_data.rb b/spec/support/models/active_record/timeseries_data.rb new file mode 100644 index 00000000..97839b0f --- /dev/null +++ b/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 --git a/spec/support/shared_examples/active_record.rb b/spec/support/shared_examples/active_record.rb index 74df5b95..855067e7 100644 --- a/spec/support/shared_examples/active_record.rb +++ b/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 %(#, value: 456>) + end + + line do + plain ' to eq ' + expected %(#, 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 %(#) + end + + line do + plain ' to eq ' + expected %(#, 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 --git a/spec/unit/active_record/object_inspection_spec.rb b/spec/unit/active_record/object_inspection_spec.rb index 4e1a1be7..4a9c9652 100644 --- a/spec/unit/active_record/object_inspection_spec.rb +++ b/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( + %(#, 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