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

[WIP] Support associations and embeds #21

Closed
wants to merge 16 commits into from

Conversation

narrowtux
Copy link
Contributor

@narrowtux narrowtux commented Jun 1, 2017

This is a WIP Pull Request it is only made this early to allow discussion about the code

TODO

  • function that extracts a flat list of version structs
  • make insert work
  • make insert work (strict mode)
  • make insert! work
  • make update work
  • make update work (strict mode)
  • make update! work
  • check if delete needs to be handled too
  • before deleting, perform some schema reflection magic to get a list of the assocs that would be deleted too
  • also check child assocs that have on_delete: :nillify_all so those get a version record that the reference was set to null
  • reduce changesets to a simple map when not using UUID keys
  • otherwise, create version records for embeds as well (should work by reusing the assoc code)
    • insert, update
    • delete

non-bang functions now return a list of assoc_versions in the result map

bang functions were upgraded to also insert assoc-related versions
@narrowtux
Copy link
Contributor Author

@izelnakri I've got some more tests to write, clean up a few things and also implement strict mode (as soon as I understand what it does exactly), but otherwise, it looks like it's going to work really well.

@izelnakri
Copy link
Owner

@narrowtux thanks for all the work you've done so far and the TODOs that make the PR more understandable! Could you explain to me what this is for: "reduce changesets to a simple map when not using UUID keys" ?

@narrowtux
Copy link
Contributor Author

Embeds use UUID keys by default, since ecto has to generate them and not a DB. That saves them from having to implement some kind of auto increment logic without a database (nearly impossible).

So if someone is using UUID keys, we can store actual versions for each embed in the normal versions table. If not, the embeds have to be handled in a special way so that PaperTrail doesn't crash when updating a struct in the DB that has an updated embed inside.

@johnhamelink
Copy link

@narrowtux I'd love to see this added to paper_trail - do you plan on finishing this PR? I may be able to help you in the coming weeks/months.

@narrowtux
Copy link
Contributor Author

Sorry I kinda gave up on this, there was too much work to be done and every time I wanted to tackle an issue a rabbit hole of new issues opened. It has become clear to me that this project wasn't what I wanted it to be, which is fine, but I couldn't use it in our product.

So in the end, I decided to write my own audit/history/revert library, don't want to plug it here though.

@paranojik
Copy link

I'm continuing the work on this in #60

@izelnakri
Copy link
Owner

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

@izelnakri izelnakri closed this Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants