issue with primary key other than id (#868)

* support reifying using primary_key field other than 'id'

* custom_primary_key_record - model to use a uuid primary key, with a weird default scope

* test custom_primary_key_record also with destroyed record

* name index on custom_primary_key_record_versions item; default is too long for mysql

* specify custom_pk_record uuid as a string primary key in a way that AR 3 supports - previous way worked with 4 and 5 but not 3

* update schema for custom_primary_key_records

* note fix in changelog

CHANGELOG.md

@@ -19,6 +19,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

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

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

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

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

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

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