Fix sort by custom date closes #496 #528

Merged
moritz merged 4 commits from issue/mitgliederverwaltung-496 into main 2026-06-15 16:34:41 +02:00
7 changed files with 202 additions and 33 deletions

View file

@ -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. - **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. - **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. - **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 ## [1.2.0] - 2026-05-08

View file

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

View file

@ -12,6 +12,7 @@ defmodule MvWeb.MemberExportController do
alias Mv.Authorization.Actor alias Mv.Authorization.Actor
alias Mv.Membership.CustomField alias Mv.Membership.CustomField
alias Mv.Membership.CustomFieldSort
alias Mv.Membership.Member alias Mv.Membership.Member
alias Mv.Membership.MemberExport alias Mv.Membership.MemberExport
alias Mv.Membership.MembersCSV alias Mv.Membership.MembersCSV
@ -523,18 +524,22 @@ defmodule MvWeb.MemberExportController do
false false
cfv -> 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
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) == "" String.trim(value) == ""
end end
defp empty_value?(_value, _type), do: false defp empty_custom_field_value?(_value, _type), do: false
defp find_cfv(member, custom_field) do defp find_cfv(member, custom_field) do
(member.custom_field_values || []) (member.custom_field_values || [])
@ -548,7 +553,7 @@ defmodule MvWeb.MemberExportController do
defp extract_member_sort_value(member, custom_field) do defp extract_member_sort_value(member, custom_field) do
case find_cfv(member, custom_field) do case find_cfv(member, custom_field) do
nil -> nil nil -> nil
cfv -> extract_sort_value(cfv.value, custom_field.value_type) cfv -> CustomFieldSort.sort_key(cfv.value, custom_field.value_type)
end end
end end
@ -670,15 +675,4 @@ defmodule MvWeb.MemberExportController do
|> String.split() |> String.split()
|> Enum.map_join(" ", &String.capitalize/1) |> Enum.map_join(" ", &String.capitalize/1)
end 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 end

View file

@ -32,6 +32,7 @@ defmodule MvWeb.MemberLive.Index do
import MvWeb.LiveHelpers, only: [current_actor: 1] import MvWeb.LiveHelpers, only: [current_actor: 1]
alias Mv.Membership alias Mv.Membership
alias Mv.Membership.CustomFieldSort
alias Mv.Membership.Member, as: MemberResource alias Mv.Membership.Member, as: MemberResource
alias Mv.MembershipFees alias Mv.MembershipFees
alias Mv.MembershipFees.MembershipFeeType alias Mv.MembershipFees.MembershipFeeType
@ -1414,8 +1415,7 @@ defmodule MvWeb.MemberLive.Index do
false false
cfv -> 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
end end
@ -1423,29 +1423,22 @@ defmodule MvWeb.MemberLive.Index do
sorted = sorted =
Enum.sort_by(members_with_values, fn member -> Enum.sort_by(members_with_values, fn member ->
cfv = get_custom_field_value(member, custom_field) 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) end)
if order == :desc, do: Enum.reverse(sorted), else: sorted if order == :desc, do: Enum.reverse(sorted), else: sorted
end 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 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 defp maybe_update_sort(socket, %{"sort_field" => sf, "sort_order" => so}) do
field = determine_field(socket.assigns.sort_field, sf) field = determine_field(socket.assigns.sort_field, sf)
order = determine_order(socket.assigns.sort_order, so) order = determine_order(socket.assigns.sort_order, so)

View file

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

View file

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

View file

@ -231,6 +231,63 @@ defmodule MvWeb.MemberLive.IndexCustomFieldsSortingTest do
assert has_element?(view, "[data-testid='custom_field_#{field.id}'][aria-label='ascending']") assert has_element?(view, "[data-testid='custom_field_#{field.id}'][aria-label='ascending']")
end 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 test "NULL values and empty strings are always sorted last (ASC)", %{conn: conn} do
system_actor = Mv.Helpers.SystemActor.get_system_actor() system_actor = Mv.Helpers.SystemActor.get_system_actor()