improvement: simplifications and clarifications around bulk callback behavior

documentation/topics/resources/changes.md

@@ -118,7 +118,7 @@ end
 
 ## Atomic Changes
 
-To make a change atomic, you have to implement the `c:Ash.Resource.Change.atomic/3` callback. This callback returns a map of changes to attributes that should be changed atomically. We will also honor any `c:Ash.Resource.Change.after_batch/3` functionality to run code after atomic changes have been applied.
+To make a change atomic, you have to implement the `c:Ash.Resource.Change.atomic/3` callback. This callback returns a map of changes to attributes that should be changed atomically. We will also honor any `c:Ash.Resource.Change.after_batch/3` functionality to run code after atomic changes have been applied (only if `atomic/3` callback has also been defined). Note that `c:Ash.Resource.Change.before_batch/3` is not supported in this scenario and will be ignored.
 
 ```elixir
 defmodule MyApp.Changes.Slugify do
@@ -160,6 +160,10 @@ end
 
 Changes can support being run on batches of changesets, using the `c:Ash.Resource.Change.batch_change/3`, `c:Ash.Resource.Change.before_batch/3`, and `c:Ash.Resource.Change.after_batch/3` callbacks.
 
+> ### batch_change/3 must be defined {: .warning}
+>
+> `c:Ash.Resource.Change.before_batch/3` must be defined for `c:Ash.Resource.Change.before_batch/3` and `c:Ash.Resource.Change.after_batch/3` to be called!
+
 For some changes, this may not be necessary at all, i.e the `Slugify` example has no benefit from batching. If no batch callbacks are added, your change will be run on a loop over the changesets. For the sake of example, however, we will show what it might look like to implement batching for our `Slugify` example.
 
 ```elixir

lib/ash/actions/create/bulk.ex

@@ -687,7 +687,8 @@ defmodule Ash.Actions.Create.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, _opts}}, _} ->
-        function_exported?(module, :before_batch, 3)
+        function_exported?(module, :batch_change, 3) &&
+          function_exported?(module, :before_batch, 3)
 
       _ ->
         false
