From 7a8b069834d6ab9c734c553c7a3805abec480aae Mon Sep 17 00:00:00 2001 From: Moritz Date: Tue, 3 Mar 2026 19:03:47 +0100 Subject: [PATCH] Fix Credo Design (AliasUsage): add aliases in lib Add module aliases at top and use short names instead of fully qualified nested modules across lib/. --- lib/accounts/user.ex | 21 +++++---- .../user/validations/oidc_email_collision.ex | 8 ++-- lib/membership/member.ex | 44 +++++++++---------- lib/mv/application.ex | 16 ++++--- .../checks/actor_is_system_user.ex | 4 +- .../checks/custom_field_value_create_scope.ex | 3 +- lib/mv/authorization/checks/has_permission.ex | 3 +- lib/mv/authorization/role.ex | 6 ++- lib/mv/membership/member_export.ex | 3 +- lib/mv/membership/member_export/build.ex | 3 +- lib/mv/membership_fees/cycle_generator.ex | 4 +- lib/mv/vereinfacht/changes/sync_contact.ex | 13 +++--- lib/mv_web/components/core_components.ex | 9 ++-- .../live/auth/link_oidc_account_live.ex | 20 +++++---- .../live/custom_field_live/index_component.ex | 4 +- lib/mv_web/live/global_settings_live.ex | 12 +++-- lib/mv_web/live/member_live/form.ex | 15 ++++--- lib/mv_web/live/member_live/index.ex | 3 +- lib/mv_web/live/member_live/show.ex | 22 ++++++---- lib/mv_web/live/user_live/form.ex | 37 +++++++++------- lib/mv_web/live/user_live/index.ex | 10 +++-- lib/mv_web/live/user_live/show.ex | 10 +++-- lib/mv_web/live_helpers.ex | 3 +- lib/mv_web/live_user_auth.ex | 9 ++-- lib/mv_web/plugs/check_page_permission.ex | 3 +- 25 files changed, 176 insertions(+), 109 deletions(-) diff --git a/lib/accounts/user.ex b/lib/accounts/user.ex index 5e24445..6b9cd1e 100644 --- a/lib/accounts/user.ex +++ b/lib/accounts/user.ex @@ -11,6 +11,11 @@ defmodule Mv.Accounts.User do require Ash.Query import Ash.Expr + alias Ash.Resource.Preparation.Builtins + alias Mv.Authorization.Role, as: RoleResource + alias Mv.Helpers.SystemActor + alias Mv.OidcRoleSync + postgres do table "users" repo Mv.Repo @@ -282,20 +287,20 @@ defmodule Mv.Accounts.User do # Sync role from OIDC groups after sign-in (e.g. admin group → Admin role) # get? true can return nil, a single %User{}, or a list; normalize to list for Enum.each - prepare Ash.Resource.Preparation.Builtins.after_action(fn query, result, _context -> + prepare Builtins.after_action(fn query, result, _context -> user_info = Ash.Query.get_argument(query, :user_info) || %{} oauth_tokens = Ash.Query.get_argument(query, :oauth_tokens) || %{} users = case result do nil -> [] - u when is_struct(u, User) -> [u] + u when is_struct(u, __MODULE__) -> [u] list when is_list(list) -> list _ -> [] end Enum.each(users, fn user -> - Mv.OidcRoleSync.apply_admin_role_from_user_info(user, user_info, oauth_tokens) + OidcRoleSync.apply_admin_role_from_user_info(user, user_info, oauth_tokens) end) {:ok, result} @@ -483,10 +488,10 @@ defmodule Mv.Accounts.User do |> Enum.map(& &1.id) # Count only non-system users with admin role (system user is for internal ops) - system_email = Mv.Helpers.SystemActor.system_user_email() + system_email = SystemActor.system_user_email() count = - Mv.Accounts.User + __MODULE__ |> Ash.Query.for_read(:read) |> Ash.Query.filter(expr(role_id in ^admin_role_ids)) |> Ash.Query.filter(expr(email != ^system_email)) @@ -512,7 +517,7 @@ defmodule Mv.Accounts.User do # Prevent modification of the system actor user (required for internal operations). # Block update/destroy on UI-exposed actions only; :update_internal is used by bootstrap/tests. validate fn changeset, _context -> - if Mv.Helpers.SystemActor.system_user?(changeset.data) do + if SystemActor.system_user?(changeset.data) do {:error, field: :email, message: @@ -641,8 +646,8 @@ defmodule Mv.Accounts.User do case Process.get({__MODULE__, :default_role_id}) do nil -> role_id = - case Mv.Authorization.Role.get_mitglied_role() do - {:ok, %Mv.Authorization.Role{id: id}} -> id + case RoleResource.get_mitglied_role() do + {:ok, %RoleResource{id: id}} -> id _ -> nil end diff --git a/lib/accounts/user/validations/oidc_email_collision.ex b/lib/accounts/user/validations/oidc_email_collision.ex index f92c7e2..7ae8510 100644 --- a/lib/accounts/user/validations/oidc_email_collision.ex +++ b/lib/accounts/user/validations/oidc_email_collision.ex @@ -26,7 +26,9 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do use Ash.Resource.Validation require Logger + alias Mv.Accounts.User alias Mv.Accounts.User.Errors.PasswordVerificationRequired + alias Mv.Helpers.SystemActor @impl true def init(opts), do: {:ok, opts} @@ -43,10 +45,10 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do # Check if a user with this oidc_id already exists # If yes, this will be an upsert (email update), not a new registration # Use SystemActor for authorization during OIDC registration (no logged-in actor) - system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_actor = SystemActor.get_system_actor() existing_oidc_user = - case Mv.Accounts.User + case User |> Ash.Query.filter(oidc_id == ^to_string(oidc_id)) |> Ash.read_one(actor: system_actor) do {:ok, user} -> user @@ -62,7 +64,7 @@ defmodule Mv.Accounts.User.Validations.OidcEmailCollision do defp check_email_collision(email, new_oidc_id, user_info, existing_oidc_user, system_actor) do # Find existing user with this email # Use SystemActor for authorization during OIDC registration (no logged-in actor) - case Mv.Accounts.User + case User |> Ash.Query.filter(email == ^to_string(email)) |> Ash.read_one(actor: system_actor) do {:ok, nil} -> diff --git a/lib/membership/member.ex b/lib/membership/member.ex index 8f24595..0519a72 100644 --- a/lib/membership/member.ex +++ b/lib/membership/member.ex @@ -39,10 +39,16 @@ defmodule Mv.Membership.Member do require Ash.Query import Ash.Expr + alias Ecto.Adapters.SQL, as: EctoSQL alias Mv.Helpers - require Logger - + alias Mv.Helpers.SystemActor alias Mv.Membership.Helpers.VisibilityConfig + alias Mv.MembershipFees.CalendarCycles + alias Mv.MembershipFees.CycleGenerator + alias Mv.MembershipFees.MembershipFeeCycle + alias Mv.Repo + + require Logger # Module constants @member_search_limit 10 @@ -813,7 +819,7 @@ defmodule Mv.Membership.Member do case Map.get(cycle, :membership_fee_type) do %{interval: interval} -> cycle_end = - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) Date.compare(cycle.cycle_start, today) in [:lt, :eq] and Date.compare(today, cycle_end) in [:lt, :eq] @@ -847,7 +853,7 @@ defmodule Mv.Membership.Member do case Map.get(cycle, :membership_fee_type) do %{interval: interval} -> cycle_end = - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) Date.compare(today, cycle_end) == :gt @@ -863,7 +869,7 @@ defmodule Mv.Membership.Member do cycles, fn cycle -> interval = Map.get(cycle, :membership_fee_type).interval - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) end, {:desc, Date} ) @@ -890,7 +896,7 @@ defmodule Mv.Membership.Member do case Map.get(cycle, :membership_fee_type) do %{interval: interval} -> cycle_end = - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) cycle.status == :unpaid and Date.compare(today, cycle_end) == :gt @@ -908,15 +914,12 @@ defmodule Mv.Membership.Member do @doc false # 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) # Use advisory lock to prevent concurrent deletion and regeneration # This ensures atomicity when multiple updates happen simultaneously - if Mv.Repo.in_transaction?() do + if Repo.in_transaction?() do regenerate_cycles_in_transaction(member, today, lock_key) else regenerate_cycles_new_transaction(member, today, lock_key) @@ -926,15 +929,15 @@ defmodule Mv.Membership.Member do # 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) do - Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) 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) do - Mv.Repo.transaction(fn -> - Ecto.Adapters.SQL.query!(Mv.Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + Repo.transaction(fn -> + EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_regenerate_cycles_on_type_change(member, today, skip_lock?: true) do {:ok, notifications} -> @@ -942,7 +945,7 @@ defmodule Mv.Membership.Member do notifications {:error, reason} -> - Mv.Repo.rollback(reason) + Repo.rollback(reason) end end) |> case do @@ -956,9 +959,6 @@ defmodule Mv.Membership.Member do # 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) @@ -968,7 +968,7 @@ defmodule Mv.Membership.Member do # Find all unpaid cycles for this member # We need to check cycle_end for each cycle using its own interval all_unpaid_cycles_query = - Mv.MembershipFees.MembershipFeeCycle + MembershipFeeCycle |> Ash.Query.filter(member_id == ^member.id) |> Ash.Query.filter(status == :unpaid) |> Ash.Query.load([:membership_fee_type]) @@ -997,7 +997,7 @@ defmodule Mv.Membership.Member do case cycle.membership_fee_type do %{interval: interval} -> cycle_end = - Mv.MembershipFees.CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) + CalendarCycles.calculate_cycle_end(cycle.cycle_start, interval) Date.compare(today, cycle_end) in [:lt, :eq] @@ -1047,7 +1047,7 @@ defmodule Mv.Membership.Member do defp regenerate_cycles(member_id, today, opts) do skip_lock? = Keyword.get(opts, :skip_lock?, false) - case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( + case CycleGenerator.generate_cycles_for_member( member_id, today: today, skip_lock?: skip_lock? @@ -1078,7 +1078,7 @@ defmodule Mv.Membership.Member do # Runs cycle generation synchronously (for test environment) defp handle_cycle_generation_sync(member, initiator) do - case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member( + case CycleGenerator.generate_cycles_for_member( member.id, today: Date.utc_today(), initiator: initiator @@ -1099,7 +1099,7 @@ defmodule Mv.Membership.Member do # Runs cycle generation asynchronously (for production environment) defp handle_cycle_generation_async(member, initiator) do Task.Supervisor.async_nolink(Mv.TaskSupervisor, fn -> - case Mv.MembershipFees.CycleGenerator.generate_cycles_for_member(member.id, + case CycleGenerator.generate_cycles_for_member(member.id, initiator: initiator ) do {:ok, cycles, notifications} -> diff --git a/lib/mv/application.ex b/lib/mv/application.ex index 1967ddd..835652f 100644 --- a/lib/mv/application.ex +++ b/lib/mv/application.ex @@ -5,22 +5,28 @@ defmodule Mv.Application do use Application + alias Mv.Helpers.SystemActor + alias Mv.Repo + alias Mv.Vereinfacht.SyncFlash + alias MvWeb.Endpoint + alias MvWeb.Telemetry + @impl true def start(_type, _args) do - Mv.Vereinfacht.SyncFlash.create_table!() + SyncFlash.create_table!() children = [ - MvWeb.Telemetry, - Mv.Repo, + Telemetry, + Repo, {Task.Supervisor, name: Mv.TaskSupervisor}, {DNSCluster, query: Application.get_env(:mv, :dns_cluster_query) || :ignore}, {Phoenix.PubSub, name: Mv.PubSub}, {AshAuthentication.Supervisor, otp_app: :my}, - Mv.Helpers.SystemActor, + SystemActor, # Start a worker by calling: Mv.Worker.start_link(arg) # {Mv.Worker, arg}, # Start to serve requests, typically the last entry - MvWeb.Endpoint + Endpoint ] # See https://hexdocs.pm/elixir/Supervisor.html diff --git a/lib/mv/authorization/checks/actor_is_system_user.ex b/lib/mv/authorization/checks/actor_is_system_user.ex index a614a83..804a24d 100644 --- a/lib/mv/authorization/checks/actor_is_system_user.ex +++ b/lib/mv/authorization/checks/actor_is_system_user.ex @@ -7,9 +7,11 @@ defmodule Mv.Authorization.Checks.ActorIsSystemUser do """ use Ash.Policy.SimpleCheck + alias Mv.Helpers.SystemActor + @impl true def describe(_opts), do: "actor is the system user" @impl true - def match?(actor, _context, _opts), do: Mv.Helpers.SystemActor.system_user?(actor) + def match?(actor, _context, _opts), do: SystemActor.system_user?(actor) end diff --git a/lib/mv/authorization/checks/custom_field_value_create_scope.ex b/lib/mv/authorization/checks/custom_field_value_create_scope.ex index 0b24e74..bbbdacc 100644 --- a/lib/mv/authorization/checks/custom_field_value_create_scope.ex +++ b/lib/mv/authorization/checks/custom_field_value_create_scope.ex @@ -22,6 +22,7 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do end """ use Ash.Policy.Check + alias Mv.Authorization.Actor alias Mv.Authorization.PermissionSets @impl true @@ -67,5 +68,5 @@ defmodule Mv.Authorization.Checks.CustomFieldValueCreateScope do end end - defp ensure_role_loaded(actor), do: Mv.Authorization.Actor.ensure_loaded(actor) + defp ensure_role_loaded(actor), do: Actor.ensure_loaded(actor) end diff --git a/lib/mv/authorization/checks/has_permission.ex b/lib/mv/authorization/checks/has_permission.ex index 721cee7..a11bf2e 100644 --- a/lib/mv/authorization/checks/has_permission.ex +++ b/lib/mv/authorization/checks/has_permission.ex @@ -81,6 +81,7 @@ defmodule Mv.Authorization.Checks.HasPermission do use Ash.Policy.Check require Ash.Query import Ash.Expr + alias Mv.Authorization.Actor alias Mv.Authorization.PermissionSets require Logger @@ -397,6 +398,6 @@ defmodule Mv.Authorization.Checks.HasPermission do # Fallback: Load role if not loaded (in case on_mount didn't run) # Delegates to centralized Actor helper defp ensure_role_loaded(actor) do - Mv.Authorization.Actor.ensure_loaded(actor) + Actor.ensure_loaded(actor) end end diff --git a/lib/mv/authorization/role.ex b/lib/mv/authorization/role.ex index 8700a33..77e0507 100644 --- a/lib/mv/authorization/role.ex +++ b/lib/mv/authorization/role.ex @@ -94,14 +94,16 @@ defmodule Mv.Authorization.Role do end end + alias Mv.Authorization.PermissionSets + validations do validate one_of( :permission_set_name, - Mv.Authorization.PermissionSets.all_permission_sets() + PermissionSets.all_permission_sets() |> Enum.map(&Atom.to_string/1) ), message: - "must be one of: #{Mv.Authorization.PermissionSets.all_permission_sets() |> Enum.map_join(", ", &Atom.to_string/1)}" + "must be one of: #{PermissionSets.all_permission_sets() |> Enum.map_join(", ", &Atom.to_string/1)}" validate fn changeset, _context -> if changeset.data.is_system_role do diff --git a/lib/mv/membership/member_export.ex b/lib/mv/membership/member_export.ex index bbfbb6e..635c832 100644 --- a/lib/mv/membership/member_export.ex +++ b/lib/mv/membership/member_export.ex @@ -13,6 +13,7 @@ defmodule Mv.Membership.MemberExport do alias Mv.Membership.CustomField alias Mv.Membership.Member alias Mv.Membership.MemberExportSort + alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index.MembershipFeeStatus @member_fields_allowlist (Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)) ++ @@ -169,7 +170,7 @@ defmodule Mv.Membership.MemberExport do if parsed.selected_ids == [] do members |> apply_cycle_status_filter(parsed.cycle_status_filter, parsed.show_current_cycle) - |> MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( + |> Index.apply_boolean_custom_field_filters( parsed.boolean_filters || %{}, Map.values(custom_fields_by_id) ) diff --git a/lib/mv/membership/member_export/build.ex b/lib/mv/membership/member_export/build.ex index e08a6f4..7159679 100644 --- a/lib/mv/membership/member_export/build.ex +++ b/lib/mv/membership/member_export/build.ex @@ -21,6 +21,7 @@ defmodule Mv.Membership.MemberExport.Build do import Ash.Expr alias Mv.Membership.{CustomField, CustomFieldValueFormatter, Member, MemberExportSort} + alias MvWeb.MemberLive.Index alias MvWeb.MemberLive.Index.MembershipFeeStatus @custom_field_prefix Mv.Constants.custom_field_prefix() @@ -169,7 +170,7 @@ defmodule Mv.Membership.MemberExport.Build do if parsed.selected_ids == [] do members |> apply_cycle_status_filter(parsed.cycle_status_filter, parsed.show_current_cycle) - |> MvWeb.MemberLive.Index.apply_boolean_custom_field_filters( + |> Index.apply_boolean_custom_field_filters( parsed.boolean_filters || %{}, Map.values(custom_fields_by_id) ) diff --git a/lib/mv/membership_fees/cycle_generator.ex b/lib/mv/membership_fees/cycle_generator.ex index 1a33ca8..52776b6 100644 --- a/lib/mv/membership_fees/cycle_generator.ex +++ b/lib/mv/membership_fees/cycle_generator.ex @@ -54,6 +54,8 @@ defmodule Mv.MembershipFees.CycleGenerator do alias Mv.MembershipFees.MembershipFeeCycle alias Mv.Repo + alias Ecto.Adapters.SQL, as: EctoSQL + require Ash.Query require Logger @@ -113,7 +115,7 @@ defmodule Mv.MembershipFees.CycleGenerator do lock_key = :erlang.phash2(member.id) Repo.transaction(fn -> - Ecto.Adapters.SQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) + EctoSQL.query!(Repo, "SELECT pg_advisory_xact_lock($1)", [lock_key]) case do_generate_cycles(member, today, opts) do {:ok, cycles, notifications} -> diff --git a/lib/mv/vereinfacht/changes/sync_contact.ex b/lib/mv/vereinfacht/changes/sync_contact.ex index 99875e0..f3679d4 100644 --- a/lib/mv/vereinfacht/changes/sync_contact.ex +++ b/lib/mv/vereinfacht/changes/sync_contact.ex @@ -14,6 +14,9 @@ defmodule Mv.Vereinfacht.Changes.SyncContact do """ use Ash.Resource.Change + alias Mv.Vereinfacht + alias Mv.Vereinfacht.SyncFlash + require Logger @synced_attributes [ @@ -60,13 +63,13 @@ defmodule Mv.Vereinfacht.Changes.SyncContact do # Ash calls after_transaction with (changeset, result) only - 2 args. defp sync_after_transaction(_changeset, {:ok, member}) do - case Mv.Vereinfacht.sync_member(member) do + case Vereinfacht.sync_member(member) do :ok -> - Mv.Vereinfacht.SyncFlash.store(to_string(member.id), :ok, "Synced to Vereinfacht.") + SyncFlash.store(to_string(member.id), :ok, "Synced to Vereinfacht.") {:ok, member} {:ok, member_updated} -> - Mv.Vereinfacht.SyncFlash.store( + SyncFlash.store( to_string(member_updated.id), :ok, "Synced to Vereinfacht." @@ -77,10 +80,10 @@ defmodule Mv.Vereinfacht.Changes.SyncContact do {:error, reason} -> Logger.warning("Vereinfacht sync failed for member #{member.id}: #{inspect(reason)}") - Mv.Vereinfacht.SyncFlash.store( + SyncFlash.store( to_string(member.id), :warning, - Mv.Vereinfacht.format_error(reason) + Vereinfacht.format_error(reason) ) {:ok, member} diff --git a/lib/mv_web/components/core_components.ex b/lib/mv_web/components/core_components.ex index 78b8bfb..3606b34 100644 --- a/lib/mv_web/components/core_components.ex +++ b/lib/mv_web/components/core_components.ex @@ -29,6 +29,7 @@ defmodule MvWeb.CoreComponents do use Phoenix.Component use Gettext, backend: MvWeb.Gettext + alias Phoenix.HTML.Form, as: HTMLForm alias Phoenix.LiveView.JS # WCAG 2.4.7 / 2.4.11: Shared focus ring for buttons and dropdown (trigger + items) @@ -669,7 +670,7 @@ defmodule MvWeb.CoreComponents do def input(%{type: "checkbox"} = assigns) do assigns = assign_new(assigns, :checked, fn -> - Phoenix.HTML.Form.normalize_value("checkbox", assigns[:value]) + HTMLForm.normalize_value("checkbox", assigns[:value]) end) # For checkboxes, we don't use HTML required attribute (means "must be checked") @@ -736,7 +737,7 @@ defmodule MvWeb.CoreComponents do {@rest} > - {Phoenix.HTML.Form.options_for_select(@options, @value)} + {HTMLForm.options_for_select(@options, @value)} <.error :for={msg <- @errors}>{msg} @@ -765,7 +766,7 @@ defmodule MvWeb.CoreComponents do @errors != [] && (@error_class || "textarea-error") ]} {@rest} - >{Phoenix.HTML.Form.normalize_value("textarea", @value)} + >{HTMLForm.normalize_value("textarea", @value)} <.error :for={msg <- @errors}>{msg} @@ -790,7 +791,7 @@ defmodule MvWeb.CoreComponents do type={@type} name={@name} id={@id} - value={Phoenix.HTML.Form.normalize_value(@type, @value)} + value={HTMLForm.normalize_value(@type, @value)} class={[ @class || "w-full input", @errors != [] && (@error_class || "input-error") diff --git a/lib/mv_web/live/auth/link_oidc_account_live.ex b/lib/mv_web/live/auth/link_oidc_account_live.ex index 01bd57b..c4b3ab0 100644 --- a/lib/mv_web/live/auth/link_oidc_account_live.ex +++ b/lib/mv_web/live/auth/link_oidc_account_live.ex @@ -18,15 +18,19 @@ defmodule MvWeb.LinkOidcAccountLive do require Ash.Query require Logger + alias AshAuthentication.Strategy.Password.Actions, as: PasswordActions + alias Mv.Accounts.User, as: UserResource + alias Mv.Helpers.SystemActor + @impl true def mount(_params, session, socket) do # Use SystemActor for authorization during OIDC linking (user is not yet logged in) - system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_actor = SystemActor.get_system_actor() with user_id when not is_nil(user_id) <- Map.get(session, "oidc_linking_user_id"), oidc_user_info when not is_nil(oidc_user_info) <- Map.get(session, "oidc_linking_user_info"), - {:ok, user} <- Ash.get(Mv.Accounts.User, user_id, actor: system_actor) do + {:ok, user} <- Ash.get(UserResource, user_id, actor: system_actor) do # Check if user is passwordless if passwordless?(user) do # Auto-link passwordless user immediately @@ -50,9 +54,9 @@ defmodule MvWeb.LinkOidcAccountLive do defp reload_user!(user_id) do # Use SystemActor for authorization during OIDC linking (user is not yet logged in) - system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_actor = SystemActor.get_system_actor() - Mv.Accounts.User + UserResource |> Ash.Query.filter(id == ^user_id) |> Ash.read_one!(actor: system_actor) end @@ -65,7 +69,7 @@ defmodule MvWeb.LinkOidcAccountLive do oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id") # Use SystemActor for authorization (passwordless user auto-linking) - system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_actor = SystemActor.get_system_actor() case user.id |> reload_user!() @@ -176,11 +180,11 @@ defmodule MvWeb.LinkOidcAccountLive do defp verify_password(email, password) do # Use AshAuthentication password strategy to verify - strategies = AshAuthentication.Info.authentication_strategies(Mv.Accounts.User) + strategies = AshAuthentication.Info.authentication_strategies(UserResource) password_strategy = Enum.find(strategies, fn s -> s.name == :password end) if password_strategy do - AshAuthentication.Strategy.Password.Actions.sign_in( + PasswordActions.sign_in( password_strategy, %{ "email" => email, @@ -197,7 +201,7 @@ defmodule MvWeb.LinkOidcAccountLive do oidc_id = Map.get(oidc_user_info, "sub") || Map.get(oidc_user_info, "id") # Use SystemActor for authorization (user just verified password but is not yet logged in) - system_actor = Mv.Helpers.SystemActor.get_system_actor() + system_actor = SystemActor.get_system_actor() # Update the user with the OIDC ID case user.id diff --git a/lib/mv_web/live/custom_field_live/index_component.ex b/lib/mv_web/live/custom_field_live/index_component.ex index 6d1fc2f..4d654e2 100644 --- a/lib/mv_web/live/custom_field_live/index_component.ex +++ b/lib/mv_web/live/custom_field_live/index_component.ex @@ -12,11 +12,13 @@ defmodule MvWeb.CustomFieldLive.IndexComponent do """ use MvWeb, :live_component + alias MvWeb.Translations.FieldTypes + require Logger @impl true def render(assigns) do - assigns = assign(assigns, :field_type_label, &MvWeb.Translations.FieldTypes.label/1) + assigns = assign(assigns, :field_type_label, &FieldTypes.label/1) ~H"""
diff --git a/lib/mv_web/live/global_settings_live.ex b/lib/mv_web/live/global_settings_live.ex index 289d721..8c4ea2c 100644 --- a/lib/mv_web/live/global_settings_live.ex +++ b/lib/mv_web/live/global_settings_live.ex @@ -26,7 +26,11 @@ defmodule MvWeb.GlobalSettingsLive do require Ash.Query import Ash.Expr + alias Mv.Helpers + alias Mv.Helpers.SystemActor alias Mv.Membership + alias Mv.Membership.Member, as: MemberResource + alias MvWeb.Helpers.MemberHelpers on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded} @@ -551,13 +555,13 @@ defmodule MvWeb.GlobalSettingsLive do end defp fetch_member_names_by_ids(ids) do - actor = Mv.Helpers.SystemActor.get_system_actor() - opts = Mv.Helpers.ash_actor_opts(actor) - query = Ash.Query.filter(Mv.Membership.Member, expr(id in ^ids)) + actor = SystemActor.get_system_actor() + opts = Helpers.ash_actor_opts(actor) + query = Ash.Query.filter(MemberResource, expr(id in ^ids)) case Ash.read(query, opts) do {:ok, members} -> - Map.new(members, fn m -> {m.id, MvWeb.Helpers.MemberHelpers.display_name(m)} end) + Map.new(members, fn m -> {m.id, MemberHelpers.display_name(m)} end) _ -> %{} diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index 88d0444..4d76f33 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -25,8 +25,11 @@ defmodule MvWeb.MemberLive.Form do alias Mv.Membership alias Mv.Membership.Helpers.VisibilityConfig + alias Mv.Membership.Member, as: MemberResource alias Mv.MembershipFees alias Mv.MembershipFees.MembershipFeeType + alias Mv.Vereinfacht.SyncFlash + alias MvWeb.Helpers.MemberHelpers alias MvWeb.Helpers.MembershipFeeHelpers @impl true @@ -51,7 +54,7 @@ defmodule MvWeb.MemberLive.Form do <%= if @member do %> - {MvWeb.Helpers.MemberHelpers.display_name(@member)} + {MemberHelpers.display_name(@member)} <% else %> {gettext("New Member")} <% end %> @@ -289,7 +292,7 @@ defmodule MvWeb.MemberLive.Form do data-testid="member-delete" aria-label={ gettext("Delete member %{name}", - name: MvWeb.Helpers.MemberHelpers.display_name(@member) + name: MemberHelpers.display_name(@member) ) } > @@ -316,7 +319,7 @@ defmodule MvWeb.MemberLive.Form do

