From cf9e6e91fdf7ea0c3e799ac0793b836eae9cb984 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 20 Jan 2026 22:09:20 +0100 Subject: [PATCH] Use system actor for cycle generation Update cycle generator, member hooks, and job to use system actor. Remove actor parameters as cycle generation is a mandatory side effect. --- lib/membership/member.ex | 82 ++++++++----------- lib/mv/membership_fees/cycle_generator.ex | 67 +++++++-------- .../show/membership_fees_component.ex | 3 +- 3 files changed, 65 insertions(+), 87 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 5a4d01c..ef3aca1 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -125,7 +125,7 @@ defmodule Mv.Membership.Member do {:ok, member} -> if member.membership_fee_type_id && member.join_date do actor = Map.get(changeset.context, :actor) - handle_cycle_generation(member, actor: actor) + handle_cycle_generation(member, []) end {:error, _} -> @@ -196,9 +196,7 @@ defmodule Mv.Membership.Member do Ash.Changeset.changing_attribute?(changeset, :membership_fee_type_id) if fee_type_changed && member.membership_fee_type_id && member.join_date do - actor = Map.get(changeset.context, :actor) - - case regenerate_cycles_on_type_change(member, actor: actor) do + case regenerate_cycles_on_type_change(member) do {:ok, notifications} -> # Return notifications to Ash - they will be sent automatically after commit {:ok, member, notifications} @@ -231,7 +229,7 @@ defmodule Mv.Membership.Member do if (join_date_changed || exit_date_changed) && member.membership_fee_type_id do actor = Map.get(changeset.context, :actor) - handle_cycle_generation(member, actor: actor) + handle_cycle_generation(member, []) end {:error, _} -> @@ -790,37 +788,37 @@ defmodule Mv.Membership.Member do # Returns {:ok, notifications} or {:error, reason} where notifications are collected # to be sent after transaction commits @doc false - def regenerate_cycles_on_type_change(member, opts \\ []) do + # Uses system actor for cycle regeneration (mandatory side effect) + def regenerate_cycles_on_type_change(member, _opts \\ []) do + alias Mv.Helpers + alias Mv.Helpers.SystemActor + today = Date.utc_today() lock_key = :erlang.phash2(member.id) - actor = Keyword.get(opts, :actor) # Use advisory lock to prevent concurrent deletion and regeneration # This ensures atomicity when multiple updates happen simultaneously if Mv.Repo.in_transaction?() do - regenerate_cycles_in_transaction(member, today, lock_key, actor: actor) + regenerate_cycles_in_transaction(member, today, lock_key) else - regenerate_cycles_new_transaction(member, today, lock_key, actor: actor) + regenerate_cycles_new_transaction(member, today, lock_key) end end # Already in transaction: use advisory lock directly # Returns {:ok, notifications} - notifications should be returned to after_action hook - defp regenerate_cycles_in_transaction(member, today, lock_key, opts) do - actor = Keyword.get(opts, :actor) + defp regenerate_cycles_in_transaction(member, today, lock_key) do Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) - do_regenerate_cycles_on_type_change(member, today, skip_lock?: true, actor: actor) + do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) end # Not in transaction: start new transaction with advisory lock # Returns {:ok, notifications} - notifications should be sent by caller (e.g., via after_action) - defp regenerate_cycles_new_transaction(member, today, lock_key, opts) do - actor = Keyword.get(opts, :actor) - + defp regenerate_cycles_new_transaction(member, today, lock_key) do Mv.Repo.transaction(fn -> Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) - case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true, actor: actor) do + case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do {:ok, notifications} -> # Return notifications - they will be sent by the caller notifications @@ -838,11 +836,16 @@ defmodule Mv.Membership.Member do # Performs the actual cycle deletion and regeneration # Returns {:ok, notifications} or {:error, reason} # notifications are collected to be sent after transaction commits + # Uses system actor for all operations defp do_regenerate_cycles_on_type_change(member, today, opts) do + alias Mv.Helpers + alias Mv.Helpers.SystemActor + require Ash.Query skip_lock? = Keyword.get(opts, :skip_lock?, false) - actor = Keyword.get(opts, :actor) + system_actor = SystemActor.get_system_actor() + actor_opts = Helpers.ash_actor_opts(system_actor) # Find all unpaid cycles for this member # We need to check cycle_end for each cycle using its own interval @@ -852,21 +855,11 @@ defmodule Mv.Membership.Member do |> Ash.Query.filter(status == :unpaid) |> Ash.Query.load([:membership_fee_type]) - result = - if actor do - Ash.read(all_unpaid_cycles_query, actor: actor) - else - Ash.read(all_unpaid_cycles_query) - end - - case result do + case Ash.read(all_unpaid_cycles_query, actor_opts) do {:ok, all_unpaid_cycles} -> cycles_to_delete = filter_future_cycles(all_unpaid_cycles, today) - delete_and_regenerate_cycles(cycles_to_delete, member.id, today, - skip_lock?: skip_lock?, - actor: actor - ) + delete_and_regenerate_cycles(cycles_to_delete, member.id, today, skip_lock?: skip_lock?) {:error, reason} -> {:error, reason} @@ -893,16 +886,16 @@ defmodule Mv.Membership.Member do # Deletes future cycles and regenerates them with the new type/amount # Passes today to ensure consistent date across deletion and regeneration # Returns {:ok, notifications} or {:error, reason} + # Uses system actor for cycle generation defp delete_and_regenerate_cycles(cycles_to_delete, member_id, today, opts) do skip_lock? = Keyword.get(opts, :skip_lock?, false) - actor = Keyword.get(opts, :actor) if Enum.empty?(cycles_to_delete) do # No cycles to delete, just regenerate - regenerate_cycles(member_id, today, skip_lock?: skip_lock?, actor: actor) + regenerate_cycles(member_id, today, skip_lock?: skip_lock?) else case delete_cycles(cycles_to_delete) do - :ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?, actor: actor) + :ok -> regenerate_cycles(member_id, today, skip_lock?: skip_lock?) {:error, reason} -> {:error, reason} end end @@ -928,13 +921,11 @@ defmodule Mv.Membership.Member do # Returns {:ok, notifications} - notifications should be returned to after_action hook defp regenerate_cycles(member_id, today, opts) do skip_lock? = Keyword.get(opts, :skip_lock?, false) - actor = Keyword.get(opts, :actor) case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( member_id, today: today, - skip_lock?: skip_lock?, - actor: actor + skip_lock?: skip_lock? ) do {:ok, _cycles, notifications} when is_list(notifications) -> {:ok, notifications} @@ -948,25 +939,22 @@ defmodule Mv.Membership.Member do # based on environment (test vs production) # This function encapsulates the common logic for cycle generation # to avoid code duplication across different hooks - defp handle_cycle_generation(member, opts) do - actor = Keyword.get(opts, :actor) - + # Uses system actor for cycle generation (mandatory side effect) + defp handle_cycle_generation(member, _opts) do if Mv.Config.sql_sandbox?() do - handle_cycle_generation_sync(member, actor: actor) + handle_cycle_generation_sync(member) else - handle_cycle_generation_async(member, actor: actor) + handle_cycle_generation_async(member) end end # Runs cycle generation synchronously (for test environment) - defp handle_cycle_generation_sync(member, opts) do + defp handle_cycle_generation_sync(member) do require Logger - actor = Keyword.get(opts, :actor) case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( member.id, - today: Date.utc_today(), - actor: actor + today: Date.utc_today() ) do {:ok, cycles, notifications} -> send_notifications_if_any(notifications) @@ -978,11 +966,9 @@ defmodule Mv.Membership.Member do end # Runs cycle generation asynchronously (for production environment) - defp handle_cycle_generation_async(member, opts) do - actor = Keyword.get(opts, :actor) - + defp handle_cycle_generation_async(member) do Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn -> - case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id, actor: actor) do + case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id) do {:ok, cycles, notifications} -> send_notifications_if_any(notifications) log_cycle_generation_success(member, cycles, notifications, sync: false) diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 0d17dd3..ec33914 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -30,12 +30,11 @@ defmodule Mv.MembershipFees.CycleGenerator do ## Authorization - This module runs systemically and accepts optional actor parameters. - When called from hooks/changes, actor is extracted from changeset context. - When called directly, actor should be provided for proper authorization. + This module runs systemically and uses the system actor for all operations. + This ensures that cycle generation always works, regardless of user permissions. - All functions accept an optional `actor` parameter in the `opts` keyword list - that is passed to Ash operations to ensure proper authorization checks are performed. + All functions use `Mv.Helpers.SystemActor.get_system_actor/0` to bypass + user permission checks, as cycle generation is a mandatory side effect. ## Examples @@ -47,6 +46,8 @@ defmodule Mv.MembershipFees.CycleGenerator do """ + alias Mv.Helpers + alias Mv.Helpers.SystemActor alias Mv.Membership.Member alias Mv.MembershipFees.CalendarCycles alias Mv.MembershipFees.Changes.SetMembershipFeeStartDate @@ -86,9 +87,7 @@ defmodule Mv.MembershipFees.CycleGenerator do def generate_cycles_for_member(member_or_id, opts \\ []) def generate_cycles_for_member(member_id, opts) when is_binary(member_id) do - actor = Keyword.get(opts, :actor) - - case load_member(member_id, actor: actor) do + case load_member(member_id) do {:ok, member} -> generate_cycles_for_member(member, opts) {:error, reason} -> {:error, reason} end @@ -98,27 +97,25 @@ defmodule Mv.MembershipFees.CycleGenerator do today = Keyword.get(opts, :today, Date.utc_today()) skip_lock? = Keyword.get(opts, :skip_lock?, false) - do_generate_cycles_with_lock(member, today, skip_lock?, opts) + do_generate_cycles_with_lock(member, today, skip_lock?) end # Generate cycles with lock handling # Returns {:ok, cycles, notifications} - notifications are never sent here, # they should be returned to the caller (e.g., via after_action hook) - defp do_generate_cycles_with_lock(member, today, true = _skip_lock?, opts) do + defp do_generate_cycles_with_lock(member, today, true = _skip_lock?) do # Lock already set by caller (e.g., regenerate_cycles_on_type_change) # Just generate cycles without additional locking - actor = Keyword.get(opts, :actor) - do_generate_cycles(member, today, actor: actor) + do_generate_cycles(member, today) end - defp do_generate_cycles_with_lock(member, today, false, opts) do + defp do_generate_cycles_with_lock(member, today, false) do lock_key = :erlang.phash2(member.id) - actor = Keyword.get(opts, :actor) Repo.transaction(fn -> Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) - case do_generate_cycles(member, today, actor: actor) do + case do_generate_cycles(member, today) do {:ok, cycles, notifications} -> # Return cycles and notifications - do NOT send notifications here # They will be sent by the caller (e.g., via after_action hook) @@ -168,12 +165,15 @@ defmodule Mv.MembershipFees.CycleGenerator do # Query ALL members with fee type assigned (including inactive/left members) # The exit_date boundary is applied during cycle generation, not here. # This allows catch-up generation for members who left but are missing cycles. + system_actor = SystemActor.get_system_actor() + opts = Helpers.ash_actor_opts(system_actor) + query = Member |> Ash.Query.filter(not is_nil(membership_fee_type_id)) |> Ash.Query.filter(not is_nil(join_date)) - case Ash.read(query) do + case Ash.read(query, opts) do {:ok, members} -> results = process_members_in_batches(members, batch_size, today) {:ok, build_results_summary(results)} @@ -235,33 +235,25 @@ defmodule Mv.MembershipFees.CycleGenerator do # Private functions - defp load_member(member_id, opts) do - actor = Keyword.get(opts, :actor) + defp load_member(member_id) do + system_actor = SystemActor.get_system_actor() + opts = Helpers.ash_actor_opts(system_actor) query = Member |> Ash.Query.filter(id == ^member_id) |> Ash.Query.load([:membership_fee_type, :membership_fee_cycles]) - result = - if actor do - Ash.read_one(query, actor: actor) - else - Ash.read_one(query) - end - - case result do + case Ash.read_one(query, opts) do {:ok, nil} -> {:error, :member_not_found} {:ok, member} -> {:ok, member} {:error, reason} -> {:error, reason} end end - defp do_generate_cycles(member, today, opts) do - actor = Keyword.get(opts, :actor) - + defp do_generate_cycles(member, today) do # Reload member with relationships to ensure fresh data - case load_member(member.id, actor: actor) do + case load_member(member.id) do {:ok, member} -> cond do is_nil(member.membership_fee_type_id) -> @@ -271,7 +263,7 @@ defmodule Mv.MembershipFees.CycleGenerator do {:error, :no_join_date} true -> - generate_missing_cycles(member, today, actor: actor) + generate_missing_cycles(member, today) end {:error, reason} -> @@ -279,8 +271,7 @@ defmodule Mv.MembershipFees.CycleGenerator do end end - defp generate_missing_cycles(member, today, opts) do - actor = Keyword.get(opts, :actor) + defp generate_missing_cycles(member, today) do fee_type = member.membership_fee_type interval = fee_type.interval amount = fee_type.amount @@ -296,7 +287,7 @@ defmodule Mv.MembershipFees.CycleGenerator do # Only generate if start_date <= end_date if start_date && Date.compare(start_date, end_date) != :gt do cycle_starts = generate_cycle_starts(start_date, end_date, interval) - create_cycles(cycle_starts, member.id, fee_type.id, amount, actor: actor) + create_cycles(cycle_starts, member.id, fee_type.id, amount) else {:ok, [], []} end @@ -391,8 +382,10 @@ defmodule Mv.MembershipFees.CycleGenerator do end end - defp create_cycles(cycle_starts, member_id, fee_type_id, amount, opts) do - actor = Keyword.get(opts, :actor) + defp create_cycles(cycle_starts, member_id, fee_type_id, amount) do + system_actor = SystemActor.get_system_actor() + opts = Helpers.ash_actor_opts(system_actor) + # Always use return_notifications?: true to collect notifications # Notifications will be returned to the caller, who is responsible for # sending them (e.g., via after_action hook returning {:ok, result, notifications}) @@ -407,7 +400,7 @@ defmodule Mv.MembershipFees.CycleGenerator do } handle_cycle_creation_result( - Ash.create(MembershipFeeCycle, attrs, return_notifications?: true, actor: actor), + Ash.create(MembershipFeeCycle, attrs, [return_notifications?: true] ++ opts), cycle_start ) end) diff --git a/lib/mv_web/live/member_live/show/membership_fees_component.ex b/lib/mv_web/live/member_live/show/membership_fees_component.ex index e85baab..186e8b7 100644 --- a/lib/mv_web/live/member_live/show/membership_fees_component.ex +++ b/lib/mv_web/live/member_live/show/membership_fees_component.ex @@ -556,9 +556,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do def handle_event("regenerate_cycles", _params, socket) do socket = assign(socket, :regenerating, true) member = socket.assigns.member - actor = current_actor(socket) - case CycleGenerator.generate_cycles_for_member(member.id, actor: actor) do + case CycleGenerator.generate_cycles_for_member(member.id) do {:ok, _new_cycles, _notifications} -> # Reload member with cycles actor = current_actor(socket)