Merge branch 'main' into feature/337_polish_import

test/mv_web/authorization_test.exs

@@ -50,14 +50,14 @@ defmodule MvWeb.AuthorizationTest do
       assert Authorization.can?(admin, :destroy, Mv.Authorization.Role) == true
     end
 
-    test "non-admin cannot manage roles" do
+    test "non-admin can read roles but cannot create/update/destroy" do
       normal_user = %{
         id: "normal-123",
         role: %{permission_set_name: "normal_user"}
       }
 
+      assert Authorization.can?(normal_user, :read, Mv.Authorization.Role) == true
       assert Authorization.can?(normal_user, :create, Mv.Authorization.Role) == false
-      assert Authorization.can?(normal_user, :read, Mv.Authorization.Role) == false
       assert Authorization.can?(normal_user, :update, Mv.Authorization.Role) == false
       assert Authorization.can?(normal_user, :destroy, Mv.Authorization.Role) == false
     end

test/mv_web/components/layouts/sidebar_test.exs

@@ -22,9 +22,14 @@ defmodule MvWeb.Layouts.SidebarTest do
   # =============================================================================
 
   # Returns assigns for an authenticated user with all required attributes.
+  # User has admin role so can_access_page? returns true for all sidebar links.
   defp authenticated_assigns(mobile \\ false) do
     %{
-      current_user: %{id: "user-123", email: "test@example.com"},
+      current_user: %{
+        id: "user-123",
+        email: "test@example.com",
+        role: %{permission_set_name: "admin"}
+      },
       club_name: "Test Club",
       mobile: mobile
     }
@@ -144,7 +149,9 @@ defmodule MvWeb.Layouts.SidebarTest do
       assert menu_item_count > 0, "Should have at least one top-level menu item"
 
       # Check that nested menu groups exist
-      assert html =~ ~s(<li role="none" class="expanded-menu-group">)
+      assert html =~
+               ~s(<li role="none" class="expanded-menu-group" data-testid="sidebar-administration">)
+
       assert html =~ ~s(role="group")
       assert has_class?(html, "expanded-menu-group")
 
@@ -193,7 +200,9 @@ defmodule MvWeb.Layouts.SidebarTest do
       html = render_sidebar(authenticated_assigns())
 
       # Check for nested menu structure
-      assert html =~ ~s(<li role="none" class="expanded-menu-group">)
+      assert html =~
+               ~s(<li role="none" class="expanded-menu-group" data-testid="sidebar-administration">)
+
       assert html =~ ~s(role="group")
       assert html =~ ~s(aria-label="Administration")
       assert has_class?(html, "expanded-menu-group")
@@ -521,7 +530,9 @@ defmodule MvWeb.Layouts.SidebarTest do
       assert html =~ ~s(role="menuitem")
 
       # Check that nested menus exist
-      assert html =~ ~s(<li role="none" class="expanded-menu-group">)
+      assert html =~
+               ~s(<li role="none" class="expanded-menu-group" data-testid="sidebar-administration">)
+
       assert html =~ ~s(role="group")
 
       # Footer section
@@ -629,7 +640,9 @@ defmodule MvWeb.Layouts.SidebarTest do
       html = render_sidebar(authenticated_assigns())
 
       # expanded-menu-group structure present
-      assert html =~ ~s(<li role="none" class="expanded-menu-group">)
+      assert html =~
+               ~s(<li role="none" class="expanded-menu-group" data-testid="sidebar-administration">)
+
       assert html =~ ~s(role="group")
       assert html =~ ~s(aria-label="Administration")
       assert has_class?(html, "expanded-menu-group")
@@ -843,7 +856,9 @@ defmodule MvWeb.Layouts.SidebarTest do
 
       # Expanded menu group should have correct structure
       # (CSS handles hover effects, but we verify structure)
-      assert html =~ ~s(<li role="none" class="expanded-menu-group">)
+      assert html =~
+               ~s(<li role="none" class="expanded-menu-group" data-testid="sidebar-administration">)
+
       assert html =~ ~s(role="group")
     end
 

test/mv_web/components/sidebar_authorization_test.exs

