fix: ensure that all notifications are sent for bulk destroy/update

closes #1186
ash-project · May 22, 2024 · 67c6e5e · 67c6e5e
1 parent e01e952
commit 67c6e5e
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 5 deletions.
diff --git a/lib/ash/actions/destroy/bulk.ex b/lib/ash/actions/destroy/bulk.ex
@@ -285,7 +285,7 @@ defmodule Ash.Actions.Destroy.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
@@ -304,9 +304,11 @@ defmodule Ash.Actions.Destroy.Bulk do
 
               %{bulk_result | notifications: notifications}
             else
-              Ash.Actions.Helpers.warn_missed!(atomic_changeset.resource, action, %{
-                resource_notifications: notifications
-              })
+              if opts[:notify?] do
+                Ash.Actions.Helpers.warn_missed!(atomic_changeset.resource, action, %{
+                  resource_notifications: notifications
+                })
+              end
 
               %{bulk_result | notifications: []}
             end

diff --git a/lib/ash/actions/update/bulk.ex b/lib/ash/actions/update/bulk.ex
@@ -237,7 +237,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/test/actions/bulk/bulk_destroy_test.exs b/test/actions/bulk/bulk_destroy_test.exs
@@ -193,6 +193,25 @@ defmodule Ash.Test.Actions.BulkDestroyTest do
     assert_received {:notification, %{data: %{title: "title2"}}}
   end
 
+  test "doesn't send notifications if not asked to" do
+    assert %Ash.BulkResult{records: [%{}, %{}]} =
+             Ash.bulk_create!([%{title: "title1"}, %{title: "title2"}], Post, :create,
+               return_stream?: true,
+               return_records?: true
+             )
+             |> Stream.map(fn {:ok, result} ->
+               result
+             end)
+             |> Ash.bulk_destroy!(:destroy, %{},
+               resource: Post,
+               strategy: :stream,
+               return_records?: true,
+               return_errors?: true
+             )
+
+    refute_received {:notification, _}
+  end
+
   test "notifications can be returned" do
     assert %Ash.BulkResult{records: [%{}, %{}], notifications: [%{}, %{}]} =
              Ash.bulk_create!([%{title: "title1"}, %{title: "title2"}], Post, :create,