ash-project / ash_paper_trail

The extension for keeping an audit log of changes to your Ash resources.
https://hexdocs.pm/ash_paper_trail
MIT License
28 stars 13 forks source link

Revert "improvement: add `atomic/3` callback to `CreateNewVersion`" #88

Closed jechol closed 1 month ago

jechol commented 1 month ago

This reverts commit 3e2a7b4458caed743c4c9fb803c27a962477da9a.

Although the existing modification should have been {:ok, change(changeset, opts, context)} instead of change(changeset, opts, context), even if we fix it that way, we are encountering the error {:error, "Cannot perform a changeset with after action hooks atomically"}, so it seems appropriate to revert it.

Contributor checklist

zachdaniel commented 1 month ago

We should return {:ok, changeset}, not revert it. This change is only compatible with main of ash, where you won't receive the error you mentioned.

zachdaniel commented 1 month ago

Would you want to PR the modification to {:ok, changeset}? We will release this only after latest ash is released.

zachdaniel commented 1 month ago

I've pushed up a fix. Keep in mind you have to use ash from main as well to be able to use this atomically.

jechol commented 1 month ago

I have tried using it with the ash main branch, but there is an issue where a version is not being created for the destroy action as shown below.

@zachdaniel Could you check this?

  1) test a new version is created on bulk_destroy (AshPaperTrailTest)
     test/ash_paper_trail_test.exs:426
     match (=) failed
     The following variables were pinned:
       post_id = "4e2ef14e-b9e7-4fa9-bda2-78f4335f3c76"
     code:  assert [
              %{subject: "subject", body: "body", version_action_type: :create, version_source_id: ^post_id},
              %{subject: "subject", body: "body", version_action_type: :destroy, version_source_id: ^post_id}
            ] = Ash.read!(Posts.Post.Version, tenant: "acme") |> Enum.sort_by(& &1.version_inserted_at)
     left:  [
              %{body: "body", subject: "subject", version_action_type: :create, version_source_id: ^post_id},
              %{subject: "subject", body: "body", version_action_type: :destroy, version_source_id: ^post_id}
            ]
     right: [
              %AshPaperTrail.Test.Posts.Post.Version{
                body: "body",
                subject: "subject",
                version_action_type: :create,
                version_source_id: "4e2ef14e-b9e7-4fa9-bda2-78f4335f3c76",
                __lateral_join_source__: nil,
                __meta__: #Ecto.Schema.Metadata<:loaded>,
                __metadata__: %{selected: [:id, :version_action_type, :version_action_name, :tenant, :subject, :body, :version_source_id, :changes, :version_inserted_at, :version_updated_at, :user_id, :news_feed_id], tenant: "acme", keyset: "g2o="},
                __order__: nil,
                aggregates: %{},
                calculations: %{},
                changes: %{author: %{id: "2910b64e-57cf-49d2-98c6-fdd4f807c3b9", first_name: "John", last_name: "Doe"}, body: "body", tenant: "acme", subject: "subject", tags: [%{id: "1779b1ed-8722-416c-9821-dec5aafecf7f", tag: "ash"}, %{id: "2f86a331-5d7a-48ff-b778-27d509610bfb", tag: "phoenix"}], published: false},
                id: "7674d024-4e5a-40f3-8998-eb5518c15332",
                news_feed: #Ash.NotLoaded<:relationship, field: :news_feed>,
                news_feed_id: nil,
                tenant: "acme",
                user: #Ash.NotLoaded<:relationship, field: :user>,
                user_id: nil,
                version_action_name: :create,
                version_inserted_at: ~U[2024-06-27 01:32:59.382621Z],
                version_source: #Ash.NotLoaded<:relationship, field: :version_source>,
                version_updated_at: ~U[2024-06-27 01:32:59.382621Z]
              }
            ]
     stacktrace:
       test/ash_paper_trail_test.exs:436: (test)

Finished in 0.3 seconds (0.00s async, 0.3s sync)
17 tests, 1 failure, 16 excluded
zachdaniel commented 1 month ago

understood, will fix the destroy behavior tomorrow.

zachdaniel commented 1 month ago

This is pretty strange actually... do you have this issue when using ash_paper_trail without the atomic change and ash from main? Or is it only when using main of ash_paper_trail and main of ash?