Merge branch 'main' into feature/ux_button_concistency

lib/mv_web/components/layouts.ex

@@ -43,11 +43,11 @@ defmodule MvWeb.Layouts do
   slot :inner_block, required: true
 
   def app(assigns) do
-    club_name = get_club_name()
-    join_form_enabled = Mv.Membership.join_form_enabled?()
+    # 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: 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.
+    # TODO: unprocessed count runs on every page load when join form 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)
 
@@ -129,12 +129,17 @@ defmodule MvWeb.Layouts do
     """
   end
 
-  # Helper function to get club name from settings
-  # Falls back to "Mitgliederverwaltung" if settings can't be loaded
-  defp get_club_name do
+  # Single settings read for layout; returns club_name and join_form_enabled to avoid duplicate get_settings().
+  defp get_layout_settings do
     case Mv.Membership.get_settings() do
-      {:ok, settings} -> settings.club_name
-      _ -> "Mitgliederverwaltung"
+      {:ok, settings} ->
+        %{
+          club_name: settings.club_name || "Mitgliederverwaltung",
+          join_form_enabled: settings.join_form_enabled == true
+        }
+
+      _ ->
+        %{club_name: "Mitgliederverwaltung", join_form_enabled: false}
     end
   end
 

lib/mv_web/live/global_settings_live.ex

@@ -54,11 +54,14 @@ defmodule MvWeb.GlobalSettingsLive do
     actor = MvWeb.LiveHelpers.current_actor(socket)
     custom_fields = load_custom_fields(actor)
 
+    environment = Application.get_env(:mv, :environment, :dev)
+
     socket =
       socket
       |> assign(:page_title, gettext("Settings"))
       |> assign(:settings, settings)
       |> assign(:locale, locale)
+      |> assign(:environment, environment)
       |> assign(:vereinfacht_env_configured, Mv.Config.vereinfacht_env_configured?())
       |> assign(:vereinfacht_api_url_env_set, Mv.Config.vereinfacht_api_url_env_set?())
       |> assign(:vereinfacht_api_key_env_set, Mv.Config.vereinfacht_api_key_env_set?())
@@ -76,7 +79,19 @@ defmodule MvWeb.GlobalSettingsLive do
       |> assign(:oidc_groups_claim_env_set, Mv.Config.oidc_groups_claim_env_set?())
       |> assign(:oidc_only_env_set, Mv.Config.oidc_only_env_set?())
       |> assign(:oidc_configured, Mv.Config.oidc_configured?())
-      |> assign(:oidc_client_secret_set, present?(settings.oidc_client_secret))
+      |> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?())
+      |> assign(:smtp_env_configured, Mv.Config.smtp_env_configured?())
+      |> assign(:smtp_host_env_set, Mv.Config.smtp_host_env_set?())
+      |> assign(:smtp_port_env_set, Mv.Config.smtp_port_env_set?())
+      |> assign(:smtp_username_env_set, Mv.Config.smtp_username_env_set?())
+      |> assign(:smtp_password_env_set, Mv.Config.smtp_password_env_set?())
+      |> assign(:smtp_ssl_env_set, Mv.Config.smtp_ssl_env_set?())
+      |> assign(:smtp_from_name_env_set, Mv.Config.mail_from_name_env_set?())
+      |> assign(:smtp_from_email_env_set, Mv.Config.mail_from_email_env_set?())
+      |> assign(:smtp_password_set, present?(Mv.Config.smtp_password()))
+      |> assign(:smtp_configured, Mv.Config.smtp_configured?())
+      |> assign(:smtp_test_result, nil)
+      |> assign(:smtp_test_to_email, "")
       |> assign_join_form_state(settings, custom_fields)
       |> assign_form()
 
@@ -137,21 +152,6 @@ defmodule MvWeb.GlobalSettingsLive do
           </label>
         </div>
 
-        <%!-- Board approval (future feature) --%>
-        <div class="flex items-center gap-3 mb-6">
-          <input
-            type="checkbox"
-            id="join-form-board-approval-checkbox"
-            class="checkbox checkbox-sm"
-            checked={false}
-            disabled
-            aria-label={gettext("Board approval required (in development)")}
-          />
-          <label for="join-form-board-approval-checkbox" class="text-base-content/60 font-medium">
-            {gettext("Board approval required (in development)")}
-          </label>
-        </div>
-
         <div :if={@join_form_enabled}>
           <%!-- Field list header + Add button (left-aligned) --%>
           <h3 class="font-medium mb-3">{gettext("Fields on the join form")}</h3>
@@ -269,6 +269,181 @@ defmodule MvWeb.GlobalSettingsLive do
           </div>
         </div>
       </.form_section>
+      <%!-- SMTP / E-Mail Section --%>
+      <.form_section title={gettext("SMTP / E-Mail")}>
+        <%= if @smtp_env_configured do %>
+          <p class="text-sm text-base-content/70 mb-4">
+            {gettext("Some values are set via environment variables. Those fields are read-only.")}
+          </p>
+        <% end %>
+
+        <%= if @environment == :prod and not @smtp_configured do %>
+          <div class="mb-4 flex items-start gap-2 p-3 rounded-lg border border-warning bg-warning/10 text-warning-aa text-sm">
+            <.icon name="hero-exclamation-triangle" class="size-5 shrink-0 mt-0.5" />
+            <span>
+              {gettext(
+                "SMTP is not configured. Transactional emails (join confirmation, password reset, etc.) will not be delivered reliably."
+              )}
+            </span>
+          </div>
+        <% end %>
+
+        <.form for={@form} id="smtp-form" phx-change="validate" phx-submit="save">
+          <div class="grid gap-4">
+            <.input
+              field={@form[:smtp_host]}
+              type="text"
+              label={gettext("Host")}
+              disabled={@smtp_host_env_set}
+              placeholder={
+                if(@smtp_host_env_set,
+                  do: gettext("From SMTP_HOST"),
+                  else: "smtp.example.com"
+                )
+              }
+            />
+            <.input
+              field={@form[:smtp_port]}
+              type="number"
+              label={gettext("Port")}
+              disabled={@smtp_port_env_set}
+              placeholder={if(@smtp_port_env_set, do: gettext("From SMTP_PORT"), else: "587")}
+            />
+            <.input
+              field={@form[:smtp_username]}
+              type="text"
+              label={gettext("Username")}
+              disabled={@smtp_username_env_set}
+              placeholder={
+                if(@smtp_username_env_set,
+                  do: gettext("From SMTP_USERNAME"),
+                  else: "user@example.com"
+                )
+              }
+            />
+            <div class="form-control">
+              <label class="label" for={@form[:smtp_password].id}>
+                <span class="label-text">{gettext("Password")}</span>
+                <%= if @smtp_password_set do %>
+                  <span class="label-text-alt">
+                    <.badge variant="neutral" size="sm">{gettext("(set)")}</.badge>
+                  </span>
+                <% end %>
+              </label>
+              <.input
+                field={@form[:smtp_password]}
+                type="password"
+                label=""
+                disabled={@smtp_password_env_set}
+                placeholder={
+                  if(@smtp_password_env_set,
+                    do: gettext("From SMTP_PASSWORD"),
+                    else:
+                      if(@smtp_password_set,
+                        do: gettext("Leave blank to keep current"),
+                        else: nil
+                      )
+                  )
+                }
+              />
+            </div>
+            <.input
+              field={@form[:smtp_ssl]}
+              type="select"
+              label={gettext("TLS/SSL")}
+              disabled={@smtp_ssl_env_set}
+              options={[
+                {gettext("TLS (port 587, recommended)"), "tls"},
+                {gettext("SSL (port 465)"), "ssl"},
+                {gettext("None (port 25, insecure)"), "none"}
+              ]}
+              placeholder={if(@smtp_ssl_env_set, do: gettext("From SMTP_SSL"), else: nil)}
+            />
+            <.input
+              field={@form[:smtp_from_email]}
+              type="email"
+              label={gettext("Sender email (From)")}
+              disabled={@smtp_from_email_env_set}
+              placeholder={
+                if(@smtp_from_email_env_set,
+                  do: gettext("From MAIL_FROM_EMAIL"),
+                  else: "noreply@example.com"
+                )
+              }
+            />
+            <.input
+              field={@form[:smtp_from_name]}
+              type="text"
+              label={gettext("Sender name (From)")}
+              disabled={@smtp_from_name_env_set}
+              placeholder={
+                if(@smtp_from_name_env_set, do: gettext("From MAIL_FROM_NAME"), else: "Mila")
+              }
+            />
+          </div>
+          <p class="mt-2 text-sm text-base-content/60">
+            {gettext(
+              "The sender email must be owned by or authorized for the SMTP user on most servers."
+            )}
+          </p>
+          <.button
+            :if={
+              not (@smtp_host_env_set and @smtp_port_env_set and @smtp_username_env_set and
+                     @smtp_password_env_set and @smtp_ssl_env_set and @smtp_from_email_env_set and
+                     @smtp_from_name_env_set)
+            }
+            phx-disable-with={gettext("Saving...")}
+            variant="primary"
+            class="mt-2"
+          >
+            {gettext("Save SMTP Settings")}
+          </.button>
+        </.form>
+
+        <%!-- Test email: use form phx-submit so the current input value is always sent (e.g. after paste without blur) --%>
+        <div class="mt-6">
+          <h3 class="font-medium mb-3">{gettext("Test email")}</h3>
+          <.form
+            for={%{}}
+            id="smtp-test-email-form"
+            data-testid="smtp-test-email-form"
+            phx-submit="send_smtp_test_email"
+            class="space-y-3"
+          >
+            <div class="flex flex-wrap items-end gap-3">
+              <div class="form-control">
+                <label class="label" for="smtp-test-to-email">
+                  <span class="label-text">{gettext("Recipient")}</span>
+                </label>
+                <input
+                  id="smtp-test-to-email"
+                  type="email"
+                  name="to_email"
+                  data-testid="smtp-test-email-input"
+                  value={@smtp_test_to_email}
+                  class="input input-bordered"
+                  placeholder="test@example.com"
+                  phx-change="update_smtp_test_to_email"
+                />
+              </div>
+              <.button
+                type="submit"
+                variant="outline"
+                data-testid="smtp-send-test-email"
+                phx-disable-with={gettext("Sending...")}
+              >
+                {gettext("Send test email")}
+              </.button>
+            </div>
+          </.form>
+          <%= if @smtp_test_result do %>
+            <div data-testid="smtp-test-result">
+              <.smtp_test_result result={@smtp_test_result} />
+            </div>
+          <% end %>
+        </div>
+      </.form_section>
+
       <%!-- Vereinfacht Integration Section --%>
       <.form_section title={gettext("Accounting-Software (Vereinfacht) Integration")}>
         <%= if @vereinfacht_env_configured do %>
@@ -516,6 +691,27 @@ defmodule MvWeb.GlobalSettingsLive do
      assign(socket, form: AshPhoenix.Form.validate(socket.assigns.form, setting_params))}
   end
 
+  # phx-change can fire without "setting" (e.g. only _target when focusing). Do not validate
+  # with previous form params to avoid surprising behaviour; wait for the next event with setting data.
+  def handle_event("validate", _params, socket) do
+    {:noreply, socket}
+  end
+
+  @impl true
+  def handle_event("update_smtp_test_to_email", %{"to_email" => email}, socket) do
+    {:noreply, assign(socket, :smtp_test_to_email, email)}
+  end
+
+  @impl true
+  def handle_event("send_smtp_test_email", params, socket) do
+    to_email =
+      (params["to_email"] || socket.assigns.smtp_test_to_email || "")
+      |> String.trim()
+
+    result = Mv.Mailer.send_test_email(to_email)
+    {:noreply, assign(socket, :smtp_test_result, result)}
+  end
+
   @impl true
   def handle_event("test_vereinfacht_connection", _params, socket) do
     result = Mv.Vereinfacht.test_connection()
@@ -560,11 +756,13 @@ defmodule MvWeb.GlobalSettingsLive do
   @impl true
   def handle_event("save", %{"setting" => setting_params}, socket) do
     actor = MvWeb.LiveHelpers.current_actor(socket)
-    # Never send blank API key / client secret so we do not overwrite stored secrets
+
+    # Never send blank API key / client secret / smtp password so we do not overwrite stored secrets
     setting_params_clean =
       setting_params
       |> drop_blank_vereinfacht_api_key()
       |> drop_blank_oidc_client_secret()
+      |> drop_blank_smtp_password()
 
     saves_vereinfacht = vereinfacht_params?(setting_params_clean)
 
@@ -579,8 +777,12 @@ defmodule MvWeb.GlobalSettingsLive do
           socket
           |> assign(:settings, fresh_settings)
           |> assign(:vereinfacht_api_key_set, present?(fresh_settings.vereinfacht_api_key))
-          |> assign(:oidc_client_secret_set, present?(fresh_settings.oidc_client_secret))
+          |> assign(:oidc_client_secret_set, Mv.Config.oidc_client_secret_set?())
           |> assign(:oidc_configured, Mv.Config.oidc_configured?())
+          |> assign(:smtp_configured, Mv.Config.smtp_configured?())
+          |> assign(:smtp_password_set, present?(Mv.Config.smtp_password()))
+          |> assign(:smtp_from_name_env_set, Mv.Config.mail_from_name_env_set?())
+          |> assign(:smtp_from_email_env_set, Mv.Config.mail_from_email_env_set?())
           |> assign(:vereinfacht_test_result, test_result)
           |> put_flash(:success, gettext("Settings updated successfully"))
           |> assign_form()
@@ -760,17 +962,29 @@ defmodule MvWeb.GlobalSettingsLive do
     end
   end
 
+  defp drop_blank_smtp_password(params) when is_map(params) do
+    case params do
+      %{"smtp_password" => v} when v in [nil, ""] ->
+        Map.delete(params, "smtp_password")
+
+      _ ->
+        params
+    end
+  end
+
   defp assign_form(%{assigns: %{settings: settings}} = socket) do
-    # Show ENV values in disabled fields (Vereinfacht and OIDC); never expose API key / client secret
+    # Show ENV values in disabled fields (Vereinfacht, OIDC, SMTP); never expose secrets in form
     settings_display =
       settings
       |> merge_vereinfacht_env_values()
       |> merge_oidc_env_values()
+      |> merge_smtp_env_values()
 
     settings_for_form = %{
       settings_display
       | vereinfacht_api_key: nil,
-        oidc_client_secret: nil
+        oidc_client_secret: nil,
+        smtp_password: nil
     }
 
     form =
@@ -845,6 +1059,28 @@ defmodule MvWeb.GlobalSettingsLive do
     end
   end
 
+  defp merge_smtp_env_values(s) do
+    s
+    |> put_if_env_set(:smtp_host, Mv.Config.smtp_host_env_set?(), Mv.Config.smtp_host())
+    |> put_if_env_set(:smtp_port, Mv.Config.smtp_port_env_set?(), Mv.Config.smtp_port())
+    |> put_if_env_set(
+      :smtp_username,
+      Mv.Config.smtp_username_env_set?(),
+      Mv.Config.smtp_username()
+    )
+    |> put_if_env_set(:smtp_ssl, Mv.Config.smtp_ssl_env_set?(), Mv.Config.smtp_ssl())
+    |> put_if_env_set(
+      :smtp_from_email,
+      Mv.Config.mail_from_email_env_set?(),
+      Mv.Config.mail_from_email()
+    )
+    |> put_if_env_set(
+      :smtp_from_name,
+      Mv.Config.mail_from_name_env_set?(),
+      Mv.Config.mail_from_name()
+    )
+  end
+
   defp enrich_sync_errors([]), do: []
 
   defp enrich_sync_errors(errors) when is_list(errors) do
@@ -1018,6 +1254,115 @@ defmodule MvWeb.GlobalSettingsLive do
     """
   end
 