@@ -0,0 +1,120 @@
+defmodule MvWeb.SidebarAuthorizationTest do
+  @moduledoc """
+  Tests for sidebar menu visibility based on user permissions (can_access_page?).
+  """
+  use MvWeb.ConnCase, async: false
+
+  import Phoenix.LiveViewTest
+  import MvWeb.Layouts.Sidebar
+
+  alias Mv.Fixtures
+
+  defp render_sidebar(assigns) do
+    render_component(&sidebar/1, assigns)
+  end
+
+  defp sidebar_assigns(current_user, opts \\ []) do
+    mobile = Keyword.get(opts, :mobile, false)
+    club_name = Keyword.get(opts, :club_name, "Test Club")
+
+    %{
+      current_user: current_user,
+      club_name: club_name,
+      mobile: mobile
+    }
+  end
+
+  describe "sidebar menu with admin user" do
+    test "shows Members, Fee Types and Administration with all subitems" do
+      user = Fixtures.user_with_role_fixture("admin")
+      html = render_sidebar(sidebar_assigns(user))
+
+      assert html =~ ~s(href="/members")
+      assert html =~ ~s(href="/membership_fee_types")
+      assert html =~ ~s(data-testid="sidebar-administration")
+      assert html =~ ~s(href="/users")
+      assert html =~ ~s(href="/groups")
+      assert html =~ ~s(href="/admin/roles")
+      assert html =~ ~s(href="/membership_fee_settings")
+      assert html =~ ~s(href="/settings")
+    end
+  end
+
+  describe "sidebar menu with read_only user (Vorstand/Buchhaltung)" do
+    test "shows Members and Groups (from Administration)" do
+      user = Fixtures.user_with_role_fixture("read_only")
+      html = render_sidebar(sidebar_assigns(user))
+
+      assert html =~ ~s(href="/members")
+      assert html =~ ~s(href="/groups")
+    end
+
+    test "does not show Fee Types, Users, Roles or Settings" do
+      user = Fixtures.user_with_role_fixture("read_only")
+      html = render_sidebar(sidebar_assigns(user))
+
+      refute html =~ ~s(href="/membership_fee_types")
+      refute html =~ ~s(href="/users")
+      refute html =~ ~s(href="/admin/roles")
+      refute html =~ ~s(href="/settings")
+    end
+  end
+
+  describe "sidebar menu with normal_user (Kassenwart)" do
+    test "shows Members and Groups" do
+      user = Fixtures.user_with_role_fixture("normal_user")
+      html = render_sidebar(sidebar_assigns(user))
+
+      assert html =~ ~s(href="/members")
+      assert html =~ ~s(href="/groups")
+    end
+
+    test "does not show Fee Types, Users, Roles or Settings" do
+      user = Fixtures.user_with_role_fixture("normal_user")
+      html = render_sidebar(sidebar_assigns(user))
+
+      refute html =~ ~s(href="/membership_fee_types")
+      refute html =~ ~s(href="/users")
+      refute html =~ ~s(href="/admin/roles")
+      refute html =~ ~s(href="/settings")
+    end
+  end
+
+  describe "sidebar menu with own_data user (Mitglied)" do
+    test "does not show Members link (no /members page access)" do
+      user = Fixtures.user_with_role_fixture("own_data")
+      html = render_sidebar(sidebar_assigns(user))
+
+      refute html =~ ~s(href="/members")
+    end
+
+    test "does not show Fee Types or Administration" do
+      user = Fixtures.user_with_role_fixture("own_data")
+      html = render_sidebar(sidebar_assigns(user))
+
+      refute html =~ ~s(href="/membership_fee_types")
+      refute html =~ ~s(href="/users")
+      refute html =~ ~s(data-testid="sidebar-administration")
+    end
+  end
+
+  describe "sidebar with nil current_user" do
+    test "does not render menu items (only header and footer when present)" do
+      html = render_sidebar(sidebar_assigns(nil))
+
+      refute html =~ ~s(role="menubar")
+      refute html =~ ~s(href="/members")
+    end
+  end
+
+  describe "sidebar with user without role" do
+    test "does not show any navigation links" do
+      user = %{id: "user-no-role", email: "noreply@test.com", role: nil}
+      html = render_sidebar(sidebar_assigns(user))
+
+      refute html =~ ~s(href="/members")
+      refute html =~ ~s(href="/membership_fee_types")
+      refute html =~ ~s(href="/users")
+    end
+  end
+end

test/mv_web/helpers/membership_fee_helpers_test.exs

@@ -134,8 +134,8 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do
       # Load cycles with membership_fee_type relationship
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: actor)
+        |> Ash.load!(:membership_fee_type, actor: actor)
 
       # Use a fixed date in 2024 to ensure 2023 is last completed
       today = ~D[2024-06-15]
@@ -180,8 +180,8 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do
       # Load cycles and fee type (will be empty)
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: actor)
+        |> Ash.load!(:membership_fee_type, actor: actor)
 
       last_cycle = MembershipFeeHelpers.get_last_completed_cycle(member, Date.utc_today())
       assert last_cycle == nil
@@ -245,8 +245,8 @@ defmodule MvWeb.Helpers.MembershipFeeHelpersTest do
       # Load cycles with membership_fee_type relationship
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: actor)
+        |> Ash.load!(:membership_fee_type, actor: actor)
 
       result = MembershipFeeHelpers.get_current_cycle(member, today)
 

test/mv_web/live/member_live_authorization_test.exs

