improvement: add create_version_on_destroy? option

Author: zachdaniel

documentation/dsls/DSL-AshPaperTrail.Resource.md

@@ -34,6 +34,7 @@ A section for configuring how versioning is derived for the resource.
 | [`relationship_opts`](#paper_trail-relationship_opts){: #paper_trail-relationship_opts } | `keyword` |  | Options to pass to the has_many :paper_trail_versions relationship that is created on this resource. For example, `public?: true` to expose the relationship over graphql. See `d:Ash.Resource.Dsl.relationships.has_many`. |
 | [`store_action_name?`](#paper_trail-store_action_name?){: #paper_trail-store_action_name? } | `boolean` | `false` | Whether or not to add the `version_action_name` attribute to the  version resource. This is useful for auditing purposes. The `version_action_type` attribute is always stored. |
 | [`store_action_inputs?`](#paper_trail-store_action_inputs?){: #paper_trail-store_action_inputs? } | `boolean` | `false` | Whether or not to add the `version_action_inputs` attribute to the version resource, which will store all attributes and arguments for the called action, redacting any sensitive values. This is useful for auditing purposes. The `version_action_inputs` attribute is always stored. |
+| [`create_version_on_destroy?`](#paper_trail-create_version_on_destroy?){: #paper_trail-create_version_on_destroy? } | `boolean` | `true` | Whether or not to create a version on destroy. You will need to set this to `false` unless you are doing soft destroys (like with `AshArchival`) |
 | [`store_resource_identifier?`](#paper_trail-store_resource_identifier?){: #paper_trail-store_resource_identifier? } | `boolean` | `false` | Whether or not to add the `version_resource_identifier` attribute to the version resource. This is useful for auditing purposes. |
 | [`resource_identifier`](#paper_trail-resource_identifier){: #paper_trail-resource_identifier } | `atom` |  | A name to use for this resource in the `version_resource_identifier`. Defaults to `Ash.Resource.Info.short_name/1`. |
 | [`version_extensions`](#paper_trail-version_extensions){: #paper_trail-version_extensions } | `keyword` | `[]` | Extensions that should be used by the version resource. For example: `extensions: [AshGraphql.Resource], notifier: [Ash.Notifiers.PubSub]` |

documentation/tutorials/getting-started-with-ash-paper-trail.md

@@ -69,17 +69,25 @@ end
 
 ## Destroy Actions
 
-If you are using `AshPostgres`, and you want to support destroy actions, you need to do one of two things:
+If you are using `AshPostgres`, and you want to support destroy actions, you will need to do one of the following:
 
-1. (preferred) use the version mixin to set `on_delete: :delete` on the ference.
+<!-- tabs-open -->
+
+### Don't version destroys
+
+1. configure versions not to be created on destroy actions
 
 ```elixir
 paper_trail do
-  mixin {MyApp.MyResource.PaperTrailMixin, :mixin, []}
+  create_version_on_destroy? false
 end
+```
+
+2. use the version mixin to set `on_delete: :delete` on the underlying reference to the parent resource.
 
-...
+First, create a paper trail mixin if you haven't already
 
+```elixir
 defmodule MyApp.MyResource.PaperTrailMixin do
   def mixin do
     # quote here is because we are returning code to be evaluated inside of the
@@ -96,17 +104,33 @@ end
 ```
 
 
-2.  use something like `AshArchival` in conjunction with this resource to ensure that destroy actions are `soft?` and do not actually result in row deletion
+Second, configure it in the resource
+
+```elixir
+paper_trail do
+  mixin {MyApp.MyResource.PaperTrailMixin, :mixin, []}
+end
+```
+
+
+### Soft Destroys
 
-3. configure `AshPaperTrail` to ignore the reference, via:
+Manually implement soft deletion, or use something like [`AshArchival`](https://hexdocs.pm/ash_archival) to ensure that destroy actions are `soft?` and do not actually result in row deletion
+
+### Disable Foreign Keys (not recommended)
+
+configure `AshPaperTrail` to ignore the reference, like so:
 
 ```elixir
 paper_trail do
   reference_source? false
 end
 ```
 
-and you could then use the `mixin` described below to add `on_delete` options to the reference.
+This will make it skip creating a foreign key for the version source attribute
+
+
+<!-- tabs-close -->
 
 ## Attributes
 

lib/resource/changes/create_new_version.ex

@@ -53,7 +53,9 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do
 
   defp valid_for_tracking?(%Ash.Changeset{} = changeset) do
     changeset.action.name not in AshPaperTrail.Resource.Info.ignore_actions(changeset.resource) &&
-      (changeset.action_type in [:create, :destroy] ||
+      (changeset.action_type == :create ||
+         (changeset.action_type == :destroy &&
+            AshPaperTrail.Resource.Info.create_version_on_destroy?(changeset.resource)) ||
          (changeset.action_type == :update &&
             changeset.action.name in AshPaperTrail.Resource.Info.on_actions(changeset.resource)))
   end
@@ -62,7 +64,9 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do
     Ash.Changeset.after_action(changeset, fn changeset, result ->
       changed? = changed?(changeset)
 
-      if changeset.action_type in [:create, :destroy] ||
+      if changeset.action_type == :create ||
+           (changeset.action_type == :destroy &&
+              AshPaperTrail.Resource.Info.create_version_on_destroy?(changeset.resource)) ||
            (changeset.action_type == :update && changed?) do
         {version_changeset, input, actor} = build_notifications(changeset, result)
         create!(changeset, version_changeset, input, actor)
@@ -78,7 +82,9 @@ defmodule AshPaperTrail.Resource.Changes.CreateNewVersion do
     |> Enum.filter(fn {changeset, _result} ->
       changed? = changed?(changeset)
 
-      changeset.action_type in [:create, :destroy] ||
+      changeset.action_type == :create ||
+        (changeset.action_type == :destroy &&
+           AshPaperTrail.Resource.Info.create_version_on_destroy?(changeset.resource)) ||
         (changeset.action_type == :update && changed?)
     end)
     |> Enum.map(fn {changeset, result} -> build_notifications(changeset, result, bulk?: true) end)

lib/resource/info.ex

@@ -107,4 +107,9 @@ defmodule AshPaperTrail.Resource.Info do
   def public_timestamps?(resource) do
     Spark.Dsl.Extension.get_opt(resource, [:paper_trail], :public_timestamps?, false)
   end
+
+  @spec create_version_on_destroy?(Spark.Dsl.t() | Ash.Resource.t()) :: boolean
+  def create_version_on_destroy?(resource) do
+    Spark.Dsl.Extension.get_opt(resource, [:paper_trail], :create_version_on_destroy?, true)
+  end
 end

lib/resource/resource.ex

@@ -115,6 +115,12 @@ defmodule AshPaperTrail.Resource do
         doc:
           "Whether or not to add the `version_action_inputs` attribute to the version resource, which will store all attributes and arguments for the called action, redacting any sensitive values. This is useful for auditing purposes. The `version_action_inputs` attribute is always stored."
       ],
+      create_version_on_destroy?: [
+        type: :boolean,
+        default: true,
+        doc:
+          "Whether or not to create a version on destroy. You will need to set this to `false` unless you are doing soft destroys (like with `AshArchival`)"
+      ],
       store_resource_identifier?: [
         type: :boolean,
         default: false,

test/ash_paper_trail_test.exs

@@ -734,6 +734,98 @@ defmodule AshPaperTrailTest do
     end
   end
 
+  describe "create_version_on_destroy?" do
+    test "when create_version_on_destroy? is false, no version is created on destroy" do
+      assert %{subject: "subject", body: "body", id: post_id} =
+               post = Posts.NoDestroyVersionPost.create!(%{subject: "subject", body: "body"})
+
+      assert :ok = Posts.NoDestroyVersionPost.destroy!(post)
+
+      assert [
+               %{
+                 subject: "subject",
+                 body: "body",
+                 version_action_type: :create,
+                 version_source_id: ^post_id
+               }
+             ] = Ash.read!(Posts.NoDestroyVersionPost.Version)
+
+      refute Enum.any?(Ash.read!(Posts.NoDestroyVersionPost.Version), &(&1.version_action_type == :destroy))
+    end
+
+    test "when create_version_on_destroy? is false, no version is created on bulk destroy with enumerable" do
+      %{subject: "subject", body: "body", id: post_id} =
+        post = Posts.NoDestroyVersionPost.create!(%{subject: "subject", body: "body"})
+
+      %Ash.BulkResult{
+        status: :success
+      } =
+        Ash.bulk_destroy!([post], :destroy, %{},
+          strategy: :stream,
+          return_errors?: true
+        )
+
+      assert [
+               %{
+                 subject: "subject",
+                 body: "body",
+                 version_action_type: :create,
+                 version_source_id: ^post_id
+               }
+             ] = Ash.read!(Posts.NoDestroyVersionPost.Version)
+
+      refute Enum.any?(Ash.read!(Posts.NoDestroyVersionPost.Version), &(&1.version_action_type == :destroy))
+    end
+
+    test "when create_version_on_destroy? is false, no version is created on bulk destroy with query" do
+      %{subject: "subject", body: "body", id: post_id} =
+        Posts.NoDestroyVersionPost.create!(%{subject: "subject", body: "body"})
+
+      %Ash.BulkResult{
+        status: :success
+      } =
+        Posts.NoDestroyVersionPost
+        |> Ash.Query.filter(id: post_id)
+        |> Ash.bulk_destroy!(:destroy, %{},
+          strategy: :stream,
+          return_errors?: true
+        )
+
+      assert [
+               %{
+                 subject: "subject",
+                 body: "body",
+                 version_action_type: :create,
+                 version_source_id: ^post_id
+               }
+             ] = Ash.read!(Posts.NoDestroyVersionPost.Version)
+
+      refute Enum.any?(Ash.read!(Posts.NoDestroyVersionPost.Version), &(&1.version_action_type == :destroy))
+    end
+
+    test "when create_version_on_destroy? is true (default), a version is created on destroy" do
+      assert %{subject: "subject", body: "body", id: post_id} =
+               post = Posts.Post.create!(@valid_attrs, tenant: "acme")
+
+      assert :ok = Posts.Post.destroy!(post, tenant: "acme")
+
+      versions = 
+        Ash.read!(Posts.Post.Version, tenant: "acme")
+        |> Enum.sort_by(& &1.version_inserted_at)
+
+      assert [
+               %{
+                 version_action_type: :create,
+                 version_source_id: ^post_id
+               },
+               %{
+                 version_action_type: :destroy,
+                 version_source_id: ^post_id
+               }
+             ] = versions
+    end
+  end
+
   describe "public_timestamps?" do
     test "when public_timestamps? is false, timestamps are not public on the version resource" do
       assert AshPaperTrail.Resource.Info.public_timestamps?(AshPaperTrail.Test.Articles.Article) ==

test/support/posts/domain.ex

@@ -11,5 +11,7 @@ defmodule AshPaperTrail.Test.Posts.Domain do
     resource AshPaperTrail.Test.Posts.FullDiffPost.Version
     resource AshPaperTrail.Test.Posts.StoreInputsPost
     resource AshPaperTrail.Test.Posts.StoreInputsPost.Version
+    resource AshPaperTrail.Test.Posts.NoDestroyVersionPost
+    resource AshPaperTrail.Test.Posts.NoDestroyVersionPost.Version
   end
 end

test/support/posts/no_destroy_version_post.ex

@@ -0,0 +1,49 @@
+defmodule AshPaperTrail.Test.Posts.NoDestroyVersionPost do
+  @moduledoc """
+  A post resource that doesn't create versions on destroy
+  """
+
+  use Ash.Resource,
+    domain: AshPaperTrail.Test.Posts.Domain,
+    data_layer: Ash.DataLayer.Ets,
+    extensions: [AshPaperTrail.Resource],
+    validate_domain_inclusion?: false
+
+  ets do
+    private? true
+  end
+
+  paper_trail do
+    attributes_as_attributes [:subject, :body]
+    create_version_on_destroy? false
+  end
+
+  code_interface do
+    define :create
+    define :read
+    define :update
+    define :destroy
+  end
+
+  actions do
+    default_accept :*
+    defaults [:create, :read, :update, :destroy]
+  end
+
+  attributes do
+    uuid_primary_key :id
+
+    attribute :subject, :string do
+      public? true
+      allow_nil? false
+    end
+
+    attribute :body, :string do
+      public? true
+      allow_nil? false
+    end
+
+    create_timestamp :inserted_at
+    update_timestamp :updated_at
+  end
+end