{gettext( "Are you sure you want to delete %{name}? This action cannot be undone.", - name: MvWeb.Helpers.MemberHelpers.display_name(@member) + name: MemberHelpers.display_name(@member) )}

@@ -371,7 +374,7 @@ defmodule MvWeb.MemberLive.Form do member = case params["id"] do nil -> nil - id -> Ash.get!(Mv.Membership.Member, id, load: [:membership_fee_type], actor: actor) + id -> Ash.get!(MemberResource, id, load: [:membership_fee_type], actor: actor) end page_title = @@ -562,7 +565,7 @@ defmodule MvWeb.MemberLive.Form do end defp maybe_put_vereinfacht_sync_flash(socket, member_id) do - case Mv.Vereinfacht.SyncFlash.take(to_string(member_id)) do + case SyncFlash.take(to_string(member_id)) do {:warning, message} -> put_flash(socket, :warning, translate_vereinfacht_flash(message)) @@ -767,7 +770,7 @@ defmodule MvWeb.MemberLive.Form do ) else AshPhoenix.Form.for_create( - Mv.Membership.Member, + MemberResource, :create_member, api: Mv.Membership, as: "member", diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index c83a204..6cf532d 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -32,6 +32,7 @@ defmodule MvWeb.MemberLive.Index do import MvWeb.LiveHelpers, only: [current_actor: 1] alias Mv.Membership + alias Mv.Membership.Member, as: MemberResource alias MvWeb.Helpers.DateFormatter alias MvWeb.MemberLive.Index.FieldSelection alias MvWeb.MemberLive.Index.FieldVisibility @@ -1012,7 +1013,7 @@ defmodule MvWeb.MemberLive.Index do defp apply_search_filter(query, search_query) do if search_query && String.trim(search_query) != "" do query - |> Mv.Membership.Member.fuzzy_search(%{query: search_query}) + |> MemberResource.fuzzy_search(%{query: search_query}) else query end diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index cc929cd..d47ee4c 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -24,7 +24,13 @@ defmodule MvWeb.MemberLive.Show do import Ash.Query import MvWeb.LiveHelpers, only: [current_actor: 1] + alias Mv.Membership.CustomField + alias Mv.Membership.CustomFieldValue + alias Mv.Membership.Member, as: MemberResource + alias Mv.Vereinfacht.Client, as: VereinfachtClient + alias MvWeb.Helpers.MemberHelpers alias MvWeb.Helpers.MembershipFeeHelpers + alias Phoenix.HTML.Engine, as: HTMLEngine @impl true def render(assigns) do @@ -41,7 +47,7 @@ defmodule MvWeb.MemberLive.Show do {gettext("Back")} - {MvWeb.Helpers.MemberHelpers.display_name(@member)} + {MemberHelpers.display_name(@member)} <:actions> <%= if can?(@current_user, :update, @member) do %> <.button @@ -329,7 +335,7 @@ defmodule MvWeb.MemberLive.Show do data-testid="member-delete" aria-label={ gettext("Delete member %{name}", - name: MvWeb.Helpers.MemberHelpers.display_name(@member) + name: MemberHelpers.display_name(@member) ) } > @@ -355,7 +361,7 @@ defmodule MvWeb.MemberLive.Show do

{gettext("Are you sure you want to delete %{name}? This action cannot be undone.", - name: MvWeb.Helpers.MemberHelpers.display_name(@member) + name: MemberHelpers.display_name(@member) )}

@@ -402,13 +408,13 @@ defmodule MvWeb.MemberLive.Show do # Load custom fields once using assign_new to avoid repeated queries socket = assign_new(socket, :custom_fields, fn -> - Mv.Membership.CustomField + CustomField |> Ash.Query.sort(name: :asc) |> Ash.read!(actor: actor) end) query = - Mv.Membership.Member + MemberResource |> filter(id == ^id) |> load([ :user, @@ -527,7 +533,7 @@ defmodule MvWeb.MemberLive.Show do def handle_event("load_vereinfacht_receipts", %{"contact_id" => contact_id}, socket) do response = - case Mv.Vereinfacht.Client.get_contact_with_receipts(contact_id) do + case VereinfachtClient.get_contact_with_receipts(contact_id) do {:ok, receipts} -> {:ok, receipts} {:error, reason} -> {:error, reason} end @@ -717,7 +723,7 @@ defmodule MvWeb.MemberLive.Show do # Handles both CustomFieldValue structs and direct values defp format_custom_field_value(nil, _type), do: render_empty_value() - defp format_custom_field_value(%Mv.Membership.CustomFieldValue{} = cfv, value_type) do + defp format_custom_field_value(%CustomFieldValue{} = cfv, value_type) do format_custom_field_value(cfv.value, value_type) end @@ -759,6 +765,6 @@ defmodule MvWeb.MemberLive.Show do # Returns safe HTML so it can be used from helpers without LiveView assigns. defp render_empty_value do text = gettext("Not set") - {:safe, ["", Phoenix.HTML.Engine.html_escape(text), ""]} + {:safe, ["", HTMLEngine.html_escape(text), ""]} end end diff --git a/lib/mv_web/live/user_live/form.ex b/lib/mv_web/live/user_live/form.ex index 06b57fc..5232ec7 100644 --- a/lib/mv_web/live/user_live/form.ex +++ b/lib/mv_web/live/user_live/form.ex @@ -35,7 +35,14 @@ defmodule MvWeb.UserLive.Form do require Jason + alias Mv.Accounts + alias Mv.Accounts.User, as: UserResource alias Mv.Authorization + alias Mv.Authorization.Role, as: RoleResource + alias Mv.Helpers.SystemActor + alias Mv.Membership + alias Mv.Membership.Member, as: MemberResource + alias MvWeb.Helpers.MemberHelpers import MvWeb.LiveHelpers, only: [current_actor: 1, submit_form: 3] import MvWeb.Authorization, only: [can?: 3] @@ -303,7 +310,7 @@ defmodule MvWeb.UserLive.Form do <% end %> <%!-- Danger zone: canonical pattern (same as member form) --%> - <%= if @user && can?(@current_user, :destroy, @user) && !Mv.Helpers.SystemActor.system_user?(@user) do %> + <%= if @user && can?(@current_user, :destroy, @user) && !SystemActor.system_user?(@user) do %>

{gettext("Danger zone")} @@ -402,9 +409,9 @@ defmodule MvWeb.UserLive.Form do defp load_user_or_redirect(nil, _actor, _socket), do: {:ok, nil} defp load_user_or_redirect(id, actor, socket) do - user = Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member], actor: actor) + user = Ash.get!(UserResource, id, domain: Accounts, load: [:member], actor: actor) - if Mv.Helpers.SystemActor.system_user?(user) do + if SystemActor.system_user?(user) do {:redirect, socket |> put_flash(:error, gettext("This user cannot be edited.")) @@ -420,9 +427,9 @@ defmodule MvWeb.UserLive.Form do page_title = action <> " " <> gettext("User") # Only admins can link/unlink users to members (permission docs; prevents privilege escalation). - can_manage_member_linking = can?(actor, :destroy, Mv.Accounts.User) + can_manage_member_linking = can?(actor, :destroy, UserResource) # Only admins can assign user roles (Role update permission). - can_assign_role = can?(actor, :update, Mv.Authorization.Role) + can_assign_role = can?(actor, :update, RoleResource) roles = if can_assign_role, do: load_roles(actor), else: [] {:ok, @@ -541,7 +548,7 @@ defmodule MvWeb.UserLive.Form do |> put_flash(:error, gettext("User not found")) |> assign(:show_delete_modal, false)} - Mv.Helpers.SystemActor.system_user?(user) -> + SystemActor.system_user?(user) -> {:noreply, socket |> put_flash(:error, gettext("System user cannot be deleted.")) @@ -634,7 +641,7 @@ defmodule MvWeb.UserLive.Form do member_name = if selected_member, - do: MvWeb.Helpers.MemberHelpers.display_name(selected_member), + do: MemberHelpers.display_name(selected_member), else: "" # Store the selected member ID and name in socket state and clear unlink flag @@ -704,17 +711,17 @@ defmodule MvWeb.UserLive.Form do defp perform_member_link_action(socket, user, actor) do # Only admins may link/unlink (backend policy also restricts update_user; UI must not call it). - if can?(actor, :destroy, Mv.Accounts.User) do + if can?(actor, :destroy, UserResource) do cond do # Selected member ID takes precedence (new link) socket.assigns.selected_member_id -> - Mv.Accounts.update_user(user, %{member: %{id: socket.assigns.selected_member_id}}, + Accounts.update_user(user, %{member: %{id: socket.assigns.selected_member_id}}, actor: actor ) # Unlink flag is set socket.assigns[:unlink_member] -> - Mv.Accounts.update_user(user, %{member: nil}, actor: actor) + Accounts.update_user(user, %{member: nil}, actor: actor) # No changes to member relationship true -> @@ -831,8 +838,8 @@ defmodule MvWeb.UserLive.Form do # For new users, use password registration if password fields are shown action = if show_password_fields, do: :register_with_password, else: :create_user - AshPhoenix.Form.for_create(Mv.Accounts.User, action, - domain: Mv.Accounts, + AshPhoenix.Form.for_create(UserResource, action, + domain: Accounts, as: "user", actor: actor ) @@ -878,7 +885,7 @@ defmodule MvWeb.UserLive.Form do search_query_str = if search_query && search_query != "", do: search_query, else: nil query = - Mv.Membership.Member + MemberResource |> Ash.Query.for_read(:available_for_linking, %{ user_email: user_email_str, search_query: search_query_str @@ -890,7 +897,7 @@ defmodule MvWeb.UserLive.Form do if is_nil(actor) do [] else - case Ash.read(query, domain: Mv.Membership, actor: actor) do + case Ash.read(query, domain: Membership, actor: actor) do {:ok, members} -> apply_email_filter(members, user_email_str) {:error, _} -> [] end @@ -902,7 +909,7 @@ defmodule MvWeb.UserLive.Form do defp apply_email_filter(members, nil), do: members defp apply_email_filter(members, user_email_str) when is_binary(user_email_str) do - Mv.Membership.Member.filter_by_email_match(members, user_email_str) + MemberResource.filter_by_email_match(members, user_email_str) end @spec load_roles(any()) :: [Mv.Authorization.Role.t()] diff --git a/lib/mv_web/live/user_live/index.ex b/lib/mv_web/live/user_live/index.ex index 4858202..882fb85 100644 --- a/lib/mv_web/live/user_live/index.ex +++ b/lib/mv_web/live/user_live/index.ex @@ -19,6 +19,10 @@ defmodule MvWeb.UserLive.Index do import MvWeb.LiveHelpers, only: [current_actor: 1] + alias Mv.Accounts + alias Mv.Accounts.User, as: UserResource + alias Mv.Helpers.SystemActor + require Ash.Query @impl true @@ -26,9 +30,9 @@ defmodule MvWeb.UserLive.Index do actor = current_actor(socket) users = - Mv.Accounts.User - |> Ash.Query.filter(email != ^Mv.Helpers.SystemActor.system_user_email()) - |> Ash.read!(domain: Mv.Accounts, load: [:member, :role], actor: actor) + UserResource + |> Ash.Query.filter(email != ^SystemActor.system_user_email()) + |> Ash.read!(domain: Accounts, load: [:member, :role], actor: actor) sorted = Enum.sort_by(users, & &1.email) diff --git a/lib/mv_web/live/user_live/show.ex b/lib/mv_web/live/user_live/show.ex index 770be82..8ac8126 100644 --- a/lib/mv_web/live/user_live/show.ex +++ b/lib/mv_web/live/user_live/show.ex @@ -29,6 +29,10 @@ defmodule MvWeb.UserLive.Show do import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.ErrorHelpers, only: [format_ash_error: 1] + alias Mv.Accounts + alias Mv.Accounts.User, as: UserResource + alias Mv.Helpers.SystemActor + @impl true def render(assigns) do ~H""" @@ -167,9 +171,9 @@ defmodule MvWeb.UserLive.Show do actor = current_actor(socket) user = - Ash.get!(Mv.Accounts.User, id, domain: Mv.Accounts, load: [:member, :role], actor: actor) + Ash.get!(UserResource, id, domain: Accounts, load: [:member, :role], actor: actor) - if Mv.Helpers.SystemActor.system_user?(user) do + if SystemActor.system_user?(user) do {:ok, socket |> put_flash(:error, gettext("This user cannot be viewed.")) @@ -221,7 +225,7 @@ defmodule MvWeb.UserLive.Show do |> put_flash(:error, gettext("User not found")) |> assign(:show_delete_modal, false)} - Mv.Helpers.SystemActor.system_user?(user) -> + SystemActor.system_user?(user) -> {:noreply, socket |> put_flash(:error, gettext("System user cannot be deleted.")) diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex index ff99ad8..dae8325 100644 --- a/lib/mv_web/live_helpers.ex +++ b/lib/mv_web/live_helpers.ex @@ -16,6 +16,7 @@ defmodule MvWeb.LiveHelpers do ``` """ import Phoenix.Component + alias Mv.Authorization.Actor alias MvWeb.Plugs.CheckPagePermission def on_mount(:default, _params, session, socket) do @@ -68,7 +69,7 @@ defmodule MvWeb.LiveHelpers do if user do # Use centralized Actor helper to ensure role is loaded - user_with_role = Mv.Authorization.Actor.ensure_loaded(user) + user_with_role = Actor.ensure_loaded(user) assign(socket, :current_user, user_with_role) else socket diff --git a/lib/mv_web/live_user_auth.ex b/lib/mv_web/live_user_auth.ex index c913caa..f3f3fc9 100644 --- a/lib/mv_web/live_user_auth.ex +++ b/lib/mv_web/live_user_auth.ex @@ -6,6 +6,9 @@ defmodule MvWeb.LiveUserAuth do import Phoenix.Component use MvWeb, :verified_routes + alias AshAuthentication.Phoenix.LiveSession + alias Phoenix.LiveView + # This is used for nested liveviews to fetch the current user. # To use, place the following at the top of that liveview: # on_mount {MvWeb.LiveUserAuth, :current_user} @@ -15,7 +18,7 @@ defmodule MvWeb.LiveUserAuth do socket = socket |> assign(:return_to, return_to) - |> AshAuthentication.Phoenix.LiveSession.assign_new_resources(session) + |> LiveSession.assign_new_resources(session) {:cont, session, socket} end @@ -29,14 +32,14 @@ defmodule MvWeb.LiveUserAuth do end def on_mount(:live_user_required, _params, session, socket) do - socket = AshAuthentication.Phoenix.LiveSession.assign_new_resources(socket, session) + socket = LiveSession.assign_new_resources(socket, session) case socket.assigns do %{current_user: %{} = user} -> {:cont, assign(socket, :current_user, user)} _ -> - socket = Phoenix.LiveView.redirect(socket, to: ~p"/sign-in") + socket = LiveView.redirect(socket, to: ~p"/sign-in") {:halt, socket} end end diff --git a/lib/mv_web/plugs/check_page_permission.ex b/lib/mv_web/plugs/check_page_permission.ex index cbbb76d..3aa6561 100644 --- a/lib/mv_web/plugs/check_page_permission.ex +++ b/lib/mv_web/plugs/check_page_permission.ex @@ -25,6 +25,7 @@ defmodule MvWeb.Plugs.CheckPagePermission do import Plug.Conn import Phoenix.Controller + alias Mv.Authorization.Actor alias Mv.Authorization.PermissionSets require Logger @@ -37,7 +38,7 @@ defmodule MvWeb.Plugs.CheckPagePermission do # Ensure role is loaded (load_from_session does not load it; required for permission check) user = conn.assigns[:current_user] - |> Mv.Authorization.Actor.ensure_loaded() + |> Actor.ensure_loaded() conn = Plug.Conn.assign(conn, :current_user, user) page_path = get_page_path(conn)