@@ -0,0 +1,102 @@
+defmodule MvWeb.MemberLiveAuthorizationTest do
+  @moduledoc """
+  Tests for UI authorization on Member LiveViews (Index and Show).
+  """
+  use MvWeb.ConnCase, async: false
+
+  import Phoenix.LiveViewTest
+
+  alias Mv.Fixtures
+
+  describe "Member Index - Vorstand (read_only)" do
+    @tag role: :read_only
+    test "sees member list but not New Member button", %{conn: conn} do
+      _member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members")
+
+      refute has_element?(view, "[data-testid=member-new]")
+    end
+
+    @tag role: :read_only
+    test "does not see Edit or Delete buttons in table", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members")
+
+      refute has_element?(view, "#row-#{member.id} [data-testid=member-edit]")
+      refute has_element?(view, "#row-#{member.id} [data-testid=member-delete]")
+    end
+  end
+
+  describe "Member Index - Kassenwart (normal_user)" do
+    @tag role: :normal_user
+    test "sees New Member and Edit buttons", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members")
+
+      assert has_element?(view, "[data-testid=member-new]")
+      assert has_element?(view, "#row-#{member.id} [data-testid=member-edit]")
+    end
+
+    @tag role: :normal_user
+    test "does not see Delete button", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members")
+
+      refute has_element?(view, "#row-#{member.id} [data-testid=member-delete]")
+    end
+  end
+
+  describe "Member Index - Admin" do
+    @tag role: :admin
+    test "sees New Member, Edit and Delete buttons", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members")
+
+      assert has_element?(view, "[data-testid=member-new]")
+      assert has_element?(view, "#row-#{member.id} [data-testid=member-edit]")
+      assert has_element?(view, "#row-#{member.id} [data-testid=member-delete]")
+    end
+  end
+
+  describe "Member Index - Mitglied (own_data)" do
+    @tag role: :member
+    test "is redirected when accessing /members", %{conn: conn, current_user: user} do
+      assert {:error, {:redirect, %{to: to}}} = live(conn, "/members")
+      assert to == "/users/#{user.id}"
+    end
+  end
+
+  describe "Member Show - Edit button visibility" do
+    @tag role: :admin
+    test "admin sees Edit button", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      assert has_element?(view, "[data-testid=member-edit]")
+    end
+
+    @tag role: :read_only
+    test "read_only does not see Edit button", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      refute has_element?(view, "[data-testid=member-edit]")
+    end
+
+    @tag role: :normal_user
+    test "normal_user sees Edit button", %{conn: conn} do
+      member = Fixtures.member_fixture()
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      assert has_element?(view, "[data-testid=member-edit]")
+    end
+  end
+end

test/mv_web/live/membership_fee_type_live/form_test.exs

@@ -50,7 +50,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do
   end
 
   describe "create form" do
