fix: handle nil notification results better

fix: don't emit after batch notifications if `notify?: false`
ash-project · May 16, 2024 · 23d7479 · 23d7479
1 parent 51512f7
commit 23d7479
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 12 deletions.
diff --git a/lib/ash/actions/destroy/bulk.ex b/lib/ash/actions/destroy/bulk.ex
@@ -282,9 +282,10 @@ defmodule Ash.Actions.Destroy.Bulk do
 
           notifications =
             if notify? do
-              List.wrap(bulk_result.notifications) ++ Process.delete(:ash_notifications)
+              List.wrap(bulk_result.notifications) ++
+                List.wrap(Process.delete(:ash_notifications))
             else
-              List.wrap(bulk_result.notifications)
+              []
             end
 
           if opts[:return_notifications?] do
@@ -401,8 +402,8 @@ defmodule Ash.Actions.Destroy.Bulk do
               %{
                 bulk_result
                 | notifications:
-                    (bulk_result.notifications || []) ++ Process.delete(:ash_notifications) ||
-                      []
+                    (bulk_result.notifications || []) ++
+                      List.wrap(Process.delete(:ash_notifications))
               }
             else
               bulk_result
@@ -1426,7 +1427,7 @@ defmodule Ash.Actions.Destroy.Bulk do
 
   defp errors(result, {:error, error}, opts) do
     if opts[:return_errors?] do
-      {result.error_count + 1, [error | result.errors]}
+      {result.error_count + 1, [error | List.wrap(result.errors)]}
     else
       {result.error_count + 1, []}
     end
@@ -1546,9 +1547,9 @@ defmodule Ash.Actions.Destroy.Bulk do
 
       new_notifications =
         if is_list(notification) do
-          notification ++ notifications
+          notification ++ List.wrap(notifications)
         else
-          [notification | notifications]
+          [notification | List.wrap(notifications)]
         end
 
       Process.put({:bulk_destroy_notifications, ref}, new_notifications)

diff --git a/lib/ash/actions/update/bulk.ex b/lib/ash/actions/update/bulk.ex
@@ -233,7 +233,7 @@ defmodule Ash.Actions.Update.Bulk do
                   List.wrap(bulk_result.notifications) ++
                     List.wrap(Process.delete(:ash_notifications))
                 else
-                  List.wrap(bulk_result.notifications)
+                  []
                 end
 
               if opts[:return_notifications?] do

diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex
@@ -3397,7 +3397,7 @@ defmodule Ash.Changeset do
         case result do
           {:ok, new_result, new_notifications} ->
             all_notifications =
-              Enum.map(notifications ++ new_notifications, fn notification ->
+              Enum.map(notifications ++ List.wrap(new_notifications), fn notification ->
                 %{
                   notification
                   | resource: notification.resource || changeset.resource,

diff --git a/lib/ash/resource/change/cascade_destroy.ex b/lib/ash/resource/change/cascade_destroy.ex
@@ -90,7 +90,7 @@ defmodule Ash.Resource.Change.CascadeDestroy do
 
       case {destroy_related(records, opts, context), opts[:return_notifications?]} do
         {_, false} -> result
-        {%{notifications: []}, true} -> result
+        {%{notifications: empty}, true} when empty in [nil, []] -> result
         {%{notifications: notifications}, true} -> Enum.concat(result, notifications)
       end
     else

diff --git a/test/resource/changes/cascade_destroy_test.exs b/test/resource/changes/cascade_destroy_test.exs
@@ -144,8 +144,6 @@ defmodule Ash.Test.Resource.Change.CascadeDestroy do
     1..Enum.random(3..5)
     |> Enum.map(fn _ -> Post.create!(%{author_id: author.id}) end)
 
-    # This doesn't work either
-    # Author.no_notification_destroy!([author])
     Ash.bulk_destroy!([author], :no_notification_destroy, %{})
 
     assert [] = Post.read!()