diff --git a/lib/mv_web/live/components/member_filter_component.ex b/lib/mv_web/live/components/member_filter_component.ex index c020fc1..7a3517b 100644 --- a/lib/mv_web/live/components/member_filter_component.ex +++ b/lib/mv_web/live/components/member_filter_component.ex @@ -487,7 +487,7 @@ defmodule MvWeb.Components.MemberFilterComponent do # Get boolean filter label (comma-separated list of active filter names) defp boolean_filter_label(_boolean_custom_fields, boolean_filters) when map_size(boolean_filters) == 0 do - gettext("All") + gettext("Apply filters") end defp boolean_filter_label(boolean_custom_fields, boolean_filters) do diff --git a/lib/mv_web/live/member_live/form.ex b/lib/mv_web/live/member_live/form.ex index 66260f4..9138236 100644 --- a/lib/mv_web/live/member_live/form.ex +++ b/lib/mv_web/live/member_live/form.ex @@ -6,7 +6,6 @@ defmodule MvWeb.MemberLive.Form do - Create new members with personal information - Edit existing member details - Grouped sections for better organization - - Tab navigation (Payments tab disabled, coming soon) - Manage custom properties (dynamic fields, displayed sorted by name) - Real-time validation with visual feedback @@ -56,23 +55,12 @@ defmodule MvWeb.MemberLive.Form do
- <%!-- Tab Navigation --%> + <%!-- Tab navigation: Payments tab not shown on new/edit (only on member show) --%>
-
<%!-- Personal Data and Custom Fields Row --%> diff --git a/lib/mv_web/live/member_live/index.ex b/lib/mv_web/live/member_live/index.ex index a7f6316..4309611 100644 --- a/lib/mv_web/live/member_live/index.ex +++ b/lib/mv_web/live/member_live/index.ex @@ -15,7 +15,6 @@ defmodule MvWeb.MemberLive.Index do - `sort_order` - Sort direction (:asc or :desc) ## Events - - `delete` - Remove a member from the database - `select_member` - Toggle individual member selection - `select_all` - Toggle selection of all visible members - `copy_emails` - Copy email addresses of selected members to clipboard @@ -157,50 +156,10 @@ defmodule MvWeb.MemberLive.Index do Handles member-related UI events. ## Supported events: - - `"delete"` - Removes a member from the database - `"select_member"` - Toggles individual member selection - `"select_all"` - Toggles selection of all visible members - `"sort"` - Sort event from SortHeaderComponent. Updates sort field/order and syncs URL """ - @impl true - def handle_event("delete", %{"id" => id}, socket) do - actor = current_actor(socket) - - case Ash.get(Mv.Membership.Member, id, actor: actor) do - {:ok, member} -> - case Ash.destroy(member, actor: actor) do - :ok -> - updated_members = Enum.reject(socket.assigns.members, &(&1.id == id)) - - {:noreply, - socket - |> assign(:members, updated_members) - |> put_flash(:success, gettext("Member deleted successfully"))} - - {:error, %Ash.Error.Forbidden{}} -> - {:noreply, - put_flash( - socket, - :error, - gettext("You do not have permission to delete this member") - )} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end - - {:error, %Ash.Error.Query.NotFound{}} -> - {:noreply, put_flash(socket, :error, gettext("Member not found"))} - - {:error, %Ash.Error.Forbidden{} = _error} -> - {:noreply, - put_flash(socket, :error, gettext("You do not have permission to access this member"))} - - {:error, error} -> - {:noreply, put_flash(socket, :error, format_error(error))} - end - end - @impl true def handle_event("select_member", %{"id" => id}, socket) do selected = @@ -343,22 +302,6 @@ defmodule MvWeb.MemberLive.Index do {:noreply, push_patch(socket, to: ~p"/members?#{query_params}", replace: true)} end - # Helper to format errors for display - defp format_error(%Ash.Error.Invalid{errors: errors}) do - error_messages = - Enum.map(errors, fn error -> - case error do - %{field: field, message: message} -> "#{field}: #{message}" - %{message: message} -> message - _ -> inspect(error) - end - end) - - Enum.join(error_messages, ", ") - end - - defp format_error(error), do: inspect(error) - # ----------------------------------------------------------------- # Handle Infos from Child Components # ----------------------------------------------------------------- diff --git a/lib/mv_web/live/member_live/index.html.heex b/lib/mv_web/live/member_live/index.html.heex index c54ec7c..a696b00 100644 --- a/lib/mv_web/live/member_live/index.html.heex +++ b/lib/mv_web/live/member_live/index.html.heex @@ -379,26 +379,10 @@ <:action :let={member}>
- <.link navigate={~p"/members/#{member}"}>{gettext("Show")} + <.link navigate={~p"/members/#{member}"} data-testid="member-show-link"> + {gettext("Show")} +
- - <%= if can?(@current_user, :update, member) do %> - <.link navigate={~p"/members/#{member}/edit"} data-testid="member-edit"> - {gettext("Edit member")} - - <% end %> - - - <:action :let={member}> - <%= if can?(@current_user, :destroy, member) do %> - <.link - phx-click={JS.push("delete", value: %{id: member.id}) |> hide("#row-#{member.id}")} - data-confirm={gettext("Are you sure?")} - data-testid="member-delete" - > - {gettext("Delete")} - - <% end %> diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index 3af0ed2..ae69c30 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -54,217 +54,283 @@ defmodule MvWeb.MemberLive.Show do
- <%!-- Tab Navigation --%> -
+ <%!-- Tab Navigation: surface only behind buttons (inline); tabs-bordered; tabs-lg; both tabs keyboard-focusable --%> +
<%= if @active_tab == :contact do %> <%!-- Contact Data Tab Content --%> - <%!-- Personal Data and Custom Fields Row --%> -
- <%!-- Personal Data Section --%> -
- <.section_box title={gettext("Personal Data")}> -
- <%!-- Name Row --%> -
- <.data_field - label={gettext("First Name")} - value={@member.first_name} - class="w-48" - /> - <.data_field label={gettext("Last Name")} value={@member.last_name} class="w-48" /> -
+
+ <%!-- Personal Data and Custom Fields Row --%> +
+ <%!-- Personal Data Section --%> +
+ <.section_box title={gettext("Personal Data")}> +
+ <%!-- Name Row --%> +
+ <.data_field + label={gettext("First Name")} + value={@member.first_name} + class="w-48" + /> + <.data_field + label={gettext("Last Name")} + value={@member.last_name} + class="w-48" + /> +
- <%!-- Address --%> -
- <.data_field label={gettext("Address")} value={format_address(@member)} /> -
+ <%!-- Address --%> +
+ <.data_field label={gettext("Address")} value={format_address(@member)} /> +
- <%!-- Email --%> -
- <.data_field label={gettext("Email")}> - - {@member.email} - - -
+ <%!-- Email --%> +
+ <.data_field label={gettext("Email")}> + + {@member.email} + + +
- <%!-- Membership Dates Row --%> -
- <.data_field - label={gettext("Join Date")} - value={format_date(@member.join_date)} - class="w-28" - /> - <.data_field - label={gettext("Exit Date")} - value={format_date(@member.exit_date)} - class="w-28" - /> -
+ <%!-- Membership Dates Row --%> +
+ <.data_field + label={gettext("Join Date")} + value={format_date(@member.join_date)} + class="w-28" + /> + <.data_field + label={gettext("Exit Date")} + value={format_date(@member.exit_date)} + class="w-28" + /> +
- <%!-- Linked User: only show when current user can see other users (e.g. admin). + <%!-- 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 %> + <%= if can_access_page?(@current_user, "/users") do %> +
+ <.data_field label={gettext("Linked User")}> + <%= if @member.user do %> + <.link + navigate={~p"/users/#{@member.user}"} + class="text-blue-700 hover:text-blue-800 underline inline-flex items-center gap-1" + > + <.icon name="hero-user" class="size-4" /> + {@member.user.email} + + <% else %> + + {gettext("No user linked")} + + <% end %> + +
+ <% end %> + + <%!-- Groups (in Personal Data) --%> + <% groups = @member.groups || [] %>
- <.data_field label={gettext("Linked User")}> - <%= if @member.user do %> - <.link - navigate={~p"/users/#{@member.user}"} - class="text-blue-700 hover:text-blue-800 underline inline-flex items-center gap-1" - > - <.icon name="hero-user" class="size-4" /> - {@member.user.email} - + <.data_field label={gettext("Groups")}> + <%= if Enum.empty?(groups) do %> + {gettext("No groups")} <% else %> - {gettext("No user linked")} +
+ <%= for group <- groups do %> + <.button + variant="outline" + size="sm" + navigate={~p"/groups/#{group.slug}"} + aria-label={gettext("Member of group %{name}", name: group.name)} + > + {group.name} + + <% end %> +
<% end %>
- <% end %> - <%!-- Groups (in Personal Data) --%> - <% groups = @member.groups || [] %> -
- <.data_field label={gettext("Groups")}> - <%= if Enum.empty?(groups) do %> - {gettext("No groups")} - <% else %> -
- <%= for group <- groups do %> - <.button - variant="outline" - size="sm" - navigate={~p"/groups/#{group.slug}"} - aria-label={gettext("Member of group %{name}", name: group.name)} - > - {group.name} - - <% end %> -
- <% end %> - -
- - <%!-- Notes --%> - <%= if @member.notes && String.trim(@member.notes) != "" do %> -
- <.data_field label={gettext("Notes")}> -

