Member show & MembershipFees: permissions, delete all, regenerate, errors

- Show: handle_info :member_updated and :put_flash; Linked User only when can_access_page? /users
- MembershipFeesComponent: can_create_cycle/can_destroy_cycle/can_update_cycle; buttons gated
- Delete all cycles via Ash.destroy (policy enforced); format_error Forbidden
- Regenerate cycles for normal_user and admin (no admin-only check)
- Member form: format_error tuple for membership_fee_type_id; Select a membership fee type (no None)
- show_membership_fees_test: read_only UI and policy tests

lib/mv_web/live/member_live/form.ex

@@ -177,7 +177,8 @@ defmodule MvWeb.MemberLive.Form do
                   phx-change="validate"
                   value={@form[:membership_fee_type_id].value || ""}
                 >
-                  <option value="">{gettext("None")}</option>
+                  <%!-- No "None" option: a membership fee type is required (validated in Member resource). --%>
+                  <option value="">{gettext("Select a membership fee type")}</option>
                   <%= for fee_type <- @available_fee_types do %>
                     <option
                       value={fee_type.id}
@@ -189,7 +190,8 @@ defmodule MvWeb.MemberLive.Form do
                     </option>
                   <% end %>
                 </select>
-                <%= for {msg, _opts} <- @form.errors[:membership_fee_type_id] || [] do %>
+                <%= for error <- List.wrap(@form.errors[:membership_fee_type_id] || []) do %>
+                  <% {msg, _opts} = if is_tuple(error), do: error, else: {error, []} %>
                   <p class="text-error text-sm mt-1">{msg}</p>
                 <% end %>
                 <%= if @interval_warning do %>

lib/mv_web/live/member_live/show.ex

@@ -125,7 +125,10 @@ defmodule MvWeb.MemberLive.Show do
                   />
                 </div>
 
-                <%!-- Linked User --%>
+                <%!-- Linked User: only show when current user can see other users (e.g. admin).
+                  read_only cannot see linked user, so hide the section to avoid "No user linked" when
+                  a user is linked but not visible. --%>
+                <%= if can_access_page?(@current_user, "/users") do %>
                   <div>
                     <.data_field label={gettext("Linked User")}>
                       <%= if @member.user do %>
@@ -141,6 +144,7 @@ defmodule MvWeb.MemberLive.Show do
                       <% end %>
                     </.data_field>
                   </div>
+                <% end %>
 
                 <%!-- Notes --%>
                 <%= if @member.notes && String.trim(@member.notes) != "" do %>
@@ -287,6 +291,23 @@ defmodule MvWeb.MemberLive.Show do
     {:noreply, assign(socket, :active_tab, :membership_fees)}
   end
 
+  # Flash set in LiveComponent is not shown in parent layout; child sends this to display flash
+  @impl true
+  def handle_info({:put_flash, type, message}, socket) do
+    {:noreply, put_flash(socket, type, message)}
+  end
+
+  # MembershipFeesComponent sends this after cycles are created/deleted/regenerated so parent keeps member in sync
+  @impl true
+  def handle_info({:member_updated, updated_member}, socket) do
+    member =
+      updated_member
+      |> Map.put(:last_cycle_status, get_last_cycle_status(updated_member))
+      |> Map.put(:current_cycle_status, get_current_cycle_status(updated_member))
+
+    {:noreply, assign(socket, :member, member)}
+  end
+
   defp page_title(:show), do: gettext("Show Member")
   defp page_title(:edit), do: gettext("Edit Member")
 

lib/mv_web/live/member_live/show/membership_fees_component.ex

@@ -14,6 +14,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
 
   require Ash.Query
   import MvWeb.LiveHelpers, only: [current_actor: 1]
+  import MvWeb.Authorization, only: [can?: 3]
 
   alias Mv.Membership
   alias Mv.MembershipFees
@@ -49,9 +50,10 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
           <% end %>
         </div>
 
-        <%!-- Action Buttons --%>
+        <%!-- Action Buttons (only when user has permission) --%>
         <div class="flex gap-2 mb-4">
           <.button
+            :if={@can_create_cycle}
             phx-click="regenerate_cycles"
             phx-target={@myself}
             class={["btn btn-sm btn-outline", if(@regenerating, do: "btn-disabled", else: "")]}
@@ -61,7 +63,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
             {if(@regenerating, do: gettext("Regenerating..."), else: gettext("Regenerate Cycles"))}
           </.button>
           <.button
-            :if={Enum.any?(@cycles)}
+            :if={Enum.any?(@cycles) and @can_destroy_cycle}
             phx-click="delete_all_cycles"
             phx-target={@myself}
             class="btn btn-sm btn-error btn-outline"
