Fix: honor bulk upsert condition (#1432)

* Require Ash.Expr to make sure upsert_conflict/1 is available

* Make sure it correctly tests opts[:upsert_condition] for nilness

* Make sure the base changeset also adheres to the upsert condition

* Revert the changes so we can show the problem with a test case

* Add a test case that shows what the problem is

* Add the upsert_condition filter to the base changeset again

* Change the way the test checks because bulk_create/4 does not return unchanged records

lib/ash/actions/create/bulk.ex

@@ -297,6 +297,12 @@ defmodule Ash.Actions.Create.Bulk do
   end
 
   defp base_changeset(resource, domain, opts, action) do
+    upsert_condition =
+      case opts[:upsert_condition] do
+        nil -> action && action.upsert_condition
+        other -> other
+      end
+
     resource
     |> Ash.Changeset.new()
     |> Map.put(:domain, domain)
@@ -308,12 +314,17 @@ defmodule Ash.Actions.Create.Bulk do
           Ash.Changeset.expand_upsert_fields(
             opts[:upsert_fields] || action.upsert_fields,
             resource
-          )
+          ),
+        upsert_condition: upsert_condition
       }
     })
     |> Ash.Actions.Helpers.add_context(opts)
     |> Ash.Changeset.set_context(opts[:context] || %{})
     |> Ash.Changeset.prepare_changeset_for_action(action, opts)
+    |> then(fn
+      changeset when upsert_condition != nil -> Ash.Changeset.filter(changeset, upsert_condition)
+      changeset -> changeset
+    end)
   end
 
   defp lazy_matching_default_values(resource) do
@@ -1185,7 +1196,11 @@ defmodule Ash.Actions.Create.Bulk do
                           opts[:upsert_fields] || action.upsert_fields,
                           resource
                         ),
-                      upsert_condition: opts[:upsert_condition] || action.upsert_condition,
+                      upsert_condition:
+                        case opts[:upsert_condition] do
+                          nil -> action && action.upsert_condition
+                          other -> other
+                        end,
                       tenant: Ash.ToTenant.to_tenant(opts[:tenant], resource)
                     }
                   )

lib/ash/changeset/changeset.ex

@@ -1245,7 +1245,11 @@ defmodule Ash.Changeset do
       get_action_entity(changeset.resource, action) ||
         raise_no_action(changeset.resource, action, :create)
 
-    upsert_condition = opts[:upsert_condition] || (action && action.upsert_condition)
+    upsert_condition =
+      case opts[:upsert_condition] do
+        nil -> action && action.upsert_condition
+        other -> other
+      end
 
     changeset
     |> set_context(%{

test/actions/bulk/bulk_create_test.exs

@@ -446,6 +446,91 @@ defmodule Ash.Test.Actions.BulkCreateTest do
     end
   end
 
+  defmodule Product do
+    @moduledoc false
+    use Ash.Resource, domain: Domain, data_layer: Ash.DataLayer.Ets
+
+    ets do
+      private?(true)
+    end
+
+    actions do
+      default_accept [:gtin, :title]
+      defaults [:read, :update, :destroy]
+
+      create :create do
+        primary? true
+
+        argument :price, :map
+        change manage_relationship(:price, type: :create)
+
+        upsert? true
+        upsert_fields {:replace_all_except, [:gtin, :id, :inserted_at]}
+        upsert_identity :unique_gtin
+        upsert_condition expr(false)
+      end
+    end
+
+    attributes do
+      uuid_v7_primary_key :id
+
+      attribute :gtin, :string do
+        allow_nil? false
+        public? true
+      end
+
+      attribute :title, :string, public?: true
+
+      timestamps()
+    end
+
+    relationships do
+      has_one :price, Ash.Test.Actions.BulkCreateTest.Price, public?: true
+    end
+
+    identities do
+      identity :unique_gtin, :gtin do
+        pre_check_with Ash.Test.Actions.BulkCreateTest.Domain
+      end
+    end
+  end
+
+  defmodule Price do
+    @moduledoc false
+    use Ash.Resource, domain: Domain, data_layer: Ash.DataLayer.Ets
+
+    ets do
+      private?(true)
+    end
+
+    actions do
+      default_accept [:max, :min, :rrp]
+
+      defaults [:read, :update, :destroy]
+
+      create :create do
+        primary? true
+        upsert? true
+        upsert_fields {:replace_all_except, [:inserted_at]}
+      end
+    end
+
+    attributes do
+      attribute :max, :integer, public?: true
+      attribute :min, :integer, public?: true
+      attribute :rrp, :integer, public?: true
+      timestamps()
+    end
+
+    relationships do
+      belongs_to :product, Product do
+        allow_nil? false
+        primary_key? true
+        public? false
+      end
+    end
+  end
+
   test "returns created records" do
     org =
       Org
@@ -632,6 +717,7 @@ defmodule Ash.Test.Actions.BulkCreateTest do
                Post,
                :create,
                tenant: org.id,
+               return_errors?: true,
                return_records?: true,
                sorted?: true,
                return_errors?: true,
@@ -652,6 +738,7 @@ defmodule Ash.Test.Actions.BulkCreateTest do
                ],
                Post,
                :create,
+               return_errors?: true,
                return_records?: true,
                tenant: org.id,
                upsert?: true,
@@ -664,6 +751,59 @@ defmodule Ash.Test.Actions.BulkCreateTest do
              )
   end
 
+  test "respects upsert_condition despite we have a relationship" do
+    product =
+      Ash.bulk_create(
+        [
+          %{
+            gtin: "1234567891011",
+            title: "Nano Bubble Gum",
+            price: %{
+              min: 123,
+              max: 456,
+              rrp: 789
+            }
+          }
+        ],
+        Product,
+        :create,
+        return_errors?: true,
+        return_records?: true
+      )
+      |> then(fn result -> List.first(result.records) end)
+
+    assert product.gtin == "1234567891011"
+    assert product.title == "Nano Bubble Gum"
+    assert product.price.min == 123
+    assert product.price.max == 456
+    assert product.price.rrp == 789
+
+    result =
+      Ash.bulk_create(
+        [
+          %{
+            gtin: "1234567891011",
+            title: "Nano Bubble Gum",
+            price: %{
+              # Note: we try to change the price here.
+              min: 234,
+              max: 567,
+              rrp: 890
+            }
+          }
+        ],
+        Product,
+        :create,
+        return_errors?: true,
+        return_records?: true,
+        upsert?: true,
+        # But the upsert_condition says "no!".
+        upsert_condition: expr(false)
+      )
+
+    assert result.records == []
+  end
+
   test "can upsert with :replace" do
     org =
       Org