local-it/mitgliederverwaltung
2
3
Fork 0
Code Issues 52 Pull requests 2 Projects 1 Releases 4 Packages 1 Activity Actions

Improve UX of join requests and fix minor bugs (#492)
All checks were successful
continuous-integration/drone/push Build is passing
Details

Browse source 
## Description of the implemented changes
The changes were:
- [x] Bugfixing
- [x] New Feature
- [ ] Breaking Change
- [ ] Refactoring

This PR improves the join-request flow and presentation quality, fixes several data-display issues in join/join-request screens, and adds a usability improvement in global settings (directly opening the join link). It also includes dependency updates and changelog maintenance.

## What has been changed?
- Join form (`JoinLive`) now renders inputs based on actual field types (including checkbox/date/number/email behavior instead of generic text-only handling).
- Join form custom-field labels are resolved from configured custom fields (fallback remains safe if lookup fails).
- Join-request details page (`JoinRequestLive.Show`) now:
  - resolves and shows custom field names instead of raw IDs,
  - formats boolean-like values (`on/true/1`, `off/false/0`) as localized `Yes/No`,
  - formats ISO date strings for better readability,
  - keeps legacy field handling while improving output consistency.
- Join-request detail layout was improved semantically and visually (`dl/dt/dd` structure for label/value rows).
- Global settings page now includes an **Open** button for the join URL (`target="_blank"`, `rel="noopener noreferrer"`, ARIA label).
- Added/updated tests around:
  - join field type rendering,
  - custom field labels in join-request views,
  - related auth/global-settings behavior.
- Updated translations (`default.pot`, `en`, `de`) for new UI strings.
- Updated dependencies/tooling (`mix.lock`, `mix.exs`, CI/renovate-related updates).
- Updated `CHANGELOG.md` entries for unreleased changes.

## Definition of Done
### Code Quality
- [x] No new technical depths
- [x] Linting passed
- [x] Documentation is added were needed

### Accessibility
- [x] New elements are properly defined with html-tags
- [x] Colour contrast follows WCAG criteria
- [x] Aria labels are added when needed
- [x] Everything is accessible by keyboard
- [x] Tab-Order is comprehensible
- [x] All interactive elements have a visible focus

### Testing
- [x] Tests for new code are written
- [ ] All tests pass
- [ ] axe-core dev tools show no critical or major issues

## Additional Notes
- Reviewer focus areas:
  - `lib/mv_web/live/join_live.ex`: input type derivation and custom field lookup strategy (`authorize?: false` read path used intentionally for field metadata).
  - `lib/mv_web/live/join_request_live/show.ex`: value-formatting logic (especially backward compatibility for legacy `form_data` payloads).
  - `lib/mv_web/live/global_settings_live.ex`: external-link behavior and accessibility attributes.
- The branch also contains dependency update commits; please review lockfile and CI-related changes separately from functional join/join-request changes.

Reviewed-on: #492
Co-authored-by: Simon <s.thiessen@local-it.org>
Co-committed-by: Simon <s.thiessen@local-it.org>
This commit is contained in:
Simon 2026-05-06 14:34:42 +02:00 • committed by simon
parent bfa33dcae2
commit 2bb01bd201
14 changed files with 781 additions and 135 deletions
Download patch file Download diff file Expand all files Collapse all files

56
lib/mv/membership/custom_field_lookup.ex Normal file
View file

@ -0,0 +1,56 @@
defmodule Mv.Membership.CustomFieldLookup do
@moduledoc """
Shared helper for loading custom fields by ID.
"""
alias Mv.Constants
alias Mv.Membership
@spec fetch_map_by_ids([String.t()], keyword()) :: map()
def fetch_map_by_ids(field_ids, opts \\ []) when is_list(field_ids) do
member_field_strings = Constants.member_fields() |> Enum.map(&Atom.to_string/1)
custom_field_ids =
field_ids
|> Enum.uniq()
|> Enum.reject(&(&1 in member_field_strings))
if custom_field_ids == [] do
%{}
else
select = Keyword.get(opts, :select, [:id, :name, :value_type])
query =
Membership.CustomField
|> Ash.Query.select(select)
read_opts =
[domain: Membership]
|> maybe_put_actor(opts)
|> maybe_put_authorize(opts)
case Ash.read(query, read_opts) do
{:ok, fields} ->
allowed_ids = MapSet.new(custom_field_ids)
fields |> Enum.filter(&MapSet.member?(allowed_ids, &1.id)) |> Map.new(&{&1.id, &1})
{:error, _} ->
%{}
end
end
end
defp maybe_put_actor(opts, read_opts) do
case Keyword.fetch(read_opts, :actor) do
{:ok, actor} -> Keyword.put(opts, :actor, actor)
:error -> opts
end
end
defp maybe_put_authorize(opts, read_opts) do
case Keyword.fetch(read_opts, :authorize?) do
{:ok, authorize?} -> Keyword.put(opts, :authorize?, authorize?)
:error -> opts
end
end
end

2
lib/mv_web/components/layouts.ex
View file

@ -138,7 +138,7 @@ defmodule MvWeb.Layouts do
# Single get_settings() for layout; derive club_name and join_form_enabled to avoid duplicate query.
%{club_name: club_name, join_form_enabled: join_form_enabled} = get_layout_settings()
# TODO: unprocessed count runs on every page load when join form enabled; consider
# NOTE: Unprocessed count runs on every page load when join form is enabled; consider
# loading only on navigation or caching briefly if performance becomes an issue.
unprocessed_join_requests_count =
get_unprocessed_join_requests_count(assigns.current_user, join_form_enabled)

10
lib/mv_web/live/global_settings_live.ex
View file

@ -186,6 +186,16 @@ defmodule MvWeb.GlobalSettingsLive do
<.icon name="hero-clipboard-document" class="size-4" />
{gettext("Copy")}
</.button>
<.link
href={@join_url}
target="_blank"
rel="noopener noreferrer"
class="btn btn-secondary btn-sm"
>
<.icon name="hero-arrow-top-right-on-square" class="size-4" aria-hidden="true" />
{pgettext("action", "Open")}
<span class="sr-only">{gettext("join page URL in a new tab")}</span>
</.link>
</div>
</div>

135
lib/mv_web/live/join_live.ex
View file

@ -5,7 +5,9 @@ defmodule MvWeb.JoinLive do
"""
use MvWeb, :live_view
alias Ash.Resource.Info
alias Mv.Membership
alias Mv.Membership.CustomFieldLookup
alias MvWeb.JoinRateLimit
alias MvWeb.Translations.MemberFields
@ -54,10 +56,6 @@ defmodule MvWeb.JoinLive do
{gettext("Become a member")}
</.header>
<p class="text-base-content/80">
{gettext("Please enter your details for the membership application here.")}
</p>
<%= if @submitted do %>
<div data-testid="join-success-message" class="alert alert-success">
<p class="font-medium">
@ -67,6 +65,9 @@ defmodule MvWeb.JoinLive do
</p>
</div>
<% else %>
<p class="text-base-content/80">
{gettext("Please enter your details for the membership application here.")}
</p>
<.form
for={@form}
id="join-form"
@ -80,19 +81,41 @@ defmodule MvWeb.JoinLive do
<% end %>
<%= for field <- @join_fields do %>
<div>
<label for={"join-field-#{field.id}"} class="label">
<span class="label-text">{field.label}{if field.required, do: " *"}</span>
<%= if field.input_type == "checkbox" do %>
<input type="hidden" name={field.id} value="off" />
<label
for={"join-field-#{field.id}"}
class="label cursor-pointer justify-start gap-3"
>
<input
type="checkbox"
name={field.id}
id={"join-field-#{field.id}"}
checked={checkbox_checked?(@form.params[field.id])}
required={field.required}
class="checkbox checkbox-sm"
/>
<span class="label-text">
{field.label}<span :if={field.required} aria-hidden="true"> *</span>
</span>
</label>
<input
type={input_type(field.id)}
name={field.id}
id={"join-field-#{field.id}"}
value={@form.params[field.id]}
required={field.required}
class="input input-bordered w-full"
/>
</div>
<% else %>
<div>
<label for={"join-field-#{field.id}"} class="label">
<span class="label-text">
{field.label}<span :if={field.required} aria-hidden="true"> *</span>
</span>
</label>
<input
type={field.input_type}
name={field.id}
id={"join-field-#{field.id}"}
value={@form.params[field.id]}
required={field.required}
class="input input-bordered w-full"
/>
</div>
<% end %>
<% end %>
<%!--
@ -216,19 +239,32 @@ defmodule MvWeb.JoinLive do
defp build_join_fields_with_labels(allowlist) do
member_field_strings = Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)
custom_field_by_id = custom_field_map(allowlist, member_field_strings)
Enum.map(allowlist, fn %{id: id, required: required} ->
label =
if id in member_field_strings do
MemberFields.label(String.to_existing_atom(id))
else
gettext("Field")
end
%{id: id, label: label, required: required}
build_join_field(id, required, member_field_strings, custom_field_by_id)
end)
end
defp build_join_field(id, required, member_field_strings, custom_field_by_id) do
if id in member_field_strings do
label = MemberFields.label(String.to_existing_atom(id))
%{id: id, label: label, required: required, input_type: member_field_input_type(id)}
else
custom_field = Map.get(custom_field_by_id, id)
label = if custom_field, do: custom_field.name, else: gettext("Field")
input_type = custom_field_input_type(custom_field && custom_field.value_type)
%{id: id, label: label, required: required, input_type: input_type}
end
end
defp custom_field_map(allowlist, _member_field_strings) do
allowlist
|> Enum.map(& &1.id)
|> CustomFieldLookup.fetch_map_by_ids(authorize?: false, select: [:id, :name, :value_type])
end
defp initial_form_params(join_fields) do
join_fields
|> Enum.map(fn f -> {f.id, ""} end)
@ -236,8 +272,42 @@ defmodule MvWeb.JoinLive do
|> Map.put(@honeypot_field, "")
end
defp input_type("email"), do: "email"
defp input_type(_), do: "text"
defp member_field_input_type("email"), do: "email"
defp member_field_input_type(field_id) when is_binary(field_id) do
case member_field_atom(field_id) do
nil ->
"text"
field_atom ->
Mv.Membership.Member
|> Info.attribute(field_atom)
|> Map.get(:type)
|> input_type_for()
end
end
defp member_field_input_type(_), do: "text"
defp member_field_atom(field_id) when is_binary(field_id) do
Mv.Constants.member_fields()
|> Enum.find(&(Atom.to_string(&1) == field_id))
end
defp custom_field_input_type(type), do: input_type_for(type)
defp input_type_for(:date), do: "date"
defp input_type_for(Ash.Type.Date), do: "date"
defp input_type_for(:integer), do: "number"
defp input_type_for(Ash.Type.Integer), do: "number"
defp input_type_for(:boolean), do: "checkbox"
defp input_type_for(Ash.Type.Boolean), do: "checkbox"
defp input_type_for(:email), do: "email"
defp input_type_for(Mv.Membership.Email), do: "email"
defp input_type_for(_), do: "text"
defp checkbox_checked?(value) when value in [true, "true", "on", "1"], do: true
defp checkbox_checked?(_), do: false
defp build_submit_attrs(params, join_fields) do
allowlist_ids = MapSet.new(Enum.map(join_fields, & &1.id))
@ -257,9 +327,12 @@ defmodule MvWeb.JoinLive do
}
form_data =
params
|> Enum.filter(fn {key, _} -> key in allowlist_ids and key not in typed end)
|> Map.new(fn {k, v} -> {k, String.trim(to_string(v))} end)
join_fields
|> Enum.filter(&(&1.id not in typed))
|> Map.new(fn field ->
{field.id, normalize_join_field_value(params[field.id], field.input_type)}
end)
|> Map.take(MapSet.to_list(allowlist_ids))
attrs = %{attrs | form_data: form_data}
{:ok, attrs}
@ -271,6 +344,10 @@ defmodule MvWeb.JoinLive do
if is_binary(v), do: String.trim(v), else: nil
end
defp normalize_join_field_value(raw, _input_type) when is_binary(raw), do: String.trim(raw)
defp normalize_join_field_value(_raw, "checkbox"), do: "off"
defp normalize_join_field_value(_raw, _input_type), do: ""
# Prefer X-Forwarded-For / X-Real-IP when behind a reverse proxy; fall back to peer_data.
# Uses :inet.ntoa/1 for correct IPv4 and IPv6 string representation.
defp client_ip_from_socket(socket) do

206
lib/mv_web/live/join_request_live/show.ex
View file

@ -21,6 +21,7 @@ defmodule MvWeb.JoinRequestLive.Show do
alias Mv.Constants
alias Mv.Membership
alias Mv.Membership.CustomFieldLookup
alias MvWeb.Helpers.DateFormatter
alias MvWeb.JoinRequestLive.Helpers, as: JoinRequestHelpers
alias MvWeb.Translations.MemberFields, as: MemberFieldsTranslations
@ -31,6 +32,7 @@ defmodule MvWeb.JoinRequestLive.Show do
{:ok,
socket
|> assign(:join_request, nil)
|> assign(:custom_field_by_id, %{})
|> assign(:join_form_field_ids, [])
|> Layouts.assign_page_title(gettext("Join request"))}
else
@ -53,9 +55,16 @@ defmodule MvWeb.JoinRequestLive.Show do
{:ok, request} ->
field_ids = Membership.get_join_form_allowlist() |> Enum.map(& &1.id)
custom_field_by_id =
CustomFieldLookup.fetch_map_by_ids(field_ids ++ Map.keys(request.form_data || %{}),
actor: actor,
select: [:id, :name, :value_type]
)
{:noreply,
socket
|> assign(:join_request, request)
|> assign(:custom_field_by_id, custom_field_by_id)
|> assign(:join_form_field_ids, field_ids)
|> Layouts.assign_page_title(gettext("Join request – %{email}", email: request.email))}
@ -131,52 +140,58 @@ defmodule MvWeb.JoinRequestLive.Show do
<%!-- Single block: all applicant-provided data in join form order --%>
<div>
<h2 class="text-lg font-semibold mb-2">{gettext("Applicant data")}</h2>
<div class="border border-base-300 rounded-lg p-4 bg-base-100 space-y-2">
<%= for {label, value} <- applicant_data_rows(@join_request, @join_form_field_ids || []) do %>
<.field_row label={label} value={value} empty_text={gettext("Not specified")} />
<% end %>
<div class="border border-base-300 rounded-lg p-4 bg-base-100">
<dl class="grid gap-1 md:grid-cols-[14rem_minmax(0,1fr)] md:gap-2">
<%= for {label, value} <-
applicant_data_rows(
@join_request,
@join_form_field_ids || [],
@custom_field_by_id || %{}
) do %>
<.field_row label={label} value={value} empty_text={gettext("Not specified")} />
<% end %>
</dl>
</div>
</div>
<%!-- Status and review (submitted_at, status; if decided: approved/rejected at, reviewed by) --%>
<div>
<h2 class="text-lg font-semibold mb-2">{gettext("Status and review")}</h2>
<div class="border border-base-300 rounded-lg p-4 bg-base-100 space-y-2">
<.field_row
label={gettext("Submitted at")}
value={DateFormatter.format_datetime(@join_request.submitted_at, @browser_timezone)}
/>
<div class="flex gap-2">
<span class="text-base-content/60 min-w-32 shrink-0">{gettext("Status")}:</span>
<span>
<div class="border border-base-300 rounded-lg p-4 bg-base-100">
<dl class="grid gap-1 md:grid-cols-[14rem_minmax(0,1fr)] md:gap-2">
<.field_row
label={gettext("Submitted at")}
value={DateFormatter.format_datetime(@join_request.submitted_at, @browser_timezone)}
/>
<.field_row label={gettext("Status")}>
<.badge variant={JoinRequestHelpers.status_badge_variant(@join_request.status)}>
{JoinRequestHelpers.format_status(@join_request.status)}
</.badge>
</span>
</div>
<%= if @join_request.status in [:approved, :rejected] do %>
<%= if @join_request.approved_at do %>
</.field_row>
<%= if @join_request.status in [:approved, :rejected] do %>
<%= if @join_request.approved_at do %>
<.field_row
label={gettext("Approved at")}
value={
DateFormatter.format_datetime(@join_request.approved_at, @browser_timezone)
}
/>
<% end %>
<%= if @join_request.rejected_at do %>
<.field_row
label={gettext("Rejected at")}
value={
DateFormatter.format_datetime(@join_request.rejected_at, @browser_timezone)
}
/>
<% end %>
<.field_row
label={gettext("Approved at")}
value={
DateFormatter.format_datetime(@join_request.approved_at, @browser_timezone)
}
label={gettext("Review by")}
value={JoinRequestHelpers.reviewer_display(@join_request)}
empty_text="-"
/>
<% end %>
<%= if @join_request.rejected_at do %>
<.field_row
label={gettext("Rejected at")}
value={
DateFormatter.format_datetime(@join_request.rejected_at, @browser_timezone)
}
/>
<% end %>
<.field_row
label={gettext("Review by")}
value={JoinRequestHelpers.reviewer_display(@join_request)}
empty_text="-"
/>
<% end %>
</dl>
</div>
</div>
@ -209,28 +224,30 @@ defmodule MvWeb.JoinRequestLive.Show do
attr :label, :string, required: true
attr :value, :any, default: nil
attr :empty_text, :string, default: nil
slot :inner_block
defp field_row(assigns) do
~H"""
<div class="flex gap-2">
<span class="text-base-content/60 min-w-32 shrink-0">{@label}:</span>
<span>
<%= if @value && @value != "" do %>
<dt class="m-0 text-base-content/60 whitespace-normal break-words">{@label}:</dt>
<dd class="m-0 min-w-0">
<%= cond do %>
<% @inner_block != [] -> %>
{render_slot(@inner_block)}
<% @value && @value != "" -> %>
{@value}
<% else %>
<% true -> %>
<span class="text-base-content/40 italic">
{@empty_text || gettext("Not specified")}
</span>
<% end %>
</span>
</div>
<% end %>
</dd>
"""
end
# Builds a single list of {label, display_value} for all applicant-provided data in join form
# order. Typed fields (email, first_name, last_name) and form_data are merged; legacy
# form_data keys (not in current join form config) are appended at the end.
defp applicant_data_rows(join_request, ordered_field_ids) do
defp applicant_data_rows(join_request, ordered_field_ids, custom_field_by_id) do
member_field_strings = Constants.member_fields() |> Enum.map(&Atom.to_string/1)
form_data = join_request.form_data || %{}
@ -244,8 +261,9 @@ defmodule MvWeb.JoinRequestLive.Show do
ordered_field_ids
|> Enum.map(fn key ->
value = Map.get(typed, key) || Map.get(form_data, key)
label = field_key_to_label(key, member_field_strings)
{label, format_applicant_value(value)}
label = field_key_to_label(key, member_field_strings, custom_field_by_id)
value_type = field_key_to_value_type(key, member_field_strings, custom_field_by_id)
{label, format_applicant_value(value, value_type)}
end)
legacy_keys =
@ -258,34 +276,66 @@ defmodule MvWeb.JoinRequestLive.Show do
legacy_entries =
Enum.map(legacy_keys, fn key ->
label = field_key_to_label(key, member_field_strings)
{label, format_applicant_value(form_data[key])}
label = field_key_to_label(key, member_field_strings, custom_field_by_id)
value_type = field_key_to_value_type(key, member_field_strings, custom_field_by_id)
{label, format_applicant_value(form_data[key], value_type)}
end)
in_order ++ legacy_entries
end
defp format_applicant_value(nil), do: nil
defp format_applicant_value(""), do: nil
defp format_applicant_value(%Date{} = date), do: DateFormatter.format_date(date)
defp format_applicant_value(nil, _type), do: nil
defp format_applicant_value("", _type), do: nil
defp format_applicant_value(%Date{} = date, _type), do: DateFormatter.format_date(date)
defp format_applicant_value(value) when is_map(value),
do: format_applicant_value_from_map(value)
defp format_applicant_value(value, type) when is_map(value),
do: format_applicant_value_from_map(value, type)
defp format_applicant_value(value) when is_boolean(value),
defp format_applicant_value(value, _type) when is_boolean(value),
do: if(value, do: gettext("Yes"), else: gettext("No"))
defp format_applicant_value(value) when is_binary(value) or is_number(value),
do: to_string(value)
defp format_applicant_value(value, type) when is_binary(value),
do: format_binary_applicant_value(value, type)
defp format_applicant_value(value), do: to_string(value)
defp format_applicant_value(value, _type) when is_number(value), do: to_string(value)
defp format_applicant_value_from_map(value) do
defp format_applicant_value(value, _type), do: to_string(value)
defp format_binary_applicant_value(value, type) do
trimmed_value = String.trim(value)
cond do
trimmed_value == "" ->
nil
String.downcase(trimmed_value) in ["on", "true", "1"] ->
gettext("Yes")
String.downcase(trimmed_value) in ["off", "false", "0"] ->
gettext("No")
type in [:date, Ash.Type.Date] ->
format_iso_date_string(trimmed_value)
true ->
trimmed_value
end
end
defp format_iso_date_string(value) do
case Date.from_iso8601(value) do
{:ok, date} -> DateFormatter.format_date(date)
_ -> value
end
end
defp format_applicant_value_from_map(value, fallback_type) do
raw = Map.get(value, "_union_value") || Map.get(value, "value")
type = Map.get(value, "_union_type") || Map.get(value, "type")
effective_type = type || fallback_type
if raw && type in ["date", :date] do
format_applicant_value(raw)
if raw && effective_type in ["date", :date, Ash.Type.Date] do
format_applicant_value(raw, :date)
else
format_applicant_value_simple(raw, value)
end
@ -299,11 +349,39 @@ defmodule MvWeb.JoinRequestLive.Show do
defp format_applicant_value_simple(raw, _value) when is_integer(raw), do: to_string(raw)
defp format_applicant_value_simple(_raw, value), do: to_string(value)
defp field_key_to_label(key, member_field_strings) when is_binary(key) do
if key in member_field_strings,
do: MemberFieldsTranslations.label(String.to_existing_atom(key)),
else: key
defp field_key_to_label(key, member_field_strings, custom_field_by_id)
when is_binary(key) do
if key in member_field_strings do
MemberFieldsTranslations.label(String.to_existing_atom(key))
else
case Map.get(custom_field_by_id, key) do
%{name: name} -> name
_ -> key
end
end
end
defp field_key_to_label(key, _), do: to_string(key)
defp field_key_to_label(key, _, _), do: to_string(key)
defp field_key_to_value_type("email", _member_field_strings, _custom_field_by_id), do: :string
defp field_key_to_value_type("first_name", _member_field_strings, _custom_field_by_id),
do: :string
defp field_key_to_value_type("last_name", _member_field_strings, _custom_field_by_id),
do: :string
defp field_key_to_value_type(key, member_field_strings, custom_field_by_id)
when is_binary(key) do
if key in member_field_strings do
:string
else
case Map.get(custom_field_by_id, key) do
%{value_type: value_type} -> value_type
_ -> nil
end
end
end
defp field_key_to_value_type(_key, _member_field_strings, _custom_field_by_id), do: nil
end

2
lib/mv_web/live/member_live/show/membership_fees_component.ex
View file

@ -1291,7 +1291,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
defp translate_receipt_status("paid"), do: gettext("Paid")
defp translate_receipt_status("unpaid"), do: gettext("Unpaid")
defp translate_receipt_status("suspended"), do: gettext("Suspended")
defp translate_receipt_status("open"), do: gettext("Open")
defp translate_receipt_status("open"), do: pgettext("status", "Open")
defp translate_receipt_status("cancelled"), do: gettext("Cancelled")
defp translate_receipt_status("draft"), do: gettext("Draft")
defp translate_receipt_status("incompleted"), do: gettext("Incomplete")