diff --git a/lib/mv_web/live/member_live/show.ex b/lib/mv_web/live/member_live/show.ex index d84fca4..7cdc875 100644 --- a/lib/mv_web/live/member_live/show.ex +++ b/lib/mv_web/live/member_live/show.ex @@ -73,12 +73,7 @@ defmodule MvWeb.MemberLive.Show do <%!-- Email --%>
<.data_field label={gettext("Email")}> - - {@member.email} - + <.mailto_link email={@member.email} display={@member.email} />
@@ -131,15 +126,14 @@ defmodule MvWeb.MemberLive.Show do <%!-- Custom Fields Section --%> - <%= if Enum.any?(@member.custom_field_values) do %> + <%= if Enum.any?(@custom_fields) do %>
<.section_box title={gettext("Custom Fields")}>
- <%= for cfv <- sort_custom_field_values(@member.custom_field_values) do %> - <% custom_field = cfv.custom_field %> - <% value_type = custom_field && custom_field.value_type %> - <.data_field label={custom_field && custom_field.name}> - {format_custom_field_value(cfv.value, value_type)} + <%= 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 %>
@@ -180,6 +174,14 @@ defmodule MvWeb.MemberLive.Show do @impl true def handle_params(%{"id" => id}, _, socket) do + # Load custom fields once using assign_new to avoid repeated queries + socket = + assign_new(socket, :custom_fields, fn -> + Mv.Membership.CustomField + |> Ash.Query.sort(name: :asc) + |> Ash.read!() + end) + query = Mv.Membership.Member |> filter(id == ^id) @@ -236,12 +238,35 @@ defmodule MvWeb.MemberLive.Show do """ end + # Renders a mailto link if email is present, otherwise renders empty value placeholder + attr :email, :string, required: true + attr :display, :string, default: nil + + defp mailto_link(assigns) do + display_text = assigns.display || assigns.email + + if assigns.email && String.trim(assigns.email) != "" do + assigns = %{email: assigns.email, display: display_text} + + ~H""" + + {@display} + + """ + else + render_empty_value() + end + end + # ----------------------------------------------------------------- # Helper Functions # ----------------------------------------------------------------- - defp display_value(nil), do: "" - defp display_value(""), do: "" + defp display_value(nil), do: render_empty_value() + defp display_value(""), do: render_empty_value() defp display_value(value), do: value defp format_address(member) do @@ -272,20 +297,31 @@ defmodule MvWeb.MemberLive.Show do defp format_date(date), do: to_string(date) - # Sorts custom field values by custom field name - defp sort_custom_field_values(custom_field_values) do - Enum.sort_by(custom_field_values, fn cfv -> - (cfv.custom_field && cfv.custom_field.name) || "" + # Finds custom field value for a given custom field id + defp find_custom_field_value(nil, _custom_field_id), do: nil + + defp find_custom_field_value(custom_field_values, custom_field_id) + when is_list(custom_field_values) do + Enum.find(custom_field_values, fn cfv -> + cfv.custom_field_id == custom_field_id or + (cfv.custom_field && cfv.custom_field.id == custom_field_id) end) end + defp find_custom_field_value(_custom_field_values, _custom_field_id), do: nil + # Formats custom field value based on type + # Handles both CustomFieldValue structs and direct values + defp format_custom_field_value(nil, _type), do: render_empty_value() + + defp format_custom_field_value(%Mv.Membership.CustomFieldValue{} = cfv, value_type) do + format_custom_field_value(cfv.value, value_type) + end + defp format_custom_field_value(%Ash.Union{value: value, type: type}, _expected_type) do format_custom_field_value(value, type) end - defp format_custom_field_value(nil, _type), do: "—" - defp format_custom_field_value(value, :boolean) when is_boolean(value) do if value, do: gettext("Yes"), else: gettext("No") end @@ -295,11 +331,15 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, :email) when is_binary(value) do - assigns = %{email: value} + if String.trim(value) == "" do + render_empty_value() + else + assigns = %{email: value} - ~H""" - {@email} - """ + ~H""" + <.mailto_link email={@email} display={@email} /> + """ + end end defp format_custom_field_value(value, :integer) when is_integer(value) do @@ -307,8 +347,22 @@ defmodule MvWeb.MemberLive.Show do end defp format_custom_field_value(value, _type) when is_binary(value) do - if String.trim(value) == "", do: "—", else: value + if String.trim(value) == "", do: render_empty_value(), else: value end defp format_custom_field_value(value, _type), do: to_string(value) + + # Renders accessible placeholder for empty values + # Uses translated text for screen readers while maintaining visual consistency + # The visual "—" is hidden from screen readers, while the translated text is only visible to screen readers + defp render_empty_value do + assigns = %{text: gettext("Not set")} + + ~H""" + + + {@text} + + """ + end end diff --git a/priv/gettext/de/LC_MESSAGES/default.po b/priv/gettext/de/LC_MESSAGES/default.po index ec6812a..1c298e9 100644 --- a/priv/gettext/de/LC_MESSAGES/default.po +++ b/priv/gettext/de/LC_MESSAGES/default.po @@ -1422,6 +1422,11 @@ msgstr "Jährliches Intervall – Beitrittszeitraum nicht einbezogen" msgid "Yearly Interval - Joining Cycle Included" msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen" +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not set" +msgstr "Nicht gesetzt" + #~ #: lib/mv_web/live/custom_field_live/show.ex #~ #, elixir-autogen, elixir-format #~ msgid "Auto-generated identifier (immutable)" @@ -1494,12 +1499,6 @@ msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen" #~ msgid "New Custom field" #~ msgstr "Benutzerdefiniertes Feld speichern" -#~ #: lib/mv_web/live/user_live/form.ex -#~ #: lib/mv_web/live/user_live/show.ex -#~ #, elixir-autogen, elixir-format -#~ msgid "Not set" -#~ msgstr "Nicht gesetzt" - #~ #: lib/mv_web/live/contribution_settings_live.ex #~ #, elixir-autogen, elixir-format #~ msgid "Quarterly Interval - Joining Period Excluded" diff --git a/priv/gettext/default.pot b/priv/gettext/default.pot index e2bbf32..5450ee0 100644 --- a/priv/gettext/default.pot +++ b/priv/gettext/default.pot @@ -1422,3 +1422,8 @@ msgstr "" #, elixir-autogen, elixir-format msgid "Yearly Interval - Joining Cycle Included" msgstr "" + +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format +msgid "Not set" +msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/default.po b/priv/gettext/en/LC_MESSAGES/default.po index d3ee646..095ec30 100644 --- a/priv/gettext/en/LC_MESSAGES/default.po +++ b/priv/gettext/en/LC_MESSAGES/default.po @@ -1423,6 +1423,11 @@ msgstr "" msgid "Yearly Interval - Joining Cycle Included" msgstr "" +#: lib/mv_web/live/member_live/show.ex +#, elixir-autogen, elixir-format, fuzzy +msgid "Not set" +msgstr "" + #~ #: lib/mv_web/live/custom_field_live/show.ex #~ #, elixir-autogen, elixir-format #~ msgid "Auto-generated identifier (immutable)" @@ -1495,11 +1500,6 @@ msgstr "" #~ msgid "New Custom field" #~ msgstr "" -#~ #: lib/mv_web/live/user_live/show.ex -#~ #, elixir-autogen, elixir-format, fuzzy -#~ msgid "Not set" -#~ msgstr "" - #~ #: lib/mv_web/live/contribution_settings_live.ex #~ #, elixir-autogen, elixir-format #~ msgid "Quarterly Interval - Joining Period Excluded" diff --git a/test/mv_web/member_live/index_field_visibility_test.exs b/test/mv_web/member_live/index_field_visibility_test.exs index 6e1642a..05fa768 100644 --- a/test/mv_web/member_live/index_field_visibility_test.exs +++ b/test/mv_web/member_live/index_field_visibility_test.exs @@ -10,7 +10,8 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do - Integration with member list display - Custom fields visibility """ - use MvWeb.ConnCase, async: true + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest require Ash.Query diff --git a/test/mv_web/member_live/show_test.exs b/test/mv_web/member_live/show_test.exs new file mode 100644 index 0000000..1e04559 --- /dev/null +++ b/test/mv_web/member_live/show_test.exs @@ -0,0 +1,175 @@ +defmodule MvWeb.MemberLive.ShowTest do + @moduledoc """ + Tests for the member show page. + + Tests cover: + - Displaying member information + - Custom Fields section visibility (Issue #282 regression test) + - Custom field values formatting + + ## Note on async: false + Tests use `async: false` (not `async: true`) to prevent PostgreSQL deadlocks + when creating members and custom fields concurrently. This is intentional and + documented here to avoid confusion in commit messages. + """ + # async: false to prevent PostgreSQL deadlocks when creating members and custom fields + use MvWeb.ConnCase, async: false + import Phoenix.LiveViewTest + require Ash.Query + use Gettext, backend: MvWeb.Gettext + + alias Mv.Membership.{CustomField, CustomFieldValue, Member} + + setup do + # Create test member + {:ok, member} = + Member + |> Ash.Changeset.for_create(:create_member, %{ + first_name: "Alice", + last_name: "Anderson", + email: "alice@example.com" + }) + |> Ash.create() + + %{member: member} + end + + describe "custom fields section visibility (Issue #282)" do + test "displays Custom Fields section even when member has no custom field values", %{ + conn: conn, + member: member + } do + # Create a custom field but no value for the member + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should be visible + assert html =~ gettext("Custom Fields") + + # Custom field label should be visible + assert html =~ custom_field.name + + # Value should show placeholder for empty value + assert html =~ "—" or html =~ gettext("Not set") + end + + test "displays Custom Fields section with multiple custom fields, some without values", %{ + conn: conn, + member: member + } do + # Create multiple custom fields + {:ok, field1} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + {:ok, field2} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "membership_number", + value_type: :integer + }) + |> Ash.create() + + # Create value only for first field + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: field1.id, + value: %{"_union_type" => "string", "_union_value" => "+49123456789"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should be visible + assert html =~ gettext("Custom Fields") + + # Both field labels should be visible + assert html =~ field1.name + assert html =~ field2.name + + # First field should show value + assert html =~ "+49123456789" + + # Second field should show placeholder + assert html =~ "—" or html =~ gettext("Not set") + end + + test "does not display Custom Fields section when no custom fields exist", %{ + conn: conn, + member: member + } do + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}") + + # Custom Fields section should NOT be visible + refute html =~ gettext("Custom Fields") + end + end + + describe "custom field value formatting" do + test "formats string custom field values", %{conn: conn, member: member} do + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "phone_mobile", + value_type: :string + }) + |> Ash.create() + + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "string", "_union_value" => "+49123456789"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}") + + assert html =~ "+49123456789" + end + + test "formats email custom field values as mailto links", %{conn: conn, member: member} do + {:ok, custom_field} = + CustomField + |> Ash.Changeset.for_create(:create, %{ + name: "private_email", + value_type: :email + }) + |> Ash.create() + + {:ok, _cfv} = + CustomFieldValue + |> Ash.Changeset.for_create(:create, %{ + member_id: member.id, + custom_field_id: custom_field.id, + value: %{"_union_type" => "email", "_union_value" => "private@example.com"} + }) + |> Ash.create() + + conn = conn_with_oidc_user(conn) + {:ok, _view, html} = live(conn, ~p"/members/#{member}") + + # Should contain mailto link + assert html =~ ~s(href="mailto:private@example.com") + assert html =~ "private@example.com" + end + end +end diff --git a/test/mv_web/user_live/form_test.exs b/test/mv_web/user_live/form_test.exs index b8f7313..334dedd 100644 --- a/test/mv_web/user_live/form_test.exs +++ b/test/mv_web/user_live/form_test.exs @@ -1,5 +1,6 @@ defmodule MvWeb.UserLive.FormTest do - use MvWeb.ConnCase, async: true + # async: false to prevent PostgreSQL deadlocks when creating members and users + use MvWeb.ConnCase, async: false import Phoenix.LiveViewTest # Helper to setup authenticated connection and live view