Save associations changes in items_changes

[abmm]

This PR is related with #17 and PR #18.

We have had some Poison.encode errors, when updating changesets with changes in its relations. Our approach has been to include in items_changes the changes of the original struct and its relations.
For this I've used a recursive function that find every changeset.changes and keep it in a single structure, if there is not relations the behaviour is similar like before.

Taking the same example as issue #17, if we have :

%Objective{
  current_status: 25,
  key_results: [
    id: 2,
    values: %{
      current_value: 15,
      desired_value: 100
    }
  ]
}

We could save in item_changes something like:

{
"key_results": [ {"id": 2, "values": {"current_value": 15, "desired_value": 100}}], 
"current_status": 25
}

With this we could know that the Objective was updated with current_status 25 and its related key_result (id:2) with "values": {"current_value": 15, "desired_value": 100}

Note that every relation will have an id and its changes.

[izelnakri]

Hi @abmm, I've read your pull request thank you for your work, time and interest. I am of the opinion that this feature shouldn't be implemented. I would like to encourage every user of this library to explicitly version all relationships for the sake of consistency.

What we can do however is to perhaps raise an exception when a situation like this occurs. So everyone knows that relationships shouldn't be nested in a single state change. If your opinion is different than mine I'm looking forward to your arguments.

[jeremyjh]

Isn't this use case also solved in #21 ?

[izelnakri]

@jeremyjh that pull request is incomplete unfortunately. Although Im not much full in favor of the feature, I'm willing to read and investigate a full proof of concept along with tests.

[jeremyjh]

@izelnakri yes but I'm just pointing out this PR is probably redundant.

#21 actually is very nearly complete, it doesn't support strict mode or embeds but has full assoc support and tests - have you looked it over?

I'm probably going to just use that branch for now in my application. I'd rather see it merged into Paper Trail though and can maybe invest some time into finishing it if that is possible. What is your opinion of what has been done there so far ?

[joshchernoff]

I think if Ecto allows for nested associations then it would not be very consistent to say this lib can't.

I've already developed a project where I have many forms that rely on this type of behavior and I'm not very much in favor of undoing that work since it works just fine with Ecto and it servers my business needs.

I think this maybe an area where being too opinionated will stifle community engagement for this project. I my self will be less inclined to participate if I can't address the needs I'm looking for.

I'd like to understand more about why we feel this type of design pattern is not favorable and to see if there is a good compromise.

[jeremyjh]

@DigitalCake I agree. If cast_assoc and put_assoc were considered harmful, that would be one thing. But these are core features of Ecto and very valuable ones.

[izelnakri]

My answer to this issue:

Although I'm not in favor of a nested versioning feature I've always been open to review and if it is good and community wants it, merge the feature. So far there has been 3 trials however none were able to finish and test the feature completely.

[izelnakri]

Closing this PR due to inactivity, please ping me when/if its ready to be reopened.