diff --git a/lib/mv_web/authorization.ex b/lib/mv_web/authorization.ex
index 95a8524..18ecd70 100644
--- a/lib/mv_web/authorization.ex
+++ b/lib/mv_web/authorization.ex
@@ -106,18 +106,14 @@ defmodule MvWeb.Authorization do
iex> can_access_page?(mitglied, "/members")
false
"""
- @spec can_access_page?(map() | nil, String.t() | Phoenix.VerifiedRoutes.unverified_path()) ::
- boolean()
+ @spec can_access_page?(map() | nil, String.t()) :: boolean()
def can_access_page?(nil, _page_path), do: false
def can_access_page?(user, page_path) do
- # Convert verified route to string if needed
- page_path_str = if is_binary(page_path), do: page_path, else: to_string(page_path)
-
with %{role: %{permission_set_name: ps_name}} when not is_nil(ps_name) <- user,
{:ok, ps_atom} <- PermissionSets.permission_set_name_to_atom(ps_name),
permissions <- PermissionSets.get_permissions(ps_atom) do
- page_matches?(permissions.pages, page_path_str)
+ page_matches?(permissions.pages, page_path)
else
_ -> false
end
diff --git a/lib/mv_web/components/layouts/navbar.ex b/lib/mv_web/components/layouts/navbar.ex
index e3e9319..692f949 100644
--- a/lib/mv_web/components/layouts/navbar.ex
+++ b/lib/mv_web/components/layouts/navbar.ex
@@ -34,9 +34,9 @@ defmodule MvWeb.Layouts.Navbar do
<.link navigate="/settings">{gettext("Global Settings")}
- <%= if can_access_page?(@current_user, ~p"/admin/roles") do %>
+ <%= if can_access_page?(@current_user, "/admin/roles") do %>
- <.link navigate={~p"/admin/roles"}>{gettext("Roles")}
+ <.link navigate="/admin/roles">{gettext("Roles")}
<% end %>
diff --git a/lib/mv_web/live/role_live/form.ex b/lib/mv_web/live/role_live/form.ex
index 7b74c7e..6388ec1 100644
--- a/lib/mv_web/live/role_live/form.ex
+++ b/lib/mv_web/live/role_live/form.ex
@@ -15,7 +15,7 @@ defmodule MvWeb.RoleLive.Form do
alias Mv.Authorization.PermissionSets
- import MvWeb.RoleLive.Helpers, only: [format_error: 1]
+ import MvWeb.RoleLive.Helpers
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
diff --git a/lib/mv_web/live/role_live/helpers.ex b/lib/mv_web/live/role_live/helpers.ex
index 8fbc544..9d4e77d 100644
--- a/lib/mv_web/live/role_live/helpers.ex
+++ b/lib/mv_web/live/role_live/helpers.ex
@@ -6,7 +6,6 @@ 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
@@ -25,20 +24,4 @@ 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
diff --git a/lib/mv_web/live/role_live/index.ex b/lib/mv_web/live/role_live/index.ex
index 9d75da6..d34cb22 100644
--- a/lib/mv_web/live/role_live/index.ex
+++ b/lib/mv_web/live/role_live/index.ex
@@ -21,8 +21,7 @@ defmodule MvWeb.RoleLive.Index do
require Ash.Query
- import MvWeb.RoleLive.Helpers,
- only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
+ import MvWeb.RoleLive.Helpers
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
@@ -126,28 +125,33 @@ defmodule MvWeb.RoleLive.Index do
end
end
- # Loads all user counts for roles using DB-side aggregation for better performance
+ # Loads all user counts for roles in a single query to avoid N+1 queries
@spec load_user_counts([Mv.Authorization.Role.t()], map() | nil) :: %{
Ecto.UUID.t() => non_neg_integer()
}
- defp load_user_counts(roles, _actor) do
+ defp load_user_counts(roles, actor) do
role_ids = Enum.map(roles, & &1.id)
- # Use Ecto directly for efficient GROUP BY COUNT query
- # This is much more performant than loading all users and counting in Elixir
- # Note: We bypass Ash here for performance, but this is a simple read-only query
- import Ecto.Query
+ # 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
- query =
- from u in Accounts.User,
- where: u.role_id in ^role_ids,
- group_by: u.role_id,
- select: {u.role_id, count(u.id)}
+ users =
+ case Ash.read(
+ Accounts.User
+ |> Ash.Query.filter(role_id in ^role_ids)
+ |> Ash.Query.select([:role_id]),
+ opts
+ ) do
+ {:ok, users_list} -> users_list
+ {:error, _} -> []
+ end
- results = Mv.Repo.all(query)
-
- results
- |> Enum.into(%{}, fn {role_id, count} -> {role_id, count} end)
+ # Group by role_id and count
+ users
+ |> Enum.group_by(& &1.role_id)
+ |> Enum.map(fn {role_id, users_list} -> {role_id, length(users_list)} end)
+ |> Map.new()
end
# Gets user count from preloaded assigns map
@@ -160,7 +164,8 @@ 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 = opts_with_actor([], actor, Mv.Accounts)
+ opts = [domain: Mv.Accounts]
+ opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
{:ok, count} -> count
diff --git a/lib/mv_web/live/role_live/show.ex b/lib/mv_web/live/role_live/show.ex
index 7184b68..3f15155 100644
--- a/lib/mv_web/live/role_live/show.ex
+++ b/lib/mv_web/live/role_live/show.ex
@@ -16,8 +16,7 @@ defmodule MvWeb.RoleLive.Show do
require Ash.Query
- import MvWeb.RoleLive.Helpers,
- only: [format_error: 1, permission_set_badge_class: 1, opts_with_actor: 3]
+ import MvWeb.RoleLive.Helpers
on_mount {MvWeb.LiveHelpers, :ensure_user_role_loaded}
@@ -144,7 +143,8 @@ 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 = opts_with_actor([], actor, Mv.Accounts)
+ opts = [domain: Mv.Accounts]
+ opts = if actor, do: Keyword.put(opts, :actor, actor), else: opts
case Ash.count(Accounts.User |> Ash.Query.filter(role_id == ^role.id), opts) do
{:ok, count} -> count
diff --git a/lib/mv_web/live_helpers.ex b/lib/mv_web/live_helpers.ex
index f217ee2..1835cba 100644
--- a/lib/mv_web/live_helpers.ex
+++ b/lib/mv_web/live_helpers.ex
@@ -49,14 +49,8 @@ defmodule MvWeb.LiveHelpers do
opts = [domain: Mv.Accounts, actor: user]
case Ash.load(user, :role, opts) do
- {:ok, loaded_user} ->
- loaded_user
-
- {:error, error} ->
- # Log warning if role loading fails - this can cause authorization issues
- require Logger
- Logger.warning("Failed to load role for user #{user.id}: #{inspect(error)}")
- user
+ {:ok, loaded_user} -> loaded_user
+ {:error, _} -> user
end
end
end
diff --git a/lib/mv_web/router.ex b/lib/mv_web/router.ex
index 682b672..e73c926 100644
--- a/lib/mv_web/router.ex
+++ b/lib/mv_web/router.ex
@@ -46,10 +46,7 @@ defmodule MvWeb.Router do
AshAuthentication-specific: We define that all routes can only be accessed when the user is signed in.
"""
ash_authentication_live_session :authentication_required,
- on_mount: [
- {MvWeb.LiveUserAuth, :live_user_required},
- {MvWeb.LiveHelpers, :ensure_user_role_loaded}
- ] do
+ on_mount: {MvWeb.LiveUserAuth, :live_user_required} do
live "/", MemberLive.Index, :index
live "/members", MemberLive.Index, :index