From 8b528ca9de1e829446b4581c2f42ded026fa9ce2 Mon Sep 17 00:00:00 2001 From: Sam Boylett Date: Tue, 5 Apr 2016 10:37:24 +0100 Subject: [PATCH] Added has_and_belongs_to_many reification HABTM associations are saved in the version_assocations table with the foreign_key_name set to the association name. They are saved if either the associated model is being paper trailed, or if the option join_tables: [:association_name] is passed to the has_paper_trail declaration. If the option is passed but the associated model is not paper trailed, only the join model will be saved. This means reification of HABTM would reify the associated objects but in their current state. If the associated model is paper trailed, this option does nothing, and the version of the model at the time is reified. --- .rubocop_todo.yml | 2 +- CHANGELOG.md | 2 + lib/paper_trail/has_paper_trail.rb | 69 +++++++++++++ lib/paper_trail/reifier.rb | 49 +++++++++- test/dummy/app/models/bar_habtm.rb | 4 + test/dummy/app/models/foo_habtm.rb | 4 + .../20110208155312_set_up_test_tables.rb | 18 ++++ test/unit/associations_test.rb | 98 +++++++++++++++++++ 8 files changed, 241 insertions(+), 5 deletions(-) create mode 100644 test/dummy/app/models/bar_habtm.rb create mode 100644 test/dummy/app/models/foo_habtm.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7e33958f..2a73277f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -12,7 +12,7 @@ Metrics/CyclomaticComplexity: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ModuleLength: - Max: 281 + Max: 299 # Offense count: 6 Metrics/PerceivedComplexity: diff --git a/CHANGELOG.md b/CHANGELOG.md index 79038dd0..f8b3725a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ ### Added +- [#771](https://github.com/airblade/paper_trail/pull/771) - + Added support for has_and_belongs_to_many associations - [#741](https://github.com/airblade/paper_trail/issues/741) / [#681](https://github.com/airblade/paper_trail/pull/681) MySQL unicode support in migration generator diff --git a/lib/paper_trail/has_paper_trail.rb b/lib/paper_trail/has_paper_trail.rb index b570c5da..6a26c429 100644 --- a/lib/paper_trail/has_paper_trail.rb +++ b/lib/paper_trail/has_paper_trail.rb @@ -6,6 +6,7 @@ module PaperTrail module Model def self.included(base) base.send :extend, ClassMethods + base.send :attr_accessor, :paper_trail_habtm end # :nodoc: @@ -46,6 +47,12 @@ module PaperTrail # the instance was reified from. Default is `:version`. # - :save_changes - Whether or not to save changes to the object_changes # column if it exists. Default is true + # - :join_tables - If the model has a has_and_belongs_to_many relation + # with an unpapertrailed model, passing the name of the association to + # the join_tables option will paper trail the join table but not save + # the other model, allowing reification of the association but with the + # other models latest state (if the other model is paper trailed, this + # option does nothing) # def has_paper_trail(options = {}) options[:on] ||= [:create, :update, :destroy] @@ -57,6 +64,42 @@ module PaperTrail setup_model_for_paper_trail(options) setup_callbacks_from_options options[:on] + + setup_callbacks_for_habtm options[:join_tables] + end + + def update_for_callback(name, callback, model, assoc) + model.paper_trail_habtm ||= {} + model.paper_trail_habtm.reverse_merge!(name => { removed: [], added: [] }) + case callback + when :before_add + model.paper_trail_habtm[name][:added] |= [assoc.id] + model.paper_trail_habtm[name][:removed] -= [assoc.id] + when :before_remove + model.paper_trail_habtm[name][:removed] |= [assoc.id] + model.paper_trail_habtm[name][:added] -= [assoc.id] + end + end + + attr_reader :paper_trail_save_join_tables + + def setup_callbacks_for_habtm(join_tables) + @paper_trail_save_join_tables = Array.wrap(join_tables) + # Adds callbacks to record changes to habtm associations such that on + # save the previous version of the association (if changed) can be + # interpreted + reflect_on_all_associations(:has_and_belongs_to_many). + reject { |a| paper_trail_options[:skip].include?(a.name.to_s) }. + each do |a| + added_callback = lambda do |*args| + update_for_callback(a.name, :before_add, args[-2], args.last) + end + removed_callback = lambda do |*args| + update_for_callback(a.name, :before_remove, args[-2], args.last) + end + send(:"before_add_for_#{a.name}").send(:<<, added_callback) + send(:"before_remove_for_#{a.name}").send(:<<, removed_callback) + end end def setup_model_for_paper_trail(options = {}) @@ -442,6 +485,11 @@ module PaperTrail # Saves associations if the join table for `VersionAssociation` exists. def save_associations(version) return unless PaperTrail.config.track_associations? + save_associations_belongs_to(version) + save_associations_has_and_belongs_to_many(version) + end + + def save_associations_belongs_to(version) self.class.reflect_on_all_associations(:belongs_to).each do |assoc| assoc_version_args = { version_id: version.id, @@ -463,6 +511,27 @@ module PaperTrail end end + def save_associations_has_and_belongs_to_many(version) + # Use the :added and :removed keys to extrapolate the HABTM associations + # to before any changes were made + self.class.reflect_on_all_associations(:has_and_belongs_to_many).each do |a| + next unless + self.class.paper_trail_save_join_tables.include?(a.name) || + a.klass.paper_trail_enabled_for_model? + assoc_version_args = { + version_id: version.id, + foreign_key_name: a.name + } + assoc_ids = + send(a.name).to_a.map(&:id) + + (@paper_trail_habtm.try(:[], a.name).try(:[], :removed) || []) - + (@paper_trail_habtm.try(:[], a.name).try(:[], :added) || []) + assoc_ids.each do |id| + PaperTrail::VersionAssociation.create(assoc_version_args.merge(foreign_key_id: id)) + end + end + end + def reset_transaction_id PaperTrail.transaction_id = nil end diff --git a/lib/paper_trail/reifier.rb b/lib/paper_trail/reifier.rb index 7d3edbc3..8ca938ad 100644 --- a/lib/paper_trail/reifier.rb +++ b/lib/paper_trail/reifier.rb @@ -14,6 +14,7 @@ module PaperTrail has_one: false, has_many: false, belongs_to: false, + has_and_belongs_to_many: false, unversioned_attributes: :nil ) @@ -104,7 +105,8 @@ module PaperTrail elsif version.event == "create" options[:mark_for_destruction] ? record.tap(&:mark_for_destruction) : nil else - version.reify(options.merge(has_many: false, has_one: false, belongs_to: false)) + version.reify(options.merge(has_many: false, has_one: false, belongs_to: false, + has_and_belongs_to_many: false)) end end @@ -113,7 +115,8 @@ module PaperTrail # associations. array.concat( versions.values.map { |v| - v.reify(options.merge(has_many: false, has_one: false, belongs_to: false)) + v.reify(options.merge(has_many: false, has_one: false, belongs_to: false, + has_and_belongs_to_many: false)) } ) @@ -128,6 +131,10 @@ module PaperTrail reify_belongs_tos version.transaction_id, model, options if options[:belongs_to] reify_has_manys version.transaction_id, model, options if options[:has_many] + + if options[:has_and_belongs_to_many] + reify_has_and_belongs_to_many version.transaction_id, model, options + end end # Restore the `model`'s has_one associations as they were when this @@ -154,7 +161,8 @@ module PaperTrail end end else - child = version.reify(options.merge(has_many: false, has_one: false, belongs_to: false)) + child = version.reify(options.merge(has_many: false, has_one: false, belongs_to: false, + has_and_belongs_to_many: false)) model.appear_as_new_record do without_persisting(child) do model.send "#{assoc.name}=", child @@ -181,7 +189,8 @@ module PaperTrail assoc.klass.where(assoc.klass.primary_key => collection_key).first else version.reify(options.merge(has_many: false, has_one: false, - belongs_to: false)) + belongs_to: false, + has_and_belongs_to_many: false)) end model.send("#{assoc.name}=".to_sym, collection) @@ -276,6 +285,38 @@ module PaperTrail end end + def reify_has_and_belongs_to_many(transaction_id, model, options = {}) + model.class.reflect_on_all_associations(:has_and_belongs_to_many).each do |a| + papertrail_enabled = a.klass.paper_trail_enabled_for_model? + next unless + model.class.paper_trail_save_join_tables.include?(a.name) || + papertrail_enabled + + version_ids = PaperTrail::VersionAssociation. + where("foreign_key_name = ?", a.name). + where("version_id = ?", transaction_id). + pluck(:foreign_key_id) + + model.send(a.name).proxy_association.target = + version_ids.map do |id| + if papertrail_enabled + version = a.klass.paper_trail_version_class. + where("item_type = ?", a.klass.name). + where("item_id = ?", id). + where("created_at >= ? OR transaction_id = ?", + options[:version_at], transaction_id). + order("id").limit(1).first + if version + next version.reify(options.merge(has_many: false, has_one: false, + belongs_to: false, + has_and_belongs_to_many: false)) + end + end + a.klass.where(a.klass.primary_key => id).first + end + end + end + # Given a `version`, return the class to reify. This method supports # Single Table Inheritance (STI) with custom inheritance columns. # diff --git a/test/dummy/app/models/bar_habtm.rb b/test/dummy/app/models/bar_habtm.rb new file mode 100644 index 00000000..2739e1d8 --- /dev/null +++ b/test/dummy/app/models/bar_habtm.rb @@ -0,0 +1,4 @@ +class BarHabtm < ActiveRecord::Base + has_and_belongs_to_many :foo_habtms + has_paper_trail +end diff --git a/test/dummy/app/models/foo_habtm.rb b/test/dummy/app/models/foo_habtm.rb new file mode 100644 index 00000000..6ce32790 --- /dev/null +++ b/test/dummy/app/models/foo_habtm.rb @@ -0,0 +1,4 @@ +class FooHabtm < ActiveRecord::Base + has_and_belongs_to_many :bar_habtms + has_paper_trail +end diff --git a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb index 28f7faf9..6c233314 100644 --- a/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb +++ b/test/dummy/db/migrate/20110208155312_set_up_test_tables.rb @@ -249,9 +249,27 @@ class SetUpTestTables < ActiveRecord::Migration create_table :citations, force: true do |t| t.integer :quotation_id end + + create_table :foo_habtms, force: true do |t| + t.string :name + end + + create_table :bar_habtms, force: true do |t| + t.string :name + end + + create_table :bar_habtms_foo_habtms, force: true, id: false do |t| + t.integer :foo_habtm_id + t.integer :bar_habtm_id + end + add_index :bar_habtms_foo_habtms, [:foo_habtm_id] + add_index :bar_habtms_foo_habtms, [:bar_habtm_id] end def down + drop_table :bar_habtms_foo_habtms + drop_table :foo_habtms + drop_table :bar_habtms drop_table :citations drop_table :quotations drop_table :animals diff --git a/test/unit/associations_test.rb b/test/unit/associations_test.rb index 91c6a9e7..ce2caa96 100644 --- a/test/unit/associations_test.rb +++ b/test/unit/associations_test.rb @@ -891,4 +891,102 @@ class AssociationsTest < ActiveSupport::TestCase end end end + + context "has_and_belongs_to_many associations" do + context "foo and bar" do + setup do + @foo = FooHabtm.create(name: "foo") + Timecop.travel 1.second.since + end + + context "where the association is created between model versions" do + setup do + @foo.update_attributes(name: "foo1", bar_habtms: [BarHabtm.create(name: "bar")]) + end + + context "when reified" do + setup { @reified = @foo.versions.last.reify(has_and_belongs_to_many: true) } + + should "see the associated as it was at the time" do + assert_equal 0, @reified.bar_habtms.length + end + + should "not persist changes to the live association" do + assert_not_equal @reified.bar_habtms, @foo.reload.bar_habtms + end + end + end + + context "where the association is changed between model versions" do + setup do + @foo.update_attributes(name: "foo2", bar_habtms: [BarHabtm.create(name: "bar2")]) + Timecop.travel 1.second.since + @foo.update_attributes(name: "foo3", bar_habtms: [BarHabtm.create(name: "bar3")]) + end + + context "when reified" do + setup { @reified = @foo.versions.last.reify(has_and_belongs_to_many: true) } + + should "see the association as it was at the time" do + assert_equal "bar2", @reified.bar_habtms.first.name + end + + should "not persist changes to the live association" do + assert_not_equal @reified.bar_habtms.first, @foo.reload.bar_habtms.first + end + end + + context "when reified with has_and_belongs_to_many: false" do + setup { @reified = @foo.versions.last.reify } + + should "see the association as it is now" do + assert_equal "bar3", @reified.bar_habtms.first.name + end + end + end + + context "where the association is destroyed between model versions" do + setup do + @foo.update_attributes(name: "foo2", bar_habtms: [BarHabtm.create(name: "bar2")]) + Timecop.travel 1.second.since + @foo.update_attributes(name: "foo3", bar_habtms: []) + end + + context "when reified" do + setup { @reified = @foo.versions.last.reify(has_and_belongs_to_many: true) } + + should "see the association as it was at the time" do + assert_equal "bar2", @reified.bar_habtms.first.name + end + + should "not persist changes to the live association" do + assert_not_equal @reified.bar_habtms.first, @foo.reload.bar_habtms.first + end + end + end + + context "where the unassociated model changes" do + setup do + @bar = BarHabtm.create(name: "bar2") + @foo.update_attributes(name: "foo2", bar_habtms: [@bar]) + Timecop.travel 1.second.since + @foo.update_attributes(name: "foo3", bar_habtms: [BarHabtm.create(name: "bar4")]) + Timecop.travel 1.second.since + @bar.update_attributes(name: "bar3") + end + + context "when reified" do + setup { @reified = @foo.versions.last.reify(has_and_belongs_to_many: true) } + + should "see the association as it was at the time" do + assert_equal "bar2", @reified.bar_habtms.first.name + end + + should "not persist changes to the live association" do + assert_not_equal @reified.bar_habtms.first, @foo.reload.bar_habtms.first + end + end + end + end + end end