Closed mpetazzoni closed 4 months ago
1 Warning | |
---|---|
:warning: | Unless you’re refactoring existing code, please update CHANGELOG.md. |
1 Message | |
---|---|
:book: | We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests! |
Here's an example of a CHANGELOG.md entry:
* [#188](https://github.com/mongoid/mongoid-history/pull/188): Add spec to test embedded documents changes in parent's history - [@mpetazzoni](https://github.com/mpetazzoni).
Generated by :no_entry_sign: danger
This one failed because of rubocop, so rubocop -a ; rubocop --auto-gen-config
. Make the spec fail in the way it fails in your bug report.
Running rubocop -a
finds close to 200 offenses over the whole source tree. rubocop --auto-gen-config
finds about 675. Both somehow modify a bunch of files in the source tree. Am I missing some configuration?
Anyway, running bundle exec rake
(which is what the continuous build is doing) correctly only show only the errors in my file. I've updated the commit.
I played with this, and it's definitely a bug. In short, when the parent is saved, changes
doesn't include the changes from the child relation during update, however it does include it during insert. I am not sure how to fix this, maybe @JagdeepSingh who wrote embedded support or @jnfeinstein who added some improvements on top could take a look?
@mpetazzoni i appreciate the research done on this bug. I will try to look into the cause.
Thanks @dblock and @JagdeepSingh, appreciated. Unfortunately I don't know enough about Rails and Mongoid to fix this myself; there's way too much magic going on and I couldn't figure out whether I was missing something in the way my models were setup, or if something was missing in mongoid-history's code. Looking forward to hearing back from you on this soon, hopefully with a fix😄 !
def modified_attributes_for_update
@modified_attributes_for_update ||= Mongoid::History::Attributes::Update.new(self).attributes
end
always returns nil when you update only the child.
Yes, @mateuspontes as i posted here, this is because the changes
hash remains empty if we update only the embedded relations AND we are not using any code/gem to track changes in them.
@JagdeepSingh in create we track changes on embedded relations, can we use the same aproach to catch changes on update?
I'll create a new PR with a new aproach to solve this without extra gems, this is working with embed_one only for now. It's a WIP
[#<Tracker _id: 58efcf927ce04a71b049aac4, created_at: 2017-04-13 19:20:50 UTC, updated_at: 2017-04-13 19:20:50 UTC, association_chain: [{"name"=>"Parent", "id"=>BSON::ObjectId('58efcf927ce04a71b049aac5')}], modified: {"name"=>"bowser", "child"=>{"_id"=>BSON::ObjectId('58efcf927ce04a71b049aac6'), "name"=>"todd"}}, original: {}, version: 1, action: "create", scope: "parent", modifier_id: nil>, #<Tracker _id: 58efcf927ce04a71b049aac7, created_at: 2017-04-13 19:20:50 UTC, updated_at: 2017-04-13 19:20:50 UTC, association_chain: [{"name"=>"Parent", "id"=>BSON::ObjectId('58efcf927ce04a71b049aac5')}], modified: {"name"=>"brow"}, original: {"name"=>"bowser", "child"=>{}}, version: 2, action: "update", scope: "parent", modifier_id: nil>, #<Tracker _id: 58efcf927ce04a71b049aac8, created_at: 2017-04-13 19:20:50 UTC, updated_at: 2017-04-13 19:20:50 UTC, association_chain: [{"name"=>"Parent", "id"=>BSON::ObjectId('58efcf927ce04a71b049aac5')}], modified: {"child"=>{"name"=>"mario"}}, original: {"child"=>{"name"=>"todd"}}, version: 3, action: "update", scope: "parent", modifier_id: nil>]
Demonstrates #187.