Merge pull request 'Fix hidden empty custom fields closes #282' (#313) from bugfix/228_hidden_empty_custom_field_ into main

Reviewed-on: #313

lib/mv_web/live/member_live/show.ex

@@ -73,12 +73,7 @@ defmodule MvWeb.MemberLive.Show do
               <%!-- Email --%>
               <div>
                 <.data_field label={gettext("Email")}>
-                  <a
-                    href={"mailto:#{MvWeb.MemberLive.Index.format_member_email(@member)}"}
-                    class="text-blue-700 hover:text-blue-800 underline"
-                  >
-                    {@member.email}
-                  </a>
+                  <.mailto_link email={@member.email} display={@member.email} />
                 </.data_field>
               </div>
 
@@ -131,15 +126,14 @@ defmodule MvWeb.MemberLive.Show do
         </div>
 
         <%!-- Custom Fields Section --%>
-        <%= if Enum.any?(@member.custom_field_values) do %>
+        <%= if Enum.any?(@custom_fields) do %>
           <div>
             <.section_box title={gettext("Custom Fields")}>
               <div class="grid grid-cols-2 gap-4">
-                <%= 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)}
                   </.data_field>
                 <% end %>
               </div>
@@ -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"""
+      <a
+        href={"mailto:#{@email}"}
+        class="text-blue-700 hover:text-blue-800 underline"
+      >
+        {@display}
+      </a>
+      """
+    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"""
-    <a href={"mailto:#{@email}"} class="text-blue-700 hover:text-blue-800 underline">{@email}</a>
-    """
+      ~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"""
+    <span class="text-base-content/50 italic">
+      <span aria-hidden="true">—</span>
+      <span class="sr-only">{@text}</span>
+    </span>
+    """
+  end
 end

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"

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 ""

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"

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

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

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