{@member.notes}

- -
- <% end %> -
- -
- - <%!-- Custom Fields Section --%> - <%= if Enum.any?(@custom_fields) do %> -
- <.section_box title={gettext("Custom Fields")}> -
- <%= for custom_field <- @custom_fields do %> - <% cfv = find_custom_field_value(@member.custom_field_values, custom_field.id) %> - <.data_field label={custom_field.name}> - {format_custom_field_value(cfv, custom_field.value_type)} - + <%!-- Notes --%> + <%= if @member.notes && String.trim(@member.notes) != "" do %> +
+ <.data_field label={gettext("Notes")}> +

{@member.notes}

+ +
<% end %>
- <% end %> -
- <%!-- Payment Data Section --%> -
- <.section_box title={gettext("Payment Data")}> - <%= if @member.membership_fee_type do %> -
- <.data_field - label={gettext("Type")} - value={@member.membership_fee_type.name} - class="min-w-32" - /> - <.data_field - label={gettext("Membership Fee")} - value={MembershipFeeHelpers.format_currency(@member.membership_fee_type.amount)} - class="min-w-24" - /> - <.data_field - label={gettext("Payment Interval")} - value={MembershipFeeHelpers.format_interval(@member.membership_fee_type.interval)} - class="min-w-32" - /> - <.data_field label={gettext("Last Cycle")} class="min-w-32"> - <%= if @member.last_cycle_status do %> - <% status = @member.last_cycle_status %> - - {format_status_label(status)} - - <% else %> - {gettext("No cycles")} - <% end %> - - <.data_field label={gettext("Current Cycle")} class="min-w-36"> - <%= if @member.current_cycle_status do %> - <% status = @member.current_cycle_status %> - - {format_status_label(status)} - - <% else %> - {gettext("No cycles")} - <% end %> - -
- <% else %> -
- {gettext("No membership fee type assigned")} + <%!-- Custom Fields Section --%> + <%= if Enum.any?(@custom_fields) do %> +
+ <.section_box title={gettext("Custom Fields")}> +
+ <%= for custom_field <- @custom_fields do %> + <% cfv = find_custom_field_value(@member.custom_field_values, custom_field.id) %> + <.data_field label={custom_field.name}> + {format_custom_field_value(cfv, custom_field.value_type)} + + <% end %> +
+
<% end %> - +
+ + <%!-- Payment Data Section --%> +
+ <.section_box title={gettext("Payment Data")}> + <%= if @member.membership_fee_type do %> +
+ <.data_field + label={gettext("Type")} + value={@member.membership_fee_type.name} + class="min-w-32" + /> + <.data_field + label={gettext("Membership Fee")} + value={MembershipFeeHelpers.format_currency(@member.membership_fee_type.amount)} + class="min-w-24" + /> + <.data_field + label={gettext("Payment Interval")} + value={ + MembershipFeeHelpers.format_interval(@member.membership_fee_type.interval) + } + class="min-w-32" + /> + <.data_field label={gettext("Last Cycle")} class="min-w-32"> + <%= if @member.last_cycle_status do %> + <% status = @member.last_cycle_status %> + + {format_status_label(status)} + + <% else %> + {gettext("No cycles")} + <% end %> + + <.data_field label={gettext("Current Cycle")} class="min-w-36"> + <%= if @member.current_cycle_status do %> + <% status = @member.current_cycle_status %> + + {format_status_label(status)} + + <% else %> + {gettext("No cycles")} + <% end %> + +
+ <% else %> +
+ {gettext("No membership fee type assigned")} +
+ <% end %> + +
<% end %> <%= if @active_tab == :membership_fees do %> <%!-- Membership Fees Tab Content --%> - <.live_component - module={MvWeb.MemberLive.Show.MembershipFeesComponent} - id={"membership-fees-#{@member.id}"} - member={@member} - current_user={@current_user} - vereinfacht_receipts={@vereinfacht_receipts} - /> +
+ <.live_component + module={MvWeb.MemberLive.Show.MembershipFeesComponent} + id={"membership-fees-#{@member.id}"} + member={@member} + current_user={@current_user} + vereinfacht_receipts={@vereinfacht_receipts} + /> +
+ <% end %> + + <%!-- Danger zone: same section pattern as section_box (h2 outside border) --%> + <%= if can?(@current_user, :destroy, @member) do %> +
+

