refactor: add opts_with_actor helper and improve error formatting

Add opts_with_actor helper function to reduce duplication when building
Ash options with actor and domain. Improve format_error documentation
and ensure consistent error message formatting.
This commit is contained in:
Moritz 2026-01-08 15:54:49 +01:00
parent ddb4da266d
commit 08182300b9
Signed by: moritz
GPG key ID: 1020A035E5DD0824
4 changed files with 26 additions and 9 deletions

View file

@ -15,7 +15,7 @@ defmodule MvWeb.RoleLive.Form do
alias Mv.Authorization.PermissionSets
import MvWeb.RoleLive.Helpers
import MvWeb.RoleLive.Helpers, only: [format_error: 1]
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}

View file

@ -6,6 +6,7 @@ defmodule MvWeb.RoleLive.Helpers do
@doc """
Formats an error for display to the user.
Extracts error messages from Ash.Error.Invalid and joins them.
"""
@spec format_error(Ash.Error.Invalid.t() | String.t() | any()) :: String.t()
def format_error(%Ash.Error.Invalid{} = error) do
@ -24,4 +25,20 @@ defmodule MvWeb.RoleLive.Helpers do
def permission_set_badge_class("normal_user"), do: "badge badge-success badge-sm"
def permission_set_badge_class("admin"), do: "badge badge-error badge-sm"
def permission_set_badge_class(_), do: "badge badge-ghost badge-sm"
@doc """
Builds Ash options with actor and domain, ensuring actor is never nil in real paths.
"""
@spec opts_with_actor(keyword(), map() | nil, atom()) :: keyword()
def opts_with_actor(base_opts \\ [], actor, domain) do
opts = Keyword.put(base_opts, :domain, domain)
if actor do
Keyword.put(opts, :actor, actor)
else
require Logger
Logger.warning("opts_with_actor called with nil actor - this may bypass policies")
opts
end
end
end

View file

@ -21,7 +21,8 @@ defmodule MvWeb.RoleLive.Index do
require Ash.Query
import MvWeb.RoleLive.Helpers
import MvWeb.RoleLive.Helpers,
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
@ -126,6 +127,7 @@ defmodule MvWeb.RoleLive.Index do
end
# Loads all user counts for roles in a single query to avoid N+1 queries
# TODO: Optimize to use DB-side aggregation instead of loading all users
@spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{
Ecto.UUID.t() => non_neg_integer()
}
@ -133,8 +135,7 @@ defmodule MvWeb.RoleLive.Index do
role_ids = Enum.map(roles, & &1.id)
# Load all users with role_id in a single query
opts = [domain: Mv.Accounts]
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
opts = opts_with_actor([], actor, Mv.Accounts)
users =
case Ash.read(
@ -164,8 +165,7 @@ defmodule MvWeb.RoleLive.Index do
# Recalculates user count for a specific role (used before deletion)
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
defp recalculate_user_count(role, actor) do
opts = [domain: Mv.Accounts]
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
opts = opts_with_actor([], actor, Mv.Accounts)
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
{:ok, count} -> count

View file

@ -16,7 +16,8 @@ defmodule MvWeb.RoleLive.Show do
require Ash.Query
import MvWeb.RoleLive.Helpers
import MvWeb.RoleLive.Helpers,
only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
@ -143,8 +144,7 @@ defmodule MvWeb.RoleLive.Show do
# Recalculates user count for a specific role (used before deletion)
@spec recalculate_user_count(Mv.Authorization.Role.t(), map() | nil) :: non_neg_integer()
defp recalculate_user_count(role, actor) do
opts = [domain: Mv.Accounts]
opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
opts = opts_with_actor([], actor, Mv.Accounts)
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
{:ok, count} -> count