Merge pull request 'Vereinfacht fixes, test cleanup, and dev seed improvements' (#467) from fix/small_fixes into main

Reviewed-on: #467

lib/mv/vereinfacht/client.ex

@@ -92,10 +92,13 @@ defmodule Mv.Vereinfacht.Client do
 
   @sync_timeout_ms 5_000
 
+  # Resolved at compile time so Mix is never called at runtime (Mix is not available in releases).
+  @env Mix.env()
+
   # In test, skip retries so sync fails fast when no API is running (avoids log spam and long waits).
   defp req_http_options do
     opts = [receive_timeout: @sync_timeout_ms]
-    if Mix.env() == :test, do: [retry: false] ++ opts, else: opts
+    if @env == :test, do: [retry: false] ++ opts, else: opts
   end
 
   defp post_and_parse_contact(url, body, api_key) do

lib/mv_web/live/member_live/show/membership_fees_component.ex

@@ -128,7 +128,7 @@ defmodule MvWeb.MemberLive.Show.MembershipFeesComponent do
             </div>
           <% else %>
             <div class="mb-4 rounded-lg border border-warning/50 bg-warning/10 p-3">
-              <p class="text-warning font-medium flex items-center gap-2">
+              <p class="text-warning-aa font-medium flex items-center gap-2">
                 <.icon name="hero-exclamation-triangle" class="size-5 shrink-0" />
                 {gettext("No Vereinfacht contact exists for this member.")}
               </p>

priv/repo/seeds_dev.exs

@@ -34,7 +34,7 @@ countries_list =
   |> List.replace_at(7, "Österreich")
   |> List.replace_at(14, "Schweiz")
 
-# 20 members: varied names, cities, join dates; fee types by index (last 2 without fee type)
+# 20 members: varied names, cities, join dates; fee types distributed over all members (round-robin)
 member_configs = [
   %{
     first_name: "Anna",
@@ -218,7 +218,7 @@ member_configs = [
   }
 ]
 
-# Fee type index per member: 0..4 round-robin for first 18, nil for last 2
+# Fee type index per member: 0..4 round-robin for all 20 (each type used 4 times)
 # Cycle status: all_paid, all_unpaid, mixed (varied)
 cycle_statuses = [
   :all_paid,
@@ -240,18 +240,20 @@ cycle_statuses = [
   :all_unpaid,
   :all_paid,
   :mixed,
-  nil
+  :all_paid
 ]
 
+# Indices of members that get an exit date (5 distributed: 3, 7, 11, 15, 19)
+exit_date_member_indices = [3, 7, 11, 15, 19]
+
 Enum.with_index(member_configs)
 |> Enum.each(fn {config, index} ->
   email = "mitglied#{index + 1}@example.de"
-  fee_type_index = if index >= 18, do: nil, else: rem(index, length(all_fee_types))
-  fee_type_id = if fee_type_index, do: Enum.at(all_fee_types, fee_type_index).id, else: nil
+  fee_type_index = rem(index, length(all_fee_types))
+  fee_type_id = Enum.at(all_fee_types, fee_type_index).id
   cycle_status = Enum.at(cycle_statuses, index)
 
-  # Do not include membership_fee_type_id in upsert so re-runs do not overwrite
-  # existing assignments; set via update below only when member has none
+  # Set fee type at create so cycles are generated with correct interval (no interval-change conflict)
   base_attrs = %{
     first_name: config.first_name,
     last_name: config.last_name,
@@ -264,6 +266,11 @@ Enum.with_index(member_configs)
     country: Enum.at(countries_list, index)
   }
 
+  base_attrs =
+    if fee_type_id,
+      do: Map.put(base_attrs, :membership_fee_type_id, fee_type_id),
+      else: base_attrs
+
   member =
     Membership.create_member!(base_attrs,
       upsert?: true,
@@ -271,26 +278,14 @@ Enum.with_index(member_configs)
       actor: admin_user_with_role
     )
 
-  final_member =
-    if is_nil(member.membership_fee_type_id) and fee_type_id do
-      {:ok, updated} =
-        Membership.update_member(member, %{membership_fee_type_id: fee_type_id},
-          actor: admin_user_with_role
-        )
-
-      updated
-    else
-      member
-    end
-
-  if not is_nil(final_member.membership_fee_type_id) and not is_nil(cycle_status) do
+  if not is_nil(member.membership_fee_type_id) and not is_nil(cycle_status) do
     member_with_cycles =
-      Ash.load!(final_member, :membership_fee_cycles, actor: admin_user_with_role)
+      Ash.load!(member, :membership_fee_cycles, actor: admin_user_with_role)
 
     cycles =
       if Enum.empty?(member_with_cycles.membership_fee_cycles) do
         {:ok, new_cycles, _} =
-          CycleGenerator.generate_cycles_for_member(final_member.id,
+          CycleGenerator.generate_cycles_for_member(member.id,
             skip_lock?: true,
             actor: admin_user_with_role
           )
@@ -330,6 +325,11 @@ Enum.with_index(member_configs)
       end
     end)
   end
+
+  if index in exit_date_member_indices do
+    exit_date = Date.add(config.join_date, 365)
+    Membership.update_member(member, %{exit_date: exit_date}, actor: admin_user_with_role)
+  end
 end)
 
 # Groups (idempotent)
@@ -482,7 +482,7 @@ for {email, values} <- custom_value_assignments do
 end
 
 IO.puts("✅ Dev seeds completed.")
-IO.puts("   - Members: 20 with country (mostly Deutschland, 1 Österreich, 1 Schweiz)")
+IO.puts("   - Members: 20 with country (mostly Deutschland, 1 Österreich, 1 Schweiz), fee types distributed, 5 with exit date")
 IO.puts("   - Test users: 4 linked to mitglied1–4 with roles Mitglied, Vorstand, Kassenwart, Buchhaltung")
 IO.puts("   - Groups: Vorstand, Jugend, Newsletter (with assignments)")
 IO.puts("   - Custom field values: ~80% filled (16 members, 4–6 fields each)")

test/membership/custom_field_slug_test.exs

@@ -192,21 +192,5 @@ defmodule Mv.Membership.CustomFieldSlugTest do
     end
   end
 
-  describe "slug-based lookup (future feature)" do
-    @tag :skip
-    test "can find custom field by slug", %{actor: actor} do
-      {:ok, custom_field} =
-        CustomField
-        |> Ash.Changeset.for_create(:create, %{
-          name: "Test Field",
-          value_type: :string
-        })
-        |> Ash.create(actor: actor)
-
-      # This test is for future implementation
-      # We might add a custom action like :by_slug
-      found = Ash.get!(CustomField, custom_field.slug, load: [:slug], actor: actor)
-      assert found.id == custom_field.id
-    end
-  end
+  # Slug-based lookup (e.g. CustomField by slug) is not implemented; primary read uses ID.
 end

test/mv_web/live/group_live/show_authorization_test.exs

@@ -248,16 +248,6 @@ defmodule MvWeb.GroupLive.ShowAuthorizationTest do
                match?({:error, {:live_redirect, %{to: "/groups"}}}, result)
     end
 
-    @tag :skip
-    test "non-existent member IDs are handled", %{conn: conn} do
-      # Future: test add_selected_members with invalid ID (would require pushing event with forged selected_member_ids)
-      group = Fixtures.group_fixture()
-
-      {:ok, view, _html} = live(conn, "/groups/#{group.slug}")
-
-      assert has_element?(view, "button", "Add Member")
-    end
-
     test "non-existent group IDs are handled", %{conn: conn} do
       # Accessing non-existent group should redirect
       non_existent_slug = "non-existent-group-#{System.unique_integer([:positive])}"

test/mv_web/live/profile_navigation_test.exs

@@ -60,17 +60,22 @@ defmodule MvWeb.ProfileNavigationTest do
       assert html =~ "Profil"
     end
 
-    @tag :skip
-    # credo:disable-for-next-line Credo.Check.Design.TagTODO
-    # TODO: Implement user initials in navbar avatar - see issue #170
-    test "shows user initials in avatar", %{conn: conn} do
-      # Setup: Create and login a user
+    test "shows first letter of email in avatar", %{conn: conn, actor: actor} do
+      # Current behavior: sidebar shows first letter of email (see issue #170 for full initials)
       user = create_test_user(%{email: "test.user@example.com"})
+      admin_role = Mv.Fixtures.role_fixture("admin")
+
+      {:ok, user} =
+        user
+        |> Ash.Changeset.for_update(:update, %{})
+        |> Ash.Changeset.manage_relationship(:role, admin_role, type: :append_and_remove)
+        |> Ash.update(actor: actor)
+
       conn = conn_with_password_user(conn, user)
       {:ok, _view, html} = live(conn, "/")
 
-      # Initials from test.user@example.com
-      assert html =~ "<span>TU</span>"
+      assert html =~ "avatar"
+      assert html =~ ~r/text-sm font-semibold[^>]*>\s*T\s*</
     end
   end
 

test/mv_web/plugs/check_page_permission_test.exs

@@ -456,88 +456,9 @@ defmodule MvWeb.Plugs.CheckPagePermissionTest do
       assert conn.status == 200
     end
 
-    # Full-router test: session may not preserve member_id; plug logic covered by unit test
-    # "own_data user with linked member can access /members/:id/edit (plug direct call)".
-    @tag role: :member
-    @tag :skip
-    test "GET /members/:id/edit (linked member edit) returns 200 when user has linked member", %{
-      conn: conn,
-      current_user: user
-    } do
-      member = Mv.Fixtures.member_fixture()
-      system_actor = Mv.Helpers.SystemActor.get_system_actor()
-
-      {:ok, user_after_update} =
-        user
-        |> Ash.Changeset.for_update(:update, %{})
-        |> Ash.Changeset.force_set_argument(:member, %{id: member.id})
-        |> Ash.update(actor: system_actor)
-
-      user_with_member =
-        user_after_update
-        |> Ash.load!([:role], domain: Mv.Accounts)
-        |> Mv.Authorization.Actor.ensure_loaded()
-        |> Map.put(:member_id, member.id)
-
-      conn = conn_with_password_user(conn, user_with_member)
-
-      conn = get(conn, "/members/#{member.id}/edit")
-      assert conn.status == 200
-    end
-
-    @tag role: :member
-    @tag :skip
-    test "GET /members/:id/show/edit (linked member show edit) returns 200 when user has linked member",
-         %{
-           conn: conn,
-           current_user: user
-         } do
-      member = Mv.Fixtures.member_fixture()
-      system_actor = Mv.Helpers.SystemActor.get_system_actor()
-
-      {:ok, user_after_update} =
-        user
-        |> Ash.Changeset.for_update(:update, %{})
-        |> Ash.Changeset.force_set_argument(:member, %{id: member.id})
-        |> Ash.update(actor: system_actor)
-
-      user_with_member =
-        user_after_update
-        |> Ash.load!([:role], domain: Mv.Accounts)
-        |> Mv.Authorization.Actor.ensure_loaded()
-        |> Map.put(:member_id, member.id)
-
-      conn = conn_with_password_user(conn, user_with_member)
-
-      conn = get(conn, "/members/#{member.id}/show/edit")
-      assert conn.status == 200
-    end
-
-    # Skipped: MemberLive.Show requires membership fee cycle data; plug allows access
-    # (page loads then LiveView may error).
-    @tag role: :member
-    @tag :skip
-    test "GET /members/:id for linked member returns 200", %{conn: conn, current_user: user} do
-      system_actor = Mv.Helpers.SystemActor.get_system_actor()
-      member = Mv.Fixtures.member_fixture()
-
-      user =
-        user
-        |> Ash.Changeset.for_update(:update_user, %{})
-        |> Ash.Changeset.force_set_argument(:member, %{id: member.id})
-        |> Ash.update(actor: system_actor)
-        |> case do
-          {:ok, u} -> Ash.load!(u, :role, domain: Mv.Accounts, actor: system_actor)
-          {:error, _} -> user
-        end
-
-      conn =
-        conn
-        |> MvWeb.ConnCase.conn_with_password_user(user)
-        |> get("/members/#{member.id}")
-
-      assert conn.status == 200
-    end
+    # Linked-member access to /members/:id and edit routes: full-router tests are not feasible
+    # (session does not preserve member_id after auth). Plug behavior is covered by the unit
+    # tests "own_data user with linked member can access ... (plug direct call)" above.
   end
 
   # read_only (Vorstand/Buchhaltung): allowed /, /members, /members/:id, /groups, /groups/:slug