mongoid / mongoid-history

Multi-user non-linear history tracking, auditing, undo, redo for mongoid.
https://rubygems.org/gems/mongoid-history
MIT License
393 stars 129 forks source link

Solution for tracking has_and_belongs_to_many relations #214

Closed mikwat closed 6 years ago

mikwat commented 6 years ago

Currently, has_and_belongs_to_many relations are not tracked consistently. For example:

Given the following models:

class Post
  include Mongoid::Document
  include Mongoid::Timestamps
  include Mongoid::History::Trackable

  field :title
  field :body
  has_and_belongs_to_many :tags
  track_history on: %i[all], track_create: true, track_update: true
end

class Tag
  include Mongoid::Document

  field :title
  has_and_belongs_to_many :posts
end

Create with tags works as expected:

tag = Tag.create!
post = Post.create!(tags: [tag])
post.history_trackers.to_a
=> [#<Tracker _id: 5a579e2e017491024889a7b9, 
  created_at: 2018-01-11 17:26:06 UTC, 
  updated_at: 2018-01-11 17:26:06 UTC, 
  association_chain: [{"name"=>"Post", "id"=>BSON::ObjectId('5a579e2e017491024889a7b8')}], 
  modified: {"tag_ids"=>[BSON::ObjectId('5a579e2e017491024889a7b7')]}, 
  original: {}, 
  version: 1, 
  action: "create", 
  scope: "post", 
  modifier_id: nil>]

However, changes are not recorded:

tag = Tag.create!
tag2 = Tag.create!
post = Post.create!(tags: [tag])
post.tags << tag2
post.history_trackers.to_a
=> [#<Tracker _id: 5a579e2e017491024889a7b9, 
  created_at: 2018-01-11 17:26:06 UTC, 
  updated_at: 2018-01-11 17:26:06 UTC, 
  association_chain: [{"name"=>"Post", "id"=>BSON::ObjectId('5a579e2e017491024889a7b8')}], 
  modified: {"tag_ids"=>[BSON::ObjectId('5a579e2e017491024889a7b7')]}, 
  original: {}, 
  version: 1, 
  action: "create", 
  scope: "post", 
  modifier_id: nil>]

Unfortunately, none of the document callbacks (i.e. around_update, around_create, around_destroy) are called when modifications are made to a has_and_belongs_to_many relation. From my reading of the Mongoid docs and code, the only way to be notified of changes to these relations is to attach a callback directly to the relation definition.

While inconvenient, this proposed solution simply makes a callback method available that must be attached to each has_and_belongs_to_many relation via before_add: :track_has_and_belongs_to_many, before_remove: :track_has_and_belongs_to_many. I don't know of a way to automatically attach these callbacks, but please educate me if this is possible.

Here's an example of this solution in use:

class Post
  include Mongoid::Document
  include Mongoid::Timestamps
  include Mongoid::History::Trackable

  field :title
  field :body
  has_and_belongs_to_many :tags, before_add: :track_has_and_belongs_to_many, before_remove: :track_has_and_belongs_to_many
  track_history on: %i[all], track_create: true, track_update: true
end

class Tag
  include Mongoid::Document

  field :title
  has_and_belongs_to_many :posts
end
tag = Tag.create!
tag2 = Tag.create!
post = Post.create!(tags: [tag])
post.tags << tag2
post.history_trackers.to_a
=> [
  #<Tracker _id: 5a57aa610174912e0c3edf88, 
    created_at: 2018-01-11 18:18:09 UTC, 
    updated_at: 2018-01-11 18:18:09 UTC, 
    association_chain: [{"name"=>"Post", "id"=>BSON::ObjectId('5a57aa610174912e0c3edf87')}], 
    modified: {"tag_ids"=>[BSON::ObjectId('5a57aa610174912e0c3edf85')]}, 
    original: {}, 
    version: 1, 
    action: "create", 
    scope: "post", 
    modifier_id: nil>, 
  #<Tracker _id: 5a57aa610174912e0c3edf89, 
    created_at: 2018-01-11 18:18:09 UTC, 
    updated_at: 2018-01-11 18:18:09 UTC, 
    association_chain: [{"name"=>"Post", "id"=>BSON::ObjectId('5a57aa610174912e0c3edf87')}], 
    modified: {"tag_ids"=>[BSON::ObjectId('5a57aa610174912e0c3edf85'), BSON::ObjectId('5a57aa610174912e0c3edf86')]}, 
    original: {"tag_ids"=>[BSON::ObjectId('5a57aa610174912e0c3edf85')]}, 
    version: 2, 
    action: "update", 
    scope: "post", 
    modifier_id: nil>
]
mongoid-bot commented 6 years ago
1 Warning
:warning: Unless you’re refactoring existing code, please update CHANGELOG.md.

Here's an example of a CHANGELOG.md entry:

* [#214](https://github.com/mongoid/mongoid-history/pull/214): Solution for tracking has_and_belongs_to_many relations - [@mikwat](https://github.com/mikwat).

Generated by :no_entry_sign: danger

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.002%) to 99.805% when pulling 7f255d6d073adc6abce28180d7bd4be03f9979e2 on spec-has-and-belongs-to-many into bc5a0fa60d404f9c119c84025c9f5cf362877962 on master.

mikwat commented 6 years ago

Is the expected behavior, @dblock ? Basically, it appears that updates to has_and_belongs_to_many relations aren't tracked.

dblock commented 6 years ago

I would expect everything to be tracked, so we should call this a bug. Feel free to try a fix!

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.004%) to 99.807% when pulling 22c7fb15a5e1e787bd9b82e44af516af9ae6c825 on spec-has-and-belongs-to-many into bc5a0fa60d404f9c119c84025c9f5cf362877962 on master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.004%) to 99.807% when pulling 4f8dbd667c4c72d069037dc35ac2e1c2fdd2fa76 on spec-has-and-belongs-to-many into bc5a0fa60d404f9c119c84025c9f5cf362877962 on master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.004%) to 99.807% when pulling 4f8dbd667c4c72d069037dc35ac2e1c2fdd2fa76 on spec-has-and-belongs-to-many into bc5a0fa60d404f9c119c84025c9f5cf362877962 on master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.006%) to 99.809% when pulling 1055a28d78ab36162987187e5c24b294f3f89008 on spec-has-and-belongs-to-many into bc5a0fa60d404f9c119c84025c9f5cf362877962 on master.

mikwat commented 6 years ago

@dblock When you have a chance, let me know your thoughts.

dblock commented 6 years ago

I think we should be able to attach those callbacks by examining all HABTM relationships in late binding, similarly to how we examine tracked fields. This would avoid the whole explicit options to the relationship, wouldn't it?

mikwat commented 6 years ago

That would be great, but I'm not sure it's possible. reflect_on_all_associations(:has_and_belongs_to_many) returns an array of Mongoid::Relations::Metadata instances, but how would you use these to add the necessary callbacks on the relation?

dblock commented 6 years ago

Maybe @estolfo can help with some pointers, please?

dblock commented 6 years ago

@mikwat Unrelated, but to just keep the parent repo clean maybe work off a fork? My OCD is kicking in :)

mikwat commented 6 years ago

Of course @dblock Sorry about that.

mikwat commented 6 years ago

Moved to https://github.com/mongoid/mongoid-history/pull/215