From 6ff462169dfe0f3f808e1b8008ce2a2ef71053d1 Mon Sep 17 00:00:00 2001 From: Zach Daniel Date: Wed, 9 Oct 2024 22:32:39 -0400 Subject: [PATCH] improvement: StaleRecordError on skipped upsert --- lib/ash.ex | 10 ++++++++++ lib/ash/actions/create/bulk.ex | 3 +++ lib/ash/actions/create/create.ex | 15 +++++++++++++++ test/actions/create_test.exs | 28 ++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+) diff --git a/lib/ash.ex b/lib/ash.ex index 76ff2f4e0..ce61b2345 100644 --- a/lib/ash.ex +++ b/lib/ash.ex @@ -284,6 +284,12 @@ defmodule Ash do doc: "If a conflict is found based on the primary key, the record is updated in the database (requires upsert support)" ], + return_skipped_upsert?: [ + type: :boolean, + default: false, + doc: + "If `true`, and a record was *not* upserted because its filter prevented the upsert, the original record (which was *not* upserted) will be returned." + ], upsert_identity: [ type: :atom, doc: @@ -578,6 +584,10 @@ defmodule Ash do doc: "If a conflict is found based on the primary key, the record is updated in the database (requires upsert support)" ], + return_skipped_upsert?: [ + type: :boolean, + hide: true + ], upsert_identity: [ type: :atom, doc: diff --git a/lib/ash/actions/create/bulk.ex b/lib/ash/actions/create/bulk.ex index dadb66db4..00975f0a3 100644 --- a/lib/ash/actions/create/bulk.ex +++ b/lib/ash/actions/create/bulk.ex @@ -1263,6 +1263,9 @@ defmodule Ash.Actions.Create.Bulk do end case result do + {:ok, %{__metadata__: %{upsert_skipped: true}}} -> + [] + {:ok, result} -> {:ok, [ diff --git a/lib/ash/actions/create/create.ex b/lib/ash/actions/create/create.ex index f7a4d81c4..cb34f2732 100644 --- a/lib/ash/actions/create/create.ex +++ b/lib/ash/actions/create/create.ex @@ -363,6 +363,21 @@ defmodule Ash.Actions.Create do opts[:upsert_identity] || changeset.action.upsert_identity ) ) + |> case do + {:ok, %{__metadata__: %{upsert_skipped: true}}} = result -> + if opts[:return_skipped_upsert?] do + result + else + {:error, + Ash.Error.Changes.StaleRecord.exception( + resource: changeset.resource, + filter: changeset.filter + )} + end + + other -> + other + end |> Helpers.rollback_if_in_transaction( changeset.resource, changeset diff --git a/test/actions/create_test.exs b/test/actions/create_test.exs index 4d8a90548..ab9dc8903 100644 --- a/test/actions/create_test.exs +++ b/test/actions/create_test.exs @@ -616,6 +616,7 @@ defmodule Ash.Test.Actions.CreateTest do }) |> Ash.create!( upsert?: true, + return_skipped_upsert?: true, upsert_identity: :unique_title, upsert_fields: [:contents, :updated_at], upsert_condition: expr(contents != upsert_conflict(:contents)) @@ -624,6 +625,33 @@ defmodule Ash.Test.Actions.CreateTest do assert Ash.Resource.get_metadata(post, :upsert_skipped) end + test "skips upsert when condition doesn't match, returning error unless asked for" do + Post + |> Ash.Changeset.new() + |> Ash.Changeset.change_attributes(%{ + title: "foo", + contents: "bar", + tag: "before" + }) + |> Ash.create!() + + assert_raise Ash.Error.Invalid, ~r/Stale/, fn -> + Post + |> Ash.Changeset.new() + |> Ash.Changeset.change_attributes(%{ + title: "foo", + contents: "bar", + tag: "after" + }) + |> Ash.create!( + upsert?: true, + upsert_identity: :unique_title, + upsert_fields: [:contents, :updated_at], + upsert_condition: expr(contents != upsert_conflict(:contents)) + ) + end + end + test "timestamps will match each other" do post = Post