Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix/fix historical relation 181 #182

Merged
merged 4 commits into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/chrono_model/time_machine/history_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions spec/chrono_model/time_machine/history_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
tagliala marked this conversation as resolved.
Show resolved Hide resolved
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) }
Expand Down
22 changes: 22 additions & 0 deletions spec/support/time_machine/structure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand Down