From 09a9092dfc6faca818d3d128e82a820ca443c2ae Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 10 Jul 2024 11:27:50 -0400 Subject: [PATCH] fix: fix bulk destroy handling improvement: pick new values off of result --- lib/change_builders/changes_only.ex | 8 ++--- lib/change_builders/full_diff/full_diff.ex | 2 +- lib/change_builders/snapshot.ex | 8 ++--- lib/resource/changes/create_new_version.ex | 32 +++++++++++-------- .../transformers/create_version_resource.ex | 2 +- test/ash_paper_trail_test.exs | 2 +- 6 files changed, 30 insertions(+), 24 deletions(-) diff --git a/lib/change_builders/changes_only.ex b/lib/change_builders/changes_only.ex index fafd131..aef2e73 100644 --- a/lib/change_builders/changes_only.ex +++ b/lib/change_builders/changes_only.ex @@ -1,12 +1,12 @@ defmodule AshPaperTrail.ChangeBuilders.ChangesOnly do @moduledoc false - def build_changes(attributes, changeset) do - Enum.reduce(attributes, %{}, &build_attribute_change(&1, changeset, &2)) + def build_changes(attributes, changeset, result) do + Enum.reduce(attributes, %{}, &build_attribute_change(&1, changeset, result, &2)) end - def build_attribute_change(attribute, changeset, changes) do + def build_attribute_change(attribute, changeset, result, changes) do if Ash.Changeset.changing_attribute?(changeset, attribute.name) do - value = Ash.Changeset.get_attribute(changeset, attribute.name) + value = Map.get(result, attribute.name) {:ok, dumped_value} = Ash.Type.dump_to_embedded(attribute.type, value, attribute.constraints) diff --git a/lib/change_builders/full_diff/full_diff.ex b/lib/change_builders/full_diff/full_diff.ex index 1cd6836..5a421e1 100644 --- a/lib/change_builders/full_diff/full_diff.ex +++ b/lib/change_builders/full_diff/full_diff.ex @@ -24,7 +24,7 @@ defmodule AshPaperTrail.ChangeBuilders.FullDiff do } """ - def build_changes(attributes, changeset) do + def build_changes(attributes, changeset, _result) do Enum.reduce(attributes, %{}, fn attribute, changes -> Map.put( changes, diff --git a/lib/change_builders/snapshot.ex b/lib/change_builders/snapshot.ex index 4c19587..c8fed96 100644 --- a/lib/change_builders/snapshot.ex +++ b/lib/change_builders/snapshot.ex @@ -1,11 +1,11 @@ defmodule AshPaperTrail.ChangeBuilders.Snapshot do @moduledoc false - def build_changes(attributes, changeset) do - Enum.reduce(attributes, %{}, &build_attribute_change(&1, changeset, &2)) + def build_changes(attributes, changeset, result) do + Enum.reduce(attributes, %{}, &build_attribute_change(&1, changeset, result, &2)) end - def build_attribute_change(attribute, changeset, changes) do - value = Ash.Changeset.get_attribute(changeset, attribute.name) + def build_attribute_change(attribute, _changeset, result, changes) do + value = Map.get(result, attribute.name) {:ok, dumped_value} = Ash.Type.dump_to_embedded(attribute.type, value, attribute.constraints) Map.put(changes, attribute.name, dumped_value) end diff --git a/lib/resource/changes/create_new_version.ex b/lib/resource/changes/create_new_version.ex index 007a4b8..c57f8f7 100644 --- a/lib/resource/changes/create_new_version.ex +++ b/lib/resource/changes/create_new_version.ex @@ -18,7 +18,13 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do @impl true def atomic(changeset, opts, context) do - {:ok, change(changeset, opts, context)} + change_tracking_mode = AshPaperTrail.Resource.Info.change_tracking_mode(changeset.resource) + + if change_tracking_mode == :full_diff do + {:not_atomic, "Cannot perform full_diff change tracking with AshPaperTrail atomically."} + else + {:ok, change(changeset, opts, context)} + end end defp create_new_version(changeset) do @@ -61,12 +67,12 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do {input, private} = resource_attributes |> Enum.filter(&(&1.name in attributes_as_attributes)) - |> Enum.reduce({%{}, %{}}, &build_inputs(changeset, &1, &2)) + |> Enum.reduce({%{}, %{}}, &build_inputs(&1, &2, result)) changes = resource_attributes |> Enum.reject(&(&1.name in to_skip)) - |> build_changes(change_tracking_mode, changeset) + |> build_changes(change_tracking_mode, changeset, result) input = Enum.reduce(belongs_to_actors, input, fn belongs_to_actor, input -> @@ -104,36 +110,36 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do notifications end - defp build_inputs(changeset, %{public?: true} = attribute, {input, private}) do + defp build_inputs(%{public?: true} = attribute, {input, private}, result) do { Map.put( input, attribute.name, - Ash.Changeset.get_attribute(changeset, attribute.name) + Map.get(result, attribute.name) ), private } end - defp build_inputs(changeset, attribute, {input, private}) do + defp build_inputs(attribute, {input, private}, result) do {input, Map.put( private, attribute.name, - Ash.Changeset.get_attribute(changeset, attribute.name) + Map.get(result, attribute.name) )} end - defp build_changes(attributes, :changes_only, changeset) do - AshPaperTrail.ChangeBuilders.ChangesOnly.build_changes(attributes, changeset) + defp build_changes(attributes, :changes_only, changeset, result) do + AshPaperTrail.ChangeBuilders.ChangesOnly.build_changes(attributes, changeset, result) end - defp build_changes(attributes, :snapshot, changeset) do - AshPaperTrail.ChangeBuilders.Snapshot.build_changes(attributes, changeset) + defp build_changes(attributes, :snapshot, changeset, result) do + AshPaperTrail.ChangeBuilders.Snapshot.build_changes(attributes, changeset, result) end - defp build_changes(attributes, :full_diff, changeset) do - AshPaperTrail.ChangeBuilders.FullDiff.build_changes(attributes, changeset) + defp build_changes(attributes, :full_diff, changeset, result) do + AshPaperTrail.ChangeBuilders.FullDiff.build_changes(attributes, changeset, result) end defp authorize?(domain), do: Ash.Domain.Info.authorize(domain) == :always diff --git a/lib/resource/transformers/create_version_resource.ex b/lib/resource/transformers/create_version_resource.ex index c4c5abc..c418581 100644 --- a/lib/resource/transformers/create_version_resource.ex +++ b/lib/resource/transformers/create_version_resource.ex @@ -51,7 +51,7 @@ defmodule AshPaperTrail.Resource.Transformers.CreateVersionResource do attributes |> Enum.map(& &1.name), :version_source_id, :changes, - belongs_to_actors |> Enum.map(&:"#{&1.name}_id") + belongs_to_actors |> Enum.map( &String.to_existing_atom("#{&1.name}_id")) ] |> List.flatten() |> Enum.reject(&is_nil/1) diff --git a/test/ash_paper_trail_test.exs b/test/ash_paper_trail_test.exs index c2d2bd4..5b7ac06 100644 --- a/test/ash_paper_trail_test.exs +++ b/test/ash_paper_trail_test.exs @@ -438,7 +438,7 @@ defmodule AshPaperTrailTest do post = Posts.Post.create!(@valid_attrs, tenant: "acme") Ash.bulk_destroy!([post], :destroy, %{}, - strategy: [:stream, :atomic, :atomic_batches], + strategy: [:atomic], return_errors?: true, tenant: "acme" )