Merge pull request #182 from ifad/bugfix/fix-historical-relation-181

Bugfix/fix historical relation 181

lib/chrono_model/time_machine/history_model.rb

@@ -221,6 +221,20 @@ def recorded_at
         ChronoModel::Conversions.string_to_utc_time attributes_before_type_cast['recorded_at']
       end
 
+      # Starting from Rails 6.0, `.read_attribute` will use the memoized
+      # `primary_key` if it detects that the attribute name is `id`.
+      #
+      # Since the `primary key` may have been changed to `hid` because of
+      # `.find` overload, the new behavior may break relations where `id` is
+      # still the correct attribute to read
+      #
+      # Ref: ifad/chronomodel#181
+      def read_attribute(attr_name, &block)
+        return super unless attr_name.to_s == 'id' && @primary_key.to_s == 'hid'
+
+        _read_attribute('id', &block)
+      end
+
       private
 
       def with_hid_pkey

spec/chrono_model/time_machine/history_spec.rb

@@ -54,6 +54,27 @@
       it { expect($t.foo.history.except(:order).select('max(id) as foo, min(id) as bar').group('id').first.attributes.keys).to match_array %w[id foo bar] }
     end
 
+    context 'when finding historical elements by hid' do
+      subject(:historical_foo) { Foo::History.find(last_foo.hid) }
+
+      let(:last_foo) { Foo.history.last }
+
+      before do
+        Tar.create(foo: last_foo)
+      end
+
+      it { is_expected.to eq last_foo }
+
+      it 'preserves associations' do
+        expect(historical_foo.bars).to eq last_foo.bars
+      end
+
+      it 'preserves associations that are using a different fkid' do
+        expect(historical_foo.tars).not_to be_empty
+        expect(historical_foo.tars).to eq(last_foo.tars)
+      end
+    end
+
     context '.sorted' do
       describe 'orders by recorded_at, hid' do
         it { expect($t.foo.history.sorted.to_sql).to match(/order by .+"recorded_at" ASC, .+"hid" ASC/i) }

spec/support/time_machine/structure.rb

@@ -46,6 +46,18 @@ module ChronoTest::TimeMachine
     t.string     :name
     t.integer    :fooity
     t.references :goo
+    t.string     :refee_foo
+  end
+
+  adapter.create_table 'tars', temporal: true do |t|
+    t.string     :name
+    t.string     :foo_refering
+  end
+
+  adapter.change_table 'tars', temporal: true do
+    def up
+      add_reference :foos, column: :foo_refering, primary_key: :refee_foo
+    end
   end
 
   adapter.create_table 'moos', temporal: true do |t|
@@ -80,6 +92,15 @@ class ::Baz < ActiveRecord::Base
     has_timeline with: :bar
   end
 
+  class ::Tar < ActiveRecord::Base
+    include ChronoModel::TimeGate
+
+    belongs_to :foo, foreign_key: :foo_refering, primary_key: :refee_foo, class_name: 'Foo'
+
+    has_timeline with: :foo
+  end
+
+
   class ::Boo < ActiveRecord::Base
     include ChronoModel::TimeMachine
 
@@ -90,6 +111,7 @@ class ::Foo < ActiveRecord::Base
     include ChronoModel::TimeMachine
 
     has_many :bars
+    has_many :tars, foreign_key: :foo_refering, primary_key: :refee_foo
     has_many :sub_bars, through: :bars
     has_many :sub_sub_bars, through: :sub_bars