+  # ---- SMTP test result component ----
+
+  attr :result, :any, required: true
+
+  defp smtp_test_result(%{result: {:ok, _}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-success bg-success/10 text-success-aa text-sm">
+      <.icon name="hero-check-circle" class="size-5 shrink-0" />
+      <span>{gettext("Test email sent successfully.")}</span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :invalid_email_address}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>{gettext("Invalid email address. Please enter a valid recipient address.")}</span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :not_implemented}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-warning bg-warning/10 text-warning-aa text-sm">
+      <.icon name="hero-exclamation-triangle" class="size-5 shrink-0" />
+      <span>{gettext("SMTP is not configured. Please set at least the SMTP host.")}</span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :sender_rejected}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>
+        {gettext(
+          "Sender address rejected. The \"Sender email\" must be owned by or authorized for the SMTP user."
+        )}
+      </span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :auth_failed}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>
+        {gettext("Authentication failed. Please check the SMTP username and password.")}
+      </span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :recipient_rejected}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>{gettext("Recipient address rejected by the server.")}</span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :tls_failed}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>
+        {gettext(
+          "TLS connection failed. Check the TLS/SSL setting and port (587 for TLS, 465 for SSL)."
+        )}
+      </span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, :connection_failed}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>
+        {gettext("Server unreachable. Check host and port.")}
+      </span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, {:smtp_error, message}}} = assigns)
+       when is_binary(message) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>
+        {gettext("SMTP error:")} {@result |> elem(1) |> elem(1)}
+      </span>
+    </div>
+    """
+  end
+
+  defp smtp_test_result(%{result: {:error, _reason}} = assigns) do
+    ~H"""
+    <div class="mt-3 flex items-center gap-2 p-3 rounded-lg border border-error bg-error/10 text-error-aa text-sm">
+      <.icon name="hero-x-circle" class="size-5 shrink-0" />
+      <span>{gettext("Failed to send test email. Please check your SMTP configuration.")}</span>
+    </div>
+    """
+  end
+
   # ---- Join form helper functions ----
 
   defp assign_join_form_state(socket, settings, custom_fields) do