paper_trail icon indicating copy to clipboard operation
paper_trail copied to clipboard

Strict Mode with UUID

Open maennchen opened this issue 5 years ago • 4 comments

Currently Strict Mode is not supported when using UUIDs.

It could however easily be supported by generating the UUIDs on the client side.

This could look something like this:

multi
|> Ecto.Multi.run(initial_version_key, fn repo, %{} ->
  version_id = get_sequence_id("versions") + 1

  changeset_data =
    Map.get(changeset, :data, changeset)
    |> Map.merge(%{
      uuid: Ecto.UUID.generate(),
      first_version_id: version_id,
      current_version_id: version_id
    })

  initial_version = make_version_struct(%{event: "insert"}, changeset_data, options)
  repo.insert(initial_version)
end)
|> Ecto.Multi.run(model_key, fn
  repo, %{^initial_version_key => initial_version} ->
    updated_changeset =
      changeset
      |> change(%{
        uuid: initial_version.item_changes["uuid"],
        first_version_id: initial_version.id,
        current_version_id: initial_version.id
      })

    repo.insert(updated_changeset, ecto_options)
end)

maennchen avatar Feb 09 '21 18:02 maennchen

(If UUIDs are also used for the Version id, the work could even be done outside of Ecto.Multi.run and 2 inserts could be used instead.)

maennchen avatar Feb 09 '21 18:02 maennchen

Hmm this could be a good breaking change on strict_mode env config, however:

  • Im still not sure why we would want to use uuids for %PaperTrail.Version{}s instead of ids.
  • Tests for strict_mode are going to be way more complicated.
  • Probably then trackings should be first_version_uuid and current_version_uuid as well.

So I'm not sure, we probably need good use-cases for this.

izelnakri avatar Feb 10 '21 19:02 izelnakri

@izelnakri Actually it's the other way around:

https://github.com/izelnakri/paper_trail/blob/main/lib/paper_trail/multi.ex#L51

            Map.get(changeset, :data, changeset)
            |> Map.merge(%{
              id: get_sequence_id(changeset) + 1,
              first_version_id: version_id,
              current_version_id: version_id
            })

get_sequence_id does not work with models that use uuids (if Version also uses uuids is not relevant). It is however not required since a new UUID can just be generated instead of calling a sequence.

This would allow to use strict_mode when using uuids in your model.

I do not think that this needs a breaking change.

maennchen avatar Feb 10 '21 19:02 maennchen

Yep depending how tests and _id to _uuid config works out i guess we might be able to ship this depending on the end-tests complexity

izelnakri avatar Feb 11 '21 23:02 izelnakri