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

Full diff change tracking mode #18

Merged
merged 79 commits into from
Dec 27, 2023

Conversation

rgraff
Copy link
Contributor

@rgraff rgraff commented Oct 2, 2023

Adds a new full_diff change tracking mode. The goal is to get a full diff that is human readable, relatively easy to render in a UI, and contains enough data that a revert and undo action could be implemented in the future.

papertrail do
  change_tracking_mode :full_diff
end

Changes look like:

// Creating a new resource:
{ subject: {to: "my subject"}, body: {to: "my body"} }

// Updating 
{ subject: {from: "my subject", to: "new subject"}, body: {unchanged: "my body"} }

// Destroying (same as updates, allowing for soft destroys to do updates
{ subject: {unchanged: "new subject"}, body: {unchanged: "my body"} }

All attributes are dumped with the dump_embedded_value, except (ironically) embedded resources.

Embedded resources are marked as created/updated/destroyed/unchanged

{ author: { to: nil } }
{ author: { created: { first_name: {to: "Bob" } } } }
{ author: { unchanged: { first_name: {unchanged: "Bob" } } } }
{ author: { updated: { first_name: {from: "Bob", to: "Joe" } } } }
{ author: { destroyed: { first_name: {unchanged: "Bob" } } } }

Arrays of embedded resources have an index change. Items in arrays move when simple types have the same value or embedded resources are updated.

{ tags: { to: []  } }
{ tags: { unchanged: [] } }
{ tags: { to: [ { index: { to: 0 }, created: { label: { to: "ash" } } } ] } }
{ tags: { to: [ { index: { to: 0 }, created: { label: { to: "phx" } }, { index: { from: 0, to: 1 }, unchanged: { label: { to: "ash" } } } ] } }

Union objects work as well and can change type.

{ source: { to: { type: "link", value: "https://www.example.com" } } } 
{ source: { from: { type: "link", value: "https://www.example.com" }, to: { type: "domain", value: "example.com" } } 
{ source: { from: { type: "domain", value: "example.com" } , to: nil } 

Embedded Union objects are created/updated/destroyed and can be mixed with non-embedded types

{ source: { to: { type: "book", created: { name: { to: "The book"}, page: { to: 2} } } } 
{ source: { to: { type: "book", updated: { name: { from: "The book", to: "The Book" },  page: { unchanged: 2 } } } 
{ source: { from: { type: "book", destroyed: { name: { unchanged: "The Book" }, page: {unchanged: 2} } }, to: { type: "link", value: "example.com" } } 

You can even have arrays of unions moving. See the full_diff_test.exs for complete coverage.

@zachdaniel
Copy link
Contributor

We should implement the behavior for all built in types but also add a callback that can be implemented by custom types to control how they diff. This looks awesome!

@zachdaniel
Copy link
Contributor

One thing to consider is that we may want to capture the input to each action (attributes/arguments) unchanged too. I.e "they provided X, the changes were Y". This is probably independent of this specific work.

@rgraff
Copy link
Contributor Author

rgraff commented Oct 2, 2023

Good ideas. I will an add an issue for both, plus I a way to set the change_tracking_mode to be an mfa for custom diffing.

@rgraff rgraff marked this pull request as ready for review December 12, 2023 20:06
@rgraff
Copy link
Contributor Author

rgraff commented Dec 12, 2023

@zachdaniel this is feature complete and well tested. I'm going to start running it production on my app. No need to merge immediately.

If this is too much code specific to my use case, I can make the change_tracking_mode accept an mfa and have this an add-on in a separate repo. Also happy to be the maintainer for now if you need support.

@zachdaniel
Copy link
Contributor

Looks pretty slick! Happy to have it built in, especially since it is opt in. A few CI related things to address and then we can get it in.

@zachdaniel zachdaniel merged commit 449cd2a into ash-project:main Dec 27, 2023
11 of 13 checks passed
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.

2 participants