Fix hidden empty custom fields closes #282 #313
7 changed files with 273 additions and 38 deletions
|
|
@ -73,12 +73,7 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
<%!-- Email --%>
|
<%!-- Email --%>
|
||||||
<div>
|
<div>
|
||||||
<.data_field label={gettext("Email")}>
|
<.data_field label={gettext("Email")}>
|
||||||
<a
|
<.mailto_link email={@member.email} display={@member.email} />
|
||||||
href={"mailto:#{MvWeb.MemberLive.Index.format_member_email(@member)}"}
|
|
||||||
class="text-blue-700 hover:text-blue-800 underline"
|
|
||||||
>
|
|
||||||
{@member.email}
|
|
||||||
</a>
|
|
||||||
</.data_field>
|
</.data_field>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|
@ -131,15 +126,14 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<%!-- Custom Fields Section --%>
|
<%!-- Custom Fields Section --%>
|
||||||
<%= if Enum.any?(@member.custom_field_values) do %>
|
<%= if Enum.any?(@custom_fields) do %>
|
||||||
<div>
|
<div>
|
||||||
<.section_box title={gettext("Custom Fields")}>
|
<.section_box title={gettext("Custom Fields")}>
|
||||||
<div class="grid grid-cols-2 gap-4">
|
<div class="grid grid-cols-2 gap-4">
|
||||||
<%= for cfv <- sort_custom_field_values(@member.custom_field_values) do %>
|
<%= for custom_field <- @custom_fields do %>
|
||||||
<% custom_field = cfv.custom_field %>
|
<% cfv = find_custom_field_value(@member.custom_field_values, custom_field.id) %>
|
||||||
<% value_type = custom_field && custom_field.value_type %>
|
<.data_field label={custom_field.name}>
|
||||||
<.data_field label={custom_field && custom_field.name}>
|
{format_custom_field_value(cfv, custom_field.value_type)}
|
||||||
{format_custom_field_value(cfv.value, value_type)}
|
|
||||||
</.data_field>
|
</.data_field>
|
||||||
<% end %>
|
<% end %>
|
||||||
</div>
|
</div>
|
||||||
|
|
@ -180,6 +174,14 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def handle_params(%{"id" => id}, _, socket) do
|
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 =
|
query =
|
||||||
Mv.Membership.Member
|
Mv.Membership.Member
|
||||||
|> filter(id == ^id)
|
|> filter(id == ^id)
|
||||||
|
|
@ -236,12 +238,35 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
"""
|
"""
|
||||||
end
|
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
|
# Helper Functions
|
||||||
# -----------------------------------------------------------------
|
# -----------------------------------------------------------------
|
||||||
|
|
||||||
defp display_value(nil), do: ""
|
defp display_value(nil), do: render_empty_value()
|
||||||
defp display_value(""), do: ""
|
defp display_value(""), do: render_empty_value()
|
||||||
defp display_value(value), do: value
|
defp display_value(value), do: value
|
||||||
|
|
||||||
defp format_address(member) do
|
defp format_address(member) do
|
||||||
|
|
@ -272,20 +297,31 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
|
|
||||||
defp format_date(date), do: to_string(date)
|
defp format_date(date), do: to_string(date)
|
||||||
|
|
||||||
# Sorts custom field values by custom field name
|
# Finds custom field value for a given custom field id
|
||||||
defp sort_custom_field_values(custom_field_values) do
|
defp find_custom_field_value(nil, _custom_field_id), do: nil
|
||||||
Enum.sort_by(custom_field_values, fn cfv ->
|
|
||||||
(cfv.custom_field && cfv.custom_field.name) || ""
|
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)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp find_custom_field_value(_custom_field_values, _custom_field_id), do: nil
|
||||||
|
|
||||||
# Formats custom field value based on type
|
# 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
|
defp format_custom_field_value(%Ash.Union{value: value, type: type}, _expected_type) do
|
||||||
format_custom_field_value(value, type)
|
format_custom_field_value(value, type)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp format_custom_field_value(nil, _type), do: "—"
|
|
||||||
|
|
||||||
defp format_custom_field_value(value, :boolean) when is_boolean(value) do
|
defp format_custom_field_value(value, :boolean) when is_boolean(value) do
|
||||||
if value, do: gettext("Yes"), else: gettext("No")
|
if value, do: gettext("Yes"), else: gettext("No")
|
||||||
end
|
end
|
||||||
|
|
@ -295,20 +331,38 @@ defmodule MvWeb.MemberLive.Show do
|
||||||
end
|
end
|
||||||
|
|
||||||
defp format_custom_field_value(value, :email) when is_binary(value) do
|
defp format_custom_field_value(value, :email) when is_binary(value) do
|
||||||
|
if String.trim(value) == "" do
|
||||||
|
render_empty_value()
|
||||||
|
else
|
||||||
assigns = %{email: value}
|
assigns = %{email: value}
|
||||||
|
|
||||||
~H"""
|
~H"""
|
||||||
<a href={"mailto:#{@email}"} class="text-blue-700 hover:text-blue-800 underline">{@email}</a>
|
<.mailto_link email={@email} display={@email} />
|
||||||
"""
|
"""
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
defp format_custom_field_value(value, :integer) when is_integer(value) do
|
defp format_custom_field_value(value, :integer) when is_integer(value) do
|
||||||
Integer.to_string(value)
|
Integer.to_string(value)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp format_custom_field_value(value, _type) when is_binary(value) do
|
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
|
end
|
||||||
|
|
||||||
defp format_custom_field_value(value, _type), do: to_string(value)
|
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
|
end
|
||||||
|
|
|
||||||
|
|
@ -1422,6 +1422,11 @@ msgstr "Jährliches Intervall – Beitrittszeitraum nicht einbezogen"
|
||||||
msgid "Yearly Interval - Joining Cycle Included"
|
msgid "Yearly Interval - Joining Cycle Included"
|
||||||
msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen"
|
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
|
#~ #: lib/mv_web/live/custom_field_live/show.ex
|
||||||
#~ #, elixir-autogen, elixir-format
|
#~ #, elixir-autogen, elixir-format
|
||||||
#~ msgid "Auto-generated identifier (immutable)"
|
#~ msgid "Auto-generated identifier (immutable)"
|
||||||
|
|
@ -1494,12 +1499,6 @@ msgstr "Jährliches Intervall – Beitrittszeitraum einbezogen"
|
||||||
#~ msgid "New Custom field"
|
#~ msgid "New Custom field"
|
||||||
#~ msgstr "Benutzerdefiniertes Feld speichern"
|
#~ 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
|
#~ #: lib/mv_web/live/contribution_settings_live.ex
|
||||||
#~ #, elixir-autogen, elixir-format
|
#~ #, elixir-autogen, elixir-format
|
||||||
#~ msgid "Quarterly Interval - Joining Period Excluded"
|
#~ msgid "Quarterly Interval - Joining Period Excluded"
|
||||||
|
|
|
||||||
|
|
@ -1422,3 +1422,8 @@ msgstr ""
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Yearly Interval - Joining Cycle Included"
|
msgid "Yearly Interval - Joining Cycle Included"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/mv_web/live/member_live/show.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Not set"
|
||||||
|
msgstr ""
|
||||||
|
|
|
||||||
|
|
@ -1423,6 +1423,11 @@ msgstr ""
|
||||||
msgid "Yearly Interval - Joining Cycle Included"
|
msgid "Yearly Interval - Joining Cycle Included"
|
||||||
msgstr ""
|
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
|
#~ #: lib/mv_web/live/custom_field_live/show.ex
|
||||||
#~ #, elixir-autogen, elixir-format
|
#~ #, elixir-autogen, elixir-format
|
||||||
#~ msgid "Auto-generated identifier (immutable)"
|
#~ msgid "Auto-generated identifier (immutable)"
|
||||||
|
|
@ -1495,11 +1500,6 @@ msgstr ""
|
||||||
#~ msgid "New Custom field"
|
#~ msgid "New Custom field"
|
||||||
#~ msgstr ""
|
#~ 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
|
#~ #: lib/mv_web/live/contribution_settings_live.ex
|
||||||
#~ #, elixir-autogen, elixir-format
|
#~ #, elixir-autogen, elixir-format
|
||||||
#~ msgid "Quarterly Interval - Joining Period Excluded"
|
#~ msgid "Quarterly Interval - Joining Period Excluded"
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,8 @@ defmodule MvWeb.MemberLive.IndexFieldVisibilityTest do
|
||||||
- Integration with member list display
|
- Integration with member list display
|
||||||
- Custom fields visibility
|
- 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
|
import Phoenix.LiveViewTest
|
||||||
require Ash.Query
|
require Ash.Query
|
||||||
|
|
|
||||||
175
test/mv_web/member_live/show_test.exs
Normal file
175
test/mv_web/member_live/show_test.exs
Normal file
|
|
@ -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
|
||||||
|
|
@ -1,5 +1,6 @@
|
||||||
defmodule MvWeb.UserLive.FormTest do
|
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
|
import Phoenix.LiveViewTest
|
||||||
|
|
||||||
# Helper to setup authenticated connection and live view
|
# Helper to setup authenticated connection and live view
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue