refactor: integrate approval ui review changes

CODE_GUIDELINES.md

@@ -86,7 +86,7 @@ lib/
 │   ├── membership.ex      # Domain definition
 │   ├── member.ex          # Member resource
 │   ├── join_request.ex    # JoinRequest (public join form, double opt-in)
-│   ├── join_request/      # JoinRequest changes (SetConfirmationToken, FilterFormDataByAllowlist, ConfirmRequest)
+│   ├── join_request/      # JoinRequest changes (Helpers, SetConfirmationToken, FilterFormDataByAllowlist, ConfirmRequest, ApproveRequest, RejectRequest)
 │   ├── custom_field.ex               # Custom field (definition) resource
 │   ├── custom_field_value.ex        # Custom field value resource
 │   ├── setting.ex         # Global settings (singleton resource; incl. join form config)

docs/onboarding-join-concept.md

@@ -133,6 +133,7 @@ Implementation spec for Subtask 5.
   - **form_data** (jsonb): keys that match `Mv.Constants.member_fields()` (atom names or string keys) → corresponding Member attributes. Keys that are custom field IDs (UUID format) → create **CustomFieldValue** records linked to the new Member.
   - **Defaults:** e.g. `join_date` = today if not in form_data; `membership_fee_type_id` = default from settings (or first available) if not provided. Handle required Member validations (e.g. email already present from JoinRequest).
 - **Implementation:** Prefer a single Ash change (e.g. `JoinRequest.Changes.PromoteToMember`) or a domain function that builds member attributes + custom_field_values from the approved JoinRequest and calls Member `create_member` (actor: reviewer or system actor as per CODE_GUIDELINES; document choice). No User created in MVP.
+- **Atomicity:** The approve flow (get JoinRequest → update to approved → promote to Member) runs inside **`Ash.transact(JoinRequest, fn -> ... end)`** so that if Member creation fails (e.g. validation, unique constraint), the JoinRequest status is rolled back and remains consistent.
 - **Idempotency:** If approve is called again by mistake (e.g. race), either reject transition when status is already `approved` or ensure Member creation is idempotent (e.g. do not create duplicate Member for same JoinRequest).
 
 #### Permission sets and routing

lib/membership/join_request/changes/approve_request.ex

@@ -8,12 +8,14 @@ defmodule Mv.Membership.JoinRequest.Changes.ApproveRequest do
   """
   use Ash.Resource.Change
 
+  alias Mv.Membership.JoinRequest.Changes.Helpers
+
   @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t()
   def change(changeset, _opts, context) do
     current_status = Ash.Changeset.get_data(changeset, :status)
 
     if current_status == :submitted do
-      reviewed_by_id = actor_id(context.actor)
+      reviewed_by_id = Helpers.actor_id(context.actor)
 
       changeset
       |> Ash.Changeset.force_change_attribute(:status, :approved)
@@ -26,12 +28,4 @@ defmodule Mv.Membership.JoinRequest.Changes.ApproveRequest do
       )
     end
   end
-
-  defp actor_id(nil), do: nil
-
-  defp actor_id(actor) when is_map(actor) do
-    Map.get(actor, :id) || Map.get(actor, "id")
-  end
-
-  defp actor_id(_), do: nil
 end

lib/membership/join_request/changes/helpers.ex

@@ -0,0 +1,19 @@
+defmodule Mv.Membership.JoinRequest.Changes.Helpers do
+  @moduledoc """
+  Shared helpers for JoinRequest change modules (e.g. ApproveRequest, RejectRequest).
+  """
+
+  @doc """
+  Extracts the actor's user id from the Ash change context.
+
+  Supports both atom and string keys for compatibility with different actor representations.
+  """
+  @spec actor_id(term()) :: String.t() | nil
+  def actor_id(nil), do: nil
+
+  def actor_id(actor) when is_map(actor) do
+    Map.get(actor, :id) || Map.get(actor, "id")
+  end
+
+  def actor_id(_), do: nil
+end

lib/membership/join_request/changes/reject_request.ex

@@ -7,12 +7,14 @@ defmodule Mv.Membership.JoinRequest.Changes.RejectRequest do
   """
   use Ash.Resource.Change
 
