diff --git a/lib/membership/setting.ex b/lib/membership/setting.ex index 4ba0794..862f4ac 100644 --- a/lib/membership/setting.ex +++ b/lib/membership/setting.ex @@ -155,12 +155,23 @@ defmodule Mv.Membership.Setting do on: [:create, :update] # Validate default_membership_fee_type_id exists if set - validate fn changeset, _context -> + validate fn changeset, context -> fee_type_id = Ash.Changeset.get_attribute(changeset, :default_membership_fee_type_id) if fee_type_id do - case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id) do + # Actor may be in changeset.context (action context) or validation context + ctx = changeset.context || %{} + + actor = + get_in(ctx, [:private, :actor]) || + Map.get(ctx, :actor) || + (context && Map.get(context, :actor)) + + # Check existence only; action is already restricted by policy (e.g. admin). + opts = [domain: Mv.MembershipFees, authorize?: false] + + case Ash.get(Mv.MembershipFees.MembershipFeeType, fee_type_id, opts) do {:ok, _} -> :ok diff --git a/lib/membership_fees/changes/set_membership_fee_start_date.ex b/lib/membership_fees/changes/set_membership_fee_start_date.ex index a2e1ad0..0e9cf00 100644 --- a/lib/membership_fees/changes/set_membership_fee_start_date.ex +++ b/lib/membership_fees/changes/set_membership_fee_start_date.ex @@ -31,12 +31,12 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do alias Mv.MembershipFees.CalendarCycles @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do # Only calculate if membership_fee_start_date is not already set if has_start_date?(changeset) do changeset else - calculate_and_set_start_date(changeset) + calculate_and_set_start_date(changeset, context) end end @@ -56,10 +56,13 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp calculate_and_set_start_date(changeset) do + defp calculate_and_set_start_date(changeset, context) do + actor = Map.get(context || %{}, :actor) + opts = if actor, do: [actor: actor], else: [] + with {:ok, join_date} <- get_join_date(changeset), {:ok, membership_fee_type_id} <- get_membership_fee_type_id(changeset), - {:ok, interval} <- get_interval(membership_fee_type_id), + {:ok, interval} <- get_interval(membership_fee_type_id, opts), {:ok, include_joining_cycle} <- get_include_joining_cycle() do start_date = calculate_start_date(join_date, interval, include_joining_cycle) Ash.Changeset.force_change_attribute(changeset, :membership_fee_start_date, start_date) @@ -118,8 +121,8 @@ defmodule Mv.MembershipFees.Changes.SetMembershipFeeStartDate do end end - defp get_interval(membership_fee_type_id) do - case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id) do + defp get_interval(membership_fee_type_id, opts) do + case Ash.get(Mv.MembershipFees.MembershipFeeType, membership_fee_type_id, opts) do {:ok, %{interval: interval}} -> {:ok, interval} {:error, _} -> {:error, :membership_fee_type_not_found} end diff --git a/lib/membership_fees/changes/validate_same_interval.ex b/lib/membership_fees/changes/validate_same_interval.ex index 8c1efb4..0ad32a1 100644 --- a/lib/membership_fees/changes/validate_same_interval.ex +++ b/lib/membership_fees/changes/validate_same_interval.ex @@ -19,9 +19,9 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do use Ash.Resource.Change @impl true - def change(changeset, _opts, _context) do + def change(changeset, _opts, context) do if changing_membership_fee_type?(changeset) do - validate_interval_match(changeset) + validate_interval_match(changeset, context) else changeset end @@ -33,9 +33,10 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end # Validate that the new type has the same interval as the current type - defp validate_interval_match(changeset) do + defp validate_interval_match(changeset, context) do current_type_id = get_current_type_id(changeset) new_type_id = get_new_type_id(changeset) + actor = Map.get(context || %{}, :actor) cond do # If no current type, allow any change (first assignment) @@ -48,13 +49,13 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do # Both types exist - validate intervals match true -> - validate_intervals_match(changeset, current_type_id, new_type_id) + validate_intervals_match(changeset, current_type_id, new_type_id, actor) end end # Validates that intervals match when both types exist - defp validate_intervals_match(changeset, current_type_id, new_type_id) do - case get_intervals(current_type_id, new_type_id) do + defp validate_intervals_match(changeset, current_type_id, new_type_id, actor) do + case get_intervals(current_type_id, new_type_id, actor) do {:ok, current_interval, new_interval} -> if current_interval == new_interval do changeset @@ -85,11 +86,16 @@ defmodule Mv.MembershipFees.Changes.ValidateSameInterval do end end - # Get intervals for both types - defp get_intervals(current_type_id, new_type_id) do + # Get intervals for both types (actor required for authorization when resource has policies) + defp get_intervals(current_type_id, new_type_id, actor) do alias Mv.MembershipFees.MembershipFeeType - case {Ash.get(MembershipFeeType, current_type_id), Ash.get(MembershipFeeType, new_type_id)} do + opts = if actor, do: [actor: actor], else: [] + + case { + Ash.get(MembershipFeeType, current_type_id, opts), + Ash.get(MembershipFeeType, new_type_id, opts) + } do {{:ok, current_type}, {:ok, new_type}} -> {:ok, current_type.interval, new_type.interval} diff --git a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex index 0e693e1..72cc10c 100644 --- a/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex +++ b/lib/mv/accounts/user/validations/email_not_used_by_other_member.ex @@ -81,7 +81,7 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do query = Mv.Membership.Member |> Ash.Query.filter(email == ^to_string(email)) - |> maybe_exclude_id(exclude_member_id) + |> Mv.Helpers.query_exclude_id(exclude_member_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -101,7 +101,4 @@ defmodule Mv.Accounts.User.Validations.EmailNotUsedByOtherMember do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/email_sync/changes/sync_user_email_to_member.ex b/lib/mv/email_sync/changes/sync_user_email_to_member.ex index eb6770c..624692b 100644 --- a/lib/mv/email_sync/changes/sync_user_email_to_member.ex +++ b/lib/mv/email_sync/changes/sync_user_email_to_member.ex @@ -27,6 +27,10 @@ defmodule Mv.EmailSync.Changes.SyncUserEmailToMember do Modified changeset with email synchronization applied, or original changeset if recursion detected. """ + # Ash 3.12+ calls this to decide whether to run the change in certain contexts. + @impl true + def has_change?, do: true + @impl true def change(changeset, _opts, context) do # Only recursion protection needed - trigger logic is in `where` clauses diff --git a/lib/mv/helpers.ex b/lib/mv/helpers.ex index e20db67..ae22e13 100644 --- a/lib/mv/helpers.ex +++ b/lib/mv/helpers.ex @@ -5,6 +5,8 @@ defmodule Mv.Helpers do Provides utilities that are not specific to a single domain or layer. """ + require Ash.Query + @doc """ Converts an actor to Ash options list for authorization. Returns empty list if actor is nil. @@ -24,4 +26,22 @@ defmodule Mv.Helpers do @spec ash_actor_opts(Mv.Accounts.User.t() | nil) :: keyword() def ash_actor_opts(nil), do: [] def ash_actor_opts(actor) when not is_nil(actor), do: [actor: actor] + + @doc """ + Returns the query unchanged if `exclude_id` is nil; otherwise adds a filter `id != ^exclude_id`. + + Used in uniqueness validations that must exclude the current record (e.g. name uniqueness + on update, duplicate association checks). Call with the record's primary key to exclude it + from the result set. + + ## Examples + + query + |> Ash.Query.filter(name == ^name) + |> Mv.Helpers.query_exclude_id(current_id) + + """ + @spec query_exclude_id(Ash.Query.t(), String.t() | nil) :: Ash.Query.t() + def query_exclude_id(query, nil), do: query + def query_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex index 1ee8ab0..1297515 100644 --- a/lib/mv/membership/member/validations/email_not_used_by_other_user.ex +++ b/lib/mv/membership/member/validations/email_not_used_by_other_user.ex @@ -56,7 +56,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do query = Mv.Accounts.User |> Ash.Query.filter(email == ^email) - |> maybe_exclude_id(exclude_user_id) + |> Mv.Helpers.query_exclude_id(exclude_user_id) system_actor = SystemActor.get_system_actor() opts = Helpers.ash_actor_opts(system_actor) @@ -76,7 +76,4 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do :ok end end - - defp maybe_exclude_id(query, nil), do: query - defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) end diff --git a/priv/repo/seeds.exs b/priv/repo/seeds.exs index 4240336..579b7cc 100644 --- a/priv/repo/seeds.exs +++ b/priv/repo/seeds.exs @@ -10,7 +10,7 @@ alias Mv.MembershipFees.CycleGenerator require Ash.Query -# Create example membership fee types +# Create example membership fee types (no admin user yet; skip authorization for bootstrap) for fee_type_attrs <- [ %{ name: "Standard (Jährlich)", @@ -39,7 +39,12 @@ for fee_type_attrs <- [ ] do MembershipFeeType |> Ash.Changeset.for_create(:create, fee_type_attrs) - |> Ash.create!(upsert?: true, upsert_identity: :unique_name) + |> Ash.create!( + upsert?: true, + upsert_identity: :unique_name, + authorize?: false, + domain: Mv.MembershipFees + ) end for attrs <- [ @@ -299,12 +304,12 @@ case Accounts.User IO.puts("SystemActor will fall back to admin user (#{admin_email})") end -# Load all membership fee types for assignment +# Load all membership fee types for assignment (admin actor for authorization) # Sort by name to ensure deterministic order all_fee_types = MembershipFeeType |> Ash.Query.sort(name: :asc) - |> Ash.read!() + |> Ash.read!(actor: admin_user_with_role, domain: Mv.MembershipFees) |> Enum.to_list() # Create sample members for testing - use upsert to prevent duplicates diff --git a/test/membership/membership_fee_settings_test.exs b/test/membership/membership_fee_settings_test.exs index 744b6bd..2c126d9 100644 --- a/test/membership/membership_fee_settings_test.exs +++ b/test/membership/membership_fee_settings_test.exs @@ -54,18 +54,26 @@ defmodule Mv.Membership.MembershipFeeSettingsTest do # Create a valid fee type {:ok, fee_type} = - Ash.create(MembershipFeeType, %{ - name: "Test Fee Type #{System.unique_integer([:positive])}", - amount: Decimal.new("100.00"), - interval: :yearly - }) + Ash.create( + MembershipFeeType, + %{ + name: "Test Fee Type #{System.unique_integer([:positive])}", + amount: Decimal.new("100.00"), + interval: :yearly + }, + actor: actor + ) # Setting a valid fee type should work {:ok, updated} = settings - |> Ash.Changeset.for_update(:update_membership_fee_settings, %{ - default_membership_fee_type_id: fee_type.id - }) + |> Ash.Changeset.for_update( + :update_membership_fee_settings, + %{ + default_membership_fee_type_id: fee_type.id + }, + actor: actor + ) |> Ash.update(actor: actor) assert updated.default_membership_fee_type_id == fee_type.id diff --git a/test/membership_fees/changes/validate_same_interval_test.exs b/test/membership_fees/changes/validate_same_interval_test.exs index 21287dd..2310537 100644 --- a/test/membership_fees/changes/validate_same_interval_test.exs +++ b/test/membership_fees/changes/validate_same_interval_test.exs @@ -52,7 +52,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: yearly_type2.id}, actor: actor ) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) assert changeset.valid? end @@ -68,7 +68,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: monthly_type.id}, actor: actor ) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) refute changeset.valid? assert %{errors: errors} = changeset @@ -90,7 +90,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: yearly_type.id}, actor: actor ) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) assert changeset.valid? end @@ -102,7 +102,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: nil}, actor: actor) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) refute changeset.valid? assert %{errors: errors} = changeset @@ -120,7 +120,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do changeset = member |> Ash.Changeset.for_update(:update_member, %{first_name: "New Name"}, actor: actor) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) assert changeset.valid? end @@ -136,7 +136,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: quarterly_type.id}, actor: actor ) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) error = Enum.find(changeset.errors, &(&1.field == :membership_fee_type_id)) assert error.message =~ "yearly" @@ -175,7 +175,7 @@ defmodule Mv.MembershipFees.Changes.ValidateSameIntervalTest do |> Ash.Changeset.for_update(:update_member, %{membership_fee_type_id: type2.id}, actor: actor ) - |> ValidateSameInterval.change(%{}, %{}) + |> ValidateSameInterval.change(%{}, %{actor: actor}) refute changeset.valid?, "Should prevent change from #{interval1} to #{interval2}" diff --git a/test/membership_fees/membership_fee_cycle_test.exs b/test/membership_fees/membership_fee_cycle_test.exs index fefc838..8770134 100644 --- a/test/membership_fees/membership_fee_cycle_test.exs +++ b/test/membership_fees/membership_fee_cycle_test.exs @@ -151,7 +151,7 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do member = create_member(%{membership_fee_type_id: fee_type.id}, actor) cycle = create_cycle(member, fee_type, %{status: :paid}, actor) - assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_unpaid) + assert {:ok, updated} = Ash.update(cycle, %{}, actor: actor, action: :mark_as_unpaid) assert updated.status == :unpaid end @@ -175,7 +175,7 @@ defmodule Mv.MembershipFees.MembershipFeeCycleTest do member = create_member(%{membership_fee_type_id: fee_type.id}, actor) cycle = create_cycle(member, fee_type, %{status: :suspended}, actor) - assert {:ok, updated} = Ash.update(cycle, %{}, action: :mark_as_unpaid) + assert {:ok, updated} = Ash.update(cycle, %{}, actor: actor, action: :mark_as_unpaid) assert updated.status == :unpaid end end diff --git a/test/membership_fees/membership_fee_type_integration_test.exs b/test/membership_fees/membership_fee_type_integration_test.exs index e716b42..88f620d 100644 --- a/test/membership_fees/membership_fee_type_integration_test.exs +++ b/test/membership_fees/membership_fee_type_integration_test.exs @@ -155,9 +155,13 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do {:ok, settings} = Mv.Membership.get_settings() settings - |> Ash.Changeset.for_update(:update_membership_fee_settings, %{ - default_membership_fee_type_id: fee_type.id - }) + |> Ash.Changeset.for_update( + :update_membership_fee_settings, + %{ + default_membership_fee_type_id: fee_type.id + }, + actor: actor + ) |> Ash.update!(actor: actor) # Try to delete @@ -176,9 +180,13 @@ defmodule Mv.MembershipFees.MembershipFeeTypeIntegrationTest do {:ok, settings} = Mv.Membership.get_settings() settings - |> Ash.Changeset.for_update(:update_membership_fee_settings, %{ - default_membership_fee_type_id: fee_type.id - }) + |> Ash.Changeset.for_update( + :update_membership_fee_settings, + %{ + default_membership_fee_type_id: fee_type.id + }, + actor: actor + ) |> Ash.update!(actor: actor) # Create a member without explicitly setting membership_fee_type_id diff --git a/test/membership_fees/membership_fee_type_test.exs b/test/membership_fees/membership_fee_type_test.exs index 80b7839..7b6c39a 100644 --- a/test/membership_fees/membership_fee_type_test.exs +++ b/test/membership_fees/membership_fee_type_test.exs @@ -264,9 +264,13 @@ defmodule Mv.MembershipFees.MembershipFeeTypeTest do {:ok, settings} = Mv.Membership.get_settings() settings - |> Ash.Changeset.for_update(:update_membership_fee_settings, %{ - default_membership_fee_type_id: fee_type.id - }) + |> Ash.Changeset.for_update( + :update_membership_fee_settings, + %{ + default_membership_fee_type_id: fee_type.id + }, + actor: actor + ) |> Ash.update!(actor: actor) # Try to delete