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

Support for tracking has_and_belongs_to_many associations. #217

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dblock
Copy link
Collaborator

@dblock dblock commented Jan 18, 2018

Replaces #215.

@mikwat Check this out? I find the overwriting of has_and_belongs_to_many very ugly and the whole code here feels a lot like a hack.

Also I bet undo, redo and such don't work in these scenarios, needs specs.

@coveralls
Copy link

coveralls commented Jan 18, 2018

Coverage Status

Coverage increased (+0.007%) to 99.809% when pulling 659698b on dblock:habtm into af17cfe on mongoid:master.

@coveralls
Copy link

coveralls commented Jan 18, 2018

Coverage Status

Coverage increased (+0.007%) to 99.809% when pulling 71b9062 on dblock:habtm into af17cfe on mongoid:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 99.375% when pulling ca44523 on dblock:habtm into af17cfe on mongoid:master.

@dblock dblock requested a review from mikwat January 18, 2018 03:14
@coveralls
Copy link

coveralls commented Jan 18, 2018

Coverage Status

Coverage increased (+0.008%) to 99.81% when pulling 61929fb on dblock:habtm into af17cfe on mongoid:master.

Copy link
Collaborator

@mikwat mikwat left a comment

Choose a reason for hiding this comment

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

Thanks for working on this, but I understand your concern.

README.md Outdated
track_history :on => [:embedded_relations] # all embedded relations will be tracked
track_history :on => [
:embedded_relations,
:rereferenced_relations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo? Should be :referenced_relations

before_add: :track_references,
before_remove: :track_references
}.merge(opts)
end
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting idea! Does feel like a bit of a hack though...

@ypicard
Copy link

ypicard commented Jan 28, 2019

Status update for this ?

@dblock
Copy link
Collaborator Author

dblock commented Jan 28, 2019

I haven't made progress on it and probably won't any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants