Apply review feedback and fix Credo in fee type filter
All checks were successful
continuous-integration/drone/push Build is passing
continuous-integration/drone/promote/production Build is passing

- Index: use FilterParams and constants; fix parse recursion; validate fee type/group
  IDs; OR semantics for :in; build_query_params/reset_all_filters map-based API;
  alias order (Credo); Map.take list deprecation fix
- MemberFilterComponent: use FilterParams and constants; fee_type_filter_part
  helper (Credo nesting); in_not_in_filter_label_class; reset_all_filters map;
  button label for :not_in and combined filter count; fieldset borders
- Gettext: Fee types, filter count plural, 'without %{name}' (en/de)
This commit is contained in:
Moritz 2026-03-09 14:28:50 +01:00 committed by moritz
parent ae07e3efc2
commit 8da22b3d88
5 changed files with 233 additions and 245 deletions

View file

@ -34,8 +34,10 @@ defmodule MvWeb.Components.MemberFilterComponent do
"""
use MvWeb, :live_component
@group_filter_prefix "group_"
@fee_type_filter_prefix "fee_type_"
alias MvWeb.MemberLive.Index.FilterParams
@group_filter_prefix Mv.Constants.group_filter_prefix()
@fee_type_filter_prefix Mv.Constants.fee_type_filter_prefix()
@impl true
def mount(socket) do
@ -201,7 +203,7 @@ defmodule MvWeb.Components.MemberFilterComponent do
<div class="max-h-60 overflow-y-auto pr-2">
<fieldset
:for={group <- @groups}
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-b border-base-200 last:border-0 border-0 p-0 m-0 min-w-0"
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-0 border-b border-base-200 last:border-b-0 p-0 m-0 min-w-0"
>
<legend class="text-sm font-medium col-start-1 float-left w-auto">
{group.name}
@ -268,7 +270,7 @@ defmodule MvWeb.Components.MemberFilterComponent do
<div class="max-h-60 overflow-y-auto pr-2">
<fieldset
:for={fee_type <- @fee_types}
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-b border-base-200 last:border-0 border-0 p-0 m-0 min-w-0"
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-0 border-b border-base-200 last:border-b-0 p-0 m-0 min-w-0"
>
<legend class="text-sm font-medium col-start-1 float-left w-auto">
{fee_type.name}
@ -335,7 +337,7 @@ defmodule MvWeb.Components.MemberFilterComponent do
<div class="max-h-60 overflow-y-auto pr-2">
<fieldset
:for={custom_field <- @boolean_custom_fields}
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-b border-base-200 last:border-0 border-0 p-0 m-0 min-w-0"
class="grid grid-cols-[1fr_auto] items-center gap-3 py-2 border-0 border-b border-base-200 last:border-b-0 p-0 m-0 min-w-0"
>
<legend class="text-sm font-medium col-start-1 float-left w-auto">
{custom_field.name}
@ -436,10 +438,10 @@ defmodule MvWeb.Components.MemberFilterComponent do
payment_filter = parse_payment_filter(params)
group_filters_parsed =
parse_prefix_filters(params, @group_filter_prefix, &parse_group_filter_value/1)
parse_prefix_filters(params, @group_filter_prefix, &FilterParams.parse_in_not_in_value/1)
fee_type_filters_parsed =
parse_prefix_filters(params, @fee_type_filter_prefix, &parse_fee_type_filter_value/1)
parse_prefix_filters(params, @fee_type_filter_prefix, &FilterParams.parse_in_not_in_value/1)
custom_boolean_filters_parsed = parse_custom_boolean_filters(params)
@ -455,7 +457,16 @@ defmodule MvWeb.Components.MemberFilterComponent do
def handle_event("reset_filters", _params, socket) do
# Send single message to reset all filters at once (performance optimization)
# This avoids N×2 load_members() calls when resetting multiple filters
send(self(), {:reset_all_filters, nil, %{}, %{}, %{}})
send(
self(),
{:reset_all_filters,
%{
cycle_status_filter: nil,
boolean_filters: %{},
group_filters: %{},
fee_type_filters: %{}
}}
)
# Close dropdown after reset
{:noreply, assign(socket, :open, false)}
@ -467,14 +478,6 @@ defmodule MvWeb.Components.MemberFilterComponent do
defp parse_tri_state("all"), do: nil
defp parse_tri_state(_), do: nil
defp parse_group_filter_value("in"), do: :in
defp parse_group_filter_value("not_in"), do: :not_in
defp parse_group_filter_value(_), do: nil
defp parse_fee_type_filter_value("in"), do: :in
defp parse_fee_type_filter_value("not_in"), do: :not_in
defp parse_fee_type_filter_value(_), do: nil
defp parse_payment_filter(params) do
case Map.get(params, "payment_filter") do
"paid" -> :paid
@ -550,24 +553,53 @@ defmodule MvWeb.Components.MemberFilterComponent do
boolean_custom_fields,
boolean_filters
) do
cond do
cycle_status_filter ->
payment_filter_label(cycle_status_filter)
active_count =
count_active_filter_categories(
cycle_status_filter,
group_filters,
fee_type_filters,
boolean_filters
)
map_size(group_filters) > 0 ->
group_filters_label(groups, group_filters)
if active_count >= 2 do
ngettext("%{count} filter active", "%{count} filters active", active_count,
count: active_count
)
else
cond do
cycle_status_filter ->
payment_filter_label(cycle_status_filter)
map_size(fee_type_filters) > 0 ->
fee_type_filters_label(fee_types, fee_type_filters)
map_size(group_filters) > 0 ->
group_filters_label(groups, group_filters)
map_size(boolean_filters) > 0 ->
boolean_filter_label(boolean_custom_fields, boolean_filters)
map_size(fee_type_filters) > 0 ->
fee_type_filters_label(fee_types, fee_type_filters)
true ->
gettext("Apply filters")
map_size(boolean_filters) > 0 ->
boolean_filter_label(boolean_custom_fields, boolean_filters)
true ->
gettext("Apply filters")
end
end
end
defp count_active_filter_categories(
cycle_status_filter,
group_filters,
fee_type_filters,
boolean_filters
) do
[
cycle_status_filter,
map_size(group_filters) > 0,
map_size(fee_type_filters) > 0,
map_size(boolean_filters) > 0
]
|> Enum.count(& &1)
end
defp group_filters_label(_groups, group_filters) when map_size(group_filters) == 0,
do: gettext("All")
@ -589,15 +621,22 @@ defmodule MvWeb.Components.MemberFilterComponent do
defp fee_type_filters_label(fee_types, fee_type_filters) do
fee_types_by_id = Map.new(fee_types, fn ft -> {to_string(ft.id), ft.name} end)
names =
parts =
fee_type_filters
|> Enum.map(fn {fee_type_id_str, _} -> Map.get(fee_types_by_id, fee_type_id_str) end)
|> Enum.map(fn {fee_type_id_str, value} ->
fee_type_filter_part(Map.get(fee_types_by_id, fee_type_id_str), value)
end)
|> Enum.reject(&is_nil/1)
label = Enum.join(names, ", ")
label = Enum.join(parts, ", ")
truncate_label(label, 30)
end
defp fee_type_filter_part(nil, _value), do: nil
defp fee_type_filter_part(name, :not_in), do: gettext("without %{name}", name: name)
defp fee_type_filter_part(name, _), do: name
# Get payment filter label
defp payment_filter_label(nil), do: gettext("All")
defp payment_filter_label(:paid), do: gettext("Paid")
@ -671,70 +710,27 @@ defmodule MvWeb.Components.MemberFilterComponent do
end
end
# Get CSS classes for per-group filter label based on current state
defp group_filter_label_class(group_filters, group_id, expected_value) do
# Shared CSS classes for in/not_in filter labels (groups and fee types)
defp in_not_in_filter_label_class(filters, id, expected_value) do
base_classes = "join-item btn btn-sm"
current_value = Map.get(group_filters, to_string(group_id))
current_value = Map.get(filters, to_string(id))
is_active = current_value == expected_value
cond do
expected_value == nil ->
if is_active do
"#{base_classes} btn-active"
else
"#{base_classes} btn"
end
expected_value == :in ->
if is_active do
"#{base_classes} btn-success btn-active"
else
"#{base_classes} btn"
end
expected_value == :not_in ->
if is_active do
"#{base_classes} btn-error btn-active"
else
"#{base_classes} btn"
end
true ->
"#{base_classes} btn-outline"
case {expected_value, is_active} do
{_, false} -> "#{base_classes} btn"
{nil, true} -> "#{base_classes} btn-active"
{:in, true} -> "#{base_classes} btn-success btn-active"
{:not_in, true} -> "#{base_classes} btn-error btn-active"
_ -> "#{base_classes} btn-outline"
end
end
# Get CSS classes for per-fee-type filter label based on current state
defp group_filter_label_class(group_filters, group_id, expected_value) do
in_not_in_filter_label_class(group_filters, group_id, expected_value)
end
defp fee_type_filter_label_class(fee_type_filters, fee_type_id, expected_value) do
base_classes = "join-item btn btn-sm"
current_value = Map.get(fee_type_filters, to_string(fee_type_id))
is_active = current_value == expected_value
cond do
expected_value == nil ->
if is_active do
"#{base_classes} btn-active"
else
"#{base_classes} btn"
end
expected_value == :in ->
if is_active do
"#{base_classes} btn-success btn-active"
else
"#{base_classes} btn"
end
expected_value == :not_in ->
if is_active do
"#{base_classes} btn-error btn-active"
else
"#{base_classes} btn"
end
true ->
"#{base_classes} btn-outline"
end
in_not_in_filter_label_class(fee_type_filters, fee_type_id, expected_value)
end
# Get CSS classes for boolean filter label based on current state