+  alias Mv.Membership.JoinRequest.Changes.Helpers
+
   @spec change(Ash.Changeset.t(), keyword(), Ash.Resource.Change.context()) :: Ash.Changeset.t()
   def change(changeset, _opts, context) do
     current_status = Ash.Changeset.get_data(changeset, :status)
 
     if current_status == :submitted do
-      reviewed_by_id = actor_id(context.actor)
+      reviewed_by_id = Helpers.actor_id(context.actor)
 
       changeset
       |> Ash.Changeset.force_change_attribute(:status, :rejected)
@@ -25,12 +27,4 @@ defmodule Mv.Membership.JoinRequest.Changes.RejectRequest do
       )
     end
   end
-
-  defp actor_id(nil), do: nil
-
-  defp actor_id(actor) when is_map(actor) do
-    Map.get(actor, :id) || Map.get(actor, "id")
-  end
-
-  defp actor_id(_), do: nil
 end

lib/membership/membership.ex

@@ -456,6 +456,20 @@ defmodule Mv.Membership do
     end
   end
 
+  @doc """
+  Returns whether the public join form is enabled in global settings.
+
+  Used by the web layer (JoinRequest LiveViews, Layouts, plugs) to decide whether
+  to show join-related UI and to gate access to join request pages.
+  """
+  @spec join_form_enabled?() :: boolean()
+  def join_form_enabled? do
+    case get_settings() do
+      {:ok, %{join_form_enabled: true}} -> true
+      _ -> false
+    end
+  end
+
   @doc """
   Returns the allowlist of fields configured for the public join form.
 
@@ -585,8 +599,15 @@ defmodule Mv.Membership do
     query = JoinRequest |> Ash.Query.filter(expr(status == :submitted))
 
     case Ash.count(query, actor: actor, domain: __MODULE__) do
-      {:ok, count} when is_integer(count) and count >= 0 -> count
-      _ -> 0
+      {:ok, count} when is_integer(count) and count >= 0 ->
+        count
+
+      {:error, error} ->
+        Logger.debug("count_submitted_join_requests failed: #{inspect(error)}")
+        0
+
+      _ ->
+        0
     end
   end
 
@@ -604,7 +625,13 @@ defmodule Mv.Membership do
   @spec get_join_request(String.t(), keyword()) :: {:ok, JoinRequest.t() | nil} | {:error, term()}
   def get_join_request(id, opts \\ []) do
     actor = Keyword.get(opts, :actor)
-    Ash.get(JoinRequest, id, actor: actor, load: [:reviewed_by_user], domain: __MODULE__)
+
+    Ash.get(JoinRequest, id,
+      actor: actor,
+      load: [:reviewed_by_user],
+      not_found_error?: false,
+      domain: __MODULE__
+    )
   end
 
   @doc """
@@ -625,6 +652,8 @@ defmodule Mv.Membership do
   def approve_join_request(id, opts \\ []) do
     actor = Keyword.get(opts, :actor)
 
+    result =
+      Ash.transact(JoinRequest, fn ->
         with {:ok, request} <- Ash.get(JoinRequest, id, actor: actor, domain: __MODULE__),
              {:ok, approved} <-
                request
@@ -633,6 +662,13 @@ defmodule Mv.Membership do
              {:ok, _member} <- promote_to_member(approved, actor) do
           {:ok, approved}
         end
+      end)
+
+    # Ash.transact returns {:ok, callback_result}; flatten so callers get {:ok, request} | {:error, term()}
+    case result do
+      {:ok, inner} -> inner
+      {:error, _} = err -> err
+    end
   end
 
   @doc """
@@ -661,6 +697,7 @@ defmodule Mv.Membership do
 
   # Builds Member attrs + custom_field_values from a JoinRequest and creates the Member.
   @uuid_pattern ~r/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i
+  # Evaluated at compile time so we do not resolve member_fields() on every reduce step.
   @member_field_strings Mv.Constants.member_fields() |> Enum.map(&Atom.to_string/1)
 
   defp promote_to_member(%JoinRequest{} = request, actor) do

lib/mv_web/components/layouts.ex

@@ -44,8 +44,10 @@ defmodule MvWeb.Layouts do
 
   def app(assigns) do
     club_name = get_club_name()
-    join_form_enabled = get_join_form_enabled()
+    join_form_enabled = Mv.Membership.join_form_enabled?()
 
+    # TODO: get_join_form_enabled and unprocessed count run on every page load; consider
+    # loading count 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)
 
@@ -136,13 +138,6 @@ defmodule MvWeb.Layouts do
     end
   end
 
-  defp get_join_form_enabled do
-    case Mv.Membership.get_settings() do
-      {:ok, %{join_form_enabled: true}} -> true
-      _ -> false
-    end
-  end
-
   defp get_unprocessed_join_requests_count(nil, _), do: 0
   defp get_unprocessed_join_requests_count(_user, false), do: 0
 

lib/mv_web/live/join_request_live/helpers.ex

@@ -0,0 +1,47 @@
+defmodule MvWeb.JoinRequestLive.Helpers do
+  @moduledoc """
+  Shared helpers for JoinRequest LiveViews (Index, Show): status display,
+  badge variants, and reviewer display.
+  """
+  use Gettext, backend: MvWeb.Gettext
+
+  @doc "Human-readable label for a join request status atom."
+  def format_status(:pending_confirmation), do: gettext("Pending confirmation")
+  def format_status(:submitted), do: gettext("Submitted")
+  def format_status(:approved), do: gettext("Approved")
+  def format_status(:rejected), do: gettext("Rejected")
+  def format_status(other), do: to_string(other)
+
+  @doc "Badge variant for the status (used with CoreComponents.badge)."
+  def status_badge_variant(:submitted), do: :info
+  def status_badge_variant(:approved), do: :success
+  def status_badge_variant(:rejected), do: :error
+  def status_badge_variant(_), do: :neutral
+
+  @doc """
+  Returns the reviewer display string (e.g. email) for a join request, or nil if none.
+
+  Accepts a join request struct or map with optional :reviewed_by_user (loaded User struct).
+  """
+  def reviewer_display(req) when is_map(req) do
+    user = Map.get(req, :reviewed_by_user)
+
+    case user do
+      nil ->
+        nil
+
+      %{email: email} when is_binary(email) ->
+        s = String.trim(email)
+        if s == "", do: nil, else: s
+
+      %{"email" => email} when is_binary(email) ->
+        s = String.trim(email)
+        if s == "", do: nil, else: s
+
+      _ ->
+        nil
+    end
+  end
+
+  def reviewer_display(_), do: nil
+end

lib/mv_web/live/join_request_live/index.ex

@@ -20,13 +20,14 @@ defmodule MvWeb.JoinRequestLive.Index do
 
   alias Mv.Membership
   alias MvWeb.Helpers.DateFormatter
+  alias MvWeb.JoinRequestLive.Helpers, as: JoinRequestHelpers
 
   @impl true
   def mount(_params, _session, socket) do
     actor = current_actor(socket)
 
     cond do
-      not join_form_enabled?() ->
+      not Membership.join_form_enabled?() ->
         {:ok, redirect(socket, to: ~p"/members")}
 
       not can_access_page?(actor, "/join_requests") ->
@@ -81,8 +82,8 @@ defmodule MvWeb.JoinRequestLive.Index do
                 {req.email}
               </:col>
               <:col :let={req} label={gettext("Status")}>
-                <.badge variant={status_badge_variant(req.status)}>
-                  {format_status(req.status)}
+                <.badge variant={JoinRequestHelpers.status_badge_variant(req.status)}>
+                  {JoinRequestHelpers.format_status(req.status)}
                 </.badge>
               </:col>
             </.table>
@@ -119,15 +120,15 @@ defmodule MvWeb.JoinRequestLive.Index do
                 </.maybe_value>
               </:col>
               <:col :let={req} label={gettext("Status")}>
-                <.badge variant={status_badge_variant(req.status)}>
-                  {format_status(req.status)}
+                <.badge variant={JoinRequestHelpers.status_badge_variant(req.status)}>
+                  {JoinRequestHelpers.format_status(req.status)}
                 </.badge>
               </:col>
               <:col :let={req} label={gettext("Reviewed at")}>
                 {review_date(req)}
               </:col>
               <:col :let={req} label={gettext("Review by")}>
-                {reviewer_display(req)}
+                {JoinRequestHelpers.reviewer_display(req) || ""}
               </:col>
             </.table>
           <% end %>
@@ -137,13 +138,6 @@ defmodule MvWeb.JoinRequestLive.Index do
     """
   end
 
-  defp join_form_enabled? do
-    case Membership.get_settings() do
-      {:ok, %{join_form_enabled: true}} -> true
-      _ -> false
-    end
-  end
-
   defp load_join_requests(socket, actor) do
     socket =
       case Membership.list_join_requests(actor: actor, status: :submitted) do
@@ -168,17 +162,6 @@ defmodule MvWeb.JoinRequestLive.Index do
     assign(socket, :page_title, gettext("Join requests"))
   end
 
-  defp format_status(:pending_confirmation), do: gettext("Pending confirmation")
-  defp format_status(:submitted), do: gettext("Submitted")
-  defp format_status(:approved), do: gettext("Approved")
-  defp format_status(:rejected), do: gettext("Rejected")
-  defp format_status(other), do: to_string(other)
-
-  defp status_badge_variant(:submitted), do: :info
-  defp status_badge_variant(:approved), do: :success
-  defp status_badge_variant(:rejected), do: :error
-  defp status_badge_variant(_), do: :neutral
-
   defp review_date(req) do
     date =
       case req.status do
@@ -189,12 +172,4 @@ defmodule MvWeb.JoinRequestLive.Index do
 
     if date, do: DateFormatter.format_datetime(date), else: ""
   end
-
-  defp reviewer_display(req) do
-    case req.reviewed_by_user do
-      nil -> ""
-      %{email: email} when not is_nil(email) -> to_string(email) |> String.trim()
-      _ -> ""
-    end
-  end
 end

lib/mv_web/live/join_request_live/show.ex

@@ -22,10 +22,12 @@ defmodule MvWeb.JoinRequestLive.Show do
   alias Mv.Constants
   alias Mv.Membership
   alias MvWeb.Helpers.DateFormatter
+  alias MvWeb.JoinRequestLive.Helpers, as: JoinRequestHelpers
+  alias MvWeb.Translations.MemberFields, as: MemberFieldsTranslations
 
   @impl true
   def mount(_params, _session, socket) do
-    if join_form_enabled?() do
+    if Membership.join_form_enabled?() do
       {:ok,
        socket
        |> assign(:join_request, nil)
@@ -40,7 +42,7 @@ defmodule MvWeb.JoinRequestLive.Show do
   def handle_params(%{"id" => id}, _url, socket) do
     actor = current_actor(socket)
 
-    if join_form_enabled?() and can_access_page?(actor, "/join_requests/:id") do
+    if Membership.join_form_enabled?() and can_access_page?(actor, "/join_requests/:id") do
       case Membership.get_join_request(id, actor: actor) do
         {:ok, nil} ->
           {:noreply,
@@ -106,13 +108,6 @@ defmodule MvWeb.JoinRequestLive.Show do
     end
   end
 
-  defp join_form_enabled? do
-    case Membership.get_settings() do
-      {:ok, %{join_form_enabled: true}} -> true
-      _ -> false
-    end
-  end
-
   @impl true
   def render(assigns) do
     ~H"""
@@ -154,8 +149,8 @@ defmodule MvWeb.JoinRequestLive.Show do
               <div class="flex gap-2">
                 <span class="text-base-content/60 min-w-32 shrink-0">{gettext("Status")}:</span>
                 <span>
-                  <.badge variant={status_badge_variant(@join_request.status)}>
-                    {format_status(@join_request.status)}
+                  <.badge variant={JoinRequestHelpers.status_badge_variant(@join_request.status)}>
+                    {JoinRequestHelpers.format_status(@join_request.status)}
                   </.badge>
                 </span>
               </div>
@@ -191,7 +186,7 @@ defmodule MvWeb.JoinRequestLive.Show do
                 <% end %>
                 <.field_row
                   label={gettext("Review by")}
-                  value={reviewer_display(@join_request)}
+                  value={JoinRequestHelpers.reviewer_display(@join_request)}
                   empty_text="-"
                 />
               </div>
@@ -245,33 +240,6 @@ defmodule MvWeb.JoinRequestLive.Show do
     """
   end
 
-  defp format_status(:pending_confirmation), do: gettext("Pending confirmation")
-  defp format_status(:submitted), do: gettext("Submitted")
-  defp format_status(:approved), do: gettext("Approved")
-  defp format_status(:rejected), do: gettext("Rejected")
-  defp format_status(other), do: to_string(other)
-
-  defp status_badge_variant(:submitted), do: :info
-  defp status_badge_variant(:approved), do: :success
-  defp status_badge_variant(:rejected), do: :error
-  defp status_badge_variant(_), do: :neutral
-
-  defp reviewer_display(%{reviewed_by_user: user}) do
-    case user do
-      nil ->
-        nil
-
-      %{email: email} when not is_nil(email) ->
-        s = to_string(email) |> String.trim()
-        if s == "", do: nil, else: s
-
-      _ ->
-        nil
-    end
-  end
-
-  defp reviewer_display(_), do: nil
-
   # Formats form_data for display in join-form order; legacy keys (not in current
   # join_form_field_ids) are appended at the end, sorted by label for stability.
   # Labels: member field keys → human-readable; UUID keys kept as-is (custom field IDs).
@@ -307,14 +275,10 @@ defmodule MvWeb.JoinRequestLive.Show do
   end
 
   defp field_key_to_label(key, member_field_strings) when is_binary(key) do
-    if key in member_field_strings, do: humanize_field(key), else: key
+    if key in member_field_strings,
+      do: MemberFieldsTranslations.label(String.to_existing_atom(key)),
+      else: key
   end
 
   defp field_key_to_label(key, _), do: to_string(key)
-
-  defp humanize_field(key) when is_binary(key) do
-    key
-    |> String.replace("_", " ")
-    |> String.capitalize()
-  end
 end

priv/gettext/de/LC_MESSAGES/default.po

@@ -3476,8 +3476,7 @@ msgstr "Genehmigen"
 msgid "Approve this join request and create a member?"
 msgstr "Diesen Mitgliedsantrag genehmigen und Mitglied anlegen?"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Approved"
 msgstr "Genehmigt"
@@ -3553,8 +3552,7 @@ msgstr "Keine eingereichten Mitgliedsanträge"
 msgid "Not submitted yet"
 msgstr "Noch nicht eingereicht"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Pending confirmation"
 msgstr "Bestätigung ausstehend"
@@ -3569,8 +3567,7 @@ msgstr "Ablehnen"
 msgid "Reject this join request?"
 msgstr "Diesen Mitgliedsantrag ablehnen?"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Rejected"
 msgstr "Abgelehnt"
@@ -3590,8 +3587,7 @@ msgstr "Antragsdaten"
 msgid "Review information"
 msgstr "Bearbeitungsinformationen"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Submitted"
 msgstr "Eingereicht"

priv/gettext/default.pot

@@ -3476,8 +3476,7 @@ msgstr ""
 msgid "Approve this join request and create a member?"
 msgstr ""
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Approved"
 msgstr ""
@@ -3553,8 +3552,7 @@ msgstr ""
 msgid "Not submitted yet"
 msgstr ""
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Pending confirmation"
 msgstr ""
@@ -3569,8 +3567,7 @@ msgstr ""
 msgid "Reject this join request?"
 msgstr ""
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Rejected"
 msgstr ""
@@ -3590,8 +3587,7 @@ msgstr ""
 msgid "Review information"
 msgstr ""
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Submitted"
 msgstr ""

priv/gettext/en/LC_MESSAGES/default.po

@@ -3476,8 +3476,7 @@ msgstr "Approve"
 msgid "Approve this join request and create a member?"
 msgstr "Approve this membership application and create a member?"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Approved"
 msgstr "Approved"
@@ -3553,8 +3552,7 @@ msgstr "No submitted membership applications"
 msgid "Not submitted yet"
 msgstr "Not submitted yet"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Pending confirmation"
 msgstr "Pending confirmation"
@@ -3569,8 +3567,7 @@ msgstr "Reject"
 msgid "Reject this join request?"
 msgstr "Reject this membership application?"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Rejected"
 msgstr "Rejected"
@@ -3590,8 +3587,7 @@ msgstr "Request data"
 msgid "Review information"
 msgstr "Review information"
 
-#: lib/mv_web/live/join_request_live/index.ex
-#: lib/mv_web/live/join_request_live/show.ex
+#: lib/mv_web/live/join_request_live/helpers.ex
 #, elixir-autogen, elixir-format
 msgid "Submitted"
 msgstr "Submitted"