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

Solution for tracking has_and_belongs_to_many relations #215

Closed
wants to merge 1 commit into from

Conversation

mikwat
Copy link
Collaborator

@mikwat mikwat commented Jan 12, 2018

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
Copy link

mongoid-bot commented Jan 12, 2018

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.

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

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

Generated by 🚫 danger

@coveralls
Copy link

coveralls commented Jan 12, 2018

Coverage Status

Coverage increased (+0.006%) to 99.809% when pulling 93acc0c on mikwat:has-and-belongs-to-many into bc5a0fa on mongoid:master.

@mikwat
Copy link
Collaborator Author

mikwat commented Jan 12, 2018

Carryover of conversation from #214:

@dblock: 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: 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: Maybe @estolfo can help with some pointers, please?

@dblock
Copy link
Collaborator

dblock commented Jan 12, 2018

I've tried to build something by following the embeds model. I can get to the association, but don't know how to insert the callback from the association. WIP in dblock@1209422.

@mikwat
Copy link
Collaborator Author

mikwat commented Jan 12, 2018

I like the direction you're going with this @dblock, but I think we're running into the same problem.

@dblock
Copy link
Collaborator

dblock commented Jan 18, 2018

So we can add the callback by overwriting has_and_belongs_to_many, but I am not sure we should. See #217. Thoughts? We can leave that one open and carry over the conversation since it passes tests and such.

@johnnyshields
Copy link
Member

johnnyshields commented Jan 18, 2018

We should only modify the callbacks in cases where the habtm attributes have been explicitly whitelisted.

@mikwat
Copy link
Collaborator Author

mikwat commented Jan 18, 2018

See #217

@mikwat mikwat closed this Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants