diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ec2036c5..fbb409729 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ recommendations of [keepachangelog.com](http://keepachangelog.com/). ### Fixed +- [#868](https://github.com/airblade/paper_trail/pull/868) + Fix usage of find_by_id when primary key is not id, affecting reifying certain records. + ## 5.2.2 (2016-09-08) ### Breaking Changes diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index be23d86cd..9511c3dac 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -31,7 +31,8 @@ def reify(version, options) klass = version_reification_class(version, attrs) # The `dup` option always returns a new object, otherwise we should # attempt to look for the item outside of default scope(s). - if options[:dup] || (item_found = klass.unscoped.find_by_id(version.item_id)).nil? + find_cond = { klass.primary_key => version.item_id } + if options[:dup] || (item_found = klass.unscoped.where(find_cond).first).nil? model = klass.new elsif options[:unversioned_attributes] == :nil model = item_found diff --git a/spec/models/custom_primary_key_record_spec.rb b/spec/models/custom_primary_key_record_spec.rb new file mode 100644 index 000000000..1a2e777a2 --- /dev/null +++ b/spec/models/custom_primary_key_record_spec.rb @@ -0,0 +1,18 @@ +require "rails_helper" + +describe CustomPrimaryKeyRecord, type: :model do + it { is_expected.to be_versioned } + + describe "#versions" do + it "returns instances of CustomPrimaryKeyRecordVersion", versioning: true do + custom_primary_key_record = described_class.create! + custom_primary_key_record.update_attributes!(name: "bob") + version = custom_primary_key_record.versions.last + expect(version).to be_a(CustomPrimaryKeyRecordVersion) + version_from_db = CustomPrimaryKeyRecordVersion.last + expect(version_from_db.reify).to be_a(CustomPrimaryKeyRecord) + custom_primary_key_record.destroy + expect(CustomPrimaryKeyRecordVersion.last.reify).to be_a(CustomPrimaryKeyRecord) + end + end +end diff --git a/test/dummy/app/models/custom_primary_key_record.rb b/test/dummy/app/models/custom_primary_key_record.rb new file mode 100644 index 000000000..0c8e9ed19 --- /dev/null +++ b/test/dummy/app/models/custom_primary_key_record.rb @@ -0,0 +1,13 @@ +require "securerandom" +class CustomPrimaryKeyRecord < ActiveRecord::Base + self.primary_key = :uuid + + has_paper_trail class_name: "CustomPrimaryKeyRecordVersion" + # this unusual default_scope is to test the case of the Version#item association + # not returning the item due to unmatched default_scope on the model. + default_scope -> { where(name: "custom_primary_key_record") } + + before_create do + self.uuid ||= SecureRandom.uuid + end +end diff --git a/test/dummy/app/versions/custom_primary_key_record_version.rb b/test/dummy/app/versions/custom_primary_key_record_version.rb new file mode 100644 index 000000000..f90c3d5ba --- /dev/null +++ b/test/dummy/app/versions/custom_primary_key_record_version.rb @@ -0,0 +1,3 @@ +class CustomPrimaryKeyRecordVersion < PaperTrail::Version + self.table_name = "custom_primary_key_record_versions" +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 3f45e9a84..c0527d5e1 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -271,6 +271,24 @@ def up end add_index :bar_habtms_foo_habtms, [:foo_habtm_id] add_index :bar_habtms_foo_habtms, [:bar_habtm_id] + + # custom_primary_key_records use a uuid column (string) + create_table :custom_primary_key_records, id: false, force: true do |t| + t.column :uuid, :string, primary_key: true + t.string :name + t.timestamps null: true + end + + # and custom_primary_key_record_versions stores the uuid in item_id, a string + create_table :custom_primary_key_record_versions, force: true do |t| + t.string :item_type, null: false + t.string :item_id, null: false + t.string :event, null: false + t.string :whodunnit + t.text :object + t.datetime :created_at + end + add_index :custom_primary_key_record_versions, [:item_type, :item_id], name: "idx_cust_pk_item" end def down diff --git a/test/dummy/db/schema.rb b/test/dummy/db/schema.rb index 9251b8a28..944538bf0 100644 --- a/test/dummy/db/schema.rb +++ b/test/dummy/db/schema.rb @@ -80,6 +80,24 @@ t.integer "quotation_id" end + create_table "custom_primary_key_record_versions", force: :cascade do |t| + t.string "item_type", null: false + t.string "item_id", null: false + t.string "event", null: false + t.string "whodunnit" + t.text "object" + t.datetime "created_at" + end + + add_index "custom_primary_key_record_versions", ["item_type", "item_id"], name: "idx_cust_pk_item" + + create_table "custom_primary_key_records", id: false, force: :cascade do |t| + t.string "uuid" + t.string "name" + t.datetime "created_at" + t.datetime "updated_at" + end + create_table "customers", force: :cascade do |t| t.string "name" end