From 897677a7824656f91a328f14e4588e98ce284df1 Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 13 Jan 2026 14:05:41 +0100 Subject: [PATCH] refactor: Replace actor option patterns with ash_actor_opts helper - Replace if actor, do: [actor: actor], else: [] with Mv.Helpers.ash_actor_opts/1 - Update email_sync/loader.ex, member validations, member.ex, cycle_generator.ex - Consistent actor handling across non-LiveView modules --- lib/membership/member.ex | 3 ++- lib/mv/email_sync/loader.ex | 16 +++++++++++++--- .../validations/email_not_used_by_other_user.ex | 5 +++-- lib/mv/membership_fees/cycle_generator.ex | 9 +++++++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 43322f6..41e02b1 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -39,6 +39,7 @@ defmodule Mv.Membership.Member do require Ash.Query import Ash.Expr + alias Mv.Helpers require Logger alias Mv.Membership.Helpers.VisibilityConfig @@ -1217,7 +1218,7 @@ defmodule Mv.Membership.Member do # Extracts custom field values from existing member data (update scenario) defp extract_existing_values(member_data, changeset) do actor = Map.get(changeset.context, :actor) - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.load(member_data, :custom_field_values, opts) do {:ok, %{custom_field_values: existing_values}} -> diff --git a/lib/mv/email_sync/loader.ex b/lib/mv/email_sync/loader.ex index f6f6ecc..91927fb 100644 --- a/lib/mv/email_sync/loader.ex +++ b/lib/mv/email_sync/loader.ex @@ -2,7 +2,17 @@ defmodule Mv.EmailSync.Loader do @moduledoc """ Helper functions for loading linked records in email synchronization. Centralizes the logic for retrieving related User/Member entities. + + ## 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. + + All functions accept an optional `actor` parameter that is passed to Ash operations + to ensure proper authorization checks are performed. """ + alias Mv.Helpers @doc """ Loads the member linked to a user, returns nil if not linked or on error. @@ -13,7 +23,7 @@ defmodule Mv.EmailSync.Loader do def get_linked_member(%{member_id: nil}, _actor), do: nil def get_linked_member(%{member_id: id}, actor) do - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.get(Mv.Membership.Member, id, opts) do {:ok, member} -> member @@ -27,7 +37,7 @@ defmodule Mv.EmailSync.Loader do Accepts optional actor for authorization. """ def get_linked_user(member, actor \\ nil) do - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.load(member, :user, opts) do {:ok, %{user: user}} -> user @@ -42,7 +52,7 @@ defmodule Mv.EmailSync.Loader do Accepts optional actor for authorization. """ def load_linked_user!(member, actor \\ nil) do - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.load(member, :user, opts) do {:ok, %{user: user}} when not is_nil(user) -> {:ok, user} 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 649493b..e1bbc4e 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 @@ -8,6 +8,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do This allows creating members with the same email as unlinked users. """ use Ash.Resource.Validation + alias Mv.Helpers @doc """ Validates email uniqueness across linked Member-User pairs. @@ -51,7 +52,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do |> Ash.Query.filter(email == ^email) |> maybe_exclude_id(exclude_user_id) - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.read(query, opts) do {:ok, []} -> @@ -69,7 +70,7 @@ defmodule Mv.Membership.Member.Validations.EmailNotUsedByOtherUser do defp maybe_exclude_id(query, id), do: Ash.Query.filter(query, id != ^id) defp get_linked_user_id(member_data, actor) do - opts = if actor, do: [actor: actor], else: [] + opts = Helpers.ash_actor_opts(actor) case Ash.load(member_data, :user, opts) do {:ok, %{user: %{id: id}}} -> id diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 32b4e79..0d17dd3 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -28,6 +28,15 @@ defmodule Mv.MembershipFees.CycleGenerator do Uses PostgreSQL advisory locks to prevent race conditions when generating cycles for the same member concurrently. + ## 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. + + 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. + ## Examples # Generate cycles for a single member