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

has_many associations defined on intermediate STI classes fail to reify #23

Closed
JaciBrunning opened this issue Jan 1, 2021 · 0 comments · Fixed by #24
Closed

has_many associations defined on intermediate STI classes fail to reify #23

JaciBrunning opened this issue Jan 1, 2021 · 0 comments · Fixed by #24

Comments

@JaciBrunning
Copy link
Contributor

JaciBrunning commented Jan 1, 2021

In an example such as the following, PT-AT fails to reify the students association, always returning the latest result as if .reify() was called without has_many: true.

class Individual < ActiveRecord::Base
  has_paper_trail
end

class Teacher < Individual
  has_many :students, class_name: "Studentship"
end

class Professor < Teacher
end

class Student < Individual
end

class Studentship < ActiveRecord::Base
  belongs_to :teacher
  belongs_to :student
  has_paper_trail
end

This is demonstrated in the following bug report script:

Bug Report Script
### Bug Report Template

require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "2.6"
  source "https://rubygems.org"
  gem "activerecord", "6.0.2"
  gem "minitest"
  gem "paper_trail", "~>10.3.0"
  gem "paper_trail-association_tracking"
  gem "sqlite3"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Please use sqlite for your bug reports, if possible.
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Base.logger = nil

ActiveRecord::Schema.define do
  # STEP TWO: Define your tables here.
  create_table :individuals, force: true do |t|
    t.text :name, null: false
    t.text :flag, null: false
    t.string :type, null: false
    t.timestamps null: false
  end

  create_table :studentships, force: true do |t|
    t.integer :teacher_id, null: false
    t.integer :student_id, null: false 
    t.timestamps null: false
  end

  create_table :versions do |t|
    t.string :item_type, null: false
    t.integer :item_id, null: false
    t.string :event, null: false
    t.string :whodunnit
    t.text :object, limit: 1_073_741_823
    t.text :object_changes, limit: 1_073_741_823
    t.integer :transaction_id
    t.datetime :created_at
  end
  add_index :versions, %i[item_type item_id]
  add_index :versions, [:transaction_id]

  create_table :version_associations do |t|
    t.integer  :version_id
    t.string   :foreign_key_name, null: false
    t.integer  :foreign_key_id
    t.string   :foreign_type            # NOTE: bug report template is out of date, this wasn't in there :) 
  end
  add_index :version_associations, [:version_id]
  add_index :version_associations, %i[foreign_key_name foreign_key_id foreign_type],
    name: "index_version_associations_on_foreign_key"
end

ActiveRecord::Base.logger = Logger.new(STDOUT)

require "paper_trail"
require "paper_trail-association_tracking"

PaperTrail.config.track_associations = true

# STEP FOUR: Define your AR models here.
class Individual < ActiveRecord::Base
  has_paper_trail
end

class Teacher < Individual
  has_many :students, class_name: "Studentship"
end

class Professor < Teacher
end

class Student < Individual
end

class Studentship < ActiveRecord::Base
  belongs_to :teacher
  belongs_to :student
  has_paper_trail
end

# STEP FIVE: Please write a test that demonstrates your issue.
class BugTest < ActiveSupport::TestCase
  def test_sti
    teach1 = Professor.create!(name: "Prof. Alice", flag: "v1")   # Flag is used to trigger a new version
    student1 = Student.create!(name: "Mr Student", flag: "v1")
    student2 = Student.create!(name: "Miss Student", flag: "v1")

    teach1.students.create!(student: student1)

    teach1.update flag: "v2"
    teach1.students.destroy_all

    teach1.update flag: "v3"
    teach1.students.create!(student: student2)

    restore_version = ->(idx) { teach1.versions[idx].reify(has_many: true) }

    # v1 - Prof. Alice should have one student - Mr Student
    v1 = restore_version.call(1)
    assert_equal 1, v1.students.size
    assert_equal student1, v1.students.first.student

    # v2 - Prof. Alice should have NO students
    v2 = restore_version.call(2)
    assert_equal 0, v2.students.size

    # v3 - Prof. Alice should have one student - Miss Student
    v3 = teach1
    assert_equal 1, v3.students.size
    assert_equal student2, v3.students.first.student
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`
# Error log:
#   1) Failure:
# BugTest#test_sti [my_bug_report copy.rb:114]:
# --- expected
# +++ actual
# @@ -1 +1 @@
# -#<Student id: 2, name: "Mr Student", flag: "v1", type: "Student", created_at: "2021-01-01 11:10:53", updated_at: "2021-01-01 11:10:53">
# +#<Student id: 3, name: "Miss Student", flag: "v1", type: "Student", created_at: "2021-01-01 11:10:53", updated_at: "2021-01-01 11:10:53">

I have a functional PR that addresses this issue that I will be submitting momentarily, just working on the tests.

JaciBrunning added a commit to JaciBrunning/paper_trail-association_tracking that referenced this issue Jan 1, 2021
JaciBrunning added a commit to JaciBrunning/paper_trail-association_tracking that referenced this issue Jan 1, 2021
westonganger pushed a commit that referenced this issue Jan 4, 2021
* Fix #23 by referencing all valid ancestors, not just the model class.

* Add tests for issue #23

* filter -> select for Ruby 2.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant