From 23ffbdc7e1ecebb37164cf0c62471c770f65484d Mon Sep 17 00:00:00 2001 From: Jared Beck Date: Sun, 1 May 2016 00:21:51 -0400 Subject: [PATCH] Fix NoMethodError in load_changeset Fixes NoMethodError when an STI parent class is unversioned and thus does not respond to `unserialize_attribute_changes_for_paper_trail!` [Fixes #738] --- lib/paper_trail/version_concern.rb | 11 ++++++++++- spec/models/car_spec.rb | 13 +++++++++++++ spec/models/truck_spec.rb | 5 +++++ spec/models/vehicle_spec.rb | 5 +++++ test/dummy/app/models/car.rb | 3 +++ test/dummy/app/models/truck.rb | 4 ++++ test/dummy/app/models/vehicle.rb | 4 ++++ .../db/migrate/20110208155312_set_up_test_tables.rb | 7 +++++++ test/dummy/db/schema.rb | 7 +++++++ test/unit/model_test.rb | 4 ++++ 10 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 spec/models/car_spec.rb create mode 100644 spec/models/truck_spec.rb create mode 100644 spec/models/vehicle_spec.rb create mode 100644 test/dummy/app/models/car.rb create mode 100644 test/dummy/app/models/truck.rb create mode 100644 test/dummy/app/models/vehicle.rb diff --git a/lib/paper_trail/version_concern.rb b/lib/paper_trail/version_concern.rb index 0d7289aa..4ab0bd4f 100644 --- a/lib/paper_trail/version_concern.rb +++ b/lib/paper_trail/version_concern.rb @@ -283,7 +283,16 @@ module PaperTrail # and appears to be responsible for custom attribute serializers. For an # example of a custom attribute serializer, see # `Person::TimeZoneSerializer` in the test suite. - item_type.constantize.unserialize_attribute_changes_for_paper_trail!(changes) + # + # Is `item.class` good enough? Does it handle `inheritance_column` + # as well as `Reifier#version_reification_class`? We were using + # `item_type.constantize`, but that is problematic when the STI parent + # is not versioned. (See `Vehicle` and `Car` in the test suite). + # + # Note: `item` returns nil if `event` is "destroy". + unless item.nil? + item.class.unserialize_attribute_changes_for_paper_trail!(changes) + end # Finally, return a Hash mapping each attribute name to # a two-element array representing before and after. diff --git a/spec/models/car_spec.rb b/spec/models/car_spec.rb new file mode 100644 index 00000000..67bbc240 --- /dev/null +++ b/spec/models/car_spec.rb @@ -0,0 +1,13 @@ +require "rails_helper" + +describe Car, type: :model do + it { is_expected.to be_versioned } + + describe "changeset", versioning: true do + it "has the expected keys (see issue 738)" do + car = Car.create!(name: "Alice") + car.update_attributes(name: "Bob") + assert_includes car.versions.last.changeset.keys, "name" + end + end +end diff --git a/spec/models/truck_spec.rb b/spec/models/truck_spec.rb new file mode 100644 index 00000000..41089db0 --- /dev/null +++ b/spec/models/truck_spec.rb @@ -0,0 +1,5 @@ +require "rails_helper" + +describe Truck, type: :model do + it { is_expected.to_not be_versioned } +end diff --git a/spec/models/vehicle_spec.rb b/spec/models/vehicle_spec.rb new file mode 100644 index 00000000..5845018c --- /dev/null +++ b/spec/models/vehicle_spec.rb @@ -0,0 +1,5 @@ +require "rails_helper" + +describe Vehicle, type: :model do + it { is_expected.to_not be_versioned } +end diff --git a/test/dummy/app/models/car.rb b/test/dummy/app/models/car.rb new file mode 100644 index 00000000..d7947b6f --- /dev/null +++ b/test/dummy/app/models/car.rb @@ -0,0 +1,3 @@ +class Car < Vehicle + has_paper_trail +end diff --git a/test/dummy/app/models/truck.rb b/test/dummy/app/models/truck.rb new file mode 100644 index 00000000..8d4ff2fb --- /dev/null +++ b/test/dummy/app/models/truck.rb @@ -0,0 +1,4 @@ +class Truck < Vehicle + # This STI child class specifically does not call `has_paper_trail`. + # Of the sub-classes of `Vehicle`, only `Car` is versioned. +end diff --git a/test/dummy/app/models/vehicle.rb b/test/dummy/app/models/vehicle.rb new file mode 100644 index 00000000..5473b10f --- /dev/null +++ b/test/dummy/app/models/vehicle.rb @@ -0,0 +1,4 @@ +class Vehicle < ActiveRecord::Base + # This STI parent class specifically does not call `has_paper_trail`. + # Of the sub-classes of `Vehicle`, only `Car` is versioned. +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index 8af5449e..1b0186de 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -9,6 +9,13 @@ class SetUpTestTables < ActiveRecord::Migration TEXT_BYTES = 1_073_741_823 def up + # Classes: Vehicle, Car, Truck + create_table :vehicles, force: true do |t| + t.string :name, null: false + t.string :type, null: false + t.timestamps null: false + end + create_table :skippers, force: true do |t| t.string :name t.datetime :another_timestamp diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 26576deb..a35649e7 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -201,6 +201,13 @@ ActiveRecord::Schema.define(version: 20110208155312) do t.string "type" end + create_table "vehicles", force: :cascade do |t| + t.string "name", null: false + t.string "type", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "version_associations", force: :cascade do |t| t.integer "version_id" t.string "foreign_key_name", null: false diff --git a/test/unit/model_test.rb b/test/unit/model_test.rb index acf8bc1b..c586aeb3 100644 --- a/test/unit/model_test.rb +++ b/test/unit/model_test.rb @@ -433,6 +433,10 @@ class HasPaperTrailModelTest < ActiveSupport::TestCase assert_equal 1, @reified_widget.fluxors.length end + should "have nil item for last version" do + assert_nil(@widget.versions.last.item) + end + should "not have changes" do assert_equal({}, @widget.versions.last.changeset) end