WIP: track changes on embedded_one

[mateuspontes]

• embeds_one with changes in Parent
• embeds_one with empty changes in Parent
• embeds_many with empty changes on Parent

[coveralls]

Coverage increased (+0.004%) to 99.752% when pulling f741e5c on mateuspontes:master into 439c0e1 on mongoid:master.

[coveralls]

Coverage increased (+0.004%) to 99.752% when pulling f741e5c on mateuspontes:master into 439c0e1 on mongoid:master.

[coveralls]

Coverage increased (+0.004%) to 99.752% when pulling f741e5c on mateuspontes:master into 439c0e1 on mongoid:master.

[dblock]

Seems strange that you'd have the changes.empty? check here. Maybe write a spec that changes fields, then embeds, then both. Then we're keeping things in @attributes, so I imagine this really should be something like

@attributes ||= begin
   changes_in_this_instance_or_something_like_that  + changes_in_embeds + ...
end

[dblock]

Should be renamed and refactored to return changes on embeds. The code for non-embeds should be changed to return changes on non-embeds, finally attributes should collect all of the things.

[dblock]

Great progress. If you solve 2/3 of the cases that's already good enough to merge, so feel free to do that if the last one turns out to be too hard.

[dblock]

Also, there's nothing wrong with taking a dependency on something like https://github.com/bopm/mongoid_relations_dirty_tracking, in fact I would prefer that since it (theoretically) specializes in one thing better.

[johnnyshields]

I'm actually doing this now using https://github.com/bopm/mongoid_relations_dirty_tracking

[mateuspontes]

@johnnyshields so I can pause this POC and wait your PR.

[mateuspontes]

@johnnyshields pls tell me if I can help you with code or something like that.

[mpetazzoni]

Hi folks,

Thanks for the work on this. What's the current status? What combination of extensions/code do I need to make #187 work?

[mateuspontes]

@mpetazzoni I'm facing the same problem. I'm using bopm/mongoid_relations_dirty_tracking to track modifications but it post all embeds_many to history_track and after that my embeds_many data are duplicated (see this commit). Without this gem I cann't track changes in embed.

I was trying to create a POC to track changes in embeds_one, but I think the best option is to use MRDT. Waiting @johnnyshields update this issue

[johnnyshields]

@mateuspontes no PR needed, this works for me now:

class Customer
  include Mongoid::History::Trackable
  include Mongoid::RelationsDirtyTracking

  embeds_many :phones

  relations_dirty_tracking only: [:phones]

  track_history on: [{ phones: %i(tag number extension) }]
end

[johnnyshields]

@mateuspontes sorry I didn't read this carefully enough, if this PR is specifically for the embeds_one case then please go ahead, my apologies I didn't realize there was an issue there.

[mateuspontes]

@johnnyshields thx for your help, your code works for me but I end up with my embeds duplicated, I'm using mongo 3.4

Something like that:

{
  "customer": {
    "name": "Doe",
    "phones": [
      {
        "comercial": "11111"
      }
    ]
  }
}

After update @customer:

{
  "customer": {
    "name": "Doe",
    "phones": [
      {
        "comercial": "11111"
      },
      {
        "comercial": "11111"
      }
    ]
  }
}

[johnnyshields]

@mateuspontes that sounds like an issue unrelated to this gem. When you are updating make sure you are using the _id field on the embedded objects.

[mateuspontes]

@johnnyshields I'm using rails 4.2.7 and mongoid 5.2, the issue is related to bopm/mongoid_relations_dirty_tracking, without mongoid_relations_dirty_tracking I cann't store relations changes.

IMHO mongoid-history should track embeds changes as it can track has_many, actually it doesn't track embeds, the code isn't work and it is confirmed as a bug.

[mateuspontes]

After update parent (with mongoid_relations_dirty) action.update(action_params).
Without mongoid_relations_dirty I cann't see embeds diff, mongoid-history doesn't track it. That was the motivation to PR.

> db.actions.findOne({"_id": ObjectId("5903a7f07ce04a617e3c3bf1")})
{
	"_id" : ObjectId("5903a7f07ce04a617e3c3bf1"),
	"name" : "Parent Action",
	"operations" : [
		{
			"_id" : ObjectId("5903ae5c7ce04a63e0d8149f"),
			"name" : "Duplicated child"
		},
		{
			"_id" : ObjectId("5903ae5c7ce04a63e0d8149f"),
			"name" : "Duplicated child"
		}
	],
	"version" : 2
}

[dblock]

Still looking forward for a mergeable PR here!