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 association versioning only ever returns the most recent version #4

Open
scande3 opened this issue May 30, 2018 · 3 comments

Comments

@scande3
Copy link

scande3 commented May 30, 2018

Originally filed at: paper-trail-gem/paper_trail#1095

This may be intentional despite the docs indicating otherwise but Association Versioning only ever returns the most recent version. I'll fill out the template but I've created a sample test case that shows two ways of such updates against the latest Master branch (I've spent hours trying dozens of ways):

Github: https://github.com/scande3/ptrail
Code: https://github.com/scande3/ptrail/blob/master/db/seeds.rb

Output of that will be:

Actual -- Title: Doc1 subject: Subject 1 with file path: path1


Actual -- Title: Doc1 UPDATED subject: Subject 2 with file path: path2

Version 1 -- Title: Doc1 subject: None with file path: path2


Actual -- Title: Doc1 FINAL subject: Subject 3 with file path: path3

Version 2 -- Title: Doc1 UPDATED subject: None with file path: path3

Version 1 -- Title: Doc1 subject: None with file path: path3

Attempt at the bug report template that worked for me (albeit the output is harder to read with all the debug output):

# frozen_string_literal: true

# Please include only the minimum code necessary to reproduce your issue.
require "bundler/inline"

# STEP ONE: What versions are you using?
gemfile(true) do
  ruby "2.4.0"
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "activerecord", "5.1.4"
  gem "minitest", "5.10.3"
  gem "paper_trail", github: 'paper-trail-gem/paper_trail'
  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 :docs do |t|
    t.string :title, limit: 355
    t.timestamps null: false
  end

  create_table :doc_subjects do |t|
    t.belongs_to :doc
    t.belongs_to :subject
  end
  add_index :doc_subjects, [:doc_id, :subject_id], unique: true, name: 'index_doc_to_subjects'

  create_table :subjects do |t|
    t.string :label, index: { unique: true }
  end

  create_table :doc_files do |t|
    t.belongs_to :doc
    t.string :path

    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
  end
  add_index :version_associations, [:version_id]
  add_index :version_associations, %i[foreign_key_name foreign_key_id],
    name: "index_version_associations_on_foreign_key"
end
ActiveRecord::Base.logger = Logger.new(STDOUT)
require "paper_trail/config"

# STEP THREE: Configure PaperTrail as you would in your initializer
PaperTrail::Config.instance.track_associations = true

require "paper_trail"

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

  # Normal file relationships
  has_many :doc_files

  # Has Many Through
  has_many :doc_subjects, dependent: :destroy
  has_many :subjects, :through=>:doc_subjects
end

class DocFile < ActiveRecord::Base
  has_paper_trail

  belongs_to :doc, optional: true
end

class DocSubject < ActiveRecord::Base
  belongs_to :doc
  belongs_to :subject
  has_paper_trail
end

class Subject < ActiveRecord::Base
  has_many :doc_subjects, dependent: :destroy
  has_many :docs, :through=>:doc_subjects
  has_paper_trail
end

# STEP FIVE: Write a test that demonstrates your issue by failing.
class BugTest < ActiveSupport::TestCase
  def test_1
    if Subject.count == 0
      Subject.create(label: 'Subject 1')
      Subject.create(label: 'Subject 2')
      Subject.create(label: 'Subject 3')
    end

# Create a document
    doc = Doc.new(title: 'Doc1')

    Doc.transaction do
      doc.subjects << Subject.first
      doc.doc_files << DocFile.create(path: 'path1')
      doc.save!
    end
    puts "Actual -- Title: " + doc.title + " subject: " + doc.subjects[0].label + " with file path: " + doc.doc_files[0].path
    puts "\n"
    puts "\n"

# Update a document using clears and << associations
    Doc.transaction do
      doc.subjects.clear
      doc.subjects << Subject.all[1]
      doc.doc_files.clear
      doc.doc_files << DocFile.create(path: 'path2')
      doc.title = "Doc1 UPDATED"
      doc.save!
    end

    puts "Actual -- Title: " + doc.title + " subject: " + doc.subjects[0].label + " with file path: " + doc.doc_files[0].path
    puts "\n"

    doc.versions.reverse.each_with_index do |doc_ver, index|
      doc_reified = doc_ver.reify(has_many: true, belongs_to: true, has_one: true)
      if doc_reified.present?
        title = doc_reified.title
        if doc_reified.subjects.present?
          subject = doc_reified.subjects[0].label
        else
          subject = 'None'
        end

        if doc_reified.doc_files.present?
          file = doc_reified.doc_files[0].path
        else
          file = 'None'
        end

        puts "Version " + (doc.versions.size - index - 1).to_s + " -- Title: " + title + " subject: " + subject + " with file path: " + file
        puts "\n"
      end
    end
    puts "\n"

# Updates a document using ids
    Doc.transaction do
      doc.subject_ids = [Subject.last.id]
      file = DocFile.create(path: 'path3')
      doc.doc_file_ids = [file.id]
      doc.title = "Doc1 FINAL"
      doc.save!
    end

    puts "Actual -- Title: " + doc.title + " subject: " + doc.subjects[0].label + " with file path: " + doc.doc_files[0].path
    puts "\n"

    doc.versions.reverse.each_with_index do |doc_ver, index|
      doc_reified = doc_ver.reify(has_many: true, belongs_to: true, has_one: true)
      if doc_reified.present?
        title = doc_reified.title
        if doc_reified.subjects.present?
          subject = doc_reified.subjects[0].label
        else
          subject = 'None'
        end

        if doc_reified.doc_files.present?
          file = doc_reified.doc_files[0].path
        else
          file = 'None'
        end

        puts "Version " + (doc.versions.size - index - 1).to_s + " -- Title: " + title + " subject: " + subject + " with file path: " + file
        puts "\n"
      end
    end
  end
end

# STEP SIX: Run this script using `ruby my_bug_report.rb`
@westonganger
Copy link
Owner

Can you please clarify this bug report. Please provide clear and explained description of the problems you are experiencing and what conditions cause it.

@scande3
Copy link
Author

scande3 commented Jun 4, 2018

The sample code shows the issue. To try some psuedo-code rather than actual code:

  • I have an obj (ObjA) with an Association Object (AssociationA).
  • I update that ObjA to now point to a new Association Object of AssociationB.
  • My ObjA versions will all point to AssociationB. It never captures or reifies to show AssociationA was part of ObjA at any point in time.

Essentially: Versions only ever point to the most recent association assigned to the object in "has_many" cases and loses any association version history. (It also returns nothing with "has_many through" cases that the documentation claims should work... or I might be misreading what object history this gem supports in these cases).

@westonganger
Copy link
Owner

If you'd like to take a stab at this a PR would be greatly appreciated. If not it would really help if you could attempt add it to/edit the Known Issue list in the README that would be great.

By the way, for the bug report template (or tests in general) you are supposed to use the built-in minitest assertions to compare actual and expected and not use puts. Sorry, I love puts debugging but this is not the right tool for this job.

@westonganger westonganger changed the title Experimental Association Tracking Doesn't Version Associations has_many association versioning only ever returns the most recent version Aug 14, 2020
@westonganger westonganger added help wanted Extra attention is needed unconfirmed labels Aug 14, 2020
@westonganger westonganger added unconfirmed and removed unconfirmed help wanted Extra attention is needed labels Sep 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants