From 22e594048fe1751f023350d4f8cbb88bcc7d67ab Mon Sep 17 00:00:00 2001 From: Thomaselucas Date: Thu, 2 Jul 2026 13:39:05 -0600 Subject: [PATCH 1/3] fix: handle notifications when running nested actions --- lib/ash.ex | 40 +++++-- lib/ash/action_input.ex | 16 +++ lib/ash/actions/action.ex | 154 ++++++++++++++++++++------ test/actions/generic_actions_test.exs | 10 ++ test/notifier/notifier_test.exs | 90 +++++++++++++++ 5 files changed, 268 insertions(+), 42 deletions(-) diff --git a/lib/ash.ex b/lib/ash.ex index aff658067..25e0868ad 100644 --- a/lib/ash.ex +++ b/lib/ash.ex @@ -890,6 +890,19 @@ defmodule Ash do load: [ type: :any, doc: "A load statement to apply on the resulting records after the action is invoked." + ], + return_notifications?: [ + type: :boolean, + default: false, + doc: """ + Use this if you're running ash actions in your own transaction and you want to manually handle sending notifications. + + If a transaction is ongoing, and this is false, notifications will be aggregated in the process dictionary, + otherwise the return value is `{:ok, result, notifications}` (or `{:ok, notifications}` for actions with no return type) + + To send notifications later, use `Ash.Notifier.notify(notifications)`. It sends any notifications + that can be sent, and returns the rest. + """ ] ] @@ -1944,7 +1957,11 @@ defmodule Ash do """ @doc spark_opts: [{1, @run_action_opts}] @spec run_action(input :: Ash.ActionInput.t(), opts :: Keyword.t()) :: - :ok | {:ok, term} | {:error, Ash.Error.t()} + :ok + | {:ok, term} + | {:ok, list(Ash.Notifier.Notification.t())} + | {:ok, term, list(Ash.Notifier.Notification.t())} + | {:error, Ash.Error.t()} def run_action(input, opts \\ []) do Ash.Helpers.expect_options!(opts) domain = Ash.Helpers.domain!(input, opts) @@ -1952,15 +1969,20 @@ defmodule Ash do with {:ok, opts} <- RunActionOpts.validate(opts), opts <- RunActionOpts.to_options(opts), input = %{input | domain: domain}, - {:ok, _resource} <- Ash.Domain.Info.resource(domain, input.resource), - {:ok, result} <- Ash.Actions.Action.run(domain, input, opts) do - {:ok, result} - else - :ok -> - :ok + {:ok, _resource} <- Ash.Domain.Info.resource(domain, input.resource) do + case Ash.Actions.Action.run(domain, input, opts) do + {:ok, result, notifications} -> + {:ok, result, notifications} - {:error, error} -> - {:error, Ash.Error.to_error_class(error)} + {:ok, result} -> + {:ok, result} + + :ok -> + :ok + + {:error, error} -> + {:error, Ash.Error.to_error_class(error)} + end end end diff --git a/lib/ash/action_input.ex b/lib/ash/action_input.ex index 5064ea2db..7a33ab56f 100644 --- a/lib/ash/action_input.ex +++ b/lib/ash/action_input.ex @@ -1827,6 +1827,12 @@ defmodule Ash.ActionInput do {:ok, new_result} when has_return?(input) -> {:ok, new_result} + {:ok, new_result, notifications} when has_return?(input) and is_list(notifications) -> + {:ok, new_result, notifications} + + {:ok, notifications} when has_no_return?(input) and is_list(notifications) -> + {:ok, notifications} + {:error, error} -> {:error, error} @@ -1835,6 +1841,7 @@ defmodule Ash.ActionInput do Invalid return value from after_transaction hook. Because this action has a return type I expected one of: * {:ok, term} + * {:ok, term, notifications} * {:error, error} Got: @@ -1847,6 +1854,7 @@ defmodule Ash.ActionInput do Invalid return value from after_transaction hook. Because this action has no return type I expected one of: * :ok + * {:ok, notifications} * {:error, error} Got: @@ -1865,6 +1873,12 @@ defmodule Ash.ActionInput do {:ok, term} when has_return?(input) -> {:ok, term} + {:ok, term, notifications} when has_return?(input) and is_list(notifications) -> + {:ok, term, notifications} + + {:ok, notifications} when has_no_return?(input) and is_list(notifications) -> + {:ok, notifications} + {:error, error} -> {:error, error} @@ -1873,6 +1887,7 @@ defmodule Ash.ActionInput do Invalid return value from around_transaction hook. Because this action has no return type, I expected one of: * :ok + * {:ok, notifications} * {:error, error} Got: @@ -1885,6 +1900,7 @@ defmodule Ash.ActionInput do Invalid return value from around_transaction hook. Because this action has a return type, I expected one of: * {:ok, term} + * {:ok, term, notifications} * {:error, error} Got: diff --git a/lib/ash/actions/action.ex b/lib/ash/actions/action.ex index 21e5f9c92..307dbec54 100644 --- a/lib/ash/actions/action.ex +++ b/lib/ash/actions/action.ex @@ -77,7 +77,9 @@ defmodule Ash.Actions.Action do else run_without_transaction(domain, input, module, run_opts, context, opts) end + |> strip_return_notifications_for_hooks(input, opts) end) + |> restore_return_notifications_after_hooks(input, opts) result = maybe_load(result, input, domain, opts) @@ -133,6 +135,20 @@ defmodule Ash.Actions.Action do defp maybe_load(:ok, _input, _domain, _opts), do: :ok defp maybe_load({:ok, nil}, _input, _domain, _opts), do: {:ok, nil} + defp maybe_load({:ok, result, notifications}, input, domain, opts) do + case maybe_load({:ok, result}, input, domain, opts) do + {:ok, loaded} -> {:ok, loaded, notifications} + other -> other + end + end + + defp maybe_load({:ok, notifications}, %{action: %{returns: nil}} = _input, _domain, %{ + return_notifications?: true + }) + when is_list(notifications) do + {:ok, notifications} + end + defp maybe_load({:ok, result}, input, domain, opts) do constraints = input.action.constraints || [] returns = input.action.returns @@ -154,6 +170,41 @@ defmodule Ash.Actions.Action do defp maybe_load(other, _input, _domain, _opts), do: other + defp strip_return_notifications_for_hooks({:ok, result, notifications}, input, %{ + return_notifications?: true + }) + when not is_nil(input.action.returns) do + Process.put(:ash_return_notifications, notifications) + {:ok, result} + end + + defp strip_return_notifications_for_hooks({:ok, notifications}, %{action: %{returns: nil}}, %{ + return_notifications?: true + }) do + Process.put(:ash_return_notifications, notifications) + :ok + end + + defp strip_return_notifications_for_hooks(result, _input, _opts), do: result + + defp restore_return_notifications_after_hooks(result, input, %{return_notifications?: true}) do + case Process.delete(:ash_return_notifications) do + nil -> + result + + notifications when not is_nil(input.action.returns) -> + case result do + {:ok, result} -> {:ok, result, notifications} + other -> other + end + + notifications -> + {:ok, notifications} + end + end + + defp restore_return_notifications_after_hooks(result, _input, _opts), do: result + defp run_with_transaction(domain, input, module, run_opts, context, opts) do # Run before_transaction hooks first case Ash.ActionInput.run_before_transaction_hooks(input) do @@ -199,30 +250,15 @@ defmodule Ash.Actions.Action do ) |> case do {:ok, {:ok, result, notifications}} -> - notifications = - if notify? && !opts[:return_notifications?] do - Enum.concat( - notifications || [], - Process.delete(:ash_notifications) || [] - ) - else - notifications || [] - end - - remaining = Ash.Notifier.notify(notifications) - - Ash.Actions.Helpers.warn_missed!(input.resource, input.action, %{ - resource_notifications: remaining - }) - final_result = - if input.action.returns do - {:ok, result} - else - :ok - end + finalize_notifications( + notifications, + input, + opts, + notify? + ) + |> build_result(result, input, opts) - # Run after_transaction hooks Ash.ActionInput.run_after_transaction_hooks(final_result, input) {:error, error} -> @@ -250,17 +286,8 @@ defmodule Ash.Actions.Action do :ok -> case run_with_hooks(module, input, run_opts, context, false) do {:ok, result, notifications} -> - remaining = Ash.Notifier.notify(notifications) - - Ash.Actions.Helpers.warn_missed!(input.resource, input.action, %{ - resource_notifications: remaining - }) - - if input.action.returns do - {:ok, result} - else - :ok - end + finalize_notifications(notifications, input, opts, false) + |> build_result(result, input, opts) {:error, error} -> {:error, error} @@ -403,6 +430,67 @@ defmodule Ash.Actions.Action do ) end + defp finalize_notifications(notifications, input, opts, notify?) do + notifications = List.wrap(notifications) + + notifications = + if notify? && !opts[:return_notifications?] do + Enum.concat(notifications, Process.delete(:ash_notifications) || []) + else + notifications + end + + if opts[:return_notifications?] do + notifications + else + if Process.get(:ash_started_transaction?) && !notify? do + current_notifications = List.wrap(Process.get(:ash_notifications, [])) + + Process.put(:ash_notifications, current_notifications ++ notifications) + else + remaining = Ash.Notifier.notify(notifications) + + Ash.Actions.Helpers.warn_missed!(input.resource, input.action, %{ + resource_notifications: remaining + }) + end + + :ok + end + end + + defp build_result(:ok, result, input, opts) do + if opts[:return_notifications?] do + if input.action.returns do + {:ok, result, []} + else + {:ok, []} + end + else + if input.action.returns do + {:ok, result} + else + :ok + end + end + end + + defp build_result(notifications, result, input, opts) when is_list(notifications) do + if opts[:return_notifications?] do + if input.action.returns do + {:ok, result, notifications} + else + {:ok, notifications} + end + else + if input.action.returns do + {:ok, result} + else + :ok + end + end + end + defp run_with_hooks(module, input, run_opts, context, in_transaction?) do # Run before_action hooks case Ash.ActionInput.run_before_actions(input) do diff --git a/test/actions/generic_actions_test.exs b/test/actions/generic_actions_test.exs index 715bff3fd..1e70d360f 100644 --- a/test/actions/generic_actions_test.exs +++ b/test/actions/generic_actions_test.exs @@ -775,6 +775,16 @@ defmodule Ash.Test.Actions.GenericActionsTest do assert result == "Processed: test" end + + test "return_notifications?: true returns notifications" do + assert {:ok, "Processed: test", notifications} = + Post + |> Ash.ActionInput.for_action(:with_notifications, %{message: "test"}) + |> Ash.run_action(return_notifications?: true) + + assert length(notifications) == 1 + assert hd(notifications).data == %{message: "test"} + end end describe "action-level preparations and validations" do diff --git a/test/notifier/notifier_test.exs b/test/notifier/notifier_test.exs index 06885c435..f1da5f1f3 100644 --- a/test/notifier/notifier_test.exs +++ b/test/notifier/notifier_test.exs @@ -185,6 +185,60 @@ defmodule Ash.Test.NotifierTest do end) end end + + action :emit_notification, :atom do + transaction? false + + run fn input, _ -> + notification = %Ash.Notifier.Notification{ + resource: __MODULE__, + domain: Ash.Resource.Info.domain(__MODULE__), + action: input.action, + data: %{generic?: true} + } + + {:ok, :emitted, [notification]} + end + end + + action :emit_notification_in_transaction, :atom do + transaction? true + + run fn input, _ -> + notification = %Ash.Notifier.Notification{ + resource: __MODULE__, + domain: Ash.Resource.Info.domain(__MODULE__), + action: input.action, + data: %{generic?: true, started_transaction?: true} + } + + {:ok, :emitted, [notification]} + end + end + + create :create_with_generic_action do + change fn changeset, _ -> + Ash.Changeset.after_action(changeset, fn _changeset, result -> + __MODULE__ + |> Ash.ActionInput.for_action(:emit_notification, %{}) + |> Ash.run_action!() + + {:ok, result} + end) + end + end + + destroy :destroy_with_generic_action do + change fn changeset, _ -> + Ash.Changeset.after_action(changeset, fn _changeset, result -> + __MODULE__ + |> Ash.ActionInput.for_action(:emit_notification, %{}) + |> Ash.run_action!() + + {:ok, result} + end) + end + end end attributes do @@ -334,6 +388,42 @@ defmodule Ash.Test.NotifierTest do assert_receive {:notification, %Ash.Notifier.Notification{data: %Comment{name: "auto"}}} end + test "a nested generic action notification is sent automatically on create" do + Post + |> Ash.Changeset.for_create(:create_with_generic_action, %{name: "foobar"}) + |> Ash.create!() + + assert_receive {:notification, %{action: %{type: :create}}} + assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} + end + + test "a nested generic action notification is sent automatically on destroy" do + post = + Post + |> Ash.Changeset.for_create(:create, %{name: "foobar"}) + |> Ash.create!() + + assert_receive {:notification, %{action: %{type: :create}}} + + post + |> Ash.Changeset.for_destroy(:destroy_with_generic_action) + |> Ash.destroy!() + + assert_receive {:notification, %{action: %{type: :destroy}}} + assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} + end + + test "a top-level generic action with transaction? true sends its notifications" do + Post + |> Ash.ActionInput.for_action(:emit_notification_in_transaction, %{}) + |> Ash.run_action!() + + assert_receive {:notification, + %Ash.Notifier.Notification{ + data: %{generic?: true, started_transaction?: true} + }} + end + test "the `load/1` change puts the loaded data into the notification" do Post |> Ash.Changeset.for_create(:create_with_comment, %{name: "foobar"}) From 00412f5c505dd40c384e36801d202eb0e9c79bf5 Mon Sep 17 00:00:00 2001 From: Thomaselucas Date: Mon, 13 Jul 2026 14:53:55 -0600 Subject: [PATCH 2/3] changed from pdict solution to threading the notifications through the hooks. --- .../topics/advanced/manual-installation.md | 3 +- lib/ash/action_input.ex | 60 ++++++++++++- lib/ash/actions/action.ex | 84 ++++++++----------- test/actions/generic_actions_test.exs | 16 ++++ test/notifier/notifier_test.exs | 4 + 5 files changed, 116 insertions(+), 51 deletions(-) diff --git a/documentation/topics/advanced/manual-installation.md b/documentation/topics/advanced/manual-installation.md index 8f61804f8..5eb6fd5e0 100644 --- a/documentation/topics/advanced/manual-installation.md +++ b/documentation/topics/advanced/manual-installation.md @@ -151,7 +151,8 @@ Update `config/config.exs`: + read_action_after_action_hooks_in_order?: true, + bulk_actions_default_to_errors?: true, + transaction_rollback_on_error?: true, -+ redact_sensitive_values_in_errors?: true ++ redact_sensitive_values_in_errors?: true, ++ many_to_many_destroy_destination_on_match?: true + config :spark, formatter: [ diff --git a/lib/ash/action_input.ex b/lib/ash/action_input.ex index 7a33ab56f..6bec5bf4e 100644 --- a/lib/ash/action_input.ex +++ b/lib/ash/action_input.ex @@ -1911,8 +1911,62 @@ defmodule Ash.ActionInput do end def run_around_transaction_hooks(%{around_transaction: [around | rest]} = input, func) do - around.(input, fn input -> - run_around_transaction_hooks(%{input | around_transaction: rest}, func) - end) + pending_ref = make_ref() + + result = + around.(input, fn input -> + case run_around_transaction_hooks(%{input | around_transaction: rest}, func) do + {:ok, term, notifications} when has_return?(input) and is_list(notifications) -> + Process.put(pending_ref, notifications) + {:ok, term} + + {:ok, notifications} when has_no_return?(input) and is_list(notifications) -> + Process.put(pending_ref, notifications) + :ok + + other -> + other + end + end) + + pending = Process.get(pending_ref, []) + Process.delete(pending_ref) + + attach_around_notifications(result, pending, input) + end + + defp attach_around_notifications({:error, _} = error, _, _), do: error + + defp attach_around_notifications(result, pending, input) do + {base, extra} = + case result do + {:ok, term, notifications} when is_list(notifications) -> {{:ok, term}, notifications} + other -> {other, []} + end + + notifications = pending ++ extra + + cond do + has_return?(input) -> + case base do + {:ok, term} -> + if notifications == [], do: {:ok, term}, else: {:ok, term, notifications} + + other -> + other + end + + has_no_return?(input) -> + case base do + :ok -> + if notifications == [], do: :ok, else: {:ok, notifications} + + {:ok, more} when is_list(more) -> + {:ok, notifications ++ more} + + other -> + other + end + end end end diff --git a/lib/ash/actions/action.ex b/lib/ash/actions/action.ex index 307dbec54..46a467ae5 100644 --- a/lib/ash/actions/action.ex +++ b/lib/ash/actions/action.ex @@ -77,9 +77,7 @@ defmodule Ash.Actions.Action do else run_without_transaction(domain, input, module, run_opts, context, opts) end - |> strip_return_notifications_for_hooks(input, opts) end) - |> restore_return_notifications_after_hooks(input, opts) result = maybe_load(result, input, domain, opts) @@ -170,41 +168,6 @@ defmodule Ash.Actions.Action do defp maybe_load(other, _input, _domain, _opts), do: other - defp strip_return_notifications_for_hooks({:ok, result, notifications}, input, %{ - return_notifications?: true - }) - when not is_nil(input.action.returns) do - Process.put(:ash_return_notifications, notifications) - {:ok, result} - end - - defp strip_return_notifications_for_hooks({:ok, notifications}, %{action: %{returns: nil}}, %{ - return_notifications?: true - }) do - Process.put(:ash_return_notifications, notifications) - :ok - end - - defp strip_return_notifications_for_hooks(result, _input, _opts), do: result - - defp restore_return_notifications_after_hooks(result, input, %{return_notifications?: true}) do - case Process.delete(:ash_return_notifications) do - nil -> - result - - notifications when not is_nil(input.action.returns) -> - case result do - {:ok, result} -> {:ok, result, notifications} - other -> other - end - - notifications -> - {:ok, notifications} - end - end - - defp restore_return_notifications_after_hooks(result, _input, _opts), do: result - defp run_with_transaction(domain, input, module, run_opts, context, opts) do # Run before_transaction hooks first case Ash.ActionInput.run_before_transaction_hooks(input) do @@ -250,16 +213,17 @@ defmodule Ash.Actions.Action do ) |> case do {:ok, {:ok, result, notifications}} -> - final_result = + finalize_result = finalize_notifications( notifications, input, opts, notify? ) - |> build_result(result, input, opts) - Ash.ActionInput.run_after_transaction_hooks(final_result, input) + to_hook_result(result, input) + |> Ash.ActionInput.run_after_transaction_hooks(input) + |> attach_return_notifications(finalize_result, input, opts) {:error, error} -> error_result = {:error, Ash.Error.to_ash_error(error)} @@ -281,24 +245,25 @@ defmodule Ash.Actions.Action do # Run before_transaction hooks even for non-transactional actions case Ash.ActionInput.run_before_transaction_hooks(input) do {:ok, input} -> - result = + {result, finalize_result} = case authorize(domain, opts[:actor], input) do :ok -> case run_with_hooks(module, input, run_opts, context, false) do {:ok, result, notifications} -> - finalize_notifications(notifications, input, opts, false) - |> build_result(result, input, opts) + {to_hook_result(result, input), + finalize_notifications(notifications, input, opts, false)} {:error, error} -> - {:error, error} + {{:error, error}, nil} end {:error, error} -> - {:error, error} + {{:error, error}, nil} end - # Run after_transaction hooks - Ash.ActionInput.run_after_transaction_hooks(result, input) + result + |> Ash.ActionInput.run_after_transaction_hooks(input) + |> attach_return_notifications_maybe(finalize_result, input, opts) {:error, error} -> {:error, error} @@ -491,6 +456,31 @@ defmodule Ash.Actions.Action do end end + defp to_hook_result(result, %{action: %{returns: returns}}) when not is_nil(returns), + do: {:ok, result} + + defp to_hook_result(_result, %{action: %{returns: nil}}), do: :ok + + defp attach_return_notifications({:error, _} = error, _finalize_result, _input, _opts), do: error + + defp attach_return_notifications(hook_result, finalize_result, input, opts) do + case hook_result do + :ok -> + build_result(finalize_result, nil, input, opts) + + {:ok, result} -> + build_result(finalize_result, result, input, opts) + end + end + + defp attach_return_notifications_maybe({:error, _} = error, _finalize_result, _input, _opts), + do: error + + defp attach_return_notifications_maybe(result, nil, _input, _opts), do: result + + defp attach_return_notifications_maybe(result, finalize_result, input, opts), + do: attach_return_notifications(result, finalize_result, input, opts) + defp run_with_hooks(module, input, run_opts, context, in_transaction?) do # Run before_action hooks case Ash.ActionInput.run_before_actions(input) do diff --git a/test/actions/generic_actions_test.exs b/test/actions/generic_actions_test.exs index 1e70d360f..fbfd60ce2 100644 --- a/test/actions/generic_actions_test.exs +++ b/test/actions/generic_actions_test.exs @@ -785,6 +785,22 @@ defmodule Ash.Test.Actions.GenericActionsTest do assert length(notifications) == 1 assert hd(notifications).data == %{message: "test"} end + + test "return_notifications?: true preserves notifications through around_transaction hooks" do + assert {:ok, "wrapped_Processed: test", notifications} = + Post + |> Ash.ActionInput.for_action(:with_notifications, %{message: "test"}) + |> Ash.ActionInput.around_transaction(fn input, callback -> + case callback.(input) do + {:ok, result} -> {:ok, "wrapped_" <> result} + error -> error + end + end) + |> Ash.run_action(return_notifications?: true) + + assert length(notifications) == 1 + assert hd(notifications).data == %{message: "test"} + end end describe "action-level preparations and validations" do diff --git a/test/notifier/notifier_test.exs b/test/notifier/notifier_test.exs index f1da5f1f3..9a85aad38 100644 --- a/test/notifier/notifier_test.exs +++ b/test/notifier/notifier_test.exs @@ -217,6 +217,8 @@ defmodule Ash.Test.NotifierTest do end create :create_with_generic_action do + require_atomic? false + change fn changeset, _ -> Ash.Changeset.after_action(changeset, fn _changeset, result -> __MODULE__ @@ -229,6 +231,8 @@ defmodule Ash.Test.NotifierTest do end destroy :destroy_with_generic_action do + require_atomic? false + change fn changeset, _ -> Ash.Changeset.after_action(changeset, fn _changeset, result -> __MODULE__ From e00ae9bb7a9a3e59a1aca05fe77adba23db152c9 Mon Sep 17 00:00:00 2001 From: Thomaselucas Date: Mon, 13 Jul 2026 15:23:59 -0600 Subject: [PATCH 3/3] Move peel/reattach out of action.ex into run_after_transaction_hooks, and cover the nested path with Mnesia-backed notifier tests. --- lib/ash/action_input.ex | 43 ++++++++++- lib/ash/actions/action.ex | 55 +++----------- test/notifier/notifier_test.exs | 122 ++++++++++++++++++++++---------- 3 files changed, 137 insertions(+), 83 deletions(-) diff --git a/lib/ash/action_input.ex b/lib/ash/action_input.ex index 6bec5bf4e..2d4a8a73b 100644 --- a/lib/ash/action_input.ex +++ b/lib/ash/action_input.ex @@ -1791,9 +1791,11 @@ defmodule Ash.ActionInput do end def run_after_transaction_hooks(result, input) do + {result_for_hooks, pending_notifications} = peel_after_transaction_notifications(result, input) + input.after_transaction |> Enum.reduce( - result, + result_for_hooks, fn after_transaction, result -> tracer = input.context[:private][:tracer] @@ -1862,8 +1864,47 @@ defmodule Ash.ActionInput do #{inspect(other)} """ end + |> attach_after_transaction_notifications(pending_notifications, input) + end + + defp peel_after_transaction_notifications({:ok, result, notifications}, input) + when has_return?(input) and is_list(notifications) do + {{:ok, result}, notifications} + end + + defp peel_after_transaction_notifications({:ok, notifications}, input) + when has_no_return?(input) and is_list(notifications) do + # Hooks for untyped actions expect `:ok`; notifications are reattached after. + {:ok, notifications} + end + + defp peel_after_transaction_notifications(result, _input), do: {result, :none} + + defp attach_after_transaction_notifications(result, :none, _input), do: result + defp attach_after_transaction_notifications({:error, _} = error, _notifications, _input), do: error + + defp attach_after_transaction_notifications({:ok, result}, notifications, input) + when has_return?(input) and is_list(notifications) do + {:ok, result, notifications} + end + + defp attach_after_transaction_notifications({:ok, result, hook_notifications}, notifications, input) + when has_return?(input) and is_list(notifications) and is_list(hook_notifications) do + {:ok, result, notifications ++ hook_notifications} + end + + defp attach_after_transaction_notifications(:ok, notifications, input) + when has_no_return?(input) and is_list(notifications) do + {:ok, notifications} + end + + defp attach_after_transaction_notifications({:ok, hook_notifications}, notifications, input) + when has_no_return?(input) and is_list(notifications) and is_list(hook_notifications) do + {:ok, notifications ++ hook_notifications} end + defp attach_after_transaction_notifications(result, _notifications, _input), do: result + @doc false def run_around_transaction_hooks(%{around_transaction: []} = input, func) do case func.(input) do diff --git a/lib/ash/actions/action.ex b/lib/ash/actions/action.ex index 46a467ae5..205cad134 100644 --- a/lib/ash/actions/action.ex +++ b/lib/ash/actions/action.ex @@ -140,9 +140,7 @@ defmodule Ash.Actions.Action do end end - defp maybe_load({:ok, notifications}, %{action: %{returns: nil}} = _input, _domain, %{ - return_notifications?: true - }) + defp maybe_load({:ok, notifications}, %{action: %{returns: nil}} = _input, _domain, _opts) when is_list(notifications) do {:ok, notifications} end @@ -213,17 +211,9 @@ defmodule Ash.Actions.Action do ) |> case do {:ok, {:ok, result, notifications}} -> - finalize_result = - finalize_notifications( - notifications, - input, - opts, - notify? - ) - - to_hook_result(result, input) + finalize_notifications(notifications, input, opts, notify?) + |> build_result(result, input, opts) |> Ash.ActionInput.run_after_transaction_hooks(input) - |> attach_return_notifications(finalize_result, input, opts) {:error, error} -> error_result = {:error, Ash.Error.to_ash_error(error)} @@ -245,25 +235,23 @@ defmodule Ash.Actions.Action do # Run before_transaction hooks even for non-transactional actions case Ash.ActionInput.run_before_transaction_hooks(input) do {:ok, input} -> - {result, finalize_result} = + result = case authorize(domain, opts[:actor], input) do :ok -> case run_with_hooks(module, input, run_opts, context, false) do {:ok, result, notifications} -> - {to_hook_result(result, input), - finalize_notifications(notifications, input, opts, false)} + finalize_notifications(notifications, input, opts, false) + |> build_result(result, input, opts) {:error, error} -> - {{:error, error}, nil} + {:error, error} end {:error, error} -> - {{:error, error}, nil} + {:error, error} end - result - |> Ash.ActionInput.run_after_transaction_hooks(input) - |> attach_return_notifications_maybe(finalize_result, input, opts) + Ash.ActionInput.run_after_transaction_hooks(result, input) {:error, error} -> {:error, error} @@ -456,31 +444,6 @@ defmodule Ash.Actions.Action do end end - defp to_hook_result(result, %{action: %{returns: returns}}) when not is_nil(returns), - do: {:ok, result} - - defp to_hook_result(_result, %{action: %{returns: nil}}), do: :ok - - defp attach_return_notifications({:error, _} = error, _finalize_result, _input, _opts), do: error - - defp attach_return_notifications(hook_result, finalize_result, input, opts) do - case hook_result do - :ok -> - build_result(finalize_result, nil, input, opts) - - {:ok, result} -> - build_result(finalize_result, result, input, opts) - end - end - - defp attach_return_notifications_maybe({:error, _} = error, _finalize_result, _input, _opts), - do: error - - defp attach_return_notifications_maybe(result, nil, _input, _opts), do: result - - defp attach_return_notifications_maybe(result, finalize_result, input, opts), - do: attach_return_notifications(result, finalize_result, input, opts) - defp run_with_hooks(module, input, run_opts, context, in_transaction?) do # Run before_action hooks case Ash.ActionInput.run_before_actions(input) do diff --git a/test/notifier/notifier_test.exs b/test/notifier/notifier_test.exs index 9a85aad38..00b774f4e 100644 --- a/test/notifier/notifier_test.exs +++ b/test/notifier/notifier_test.exs @@ -185,6 +185,47 @@ defmodule Ash.Test.NotifierTest do end) end end + end + + attributes do + uuid_primary_key :id + + attribute :name, :string do + public?(true) + end + end + + relationships do + many_to_many :related_posts, __MODULE__, + public?: true, + through: PostLink, + source_attribute_on_join_resource: :source_post_id, + destination_attribute_on_join_resource: :destination_post_id + + has_many :comments, Comment, destination_attribute: :post_id, public?: true + end + end + + defmodule MnesiaPost do + @moduledoc false + use Ash.Resource, + domain: Domain, + data_layer: Ash.DataLayer.Mnesia, + simple_notifiers: [ + Notifier + ] + + mnesia do + table :notifier_test_mnesia_posts + end + + actions do + default_accept :* + defaults [:read, create: :*] + + destroy :destroy do + primary? true + end action :emit_notification, :atom do transaction? false @@ -252,16 +293,6 @@ defmodule Ash.Test.NotifierTest do public?(true) end end - - relationships do - many_to_many :related_posts, __MODULE__, - public?: true, - through: PostLink, - source_attribute_on_join_resource: :source_post_id, - destination_attribute_on_join_resource: :destination_post_id - - has_many :comments, Comment, destination_attribute: :post_id, public?: true - end end defmodule PostWithConflictingLoadNotifiers do @@ -392,40 +423,59 @@ defmodule Ash.Test.NotifierTest do assert_receive {:notification, %Ash.Notifier.Notification{data: %Comment{name: "auto"}}} end - test "a nested generic action notification is sent automatically on create" do - Post - |> Ash.Changeset.for_create(:create_with_generic_action, %{name: "foobar"}) - |> Ash.create!() + describe "nested generic action notifications" do + setup do + import ExUnit.CaptureLog - assert_receive {:notification, %{action: %{type: :create}}} - assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} - end + capture_log(fn -> + Ash.DataLayer.Mnesia.start(Domain, [MnesiaPost]) + end) - test "a nested generic action notification is sent automatically on destroy" do - post = - Post - |> Ash.Changeset.for_create(:create, %{name: "foobar"}) + on_exit(fn -> + capture_log(fn -> + :mnesia.stop() + :mnesia.delete_schema([node()]) + end) + end) + + :ok + end + + test "a nested generic action notification is sent automatically on create" do + MnesiaPost + |> Ash.Changeset.for_create(:create_with_generic_action, %{name: "foobar"}) |> Ash.create!() - assert_receive {:notification, %{action: %{type: :create}}} + assert_receive {:notification, %{action: %{type: :create}}} + assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} + end - post - |> Ash.Changeset.for_destroy(:destroy_with_generic_action) - |> Ash.destroy!() + test "a nested generic action notification is sent automatically on destroy" do + post = + MnesiaPost + |> Ash.Changeset.for_create(:create, %{name: "foobar"}) + |> Ash.create!() - assert_receive {:notification, %{action: %{type: :destroy}}} - assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} - end + assert_receive {:notification, %{action: %{type: :create}}} - test "a top-level generic action with transaction? true sends its notifications" do - Post - |> Ash.ActionInput.for_action(:emit_notification_in_transaction, %{}) - |> Ash.run_action!() + post + |> Ash.Changeset.for_destroy(:destroy_with_generic_action) + |> Ash.destroy!() - assert_receive {:notification, - %Ash.Notifier.Notification{ - data: %{generic?: true, started_transaction?: true} - }} + assert_receive {:notification, %{action: %{type: :destroy}}} + assert_receive {:notification, %Ash.Notifier.Notification{data: %{generic?: true}}} + end + + test "a top-level generic action with transaction? true sends its notifications" do + MnesiaPost + |> Ash.ActionInput.for_action(:emit_notification_in_transaction, %{}) + |> Ash.run_action!() + + assert_receive {:notification, + %Ash.Notifier.Notification{ + data: %{generic?: true, started_transaction?: true} + }} + end end test "the `load/1` change puts the loaded data into the notification" do