Drop association changes on update

[itSQualL]

Related to #17

PR purpose

As I explained on the associated issue, there was a problem with changesets including changes in their associations.

So, in the same way, you drop associations on serialize(model), you need to drop it from changes map.

Please, take a look and let me know if you want I change something here.

[izelnakri]

Thank you for the contribution! I will definitely take a look into this when I find the time.

[izelnakri]

Hi again @itSQualL, thanks again for your taking the initiative and creating this pull request with tests!

I've considered this approach and I would like this feature to be discussed with others. Personally I never use cast_assoc(), and I discourage people from using it. One can always create a transaction and do the operations individually on the relationships. If we introduce this feature, some people might lose data on tracked models simply due to the fact that they are being filtered from the changeset. In other words problem this PR tries to fix could easily be avoided by not casting the relationships key to the changeset, in addition this fix could create a caveat/bug for users who actually want to track both the model and their relationships.

I'm going to leave this PR open, perhaps others might want to share their opinion. For now I feel this PR shouldnt be merged to the library. Feel free to convince me otherwise, I'm just not convinced yet ;)

[narrowtux]

Would you pull a PR that implemented full support for associations (and embeds)?

Meaning, if I were to pass a changeset that mutates some associations, PaperTrail would then create a version entry for each of those changed associations.

Example:

ch = Post.changeset(%Post{}, %{
  body: "hello world",
  comments: [
    %Comment{ body: "I'm a comment" }
  ]
}
PaperTrail.insert(ch, user: user)

this would insert 2 %PaperTrail.Version{}s into the versions table.

[izelnakri]

Hi, I need to investigate ecto internals to see how they implement the embedded inserts. I need to see if it would be an elegant way to insert records. Feel free to submit a pull request, I will certainly review it.

[narrowtux]

Embedded inserts (or updates) have nested changesets.

In the above example, the changeset would look like this:

%Ecto.Changeset{
  changes: %{
    body: "hello world",
    comments: [
      %Ecto.Changeset{changes: %{body: "I'm a comment"}, ...}
    ]
  },
  ...
}

where ... is the normal changeset stuff that has the schema module, the action, valid? and errors fields.

[izelnakri]

Does these happen in a single transaction? Does Ecto intelligently guesses which records will be inserted OR updated based on the changeset data structure? I need to read about these as well.

Additionally I assume Ecto wont remove unused records, if one wants such a behavior one would have to resort to explicitly inserting/updating/removing records in a single transaction anyway.

This is not a blocking thing for embedded version creation feature, the 2 questions I raised above are however.

[narrowtux]

Does these happen in a single transaction?

I couldn't find documentation about this but I think it's safe to assume that it will work when you call Repo.insert, Repo.update or others from inside a transaction.

Does Ecto intelligently guesses with records will be inserted OR updated based on the data?

Short answer: yes.

The changeset contains everything that will be done. If there are changesets inside that have the action :delete, it will delete those items. The decision is made in the changeset function of your schema.

Additionally I assume Ecto wont remove unused records

By default, the changeset function will raise when you omit a member of the association in the params. However, you can use the option :on_replace to do other things, like :update or :delete.

At the point where PaperTrail gets the changeset to store, everything of this is already handled. This library's only concern would be to find out changes that are done to associations and store versions for each of those.

For embeds, it depends:

• A cleaner way would be to also make a version for each embed (since they're normally tagged with a UUID), but you could also
• bunch them all together in a single JSON array (for embeds_many) or object (for embeds_one)

[izelnakri]

Seems very interesting, thanks for taking the time to answer my points. Im looking forward to the PR!

[narrowtux]

See #21

[izelnakri]

Closing this PR due to inactivity & incompleteness, please let me know if you would like it to be reopened.