-    test "creates new membership fee type", %{conn: conn} do
+    test "creates new membership fee type", %{conn: conn, user: user} do
       {:ok, view, _html} = live(conn, "/membership_fee_types/new")
 
       form_data = %{
@@ -67,12 +67,13 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do
 
       assert to == "/membership_fee_types"
 
-      # Verify type was created
+      # Verify type was created (use actor so read is authorized)
       type =
         MembershipFeeType
         |> Ash.Query.filter(name == "New Type")
-        |> Ash.read_one!()
+        |> Ash.read_one!(domain: Mv.MembershipFees, actor: user)
 
+      assert type != nil, "Expected membership fee type to be created"
       assert type.amount == Decimal.new("75.00")
       assert type.interval == :yearly
     end
@@ -140,7 +141,7 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do
       assert html =~ "3" || html =~ "members" || html =~ "Mitglieder"
     end
 
-    test "amount change can be confirmed", %{conn: conn} do
+    test "amount change can be confirmed", %{conn: conn, user: user} do
       fee_type = create_fee_type(%{amount: Decimal.new("50.00")})
 
       {:ok, view, _html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit")
@@ -159,12 +160,17 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do
       |> form("#membership-fee-type-form", %{"membership_fee_type[amount]" => "75.00"})
       |> render_submit()
 
-      # Amount should be updated
-      updated_type = Ash.read_one!(MembershipFeeType |> Ash.Query.filter(id == ^fee_type.id))
+      # Amount should be updated (use actor so read is authorized)
+      updated_type =
+        MembershipFeeType
+        |> Ash.Query.filter(id == ^fee_type.id)
+        |> Ash.read_one!(domain: Mv.MembershipFees, actor: user)
+
+      assert updated_type != nil
       assert updated_type.amount == Decimal.new("75.00")
     end
 
-    test "amount change can be cancelled", %{conn: conn} do
+    test "amount change can be cancelled", %{conn: conn, user: user} do
       fee_type = create_fee_type(%{amount: Decimal.new("50.00")})
 
       {:ok, view, _html} = live(conn, "/membership_fee_types/#{fee_type.id}/edit")
@@ -178,8 +184,13 @@ defmodule MvWeb.MembershipFeeTypeLive.FormTest do
       |> element("button[phx-click='cancel_amount_change']")
       |> render_click()
 
-      # Amount should remain unchanged
-      updated_type = Ash.read_one!(MembershipFeeType |> Ash.Query.filter(id == ^fee_type.id))
+      # Amount should remain unchanged (use actor so read is authorized)
+      updated_type =
+        MembershipFeeType
+        |> Ash.Query.filter(id == ^fee_type.id)
+        |> Ash.read_one!(domain: Mv.MembershipFees, actor: user)
+
+      assert updated_type != nil
       assert updated_type.amount == Decimal.new("50.00")
     end
 

test/mv_web/live/profile_navigation_test.exs

@@ -61,6 +61,7 @@ defmodule MvWeb.ProfileNavigationTest do
     end
 
     @tag :skip
+    # credo:disable-for-next-line Credo.Check.Design.TagTODO
     # TODO: Implement user initials in navbar avatar - see issue #170
     test "shows user initials in avatar", %{conn: conn} do
       # Setup: Create and login a user

test/mv_web/live/role_live/show_test.exs

@@ -18,7 +18,7 @@ defmodule MvWeb.RoleLive.ShowTest do
   alias Mv.Authorization
   alias Mv.Authorization.Role
 
-  # Helper to create a role
+  # Helper to create a role (authorize?: false for test data setup)
   defp create_role(attrs \\ %{}) do
     default_attrs = %{
       name: "Test Role #{System.unique_integer([:positive])}",
@@ -28,7 +28,7 @@ defmodule MvWeb.RoleLive.ShowTest do
 
     attrs = Map.merge(default_attrs, attrs)
 
-    case Authorization.create_role(attrs) do
+    case Authorization.create_role(attrs, authorize?: false) do
       {:ok, role} -> role
       {:error, error} -> raise "Failed to create role: #{inspect(error)}"
     end
@@ -38,7 +38,7 @@ defmodule MvWeb.RoleLive.ShowTest do
   defp create_admin_user(conn, actor) do
     # Create admin role
     admin_role =
-      case Authorization.list_roles() do
+      case Authorization.list_roles(authorize?: false) do
         {:ok, roles} ->
           case Enum.find(roles, &(&1.name == "Admin")) do
             nil ->

test/mv_web/live/role_live_test.exs

@@ -9,7 +9,7 @@ defmodule MvWeb.RoleLiveTest do
   alias Mv.Authorization
   alias Mv.Authorization.Role
 
-  # Helper to create a role
+  # Helper to create a role (authorize?: false for test data setup)
   defp create_role(attrs \\ %{}) do
     default_attrs = %{
       name: "Test Role #{System.unique_integer([:positive])}",
@@ -19,7 +19,7 @@ defmodule MvWeb.RoleLiveTest do
 
     attrs = Map.merge(default_attrs, attrs)
 
-    case Authorization.create_role(attrs) do
+    case Authorization.create_role(attrs, authorize?: false) do
       {:ok, role} -> role
       {:error, error} -> raise "Failed to create role: #{inspect(error)}"
     end
@@ -29,7 +29,7 @@ defmodule MvWeb.RoleLiveTest do
   defp create_admin_user(conn, actor) do
     # Create admin role
     admin_role =
-      case Authorization.list_roles() do
+      case Authorization.list_roles(authorize?: false) do
         {:ok, roles} ->
           case Enum.find(roles, &(&1.name == "Admin")) do
             nil ->
@@ -332,7 +332,7 @@ defmodule MvWeb.RoleLiveTest do
       assert match?({:error, {:redirect, %{to: "/admin/roles"}}}, result)
     end
 
-    test "updates role name", %{conn: conn, role: role} do
+    test "updates role name", %{conn: conn, role: role, actor: actor} do
       {:ok, view, _html} = live(conn, "/admin/roles/#{role.id}/edit?return_to=show")
 
       attrs = %{
@@ -348,7 +348,7 @@ defmodule MvWeb.RoleLiveTest do
       assert_redirect(view, "/admin/roles/#{role.id}")
 
       # Verify update
-      {:ok, updated_role} = Authorization.get_role(role.id)
+      {:ok, updated_role} = Authorization.get_role(role.id, actor: actor)
       assert updated_role.name == "Updated Role Name"
     end
 
@@ -377,7 +377,7 @@ defmodule MvWeb.RoleLiveTest do
       assert_redirect(view, "/admin/roles/#{system_role.id}")
 
       # Verify update
-      {:ok, updated_role} = Authorization.get_role(system_role.id)
+      {:ok, updated_role} = Authorization.get_role(system_role.id, actor: actor)
       assert updated_role.permission_set_name == "read_only"
     end
   end
@@ -390,7 +390,7 @@ defmodule MvWeb.RoleLiveTest do
     end
 
     @tag :slow
-    test "deletes non-system role", %{conn: conn} do
+    test "deletes non-system role", %{conn: conn, actor: actor} do
       role = create_role()
 
       {:ok, view, html} = live(conn, "/admin/roles")
@@ -404,7 +404,7 @@ defmodule MvWeb.RoleLiveTest do
 
       # Verify deletion by checking database
       assert {:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{}]}} =
-               Authorization.get_role(role.id)
+               Authorization.get_role(role.id, actor: actor)
     end
 
     test "fails to delete system role with error message", %{conn: conn, actor: actor} do
@@ -430,7 +430,7 @@ defmodule MvWeb.RoleLiveTest do
       assert render(view) =~ "System roles cannot be deleted"
 
       # Role should still exist
-      {:ok, _role} = Authorization.get_role(system_role.id)
+      {:ok, _role} = Authorization.get_role(system_role.id, actor: actor)
     end
   end
 

test/mv_web/live/user_live_authorization_test.exs

@@ -0,0 +1,81 @@
+defmodule MvWeb.UserLiveAuthorizationTest do
+  @moduledoc """
+  Tests for UI authorization on User LiveViews (Index and Show).
+  """
+  use MvWeb.ConnCase, async: false
+
+  import Phoenix.LiveViewTest
+
+  alias Mv.Fixtures
+
+  describe "User Index - Admin" do
+    @tag role: :admin
+    test "sees New User, Edit and Delete buttons", %{conn: conn} do
+      user = Fixtures.user_with_role_fixture("admin")
+
+      {:ok, view, _html} = live(conn, "/users")
+
+      assert has_element?(view, "[data-testid=user-new]")
+      assert has_element?(view, "#row-#{user.id} [data-testid=user-edit]")
+      assert has_element?(view, "#row-#{user.id} [data-testid=user-delete]")
+    end
+  end
+
+  describe "User Index - Non-Admin is redirected" do
+    @tag role: :read_only
+    test "read_only is redirected when accessing /users", %{conn: conn, current_user: user} do
+      assert {:error, {:redirect, %{to: to}}} = live(conn, "/users")
+      assert to == "/users/#{user.id}"
+    end
+
+    @tag role: :member
+    test "member is redirected when accessing /users", %{conn: conn, current_user: user} do
+      assert {:error, {:redirect, %{to: to}}} = live(conn, "/users")
+      assert to == "/users/#{user.id}"
+    end
+
+    @tag role: :normal_user
+    test "normal_user is redirected when accessing /users", %{conn: conn, current_user: user} do
+      assert {:error, {:redirect, %{to: to}}} = live(conn, "/users")
+      assert to == "/users/#{user.id}"
+    end
+  end
+
+  describe "User Show - own profile" do
+    @tag role: :member
+    test "member sees Edit button on own profile", %{conn: conn, current_user: user} do
+      {:ok, view, _html} = live(conn, "/users/#{user.id}")
+
+      assert has_element?(view, "[data-testid=user-edit]")
+    end
+
+    @tag role: :read_only
+    test "read_only sees Edit button on own profile", %{conn: conn, current_user: user} do
+      {:ok, view, _html} = live(conn, "/users/#{user.id}")
+
+      assert has_element?(view, "[data-testid=user-edit]")
+    end
+
+    @tag role: :admin
+    test "admin sees Edit button on user show", %{conn: conn} do
+      user = Fixtures.user_with_role_fixture("read_only")
+
+      {:ok, view, _html} = live(conn, "/users/#{user.id}")
+
+      assert has_element?(view, "[data-testid=user-edit]")
+    end
+  end
+
+  describe "User Show - other user (non-admin redirected)" do
+    @tag role: :member
+    test "member is redirected when accessing other user's profile", %{
+      conn: conn,
+      current_user: current_user
+    } do
+      other_user = Fixtures.user_with_role_fixture("admin")
+
+      assert {:error, {:redirect, %{to: to}}} = live(conn, "/users/#{other_user.id}")
+      assert to == "/users/#{current_user.id}"
+    end
+  end
+end

test/mv_web/member_live/index/membership_fee_status_test.exs

@@ -127,10 +127,12 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
 
       # Load cycles with membership_fee_type relationship
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+        |> Ash.load!(:membership_fee_type, actor: system_actor)
 
       # Use fixed date in 2024 to ensure 2023 is last completed
       # We need to manually set the date for the helper function
@@ -183,8 +185,8 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       # Load cycles with membership_fee_type relationship
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+        |> Ash.load!(:membership_fee_type, actor: system_actor)
 
       status = MembershipFeeStatus.get_cycle_status_for_member(member, true)
 
@@ -222,8 +224,8 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       # Load cycles and fee type first (will be empty)
       member =
         member
-        |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-        |> Ash.load!(:membership_fee_type)
+        |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+        |> Ash.load!(:membership_fee_type, actor: system_actor)
 
       status = MembershipFeeStatus.get_cycle_status_for_member(member, false)
 
@@ -273,12 +275,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       member2 = create_member(%{membership_fee_type_id: fee_type.id})
       create_cycle(member2, fee_type, %{cycle_start: last_year_start, status: :unpaid})
 
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       members =
         [member1, member2]
         |> Enum.map(fn m ->
           m
-          |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-          |> Ash.load!(:membership_fee_type)
+          |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+          |> Ash.load!(:membership_fee_type, actor: system_actor)
         end)
 
       filtered = MembershipFeeStatus.filter_members_by_cycle_status(members, :paid, false)
@@ -300,12 +304,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       member2 = create_member(%{membership_fee_type_id: fee_type.id})
       create_cycle(member2, fee_type, %{cycle_start: last_year_start, status: :unpaid})
 
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       members =
         [member1, member2]
         |> Enum.map(fn m ->
           m
-          |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-          |> Ash.load!(:membership_fee_type)
+          |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+          |> Ash.load!(:membership_fee_type, actor: system_actor)
         end)
 
       filtered = MembershipFeeStatus.filter_members_by_cycle_status(members, :unpaid, false)
@@ -327,12 +333,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       member2 = create_member(%{membership_fee_type_id: fee_type.id})
       create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :unpaid})
 
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       members =
         [member1, member2]
         |> Enum.map(fn m ->
           m
-          |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-          |> Ash.load!(:membership_fee_type)
+          |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+          |> Ash.load!(:membership_fee_type, actor: system_actor)
         end)
 
       filtered = MembershipFeeStatus.filter_members_by_cycle_status(members, :paid, true)
@@ -354,12 +362,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       member2 = create_member(%{membership_fee_type_id: fee_type.id})
       create_cycle(member2, fee_type, %{cycle_start: current_year_start, status: :unpaid})
 
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       members =
         [member1, member2]
         |> Enum.map(fn m ->
           m
-          |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-          |> Ash.load!(:membership_fee_type)
+          |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+          |> Ash.load!(:membership_fee_type, actor: system_actor)
         end)
 
       filtered = MembershipFeeStatus.filter_members_by_cycle_status(members, :unpaid, true)
@@ -373,12 +383,14 @@ defmodule MvWeb.MemberLive.Index.MembershipFeeStatusTest do
       member1 = create_member(%{membership_fee_type_id: fee_type.id})
       member2 = create_member(%{membership_fee_type_id: fee_type.id})
 
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
       members =
         [member1, member2]
         |> Enum.map(fn m ->
           m
-          |> Ash.load!(membership_fee_cycles: [:membership_fee_type])
-          |> Ash.load!(:membership_fee_type)
+          |> Ash.load!([membership_fee_cycles: [:membership_fee_type]], actor: system_actor)
+          |> Ash.load!(:membership_fee_type, actor: system_actor)
         end)
 
       # filter_unpaid_members should still work for backwards compatibility

test/mv_web/member_live/show_membership_fees_test.exs

@@ -28,21 +28,6 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
     |> Ash.create!(actor: system_actor)
   end
 
-  # Helper to create a member
-  defp create_member(attrs) do
-    system_actor = Mv.Helpers.SystemActor.get_system_actor()
-
-    default_attrs = %{
-      first_name: "Test",
-      last_name: "Member",
-      email: "test.member.#{System.unique_integer([:positive])}@example.com"
-    }
-
-    attrs = Map.merge(default_attrs, attrs)
-    {:ok, member} = Mv.Membership.create_member(attrs, actor: system_actor)
-    member
-  end
-
   # Helper to create a cycle
   defp create_cycle(member, fee_type, attrs) do
     system_actor = Mv.Helpers.SystemActor.get_system_actor()
@@ -73,7 +58,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
   describe "cycles table display" do
     test "displays all cycles for member", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       _cycle1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid})
       _cycle2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