@@ -71,7 +73,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
             {gettext("Delete All Cycles")}
           </.button>
           <.button
-            :if={@member.membership_fee_type}
+            :if={@member.membership_fee_type != nil and @can_create_cycle}
             phx-click="open_create_cycle_modal"
             phx-target={@myself}
             class="btn btn-sm btn-primary"
@@ -103,6 +105,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
             </:col>
 
             <:col :let={cycle} label={gettext("Amount")}>
+              <%= if @can_update_cycle do %>
                 <span
                   class="font-mono cursor-pointer hover:text-primary"
                   phx-click="edit_cycle_amount"
@@ -112,6 +115,9 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
                 >
                   {MembershipFeeHelpers.format_currency(cycle.amount)}
                 </span>
+              <% else %>
+                <span class="font-mono">{MembershipFeeHelpers.format_currency(cycle.amount)}</span>
+              <% end %>
             </:col>
 
             <:col :let={cycle} label={gettext("Status")}>
@@ -125,6 +131,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
 
             <:action :let={cycle}>
               <div class="flex gap-1">
+                <%= if @can_update_cycle do %>
                   <button
                     :if={cycle.status != :paid}
                     type="button"
@@ -164,6 +171,8 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
                     <.icon name="hero-x-circle" class="size-4" />
                     {gettext("Unpaid")}
                   </button>
+                <% end %>
+                <%= if @can_destroy_cycle do %>
                   <button
                     type="button"
                     phx-click="delete_cycle"
@@ -175,6 +184,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
                     <.icon name="hero-trash" class="size-4" />
                     {gettext("Delete")}
                   </button>
+                <% end %>
               </div>
             </:action>
           </.table>
@@ -408,11 +418,19 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
     # Get available fee types (filtered to same interval if member has a type)
     available_fee_types = get_available_fee_types(member, actor)
 
+    # Permission flags for cycle actions (so read_only does not see create/update/destroy UI)
+    can_create_cycle = can?(actor, :create, MembershipFeeCycle)
+    can_destroy_cycle = can?(actor, :destroy, MembershipFeeCycle)
+    can_update_cycle = can?(actor, :update, MembershipFeeCycle)
+
     {:ok,
      socket
      |> assign(assigns)
      |> assign(:cycles, cycles)
      |> assign(:available_fee_types, available_fee_types)
+     |> assign(:can_create_cycle, can_create_cycle)
+     |> assign(:can_destroy_cycle, can_destroy_cycle)
+     |> assign(:can_update_cycle, can_update_cycle)
      |> assign_new(:interval_warning, fn -> nil end)
      |> assign_new(:editing_cycle, fn -> nil end)
      |> assign_new(:deleting_cycle, fn -> nil end)
@@ -554,17 +572,12 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
   end
 
   def handle_event("regenerate_cycles", _params, socket) do
-    actor = current_actor(socket)
+    # Button is only shown when can_create_cycle (normal_user and admin). Cycle generation uses system actor.
-
-    # SECURITY: Only admins can manually regenerate cycles via UI
-    # Cycle generation itself uses system actor, but UI access should be restricted
-    if actor.role && actor.role.permission_set_name == "admin" do
     socket = assign(socket, :regenerating, true)
     member = socket.assigns.member
 
     case CycleGenerator.generate_cycles_for_member(member.id) do
       {:ok, _new_cycles, _notifications} ->
-          # Reload member with cycles
         actor = current_actor(socket)
 
         updated_member =
@@ -599,11 +612,6 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
          |> assign(:regenerating, false)
          |> put_flash(:error, format_error(error))}
     end
-    else
-      {:noreply,
-       socket
-       |> put_flash(:error, gettext("Only administrators can regenerate cycles"))}
-    end
   end
 
   def handle_event("edit_cycle_amount", %{"cycle_id" => cycle_id}, socket) do
@@ -940,6 +948,10 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
     Enum.map_join(error.errors, ", ", fn e -> e.message end)
   end
 
+  defp format_error(%Ash.Error.Forbidden{}) do
+    gettext("You are not allowed to perform this action.")
+  end
+
   defp format_error(error) when is_binary(error), do: error
   defp format_error(_error), do: gettext("An error occurred")
 

test/mv_web/member_live/show_membership_fees_test.exs

@@ -274,4 +274,65 @@ 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 = create_member(%{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 = create_member(%{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 "confirm_delete_all_cycles returns error for read_only user", %{
+      current_user: read_only_user
+    } do
+      # Backend policy test: read_only cannot destroy any cycle.
+      # The UI hides the Delete All button for read_only; this test ensures
+      # that if the handler were triggered (e.g. via dev tools), the server
+      # would enforce policy and return Forbidden.
+      fee_type = create_fee_type(%{interval: :yearly})
+      member = create_member(%{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
 end