From afbcb0eaf89baac4d515db2473f856cac2535099 Mon Sep 17 00:00:00 2001 From: Jinkyou Son Date: Wed, 18 Mar 2026 12:58:31 +0900 Subject: [PATCH 1/2] fix: on_match :destroy for many_to_many now destroys both join and destination --- lib/ash/actions/managed_relationships.ex | 119 +++++++++++++--- lib/ash/actions/read/calculations.ex | 6 +- lib/ash/changeset/changeset.ex | 7 +- .../changeset/managed_relationship_helpers.ex | 6 +- test/manage_relationship_test.exs | 128 ++++++++++++++++++ 5 files changed, 243 insertions(+), 23 deletions(-) diff --git a/lib/ash/actions/managed_relationships.ex b/lib/ash/actions/managed_relationships.ex index aec5de0d62..8eb1597345 100644 --- a/lib/ash/actions/managed_relationships.ex +++ b/lib/ash/actions/managed_relationships.ex @@ -2306,6 +2306,25 @@ defmodule Ash.Actions.ManagedRelationships do :missing -> {:ok, current_value, []} + {:destroy, action_name, join_action_name} -> + case destroy_data_m2m( + source_record, + match, + domain, + actor, + opts, + action_name, + join_action_name, + changeset, + relationship + ) do + {:ok, notifications} -> + {:ok, current_value, notifications} + + {:error, error} -> + {:error, add_bread_crumb(error, relationship, :destroy)} + end + {:destroy, action_name} -> case destroy_data( source_record, @@ -3277,6 +3296,38 @@ defmodule Ash.Actions.ManagedRelationships do {:ok, []} end + # Destroys both the join record and the destination record for a many_to_many relationship. + # Used by on_match: {:destroy, dest_action, join_action} (3-tuple) and on_match: :destroy shorthand. + defp destroy_data_m2m( + source_record, + record, + domain, + actor, + opts, + dest_action_name, + join_action_name, + changeset, + %{type: :many_to_many} = relationship + ) do + with {:ok, join_notifications} <- + destroy_m2m_join_record( + source_record, + record, + actor, + opts, + join_action_name, + changeset, + relationship + ), + {:ok, dest_notifications} <- + destroy_record(record, domain, actor, opts, dest_action_name, changeset, relationship) do + {:ok, join_notifications ++ dest_notifications} + end + end + + # Destroys only the join record for a many_to_many relationship. + # Used by on_match: {:destroy, action} (2-tuple) for backward compatibility. + # Functionally equivalent to :unrelate — the destination record is left intact. defp destroy_data( source_record, record, @@ -3287,6 +3338,50 @@ defmodule Ash.Actions.ManagedRelationships do changeset, %{type: :many_to_many} = relationship ) do + destroy_m2m_join_record( + source_record, + record, + actor, + opts, + action_name, + changeset, + relationship + ) + end + + # Destroys the destination record directly. Used by non-many_to_many relationships. + defp destroy_data( + _source_record, + record, + domain, + actor, + opts, + action_name, + changeset, + relationship + ) do + destroy_record( + record, + domain, + actor, + opts, + action_name, + changeset, + relationship + ) + end + + # Finds and destroys a single join record for a many_to_many relationship. + # Looks up the join record by source and destination attribute values, then destroys it. + defp destroy_m2m_join_record( + source_record, + record, + actor, + opts, + action_name, + changeset, + relationship + ) do tenant = changeset.tenant source_value = Map.get(source_record, relationship.source_attribute) @@ -3312,6 +3407,10 @@ defmodule Ash.Actions.ManagedRelationships do domain: domain(changeset, join_relationship) ) |> case do + {:ok, nil} -> + debug_log(relationship.name, changeset, :read, :ok, opts[:debug?]) + {:ok, []} + {:ok, result} -> debug_log(relationship.name, changeset, :read, :ok, opts[:debug?]) @@ -3332,12 +3431,10 @@ defmodule Ash.Actions.ManagedRelationships do |> case do {:ok, notifications} -> debug_log(relationship.name, changeset, :destroy, :ok, opts[:debug?]) - {:ok, notifications} {:ok, _record, notifications} -> debug_log(relationship.name, changeset, :destroy, :ok, opts[:debug?]) - {:ok, notifications} {:error, error} -> @@ -3358,16 +3455,8 @@ defmodule Ash.Actions.ManagedRelationships do end end - defp destroy_data( - _source_record, - record, - domain, - actor, - opts, - action_name, - changeset, - relationship - ) do + # Destroys a single record using the given action. + defp destroy_record(record, domain, actor, opts, action_name, changeset, relationship) do tenant = changeset.tenant record @@ -3385,18 +3474,16 @@ defmodule Ash.Actions.ManagedRelationships do |> Ash.Changeset.set_tenant(tenant) |> Ash.destroy(return_notifications?: true) |> case do - {:ok, _record, notifications} -> + {:ok, notifications} -> debug_log(relationship.name, changeset, :destroy, :ok, opts[:debug?]) - {:ok, notifications} - {:ok, notifications} -> + {:ok, _record, notifications} -> debug_log(relationship.name, changeset, :destroy, :ok, opts[:debug?]) {:ok, notifications} {:error, error} -> debug_log(relationship.name, changeset, :destroy, {:error, error}, opts[:debug?]) - {:error, add_bread_crumb(error, relationship, :destroy)} end end diff --git a/lib/ash/actions/read/calculations.ex b/lib/ash/actions/read/calculations.ex index 4b392f9cfe..0091e1244a 100644 --- a/lib/ash/actions/read/calculations.ex +++ b/lib/ash/actions/read/calculations.ex @@ -1001,8 +1001,7 @@ defmodule Ash.Actions.Read.Calculations do calculation.context.tracer, domain, ash_query.resource, - parent_stack: - Ash.Actions.Read.parent_stack_from_context(ash_query.context), + parent_stack: Ash.Actions.Read.parent_stack_from_context(ash_query.context), source_context: ash_query.context ) @@ -1187,8 +1186,7 @@ defmodule Ash.Actions.Read.Calculations do calculation.context.tracer, ash_query.domain, ash_query.resource, - parent_stack: - Ash.Actions.Read.parent_stack_from_context(ash_query.context), + parent_stack: Ash.Actions.Read.parent_stack_from_context(ash_query.context), source_context: ash_query.context ) diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index b73c76c397..017baa7d05 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -5654,11 +5654,14 @@ defmodule Ash.Changeset do * `:update_join` - update only the join record (only valid for many to many) * `{:update_join, :join_table_action_name}` - use the specified update action on a join resource * `{:update_join, :join_table_action_name, [:list, :of, :params]}` - pass specified params from input into a join resource update action - * `{:destroy, :action_name}` - the record is destroyed using the specified action on the destination resource. The action should be: - * `many_to_many` - a destroy action on the join record + * `:destroy` - destroys the record using primary destroy actions. For `many_to_many`, destroys both the join record and the destination record. + * `{:destroy, :action_name}` - the record is destroyed using the specified action. The action should be: + * `many_to_many` - a destroy action on the join resource only (the destination record is NOT destroyed) * `has_many` - a destroy action on the destination resource * `has_one` - a destroy action on the destination resource * `belongs_to` - a destroy action on the destination resource + * `{:destroy, :destination_action_name, :join_action_name}` - (many_to_many only) destroys both the destination record + using the first action and the join record using the second action * `:error` - an error is returned indicating that a record would have been updated * `:no_match` - follows the `on_no_match` instructions with these records * `:missing` - follows the `on_missing` instructions with these records diff --git a/lib/ash/changeset/managed_relationship_helpers.ex b/lib/ash/changeset/managed_relationship_helpers.ex index dc21d0e1dd..2a3db2862b 100644 --- a/lib/ash/changeset/managed_relationship_helpers.ex +++ b/lib/ash/changeset/managed_relationship_helpers.ex @@ -110,8 +110,9 @@ defmodule Ash.Changeset.ManagedRelationshipHelpers do {:unrelate, nil} :destroy when is_many_to_many -> + destroy = primary_action_name!(relationship.destination, :destroy) join_destroy = primary_action_name!(relationship.through, :destroy) - {:destroy, join_destroy} + {:destroy, destroy, join_destroy} :destroy -> destroy = primary_action_name!(relationship.destination, :destroy) @@ -180,6 +181,9 @@ defmodule Ash.Changeset.ManagedRelationshipHelpers do {:unrelate, _nil} -> nil + {:destroy, destroy, join_destroy} when relationship.type == :many_to_many -> + all([destination(destroy), join(join_destroy, :*)]) + {:destroy, join_destroy} when relationship.type == :many_to_many -> all(join(join_destroy, :*)) diff --git a/test/manage_relationship_test.exs b/test/manage_relationship_test.exs index 8397187e41..ab6ffa054e 100644 --- a/test/manage_relationship_test.exs +++ b/test/manage_relationship_test.exs @@ -64,6 +64,36 @@ defmodule Ash.Test.ManageRelationshipTest do ) end + update :update_many_on_match_destroy do + require_atomic? false + argument :many_to_many_resources, {:array, :map} + + change manage_relationship(:many_to_many_resources, + on_no_match: :error, + on_match: :destroy + ) + end + + update :update_many_on_match_destroy_join_only do + require_atomic? false + argument :many_to_many_resources, {:array, :map} + + change manage_relationship(:many_to_many_resources, + on_no_match: :error, + on_match: {:destroy, :destroy_hard} + ) + end + + update :update_many_on_match_destroy_both do + require_atomic? false + argument :many_to_many_resources, {:array, :map} + + change manage_relationship(:many_to_many_resources, + on_no_match: :error, + on_match: {:destroy, :destroy_hard, :destroy_hard} + ) + end + update :remove_other_resource do require_atomic? false argument :other_resource_id, :uuid @@ -612,6 +642,104 @@ defmodule Ash.Test.ManageRelationshipTest do assert is_nil(hd(third.join_resources).archived_at) end + test "on_match :destroy shorthand destroys both join and destination for many_to_many" do + assert {:ok, parent} = + ParentResource + |> Ash.Changeset.for_create(:create, %{ + name: "Test Parent Resource", + many_to_many_resources: [%{name: "match_destroy_1"}, %{name: "match_destroy_2"}] + }) + |> Ash.create!() + |> Ash.load([:many_to_many_resources]) + + first_id = + Enum.find(parent.many_to_many_resources, &(&1.name == "match_destroy_1")).id + + join_count_before = Ash.read!(JoinResource) |> length() + + assert {:ok, updated_parent} = + parent + |> Ash.Changeset.for_update(:update_many_on_match_destroy, %{ + many_to_many_resources: [%{id: first_id}] + }) + |> Ash.update!() + |> Ash.load([:many_to_many_resources]) + + # matched destination record should be destroyed + assert Enum.find(updated_parent.many_to_many_resources, &(&1.name == "match_destroy_1")) == + nil + + # join record for matched item should be destroyed + join_count_after = Ash.read!(JoinResource) |> length() + assert join_count_after == join_count_before - 1 + end + + test "on_match {:destroy, action} 2-tuple only destroys join for many_to_many (backward compat)" do + assert {:ok, parent} = + ParentResource + |> Ash.Changeset.for_create(:create, %{ + name: "Test Parent Resource", + many_to_many_resources: [%{name: "join_only_1"}, %{name: "join_only_2"}] + }) + |> Ash.create!() + |> Ash.load([:many_to_many_resources]) + + first_id = + Enum.find(parent.many_to_many_resources, &(&1.name == "join_only_1")).id + + join_count_before = Ash.read!(JoinResource) |> length() + + assert {:ok, updated_parent} = + parent + |> Ash.Changeset.for_update(:update_many_on_match_destroy_join_only, %{ + many_to_many_resources: [%{id: first_id}] + }) + |> Ash.update!() + |> Ash.load([:many_to_many_resources]) + + # destination record should still exist (only join destroyed) + assert Enum.find(updated_parent.many_to_many_resources, &(&1.name == "join_only_1")) == nil + + # but the destination record itself still exists in the table + assert Ash.read!(ManyToManyResource) |> Enum.find(&(&1.name == "join_only_1")) != nil + + # join record should be destroyed + join_count_after = Ash.read!(JoinResource) |> length() + assert join_count_after == join_count_before - 1 + end + + test "on_match {:destroy, dest_action, join_action} 3-tuple destroys both for many_to_many" do + assert {:ok, parent} = + ParentResource + |> Ash.Changeset.for_create(:create, %{ + name: "Test Parent Resource", + many_to_many_resources: [%{name: "both_destroy_1"}, %{name: "both_destroy_2"}] + }) + |> Ash.create!() + |> Ash.load([:many_to_many_resources]) + + first_id = + Enum.find(parent.many_to_many_resources, &(&1.name == "both_destroy_1")).id + + join_count_before = Ash.read!(JoinResource) |> length() + + assert {:ok, updated_parent} = + parent + |> Ash.Changeset.for_update(:update_many_on_match_destroy_both, %{ + many_to_many_resources: [%{id: first_id}] + }) + |> Ash.update!() + |> Ash.load([:many_to_many_resources]) + + # matched destination record should be destroyed + assert Enum.find(updated_parent.many_to_many_resources, &(&1.name == "both_destroy_1")) == + nil + + # join record should also be destroyed + join_count_after = Ash.read!(JoinResource) |> length() + assert join_count_after == join_count_before - 1 + end + test "removing non-existent relationship returns NotFound error" do parent = ParentResource From 48771710ef6db004855a3931b41206647f7696d1 Mon Sep 17 00:00:00 2001 From: Jinkyou Son Date: Thu, 19 Mar 2026 12:31:32 +0900 Subject: [PATCH 2/2] chore: add backward compat config for many_to_many {:destroy, action} on_match --- .../backwards-compatibility-config.md | 20 ++++++++++ lib/ash/changeset/changeset.ex | 4 +- .../changeset/managed_relationship_helpers.ex | 8 ++++ lib/mix/tasks/install/ash.install.ex | 6 +++ test/manage_relationship_test.exs | 38 +++++++++++++++++++ 5 files changed, 75 insertions(+), 1 deletion(-) diff --git a/documentation/topics/development/backwards-compatibility-config.md b/documentation/topics/development/backwards-compatibility-config.md index 58ac1991ca..9650df3736 100644 --- a/documentation/topics/development/backwards-compatibility-config.md +++ b/documentation/topics/development/backwards-compatibility-config.md @@ -252,3 +252,23 @@ When a field is marked `sensitive?: true`, its value in validation error structs replaced with a redacted placeholder via `Ash.Helpers.redact/1`. This applies to both the non-atomic (`validate/3`) and atomic (`atomic/3`) code paths across all built-in validations. + +## many_to_many_destroy_destination_on_match? + +```elixir +config :ash, many_to_many_destroy_destination_on_match?: true +``` + +### Old Behavior + +When using `on_match: {:destroy, :action_name}` (2-tuple form) with a `many_to_many` +relationship, only the join record was destroyed using the given action. The +destination record was left intact, making it effectively the same as `:unrelate`. + +### New Behavior + +The 2-tuple `{:destroy, :action_name}` for `many_to_many` now destroys both +the destination record (using the given action) and the join record (using the +primary destroy action on the join resource). This makes the 2-tuple behavior +consistent with the `:destroy` shorthand and the explicit 3-tuple +`{:destroy, :dest_action, :join_action}`. diff --git a/lib/ash/changeset/changeset.ex b/lib/ash/changeset/changeset.ex index 017baa7d05..10e242814b 100644 --- a/lib/ash/changeset/changeset.ex +++ b/lib/ash/changeset/changeset.ex @@ -5656,7 +5656,9 @@ defmodule Ash.Changeset do * `{:update_join, :join_table_action_name, [:list, :of, :params]}` - pass specified params from input into a join resource update action * `:destroy` - destroys the record using primary destroy actions. For `many_to_many`, destroys both the join record and the destination record. * `{:destroy, :action_name}` - the record is destroyed using the specified action. The action should be: - * `many_to_many` - a destroy action on the join resource only (the destination record is NOT destroyed) + * `many_to_many` - by default, a destroy action on the join resource only (the destination record is NOT destroyed). + When `config :ash, many_to_many_destroy_destination_on_match?: true` is set, destroys both the destination record + (using the given action) and the join record (using the primary destroy action). * `has_many` - a destroy action on the destination resource * `has_one` - a destroy action on the destination resource * `belongs_to` - a destroy action on the destination resource diff --git a/lib/ash/changeset/managed_relationship_helpers.ex b/lib/ash/changeset/managed_relationship_helpers.ex index 2a3db2862b..07189a49d9 100644 --- a/lib/ash/changeset/managed_relationship_helpers.ex +++ b/lib/ash/changeset/managed_relationship_helpers.ex @@ -114,6 +114,14 @@ defmodule Ash.Changeset.ManagedRelationshipHelpers do join_destroy = primary_action_name!(relationship.through, :destroy) {:destroy, destroy, join_destroy} + {:destroy, action} when is_many_to_many -> + if Application.get_env(:ash, :many_to_many_destroy_destination_on_match?, false) do + join_destroy = primary_action_name!(relationship.through, :destroy) + {:destroy, action, join_destroy} + else + {:destroy, action} + end + :destroy -> destroy = primary_action_name!(relationship.destination, :destroy) {:destroy, destroy} diff --git a/lib/mix/tasks/install/ash.install.ex b/lib/mix/tasks/install/ash.install.ex index 0464dc2a82..900073809b 100644 --- a/lib/mix/tasks/install/ash.install.ex +++ b/lib/mix/tasks/install/ash.install.ex @@ -240,6 +240,12 @@ if Code.ensure_loaded?(Igniter) do [:redact_sensitive_values_in_errors?], true ) + |> Igniter.Project.Config.configure( + "config.exs", + :ash, + [:many_to_many_destroy_destination_on_match?], + true + ) end) end ) diff --git a/test/manage_relationship_test.exs b/test/manage_relationship_test.exs index ab6ffa054e..47cb35298f 100644 --- a/test/manage_relationship_test.exs +++ b/test/manage_relationship_test.exs @@ -740,6 +740,44 @@ defmodule Ash.Test.ManageRelationshipTest do assert join_count_after == join_count_before - 1 end + test "on_match {:destroy, action} 2-tuple destroys both when config enabled" do + Application.put_env(:ash, :many_to_many_destroy_destination_on_match?, true) + + on_exit(fn -> + Application.delete_env(:ash, :many_to_many_destroy_destination_on_match?) + end) + + assert {:ok, parent} = + ParentResource + |> Ash.Changeset.for_create(:create, %{ + name: "Test Parent Resource", + many_to_many_resources: [%{name: "config_1"}, %{name: "config_2"}] + }) + |> Ash.create!() + |> Ash.load([:many_to_many_resources]) + + first_id = + Enum.find(parent.many_to_many_resources, &(&1.name == "config_1")).id + + join_count_before = Ash.read!(JoinResource) |> length() + + assert {:ok, updated_parent} = + parent + |> Ash.Changeset.for_update(:update_many_on_match_destroy_join_only, %{ + many_to_many_resources: [%{id: first_id}] + }) + |> Ash.update!() + |> Ash.load([:many_to_many_resources]) + + # destination record should be destroyed (config enabled) + assert Enum.find(updated_parent.many_to_many_resources, &(&1.name == "config_1")) == nil + assert Ash.read!(ManyToManyResource) |> Enum.find(&(&1.name == "config_1")) == nil + + # join record should also be destroyed + join_count_after = Ash.read!(JoinResource) |> length() + assert join_count_after == join_count_before - 1 + end + test "removing non-existent relationship returns NotFound error" do parent = ParentResource