@@ -95,7 +80,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
 
     test "table columns show correct data", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly, amount: Decimal.new("60.00")})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       create_cycle(member, fee_type, %{
         cycle_start: ~D[2023-01-01],
@@ -124,7 +109,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
       yearly_type = create_fee_type(%{interval: :yearly, name: "Yearly Type"})
       _monthly_type = create_fee_type(%{interval: :monthly, name: "Monthly Type"})
 
-      member = create_member(%{membership_fee_type_id: yearly_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: yearly_type.id})
 
       {:ok, _view, html} = live(conn, "/members/#{member.id}")
 
@@ -132,20 +117,30 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
       assert html =~ "Yearly Type"
     end
 
-    test "shows no type message when no type assigned", %{conn: conn} do
-      member = create_member(%{})
+    test "shows no type message when no type assigned and Regenerate Cycles button is hidden", %{
+      conn: conn
+    } do
+      member = Mv.Fixtures.member_fixture(%{})
 
-      {:ok, _view, html} = live(conn, "/members/#{member.id}")
+      {:ok, view, html} = live(conn, "/members/#{member.id}")
 
       # Should show message about no type assigned
       assert html =~ "No membership fee type assigned" || html =~ "No type"
+
+      # Switch to membership fees tab: message and no Regenerate Cycles button
+      view
+      |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']")
+      |> render_click()
+
+      refute has_element?(view, "button[phx-click='regenerate_cycles']"),
+             "Regenerate Cycles should be hidden when no membership fee type is assigned"
     end
   end
 
   describe "status change actions" do
     test "mark as paid works", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
 
@@ -176,7 +171,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
 
     test "mark as suspended works", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
 
@@ -207,7 +202,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
 
     test "mark as unpaid works", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :paid})
 
