From e5979494246e28d5cf04b4d48a879c85344a6eff Mon Sep 17 00:00:00 2001 From: Rebecca Le Date: Tue, 27 Oct 2026 14:27:53 +0800 Subject: [PATCH 1/2] feat: add `on_delete_message` and `on_update_message` options to references Deleting or updating a destination record that is still referenced returned a hardcoded "would leave records behind" error. The message can now be customised per reference with `on_delete_message` and `on_update_message`, falling back to the existing message when unset. Also flattens `add_related_foreign_key_constraints` so the constraint name and message are resolved once, matching `add_my_foreign_key_constraints`. --- .formatter.exs | 2 + .../dsls/DSL-AshPostgres.DataLayer.md | 2 + lib/data_layer.ex | 65 +++++++++---------- lib/reference.ex | 12 ++++ test/sourced_foreign_key_test.exs | 27 ++++++-- test/support/resources/sourced_fk_child.ex | 5 +- test/support/resources/sourced_fk_parent.ex | 4 +- test/update_test.exs | 25 ++++++- 8 files changed, 99 insertions(+), 43 deletions(-) diff --git a/.formatter.exs b/.formatter.exs index baf5d3ff..28d567b0 100644 --- a/.formatter.exs +++ b/.formatter.exs @@ -40,7 +40,9 @@ spark_locals_without_parens = [ name: 1, nulls_distinct: 1, on_delete: 1, + on_delete_message: 1, on_update: 1, + on_update_message: 1, polymorphic?: 1, polymorphic_on_delete: 1, polymorphic_on_update: 1, diff --git a/documentation/dsls/DSL-AshPostgres.DataLayer.md b/documentation/dsls/DSL-AshPostgres.DataLayer.md index 89a91f1a..fe9c3e4a 100644 --- a/documentation/dsls/DSL-AshPostgres.DataLayer.md +++ b/documentation/dsls/DSL-AshPostgres.DataLayer.md @@ -321,6 +321,8 @@ reference :post, on_delete: :delete, on_update: :update, name: "comments_to_post |------|------|---------|------| | [`ignore?`](#postgres-references-reference-ignore?){: #postgres-references-reference-ignore? } | `boolean` | | If set to true, no reference is created for the given relationship. This is useful if you need to define it in some custom way | | [`on_delete`](#postgres-references-reference-on_delete){: #postgres-references-reference-on_delete } | `:delete \| :nilify \| :nothing \| :restrict \| {:nilify, atom \| list(atom)}` | | What should happen to records of this resource when the referenced record of the *destination* resource is deleted. | +| [`on_delete_message`](#postgres-references-reference-on_delete_message){: #postgres-references-reference-on_delete_message } | `String.t` | | A custom message to use when deleting a *destination* record is prevented by this foreign key, e.g. with `on_delete: :restrict` | +| [`on_update_message`](#postgres-references-reference-on_update_message){: #postgres-references-reference-on_update_message } | `String.t` | | A custom message to use when updating the referenced attribute of a *destination* record is prevented by this foreign key, e.g. with `on_update: :restrict` | | [`on_update`](#postgres-references-reference-on_update){: #postgres-references-reference-on_update } | `:update \| :nilify \| :nothing \| :restrict` | | What should happen to records of this resource when the referenced destination_attribute of the *destination* record is update. | | [`deferrable`](#postgres-references-reference-deferrable){: #postgres-references-reference-deferrable } | `false \| true \| :initially` | `false` | Whether or not the constraint is deferrable. This only affects the migration generator. | | [`name`](#postgres-references-reference-name){: #postgres-references-reference-name } | `String.t` | | The name of the foreign key to generate in the database. Defaults to __fkey | diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 64391b99..99d42d51 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -3590,11 +3590,11 @@ defmodule AshPostgres.DataLayer do type when type in [:upsert, :update] -> ecto_changeset |> add_my_foreign_key_constraints(record.__struct__, repo) - |> add_related_foreign_key_constraints(record.__struct__, repo) + |> add_related_foreign_key_constraints(record.__struct__, repo, :update) :delete -> ecto_changeset - |> add_related_foreign_key_constraints(record.__struct__, repo) + |> add_related_foreign_key_constraints(record.__struct__, repo, :delete) end end @@ -4170,7 +4170,7 @@ defmodule AshPostgres.DataLayer do end) end - defp add_related_foreign_key_constraints(changeset, resource, repo) do + defp add_related_foreign_key_constraints(changeset, resource, repo, type) do # TODO: this doesn't guarantee us to get all of them, because if something is related to this # schema and there is no back-relation, then this won't catch it's foreign key constraints resource @@ -4190,43 +4190,38 @@ defmodule AshPostgres.DataLayer do name: relationship_name }, changeset -> - case AshPostgres.DataLayer.Info.reference(source, relationship_name) do - %{name: name} when not is_nil(name) -> - case repo.default_constraint_match_type(:foreign, name) do - {:regex, regex} -> - Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, - name: regex, - message: "would leave records behind", - match: :exact - ) + reference = AshPostgres.DataLayer.Info.reference(source, relationship_name) - match -> - Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, - name: name, - message: "would leave records behind", - match: match - ) - end + name = + case reference do + %{name: name} when not is_nil(name) -> + name - _ -> - name = + _ -> "#{AshPostgres.DataLayer.Info.table(source)}_#{attribute_source(source, source_attribute)}_fkey" + end - case repo.default_constraint_match_type(:foreign, name) do - {:regex, regex} -> - Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, - name: regex, - message: "would leave records behind", - match: :exact - ) + message = + case {type, reference} do + {:delete, %{on_delete_message: message}} when not is_nil(message) -> message + {:update, %{on_update_message: message}} when not is_nil(message) -> message + _ -> "would leave records behind" + end - match -> - Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, - name: name, - message: "would leave records behind", - match: match - ) - end + case repo.default_constraint_match_type(:foreign, name) do + {:regex, regex} -> + Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, + name: regex, + message: message, + match: :exact + ) + + match -> + Ecto.Changeset.foreign_key_constraint(changeset, destination_attribute, + name: name, + message: message, + match: match + ) end end) end diff --git a/lib/reference.ex b/lib/reference.ex index e1e0ea48..0a70582c 100644 --- a/lib/reference.ex +++ b/lib/reference.ex @@ -8,6 +8,8 @@ defmodule AshPostgres.Reference do :relationship, :on_delete, :on_update, + :on_delete_message, + :on_update_message, :name, :match_with, :match_type, @@ -42,6 +44,16 @@ defmodule AshPostgres.Reference do What should happen to records of this resource when the referenced record of the *destination* resource is deleted. """ ], + on_delete_message: [ + type: :string, + doc: + "A custom message to use when deleting a *destination* record is prevented by this foreign key, e.g. with `on_delete: :restrict`" + ], + on_update_message: [ + type: :string, + doc: + "A custom message to use when updating the referenced attribute of a *destination* record is prevented by this foreign key, e.g. with `on_update: :restrict`" + ], on_update: [ type: {:one_of, [:update, :nilify, :nothing, :restrict]}, doc: """ diff --git a/test/sourced_foreign_key_test.exs b/test/sourced_foreign_key_test.exs index 0e82fdb5..7aecdcda 100644 --- a/test/sourced_foreign_key_test.exs +++ b/test/sourced_foreign_key_test.exs @@ -14,18 +14,37 @@ defmodule AshPostgres.SourcedForeignKeyTest do |> Ash.create() end - test "destroying a parent with children returns an invalid error when the fk attribute has a different source" do + test "destroying a parent with children returns the reference's on_delete_message when the fk attribute has a different source" do parent = Ash.create!(SourcedFkParent, %{}) Ash.create!(SourcedFkChild, %{parent_ref: parent.id}) assert {:error, %Ash.Error.Invalid{errors: errors}} = Ash.destroy(parent) assert Enum.any?(errors, fn - %Ash.Error.Changes.InvalidAttribute{message: message} -> - message =~ "would leave records behind" + %Ash.Error.Changes.InvalidAttribute{message: "parent still has children"} -> true + _ -> false + end), + "Expected custom on_delete_message, got: #{inspect(errors)}" + end + + test "updating a referenced parent attribute returns the reference's on_update_message" do + parent = Ash.create!(SourcedFkParent, %{}) + Ash.create!(SourcedFkChild, %{parent_ref: parent.id}) + + assert {:error, %Ash.Error.Invalid{errors: errors}} = + parent + |> Ash.Changeset.for_update(:update, %{id: Ash.UUID.generate()}) + |> Ash.update() + + assert Enum.any?(errors, fn + %Ash.Error.Changes.InvalidAttribute{ + message: "parent is still referenced by children" + } -> + true _ -> false - end) + end), + "Expected custom on_update_message, got: #{inspect(errors)}" end end diff --git a/test/support/resources/sourced_fk_child.ex b/test/support/resources/sourced_fk_child.ex index 1d9be997..4ac41dea 100644 --- a/test/support/resources/sourced_fk_child.ex +++ b/test/support/resources/sourced_fk_child.ex @@ -37,7 +37,10 @@ defmodule AshPostgres.Test.SourcedFkChild do repo AshPostgres.TestRepo references do - reference :parent, on_delete: :restrict + reference :parent, + on_delete: :restrict, + on_delete_message: "parent still has children", + on_update_message: "parent is still referenced by children" end end end diff --git a/test/support/resources/sourced_fk_parent.ex b/test/support/resources/sourced_fk_parent.ex index 30f24631..d189244d 100644 --- a/test/support/resources/sourced_fk_parent.ex +++ b/test/support/resources/sourced_fk_parent.ex @@ -11,11 +11,11 @@ defmodule AshPostgres.Test.SourcedFkParent do actions do default_accept(:*) - defaults([:create, :read, :destroy]) + defaults([:create, :read, :destroy, :update]) end attributes do - uuid_primary_key(:id) + uuid_primary_key(:id, writable?: true, public?: true) end relationships do diff --git a/test/update_test.exs b/test/update_test.exs index a9e6797b..6bf11ba0 100644 --- a/test/update_test.exs +++ b/test/update_test.exs @@ -4,11 +4,34 @@ defmodule AshPostgres.UpdateTest do use AshPostgres.RepoCase, async: false - alias AshPostgres.Test.Post + alias AshPostgres.Test.{Permalink, Post} require Ash.Query import ExUnit.CaptureLog import Ash.Expr + test "updating a referenced attribute returns would leave records behind error" do + post = + Post + |> Ash.Changeset.for_create(:create, %{}) + |> Ash.create!() + + Permalink |> Ash.Changeset.for_create(:create, %{post_id: post.id}) |> Ash.create!() + + assert {:error, %Ash.Error.Invalid{errors: errors}} = + post + |> Ash.Changeset.for_update(:update, %{id: Ash.UUID.generate()}) + |> Ash.update() + + assert Enum.any?(errors, fn + %Ash.Error.Changes.InvalidAttribute{message: msg} -> + msg =~ "would leave records behind" + + _ -> + false + end), + "Expected 'would leave records behind' error, got: #{inspect(errors)}" + end + test "can update a keyword type attribute" do post = Post From 2978893c67c07de0b0bcc8efd6f0ab92c3b3077d Mon Sep 17 00:00:00 2001 From: Rebecca Le Date: Tue, 27 Oct 2026 14:37:54 +0800 Subject: [PATCH 2/2] fix: `foreign_key_names` take precedence over relationship-derived foreign keys Ecto uses the most recently added matching constraint, and since 1f6621b8 `add_configured_foreign_key_constraints` ran before `add_my_foreign_key_constraints` and `add_related_foreign_key_constraints`. Any `foreign_key_names` entry for a foreign key that a relationship already covered had its key and message shadowed. Configured foreign keys are now added last again. --- lib/data_layer.ex | 6 +++++- test/references_test.exs | 28 ++++++++++++++++++++++++++++ test/support/resources/container.ex | 4 +++- test/support/resources/permalink.ex | 2 ++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/lib/data_layer.ex b/lib/data_layer.ex index 99d42d51..0c49b85c 100644 --- a/lib/data_layer.ex +++ b/lib/data_layer.ex @@ -3577,24 +3577,28 @@ defmodule AshPostgres.DataLayer do |> set_table(changeset, type, table_error?) |> Ecto.Changeset.cast(%{}, []) |> force_changes(Map.take(changeset.attributes, attributes_to_change)) - |> add_configured_foreign_key_constraints(record.__struct__, repo) |> add_unique_indexes(record.__struct__, changeset, repo) |> add_check_constraints(record.__struct__, repo) |> add_exclusion_constraints(record.__struct__, repo) + # Ecto uses the most recently added matching constraint, so configured + # `foreign_key_names` are added last to override the relationship-derived ones. case type do :create -> ecto_changeset |> add_my_foreign_key_constraints(record.__struct__, repo) + |> add_configured_foreign_key_constraints(record.__struct__, repo) type when type in [:upsert, :update] -> ecto_changeset |> add_my_foreign_key_constraints(record.__struct__, repo) |> add_related_foreign_key_constraints(record.__struct__, repo, :update) + |> add_configured_foreign_key_constraints(record.__struct__, repo) :delete -> ecto_changeset |> add_related_foreign_key_constraints(record.__struct__, repo, :delete) + |> add_configured_foreign_key_constraints(record.__struct__, repo) end end diff --git a/test/references_test.exs b/test/references_test.exs index f18a041b..e3768260 100644 --- a/test/references_test.exs +++ b/test/references_test.exs @@ -198,4 +198,32 @@ defmodule AshPostgres.ReferencesTest do assert Keyword.get(private_vars, :constraint) == "special_name_fkey" assert Keyword.get(private_vars, :constraint_type) == :foreign_key end + + test "foreign_key_names message takes precedence over the relationship's foreign key on create" do + assert {:error, %Ash.Error.Invalid{errors: errors}} = + AshPostgres.Test.Permalink + |> Ash.Changeset.for_create(:create, %{post_id: Ash.UUID.generate()}) + |> Ash.create() + + assert [ + %Ash.Error.Changes.InvalidAttribute{ + field: :post_id, + message: "post must exist" + } + ] = errors + end + + test "foreign_key_names message takes precedence over the related foreign key on destroy" do + container = Ash.create!(AshPostgres.Test.Container, %{}) + Ash.create!(AshPostgres.Test.Item, %{container_id: container.id}) + + assert {:error, %Ash.Error.Invalid{errors: errors}} = Ash.destroy(container) + + assert [ + %Ash.Error.Changes.InvalidAttribute{ + field: :id, + message: "container still has items" + } + ] = errors + end end diff --git a/test/support/resources/container.ex b/test/support/resources/container.ex index 08d0b647..72c4c947 100644 --- a/test/support/resources/container.ex +++ b/test/support/resources/container.ex @@ -11,11 +11,13 @@ defmodule AshPostgres.Test.Container do postgres do table "containers" repo AshPostgres.TestRepo + + foreign_key_names [{:id, "items_container_id_fkey", "container still has items"}] end actions do default_accept(:*) - defaults([:read, create: :*]) + defaults([:read, :destroy, create: :*]) end attributes do diff --git a/test/support/resources/permalink.ex b/test/support/resources/permalink.ex index 8eaf30d7..2ad6bf8b 100644 --- a/test/support/resources/permalink.ex +++ b/test/support/resources/permalink.ex @@ -30,6 +30,8 @@ defmodule AshPostgres.Test.Permalink do table "post_permalinks" repo AshPostgres.TestRepo + foreign_key_names [{:post_id, "post_permalinks_post_id_fkey", "post must exist"}] + references do reference :post, on_delete: :nothing end