Merge pull request 'Fix sort by custom date closes #496' (#528) from issue/mitgliederverwaltung-496 into main

Reviewed-on: #528

CHANGELOG.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **CSV import – fee-status columns ignored** – Columns such as `Bezahlstatus` / `Membership Fee Status` are always ignored on import and never stored as a custom-field value, even when a custom field of the same name exists.
 - **Column-header tooltips clipped** – Tooltips on the members-overview column headers are no longer clipped by the sticky table header.
 - **Text selection opens member** – Dragging to select text in a members-overview row (for example to copy an email) no longer opens the member details; a plain click still opens them.
+- **Sort by custom date** – Sorting the member list or member export by a custom date field now orders rows chronologically instead of like text, so e.g. 29.01.1981 correctly comes before 01.03.1982.
 
 ## [1.2.0] - 2026-05-08
 

lib/mv/membership/custom_field_sort.ex

@@ -0,0 +1,30 @@
+defmodule Mv.Membership.CustomFieldSort do
+  @moduledoc """
+  Derives a term-order-comparable sort key from a custom-field value.
+
+  Custom-field values are stored in an Ash `:union` attribute, so a value
+  arrives either as an `%Ash.Union{}` or as its already-unwrapped term. This
+  module is the single source of truth for turning such a value into a key that
+  `Enum.sort_by/2` can order correctly per `value_type`.
+
+  nil / empty handling is intentionally NOT this function's concern — the call
+  sites split present from absent values before sorting.
+  """
+
+  @doc """
+  Returns a term-order-comparable sort key for `value`, given its `value_type`.
+
+  For every non-date type the natural unwrapped value is returned, so
+  `:integer` sorts numerically and `:string` / `:email` sort lexicographically.
+  """
+  def sort_key(%Ash.Union{value: value, type: type}, _expected_type),
+    do: sort_key(value, type)
+
+  def sort_key(value, :string) when is_binary(value), do: value
+  def sort_key(value, :integer) when is_integer(value), do: value
+  def sort_key(value, :boolean) when is_boolean(value), do: value
+  # Gregorian day count is a chronological integer key (earlier date ⇒ smaller).
+  def sort_key(%Date{} = date, :date), do: Date.to_gregorian_days(date)
+  def sort_key(value, :email) when is_binary(value), do: value
+  def sort_key(value, _type), do: to_string(value)
+end

lib/mv_web/controllers/member_export_controller.ex

@@ -12,6 +12,7 @@ defmodule MvWeb.MemberExportController do
 
   alias Mv.Authorization.Actor
   alias Mv.Membership.CustomField
+  alias Mv.Membership.CustomFieldSort
   alias Mv.Membership.Member
   alias Mv.Membership.MemberExport
   alias Mv.Membership.MembersCSV
@@ -523,18 +524,22 @@ defmodule MvWeb.MemberExportController do
         false
 
       cfv ->
-        extracted = extract_sort_value(cfv.value, custom_field.value_type)
+        not empty_custom_field_value?(cfv.value, custom_field.value_type)
-        not empty_value?(extracted, custom_field.value_type)
     end
   end
 
-  defp empty_value?(nil, _type), do: true
+  defp empty_custom_field_value?(%Ash.Union{value: value, type: type}, _expected_type) do
+    empty_custom_field_value?(value, type)
+  end
 
-  defp empty_value?(value, type) when type in [:string, :email] and is_binary(value) do
+  defp empty_custom_field_value?(nil, _type), do: true
+
+  defp empty_custom_field_value?(value, type)
+       when type in [:string, :email] and is_binary(value) do
     String.trim(value) == ""
   end
 
-  defp empty_value?(_value, _type), do: false
+  defp empty_custom_field_value?(_value, _type), do: false
 
   defp find_cfv(member, custom_field) do
     (member.custom_field_values || [])
@@ -548,7 +553,7 @@ defmodule MvWeb.MemberExportController do
   defp extract_member_sort_value(member, custom_field) do
     case find_cfv(member, custom_field) do
       nil -> nil
-      cfv -> extract_sort_value(cfv.value, custom_field.value_type)
+      cfv -> CustomFieldSort.sort_key(cfv.value, custom_field.value_type)
     end
   end
 
@@ -670,15 +675,4 @@ defmodule MvWeb.MemberExportController do
     |> String.split()
     |> Enum.map_join(" ", &String.capitalize/1)
   end
-
-  defp extract_sort_value(%Ash.Union{value: value, type: type}, _),
-    do: extract_sort_value(value, type)
-
-  defp extract_sort_value(nil, _), do: nil
-  defp extract_sort_value(value, :string) when is_binary(value), do: value
-  defp extract_sort_value(value, :integer) when is_integer(value), do: value
-  defp extract_sort_value(value, :boolean) when is_boolean(value), do: value
-  defp extract_sort_value(%Date{} = d, :date), do: d
-  defp extract_sort_value(value, :email) when is_binary(value), do: value
-  defp extract_sort_value(value, _), do: to_string(value)
 end

lib/mv_web/live/member_live/index.ex

@@ -32,6 +32,7 @@ defmodule MvWeb.MemberLive.Index do
   import MvWeb.LiveHelpers, only: [current_actor: 1]
 
   alias Mv.Membership
+  alias Mv.Membership.CustomFieldSort
   alias Mv.Membership.Member, as: MemberResource
   alias Mv.MembershipFees
   alias Mv.MembershipFees.MembershipFeeType
@@ -1414,8 +1415,7 @@ defmodule MvWeb.MemberLive.Index do
         false
 
       cfv ->
-        extracted = extract_sort_value(cfv.value, custom_field.value_type)
+        not empty_value?(cfv.value, custom_field.value_type)
-        not empty_value?(extracted, custom_field.value_type)
     end
   end
 
@@ -1423,29 +1423,22 @@ defmodule MvWeb.MemberLive.Index do
     sorted =
       Enum.sort_by(members_with_values, fn member ->
         cfv = get_custom_field_value(member, custom_field)
-        extracted = extract_sort_value(cfv.value, custom_field.value_type)
+        CustomFieldSort.sort_key(cfv.value, custom_field.value_type)
-        normalize_sort_value(extracted, order)
       end)
 
     if order == :desc, do: Enum.reverse(sorted), else: sorted
   end
 
-  defp extract_sort_value(%Ash.Union{value: value, type: type}, _expected_type),
+  defp empty_value?(%Ash.Union{value: value, type: type}, _expected_type),
-    do: extract_sort_value(value, type)
+    do: empty_value?(value, type)
 
-  defp extract_sort_value(value, :string) when is_binary(value), do: value
+  defp empty_value?(nil, _type), do: true
-  defp extract_sort_value(value, :integer) when is_integer(value), do: value
+
-  defp extract_sort_value(value, :boolean) when is_boolean(value), do: value
+  defp empty_value?(value, type) when type in [:string, :email] and is_binary(value),
-  defp extract_sort_value(%Date{} = date, :date), do: date
+    do: String.trim(value) == ""
-  defp extract_sort_value(value, :email) when is_binary(value), do: value
-  defp extract_sort_value(value, _type), do: to_string(value)
 
-  defp empty_value?(value, :string) when is_binary(value), do: String.trim(value) == ""
-  defp empty_value?(value, :email) when is_binary(value), do: String.trim(value) == ""
   defp empty_value?(_value, _type), do: false
 
-  defp normalize_sort_value(value, _order), do: value
-
   defp maybe_update_sort(socket, %{"sort_field" => sf, "sort_order" => so}) do
     field = determine_field(socket.assigns.sort_field, sf)
     order = determine_order(socket.assigns.sort_order, so)

test/mv/membership/custom_field_sort_property_test.exs

@@ -0,0 +1,65 @@
+defmodule Mv.Membership.CustomFieldSortPropertyTest do
+  use ExUnit.Case, async: true
+  use ExUnitProperties
+
+  alias Mv.Membership.CustomFieldSort
+
+  defp date_generator do
+    gen all(
+          year <- integer(1..9999),
+          month <- integer(1..12),
+          day <- integer(1..28)
+        ) do
+      Date.new!(year, month, day)
+    end
+  end
+
+  # The production load path always delivers a :date value as
+  # %Ash.Union{type: :date, value: %Date{}}, so the property exercises both the
+  # bare %Date{} form and the union-wrapped form to pin the union-dispatch clause.
+  defp shape_generator do
+    member_of([:bare, :union])
+  end
+
+  defp wrap_date(date, :bare), do: date
+  defp wrap_date(date, :union), do: %Ash.Union{type: :date, value: date}
+
+  property "sort_key/2 orders :date values chronologically in both directions" do
+    check all(
+            raw_dates <- list_of(date_generator(), min_length: 1),
+            shape <- shape_generator()
+          ) do
+      values = Enum.map(raw_dates, &wrap_date(&1, shape))
+
+      ascending = Enum.sort_by(values, &CustomFieldSort.sort_key(&1, :date))
+      ascending_dates = Enum.map(ascending, &unwrap/1)
+      descending_dates = Enum.reverse(ascending_dates)
+
+      assert non_decreasing?(ascending_dates)
+      assert non_increasing?(descending_dates)
+
+      # The key must be an integer Gregorian-day count, not a stringified date:
+      # this pins the dedicated :date branch and guards against a string-coerced
+      # key whose chronological correctness would silently depend on zero-padded
+      # ISO formatting.
+      Enum.each(values, fn value ->
+        assert CustomFieldSort.sort_key(value, :date) == Date.to_gregorian_days(unwrap(value))
+      end)
+    end
+  end
+
+  defp unwrap(%Ash.Union{value: value}), do: value
+  defp unwrap(%Date{} = date), do: date
+
+  defp non_decreasing?(dates) do
+    dates
+    |> Enum.chunk_every(2, 1, :discard)
+    |> Enum.all?(fn [a, b] -> Date.compare(a, b) != :gt end)
+  end
+
+  defp non_increasing?(dates) do
+    dates
+    |> Enum.chunk_every(2, 1, :discard)
+    |> Enum.all?(fn [a, b] -> Date.compare(a, b) != :lt end)
+  end
+end

test/mv/membership/custom_field_sort_test.exs

@@ -0,0 +1,29 @@
+defmodule Mv.Membership.CustomFieldSortTest do
+  use ExUnit.Case, async: true
+
+  alias Mv.Membership.CustomFieldSort
+
+  describe "sort_key/2" do
+    test "keeps :integer values numerically comparable" do
+      values = [10, 100, 2]
+
+      sorted = Enum.sort_by(values, &CustomFieldSort.sort_key(&1, :integer))
+
+      assert sorted == [2, 10, 100]
+    end
+
+    test "passes :string values through to their natural term-order key" do
+      assert CustomFieldSort.sort_key("Zebra", :string) == "Zebra"
+    end
+
+    test "passes :email values through to their natural term-order key" do
+      assert CustomFieldSort.sort_key("a@example.com", :email) == "a@example.com"
+    end
+
+    test "unwraps an %Ash.Union{} value before deriving the key" do
+      union = %Ash.Union{type: :integer, value: 42}
+
+      assert CustomFieldSort.sort_key(union, :integer) == 42
+    end
+  end
+end

test/mv_web/member_live/index_custom_fields_sorting_test.exs

@@ -231,6 +231,63 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do
     assert has_element?(view, "[data-testid='custom_field_#{field.id}'][aria-label='ascending']")
   end
 
+  test "sorts members chronologically by a :date custom field (ascending)", %{conn: conn} do
+    system_actor = Mv.Helpers.SystemActor.get_system_actor()
+
+    # Dates chosen to expose the day-first term-ordering trap: term order of the
+    # %Date{} structs compares day, then month, then year, which would place
+    # 02.07.1986 (day 02) before 29.01.1981 (day 29). Chronologically 1981 < 1982 < 1986.
+    members_and_dates = [
+      {"EightySix", ~D[1986-07-02]},
+      {"EightyOne", ~D[1981-01-29]},
+      {"EightyTwo", ~D[1982-03-01]}
+    ]
+
+    {:ok, field} =
+      CustomField
+      |> Ash.Changeset.for_create(:create, %{
+        name: "birth_date",
+        value_type: :date,
+        show_in_overview: true
+      })
+      |> Ash.create(actor: system_actor)
+
+    for {first_name, date} <- members_and_dates do
+      {:ok, member} =
+        Mv.Membership.create_member(
+          %{
+            first_name: first_name,
+            last_name: "Test",
+            email: "#{String.downcase(first_name)}@example.com"
+          },
+          actor: system_actor
+        )
+
+      {:ok, _cfv} =
+        CustomFieldValue
+        |> Ash.Changeset.for_create(:create, %{
+          member_id: member.id,
+          custom_field_id: field.id,
+          value: %{"_union_type" => "date", "_union_value" => Date.to_iso8601(date)}
+        })
+        |> Ash.create(actor: system_actor)
+    end
+
+    conn = conn_with_oidc_user(conn)
+
+    {:ok, view, _html} =
+      live(conn, "/members?query=&sort_field=custom_field_#{field.id}&sort_order=asc")
+
+    html = render(view)
+
+    {one_idx, _} = :binary.match(html, "EightyOne")
+    {two_idx, _} = :binary.match(html, "EightyTwo")
+    {six_idx, _} = :binary.match(html, "EightySix")
+
+    assert one_idx < two_idx, "29.01.1981 must come before 01.03.1982 in ascending order"
+    assert two_idx < six_idx, "01.03.1982 must come before 02.07.1986 in ascending order"
+  end
+
   test "NULL values and empty strings are always sorted last (ASC)", %{conn: conn} do
     system_actor = Mv.Helpers.SystemActor.get_system_actor()