@@ -240,7 +235,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
   describe "cycle regeneration" do
     test "manual regeneration button exists and can be clicked", %{conn: conn} do
       fee_type = create_fee_type(%{interval: :yearly})
-      member = create_member(%{membership_fee_type_id: fee_type.id})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
 
       {:ok, view, _html} = live(conn, "/members/#{member.id}")
 
@@ -266,7 +261,7 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
   describe "edge cases" do
     test "handles members without membership fee type gracefully", %{conn: conn} do
       # No fee type
-      member = create_member(%{})
+      member = Mv.Fixtures.member_fixture(%{})
 
       {:ok, _view, html} = live(conn, "/members/#{member.id}")
 
@@ -274,4 +269,120 @@ defmodule MvWeb.MemberLive.ShowMembershipFeesTest do
       assert html =~ member.first_name
     end
   end
+
+  describe "read_only user (Vorstand/Buchhaltung) - no cycle action buttons" do
+    @tag role: :read_only
+    test "read_only does not see Regenerate Cycles, Delete All Cycles, or Create Cycle buttons",
+         %{
+           conn: conn
+         } do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
+      _cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      view
+      |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']")
+      |> render_click()
+
+      refute has_element?(view, "button[phx-click='regenerate_cycles']")
+      refute has_element?(view, "button[phx-click='delete_all_cycles']")
+      refute has_element?(view, "button[phx-click='open_create_cycle_modal']")
+    end
+
+    @tag role: :read_only
+    test "read_only does not see Paid, Unpaid, Suspended, or Delete buttons in cycles table", %{
+      conn: conn
+    } do
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
+      cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      view
+      |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']")
+      |> render_click()
+
+      # Row action buttons must not be present for read_only
+      refute has_element?(view, "button[phx-click='mark_cycle_status']")
+      refute has_element?(view, "button[phx-click='delete_cycle']")
+      # Sanity: cycle row is present (read is allowed)
+      assert has_element?(view, "tr[id='cycle-#{cycle.id}']")
+    end
+  end
+
+  describe "read_only cannot delete all cycles (policy enforced via Ash.destroy)" do
+    @tag role: :read_only
+    test "Ash.destroy returns Forbidden for read_only so handler would reject", %{
+      current_user: read_only_user
+    } do
+      # The handler uses Ash.destroy per cycle, so if the handler were triggered
+      # (e.g. via dev tools), the server would enforce policy and show an error.
+      # This test verifies that Ash.destroy(cycle, actor: read_only_user) returns Forbidden.
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
+      cycle = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
+
+      assert {:error, %Ash.Error.Forbidden{}} =
+               Ash.destroy(cycle, domain: Mv.MembershipFees, actor: read_only_user)
+    end
+  end
+
+  describe "read_only cannot trigger regenerate_cycles (handler enforces can?)" do
+    @tag role: :read_only
+    test "read_only cannot create MembershipFeeCycle so regenerate_cycles handler would show flash error",
+         %{current_user: read_only_user} do
+      # The regenerate_cycles handler checks can?(actor, :create, MembershipFeeCycle) before
+      # calling the generator. If a read_only user triggered the event (e.g. via DevTools),
+      # the handler returns flash error and no new cycles are created.
+      # This test verifies the condition the handler uses.
+      refute MvWeb.Authorization.can?(read_only_user, :create, MembershipFeeCycle),
+             "read_only must not be allowed to create MembershipFeeCycle so handler rejects regenerate_cycles"
+    end
+  end
+
+  describe "confirm_delete_all_cycles handler (policy enforced)" do
+    @tag role: :admin
+    test "admin can delete all cycles via UI and cycles are removed", %{conn: conn} do
+      # Use English locale so confirmation "Yes" matches gettext("Yes")
+      conn = put_session(conn, :locale, "en")
+
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = Mv.Fixtures.member_fixture(%{membership_fee_type_id: fee_type.id})
+      _c1 = create_cycle(member, fee_type, %{cycle_start: ~D[2022-01-01], status: :paid})
+      _c2 = create_cycle(member, fee_type, %{cycle_start: ~D[2023-01-01], status: :unpaid})
+
+      {:ok, view, _html} = live(conn, "/members/#{member.id}")
+
+      view
+      |> element("button[phx-click='switch_tab'][phx-value-tab='membership_fees']")
+      |> render_click()
+
+      view
+      |> element("button[phx-click='delete_all_cycles']")
+      |> render_click()
+
+      view
+      |> element("input[phx-keyup='update_delete_all_confirmation']")
+      |> render_keyup(%{"value" => "Yes"})
+
+      view
+      |> element("button[phx-click='confirm_delete_all_cycles']")
+      |> render_click()
+
+      _html = render(view)
+
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
+      remaining =
+        Mv.MembershipFees.MembershipFeeCycle
+        |> Ash.Query.filter(member_id == ^member.id)
+        |> Ash.read!(actor: system_actor)
+
+      assert remaining == [],
+             "Expected all cycles to be deleted (handler enforces policy via Ash.destroy)"
+    end
+  end
 end

test/mv_web/plugs/check_page_permission_test.exs

@@ -742,6 +742,18 @@ defmodule MvWeb.Plugs.CheckPagePermissionTest do
       assert conn.status == 200
     end
 
+    @tag role: :normal_user
+    test "GET /groups/new returns 200", %{conn: conn} do
+      conn = get(conn, "/groups/new")
+      assert conn.status == 200
+    end
+
+    @tag role: :normal_user
+    test "GET /groups/:slug/edit returns 200", %{conn: conn, group_slug: slug} do
+      conn = get(conn, "/groups/#{slug}/edit")
+      assert conn.status == 200
+    end
+
     @tag role: :normal_user
     test "GET /members/:id/show/edit returns 200", %{conn: conn, member_id: id} do
       conn = get(conn, "/members/#{id}/show/edit")
@@ -830,22 +842,6 @@ defmodule MvWeb.Plugs.CheckPagePermissionTest do
       assert redirected_to(conn) == "/users/#{user.id}"
     end
 
-    @tag role: :normal_user
-    test "GET /groups/new redirects to user profile", %{conn: conn, current_user: user} do
-      conn = get(conn, "/groups/new")
-      assert redirected_to(conn) == "/users/#{user.id}"
-    end
-
-    @tag role: :normal_user
-    test "GET /groups/:slug/edit redirects to user profile", %{
-      conn: conn,
-      current_user: user,
-      group_slug: slug
-    } do
-      conn = get(conn, "/groups/#{slug}/edit")
-      assert redirected_to(conn) == "/users/#{user.id}"
-    end
-
     @tag role: :normal_user
     test "GET /admin/roles redirects to user profile", %{conn: conn, current_user: user} do
       conn = get(conn, "/admin/roles")

