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

Added has_and_belongs_to_many reification #771

Merged
merged 1 commit into from
Apr 26, 2016
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Metrics/CyclomaticComplexity:
# Offense count: 2
# Configuration parameters: CountComments.
Metrics/ModuleLength:
Max: 281
Max: 299

# Offense count: 6
Metrics/PerceivedComplexity:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
69 changes: 69 additions & 0 deletions lib/paper_trail/has_paper_trail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -46,6 +47,12 @@ module ClassMethods
# 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(if the other model is paper trailed, this option does nothing)

Are you sure that's the case? I just read through the code and don't see where that would happen. It seems like if one side of the association has join_tables set, and the other side of the association has paper_trail enabled without join_tables, this code will correctly load the versions for both sides of the association.

Perhaps you meant to say that there's no point enabling join_tables on both models?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I must have misunderstood -- HABTM models are tracked automatically, and this option exists to disable that.

I'm curious why someone would want to do that, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, if both models are paper trailed then the join table is also paper trailed, allowing a complete history of the models and associations. If only one model is paper trailed, and the other is not, then by default nothing is saved in relation to both models. However, by passing the join_tables option, the association changes are saved. You would only ever need join_tables when you want to paper trail an association but not the associated model. If you take a look at save_associations_has_and_belongs_to_many you can see the logic behind the saving.

#
def has_paper_trail(options = {})
options[:on] ||= [:create, :update, :destroy]
Expand All @@ -57,6 +64,42 @@ def has_paper_trail(options = {})
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: [] })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not do this? It seems easier to understand

model.paper_trail_habtm[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 = {})
Expand Down Expand Up @@ -442,6 +485,11 @@ def record_destroy
# 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,
Expand All @@ -463,6 +511,27 @@ def save_associations(version)
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) +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_a is unnecessary as map is implemented on ActiveRecord Relation

(@paper_trail_habtm.try(:[], a.name).try(:[], :removed) || []) -
(@paper_trail_habtm.try(:[], a.name).try(:[], :added) || [])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of this?

history = @paper_trail_habtm || {}
history = history[a.name] || Hash.new([])

assoc_ids = send(a.name).map(&:id) + history[:removed] - history[: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
Expand Down
49 changes: 45 additions & 4 deletions lib/paper_trail/reifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def reify(version, options)
has_one: false,
has_many: false,
belongs_to: false,
has_and_belongs_to_many: false,
unversioned_attributes: :nil
)

Expand Down Expand Up @@ -104,7 +105,8 @@ def prepare_array_for_has_many(array, options, versions)
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

Expand All @@ -113,7 +115,8 @@ def prepare_array_for_has_many(array, options, versions)
# 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))
}
)

Expand All @@ -128,6 +131,10 @@ def reify_associations(model, options, version)
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
Expand All @@ -154,7 +161,8 @@ def reify_has_ones(transaction_id, model, options = {})
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
Expand All @@ -181,7 +189,8 @@ def reify_belongs_tos(transaction_id, model, options = {})
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)
Expand Down Expand Up @@ -276,6 +285,38 @@ def reify_has_many_through(transaction_id, associations, model, options = {})
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.
#
Expand Down
4 changes: 4 additions & 0 deletions test/dummy/app/models/bar_habtm.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class BarHabtm < ActiveRecord::Base
has_and_belongs_to_many :foo_habtms
has_paper_trail
end
4 changes: 4 additions & 0 deletions test/dummy/app/models/foo_habtm.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
class FooHabtm < ActiveRecord::Base
has_and_belongs_to_many :bar_habtms
has_paper_trail
end
18 changes: 18 additions & 0 deletions test/dummy/db/migrate/20110208155312_set_up_test_tables.rb
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,27 @@ def up
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
Expand Down
98 changes: 98 additions & 0 deletions test/unit/associations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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