+ {gettext("Danger zone")} +

+
+

+ {gettext( + "Deleting this member cannot be undone. All related data (e.g. membership fee cycles) will be removed." + )} +

+ <.button + variant="danger" + phx-click="delete" + phx-value-id={@member.id} + data-confirm={ + gettext("Are you sure you want to delete %{name}? This action cannot be undone.", + name: MvWeb.Helpers.MemberHelpers.display_name(@member) + ) + } + data-testid="member-delete" + aria-label={ + gettext("Delete member %{name}", + name: MvWeb.Helpers.MemberHelpers.display_name(@member) + ) + } + > + <.icon name="hero-trash" class="size-4" /> + {gettext("Delete member")} + +
+
<% end %>
@@ -328,6 +394,35 @@ defmodule MvWeb.MemberLive.Show do {:noreply, assign(socket, :active_tab, :membership_fees)} end + @impl true + def handle_event("delete", %{"id" => id}, socket) do + member = socket.assigns.member + actor = current_actor(socket) + + if to_string(id) != to_string(member.id) do + {:noreply, put_flash(socket, :error, gettext("Member not found"))} + else + case Ash.destroy(member, actor: actor) do + :ok -> + {:noreply, + socket + |> put_flash(:success, gettext("Member deleted successfully")) + |> push_navigate(to: ~p"/members")} + + {:error, %Ash.Error.Forbidden{}} -> + {:noreply, + put_flash( + socket, + :error, + gettext("You do not have permission to delete this member") + )} + + {:error, error} -> + {:noreply, put_flash(socket, :error, format_error(error))} + end + end + end + def handle_event("load_vereinfacht_receipts", %{"contact_id" => contact_id}, socket) do response = case Mv.Vereinfacht.Client.get_contact_with_receipts(contact_id) do @@ -358,6 +453,19 @@ defmodule MvWeb.MemberLive.Show do defp page_title(:show), do: gettext("Show Member") defp page_title(:edit), do: gettext("Edit Member") + defp format_error(%Ash.Error.Invalid{errors: errors}) do + error_messages = + Enum.map(errors, fn + %{field: field, message: message} -> "#{field}: #{message}" + %{message: message} -> message + _ -> inspect(errors) + end) + + Enum.join(error_messages, ", ") + end + + defp format_error(error), do: inspect(error) + # ----------------------------------------------------------------- # Helper Components # ----------------------------------------------------------------- diff --git a/test/mv_web/member_live/form_error_handling_test.exs b/test/mv_web/member_live/form_error_handling_test.exs index d61d3fd..fec7df4 100644 --- a/test/mv_web/member_live/form_error_handling_test.exs +++ b/test/mv_web/member_live/form_error_handling_test.exs @@ -3,11 +3,38 @@ defmodule MvWeb.MemberLive.FormErrorHandlingTest do Tests for error handling in the member form, specifically flash message display. """ use MvWeb.ConnCase, async: false + use Gettext, backend: MvWeb.Gettext import Phoenix.LiveViewTest require Ash.Query + describe "tab visibility" do + @tag :ui + test "Payments tab is not visible on new member form", %{conn: conn} do + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, "/members/new") + + refute html =~ gettext("Payments") + end + + @tag :ui + test "Payments tab is not visible on edit member form", %{conn: conn} do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + + {:ok, member} = + Mv.Membership.create_member( + %{first_name: "Edit", last_name: "Member", email: "edit@example.com"}, + actor: system_actor + ) + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}/edit") + + refute html =~ gettext("Payments") + end + end + describe "error handling - flash messages" do setup do {:ok, settings} = Mv.Membership.get_settings() diff --git a/test/mv_web/member_live/index_test.exs b/test/mv_web/member_live/index_test.exs index 53a2815..d8846ea 100644 --- a/test/mv_web/member_live/index_test.exs +++ b/test/mv_web/member_live/index_test.exs @@ -266,36 +266,42 @@ defmodule MvWeb.MemberLive.IndexTest do assert is_list(state.socket.assigns.members) end - test "can delete a member without error", %{conn: conn} do + @tag :ui + test "member index does not render Edit or Delete actions", %{conn: conn} do system_actor = Mv.Helpers.SystemActor.get_system_actor() - # Create a test member first - {:ok, member} = + {:ok, _member} = Mv.Membership.create_member( - %{ - first_name: "Test", - last_name: "User", - email: "test@example.com" - }, + %{first_name: "Test", last_name: "User", email: "test@example.com"}, actor: system_actor ) conn = conn_with_oidc_user(conn) - {:ok, index_view, _html} = live(conn, "/members") + {:ok, view, html} = live(conn, "/members") - # Verify the member is displayed - assert has_element?(index_view, "#members", "Test User") + refute has_element?(view, "[data-testid='member-edit']") + refute html =~ ~s(data-testid="member-delete") + end - # Click the delete link for this member - index_view - |> element("a", "Delete") + @tag :ui + test "row click navigates to member show", %{conn: conn} do + system_actor = Mv.Helpers.SystemActor.get_system_actor() + + {:ok, member} = + Mv.Membership.create_member( + %{first_name: "Row", last_name: "Click", email: "rowclick@example.com"}, + actor: system_actor + ) + + conn = conn_with_oidc_user(conn) + {:ok, view, _html} = live(conn, "/members") + + # Click a data cell (e.g. second column = first name) to trigger row navigation + view + |> element("#row-#{member.id} td:nth-child(2)") |> render_click() - # Verify the member is no longer displayed - refute has_element?(index_view, "#members", "Test User") - - # Verify the member was actually deleted from the database - assert not (Mv.Membership.Member |> Ash.Query.filter(id == ^member.id) |> Ash.exists?()) + assert_redirect(view, ~p"/members/#{member}") end describe "copy_emails feature" do diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs index 26c3f00..8c7a23a 100644 --- a/test/mv_web/member_live/show_test.exs +++ b/test/mv_web/member_live/show_test.exs @@ -134,6 +134,35 @@ defmodule MvWeb.MemberLive.ShowTest do end end + describe "delete action" do + test "renders Delete button when user can destroy member", %{ + conn: conn, + member: member + } do + conn = conn_with_oidc_user(conn) + {:ok, view, _html} = live(conn, ~p"/members/#{member}") + + assert has_element?(view, "[data-testid='member-delete']") + end + + test "delete event removes member and redirects to index", %{ + conn: conn, + member: member + } do + conn = conn_with_oidc_user(conn) + {:ok, view, _html} = live(conn, ~p"/members/#{member}") + + view + |> render_click("delete", %{"id" => member.id}) + + assert_redirect(view, ~p"/members") + + refute Mv.Membership.Member + |> Ash.Query.filter(id == ^member.id) + |> Ash.exists?() + end + end + describe "custom field value formatting" do test "formats string custom field values", %{conn: conn, member: member, actor: actor} do {:ok, custom_field} =