test/mv_web/user_live/form_test.exs

@@ -213,6 +213,35 @@ defmodule MvWeb.UserLive.FormTest do
       assert not is_nil(updated_user.hashed_password)
       assert updated_user.hashed_password != ""
     end
+
+    test "admin can change user role and change persists", %{conn: conn} do
+      system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
+      role_a = Mv.Fixtures.role_fixture("normal_user")
+      role_b = Mv.Fixtures.role_fixture("read_only")
+
+      user = create_test_user(%{email: "rolechange@example.com"})
+      {:ok, user} = Mv.Accounts.update_user(user, %{role_id: role_a.id}, actor: system_actor)
+      assert user.role_id == role_a.id
+
+      {:ok, view, _html} = setup_live_view(conn, "/users/#{user.id}/edit")
+
+      view
+      |> form("#user-form",
+        user: %{
+          email: "rolechange@example.com",
+          role_id: role_b.id
+        }
+      )
+      |> render_submit()
+
+      assert_redirected(view, "/users")
+
+      updated_user = Ash.reload!(user, domain: Mv.Accounts, actor: system_actor)
+
+      assert updated_user.role_id == role_b.id,
+             "Expected role_id to persist as #{role_b.id}, got #{inspect(updated_user.role_id)}"
+    end
   end
 
   describe "edit user form - validation" do

test/mv_web/user_live/index_test.exs

@@ -55,7 +55,6 @@ defmodule MvWeb.UserLive.IndexTest do
 
       # Should show ascending indicator (up arrow)
       assert html =~ "hero-chevron-up"
-      assert html =~ ~s(aria-sort="ascending")
 
       # Test actual sort order: alpha should appear before mike, mike before zulu
       alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0)
@@ -76,7 +75,6 @@ defmodule MvWeb.UserLive.IndexTest do
 
       # Should now show descending indicator (down arrow)
       assert html =~ "hero-chevron-down"
-      assert html =~ ~s(aria-sort="descending")
 
       # Test actual sort order reversed: zulu should now appear before mike, mike before alpha
       alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0)
@@ -107,7 +105,6 @@ defmodule MvWeb.UserLive.IndexTest do
       # Click again to toggle back to ascending
       html = view |> element("button[phx-value-field='email']") |> render_click()
       assert html =~ "hero-chevron-up"
-      assert html =~ ~s(aria-sort="ascending")
 
       # Should be back to original ascending order
       alpha_pos = html |> :binary.match("alpha@example.com") |> elem(0)
@@ -379,6 +376,45 @@ defmodule MvWeb.UserLive.IndexTest do
     end
   end
 
+  describe "Password column display" do
+    test "user without password shows em dash in Password column", %{conn: conn} do
+      # User created with hashed_password: nil (no password) - must not get default password
+      user_no_pw =
+        create_test_user(%{
+          email: "no-password@example.com",
+          hashed_password: nil
+        })
+
+      conn = conn_with_oidc_user(conn)
+      {:ok, view, html} = live(conn, "/users")
+
+      assert html =~ "no-password@example.com"
+
+      # Password column must show "—" (em dash) for user without password, not "Enabled"
+      row = view |> element("tr#row-#{user_no_pw.id}") |> render()
+      assert row =~ "—", "Password column should show em dash for user without password"
+
+      refute row =~ "Enabled",
+             "Password column must not show Enabled when user has no password"
+    end
+
+    test "user with password shows Enabled in Password column", %{conn: conn} do
+      user_with_pw =
+        create_test_user(%{
+          email: "with-password@example.com",
+          password: "test123"
+        })
+
+      conn = conn_with_oidc_user(conn)
+      {:ok, view, html} = live(conn, "/users")
+
+      assert html =~ "with-password@example.com"
+
+      row = view |> element("tr#row-#{user_with_pw.id}") |> render()
+      assert row =~ "Enabled", "Password column should show Enabled when user has password"
+    end
+  end
+
   describe "member linking display" do
     @tag :slow
     test "displays linked member name in user list", %{conn: conn} do