@@ -1264,7 +1265,8 @@ defmodule Ash.Actions.Create.Bulk do
     all_changes
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
-        function_exported?(module, :after_batch, 3) &&
+        function_exported?(module, :batch_change, 3) &&
+          function_exported?(module, :after_batch, 3) &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -1435,7 +1437,8 @@ defmodule Ash.Actions.Create.Bulk do
                 Enum.any?(batch, fn item ->
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
-                (function_exported?(module, :after_batch, 3) &&
+                (function_exported?(module, :batch_change, 3) &&
+                   function_exported?(module, :after_batch, 3) &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -1486,7 +1489,8 @@ defmodule Ash.Actions.Create.Bulk do
                   Enum.any?(batch, fn item ->
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
-                  (function_exported?(module, :after_batch, 3) &&
+                  (function_exported?(module, :batch_change, 3) &&
+                     function_exported?(module, :after_batch, 3) &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{

lib/ash/actions/destroy/bulk.ex

@@ -412,7 +412,8 @@ defmodule Ash.Actions.Destroy.Bulk do
       |> Map.new()
       |> Map.put(:calculations, calculations)
 
-    with {:ok, query} <- authorize_bulk_query(query, atomic_changeset, opts),
+    with {:ok, query} <-
+           authorize_bulk_query(query, atomic_changeset, opts),
          {:ok, atomic_changeset, query} <-
            authorize_atomic_changeset(query, atomic_changeset, opts),
          {query, atomic_changeset} <- add_changeset_filters(query, atomic_changeset),
@@ -1802,6 +1803,7 @@ defmodule Ash.Actions.Destroy.Bulk do
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
         function_exported?(module, :after_batch, 3) &&
+          function_exported?(module, :batch_change, 3) &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -1960,6 +1962,7 @@ defmodule Ash.Actions.Destroy.Bulk do
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
                 (function_exported?(module, :after_batch, 3) &&
+                   function_exported?(module, :batch_change, 3) &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -2010,6 +2013,7 @@ defmodule Ash.Actions.Destroy.Bulk do
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
                   (function_exported?(module, :after_batch, 3) &&
+                     function_exported?(module, :batch_change, 3) &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{

lib/ash/actions/update/bulk.ex

@@ -2144,6 +2144,7 @@ defmodule Ash.Actions.Update.Bulk do
     |> Enum.filter(fn
       {%{change: {module, change_opts}}, _} ->
         function_exported?(module, :after_batch, 3) &&
+          function_exported?(module, :batch_change, 3) &&
           module.batch_callbacks?(changesets, change_opts, context)
 
       _ ->
@@ -2314,6 +2315,7 @@ defmodule Ash.Actions.Update.Bulk do
                   item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                 end) ||
                 (function_exported?(module, :after_batch, 3) &&
+                   function_exported?(module, :batch_change, 3) &&
                    module.batch_callbacks?(batch, change_opts, context))
 
             %{
@@ -2364,6 +2366,7 @@ defmodule Ash.Actions.Update.Bulk do
                     item.relationships not in [nil, %{}] || !Enum.empty?(item.after_action)
                   end) ||
                   (function_exported?(module, :after_batch, 3) &&
+                     function_exported?(module, :batch_change, 3) &&
                      module.batch_callbacks?(batch, change_opts, context))
 
               %{

lib/ash/policy/filter_check.ex

@@ -146,21 +146,28 @@ defmodule Ash.Policy.FilterCheck do
                public?: false
              }) do
           {:ok, hydrated} ->
+            opts = [
+              resource: resource,
+              unknown_on_unknown_refs?: true
+            ]
+
             # We don't want to authorize on stale data in real life
             # but when using utilities to check if something *will* be authorized
             # that is our intent
-            data =
+            opts =
               if changeset.context[:private][:pre_flight_authorization?] do
-                data
+                case data do
+                  %Ash.Changeset.OriginalDataNotAvailable{reason: :atomic_query_destroy} ->
+                    opts
+
+                  data ->
+                    Keyword.put(opts, :record, data)
+                end
               else
-                nil
+                opts
               end
 
-            Ash.Expr.eval_hydrated(hydrated,
-              record: data,
-              resource: resource,
-              unknown_on_unknown_refs?: true
-            )
+            Ash.Expr.eval_hydrated(hydrated, opts)
 
           {:error, error} ->
             {:error, error}

test/actions/bulk/bulk_create_test.exs

@@ -11,6 +11,10 @@ defmodule Ash.Test.Actions.BulkCreateTest do
       changeset
     end
 
+    def batch_change(changesets, _, _) do
+      changesets
+    end
+
     def after_batch(results, _, _) do
       Stream.map(results, fn {_changeset, result} ->
         {:ok, %{result | title: result.title <> "_after"}}
@@ -25,6 +29,10 @@ defmodule Ash.Test.Actions.BulkCreateTest do
       changeset
     end
 
+    def batch_change(changesets, _, _) do
+      changesets
+    end
+
     def before_batch(changesets, _, _) do
       changesets
       |> Stream.map(fn changeset ->

test/actions/bulk/bulk_destroy_test.exs

@@ -8,12 +8,16 @@ defmodule Ash.Test.Actions.BulkDestroyTest do
   defmodule AddAfterToTitle do
     use Ash.Resource.Change
 
-    def change(changeset, _, %{bulk?: true}) do
+    def change(changeset, _, _) do
       changeset
     end
 
     def atomic(_, _, _), do: :ok
 
+    def batch_change(changesets, _, _) do
+      changesets
+    end
+
     def after_batch(results, _, _) do
       Stream.map(results, fn {_changeset, result} ->
         {:ok, %{result | title: result.title <> "_after"}}

test/actions/bulk/bulk_update_test.exs

@@ -8,10 +8,15 @@ defmodule Ash.Test.Actions.BulkUpdateTest do
     use Ash.Resource.Change
 
     @impl true
-    def change(changeset, _, %{bulk?: true}) do
+    def change(changeset, _, _) do
       changeset
     end
 
+    @impl true
+    def batch_change(changesets, _opts, _context) do
+      changesets
+    end
+
     @impl true
     def after_batch(results, _, _) do
       Stream.map(results, fn {_changeset, result} ->