Merge pull request 'finalize groups' (#437) from feature/finalize-groups into main

Reviewed-on: #437

lib/mv_web/live/group_live/show.ex

@@ -17,10 +17,12 @@ defmodule MvWeb.GroupLive.Show do
 
   require Logger
 
+  import Ash.Expr
   import MvWeb.LiveHelpers, only: [current_actor: 1]
   import MvWeb.Authorization
 
   alias Mv.Membership
+  alias MvWeb.Helpers.MemberHelpers, as: MemberHelpers
 
   @impl true
   def mount(_params, _session, socket) do
@@ -29,6 +31,7 @@ defmodule MvWeb.GroupLive.Show do
      |> assign(:show_add_member_input, false)
      |> assign(:member_search_query, "")
      |> assign(:available_members, [])
+     |> assign(:add_member_candidates, [])
      |> assign(:selected_member_ids, [])
      |> assign(:selected_members, [])
      |> assign(:show_member_dropdown, false)
@@ -94,13 +97,21 @@ defmodule MvWeb.GroupLive.Show do
         </h1>
 
         <div class="flex gap-2">
-          <%= if can?(@current_user, :update, Mv.Membership.Group) do %>
-            <.button variant="primary" navigate={~p"/groups/#{@group.slug}/edit"}>
+          <%= if can?(@current_user, :update, @group) do %>
+            <.button
+              variant="primary"
+              navigate={~p"/groups/#{@group.slug}/edit"}
+              data-testid="group-show-edit-btn"
+            >
               {gettext("Edit")}
             </.button>
           <% end %>
-          <%= if can?(@current_user, :destroy, Mv.Membership.Group) do %>
-            <.button class="btn-error" phx-click="open_delete_modal">
+          <%= if can?(@current_user, :destroy, @group) do %>
+            <.button
+              class="btn-error"
+              phx-click="open_delete_modal"
+              data-testid="group-show-delete-btn"
+            >
               {gettext("Delete")}
             </.button>
           <% end %>
@@ -123,7 +134,7 @@ defmodule MvWeb.GroupLive.Show do
         <div>
           <h2 class="text-lg font-semibold mb-2">{gettext("Members")}</h2>
           <div class="border border-base-300 rounded-lg p-4 bg-base-100">
-            <p class="mb-4">
+            <p class="mb-4" data-testid="group-show-member-count">
               {ngettext(
                 "Total: %{count} member",
                 "Total: %{count} members",
@@ -132,7 +143,7 @@ defmodule MvWeb.GroupLive.Show do
               )}
             </p>
 
-            <%= if can?(@current_user, :update, Mv.Membership.Group) do %>
+            <%= if can?(@current_user, :update, @group) do %>
               <div class="mb-4">
                 <%= if assigns[:show_add_member_input] do %>
                   <div class="join w-full">
@@ -160,6 +171,7 @@ defmodule MvWeb.GroupLive.Show do
                           <input
                             type="text"
                             id="member-search-input"
+                            data-testid="group-show-member-search-input"
                             role="combobox"
                             phx-hook="ComboBox"
                             phx-focus="show_member_dropdown"
@@ -228,6 +240,7 @@ defmodule MvWeb.GroupLive.Show do
                       type="button"
                       class="btn btn-primary join-item"
                       phx-click="add_selected_members"
+                      data-testid="group-show-add-selected-members-btn"
                       disabled={Enum.empty?(@selected_member_ids)}
                       aria-label={gettext("Add members")}
                     >
@@ -255,15 +268,17 @@ defmodule MvWeb.GroupLive.Show do
             <% end %>
 
             <%= if Enum.empty?(@group.members || []) do %>
-              <p class="text-base-content/50 italic">{gettext("No members in this group")}</p>
+              <p class="text-base-content/50 italic" data-testid="group-show-no-members">
+                {gettext("No members in this group")}
+              </p>
             <% else %>
-              <div class="overflow-x-auto">
+              <div class="overflow-x-auto" data-testid="group-show-members-table">
                 <table class="table table-zebra">
                   <thead>
                     <tr>
                       <th>{gettext("Name")}</th>
                       <th>{gettext("Email")}</th>
-                      <%= if can?(@current_user, :update, Mv.Membership.Group) do %>
+                      <%= if can?(@current_user, :update, @group) do %>
                         <th class="w-0">{gettext("Actions")}</th>
                       <% end %>
                     </tr>
@@ -291,13 +306,14 @@ defmodule MvWeb.GroupLive.Show do
                             <span class="text-base-content/50 italic">—</span>
                           <% end %>
                         </td>
-                        <%= if can?(@current_user, :update, Mv.Membership.Group) do %>
+                        <%= if can?(@current_user, :update, @group) do %>
                           <td>
                             <button
                               type="button"
                               class="btn btn-ghost btn-sm text-error"
                               phx-click="remove_member"
                               phx-value-member_id={member.id}
+                              data-testid="group-show-remove-member"
                               aria-label={gettext("Remove member from group")}
                               data-tooltip={gettext("Remove")}
                             >
@@ -431,28 +447,31 @@ defmodule MvWeb.GroupLive.Show do
   # Add Member Events
   @impl true
   def handle_event("show_add_member_input", _params, socket) do
-    # Reload group to ensure we have the latest members list
-    actor = current_actor(socket)
-    group = socket.assigns.group
-    socket = reload_group(socket, group.slug, actor)
+    # Load candidate members once (single DB read). Search/focus then filter in memory (R2).
+    socket =
+      socket
+      |> assign(:show_add_member_input, true)
+      |> assign(:member_search_query, "")
+      |> assign(:selected_member_ids, [])
+      |> assign(:selected_members, [])
+      |> assign(:show_member_dropdown, false)
+      |> assign(:focused_member_index, nil)
+      |> load_add_member_candidates()
 
-    {:noreply,
-     socket
-     |> assign(:show_add_member_input, true)
-     |> assign(:member_search_query, "")
-     |> assign(:available_members, [])
-     |> assign(:selected_member_ids, [])
-     |> assign(:selected_members, [])
-     |> assign(:show_member_dropdown, false)
-     |> assign(:focused_member_index, nil)}
+    {:noreply, socket}
   end
 
   @impl true
   def handle_event("show_member_dropdown", _params, socket) do
-    # Use existing group.members for filtering; reload only on add/remove
+    # Filter in memory from preloaded candidates; no DB read (R2).
+    query = socket.assigns.member_search_query || ""
+
     socket =
       socket
-      |> load_available_members("")
+      |> assign(
+        :available_members,
+        filter_candidates_in_memory(socket.assigns.add_member_candidates, query)
+      )
       |> assign(:show_member_dropdown, true)
       |> assign(:focused_member_index, nil)
 
@@ -466,6 +485,7 @@ defmodule MvWeb.GroupLive.Show do
      |> assign(:show_add_member_input, false)
      |> assign(:member_search_query, "")
      |> assign(:available_members, [])
+     |> assign(:add_member_candidates, [])
      |> assign(:selected_member_ids, [])
      |> assign(:selected_members, [])
      |> assign(:show_member_dropdown, false)
@@ -532,11 +552,13 @@ defmodule MvWeb.GroupLive.Show do
 
   @impl true
   def handle_event("search_members", %{"member_search" => query}, socket) do
-    # Use existing group.members for filtering; reload only on add/remove
+    # Filter in memory from preloaded candidates; no DB read (R2).
+    candidates = socket.assigns.add_member_candidates || []
+
     socket =
       socket
       |> assign(:member_search_query, query)
-      |> load_available_members(query)
+      |> assign(:available_members, filter_candidates_in_memory(candidates, query))
       |> assign(:show_member_dropdown, true)
       |> assign(:focused_member_index, nil)
 
@@ -660,47 +682,69 @@ defmodule MvWeb.GroupLive.Show do
     end
   end
 
-  defp load_available_members(socket, query) do
+  # Load candidate members once when opening add-member UI (single DB read).
+  defp load_add_member_candidates(socket) do
     require Ash.Query
 
-    current_member_ids = group_member_ids_set(socket.assigns.group)
-    base_query = available_members_base_query(query)
-
-    # Fetch more than 10, then exclude already-in-group and take 10 (avoids empty dropdown when first N are all in group)
-    fetch_limit = 50
-    limited_query = Ash.Query.limit(base_query, fetch_limit)
+    group = socket.assigns.group
+    exclude_ids = group_member_ids_set(group) |> MapSet.to_list()
     actor = current_actor(socket)
 
-    case Ash.read(limited_query, actor: actor, domain: Mv.Membership) do
-      {:ok, members} ->
-        available =
-          members
-          |> Enum.reject(fn m -> MapSet.member?(current_member_ids, m.id) end)
-          |> Enum.take(10)
+    if exclude_ids == [] do
+      # No members in group; load first N members
+      query =
+        Mv.Membership.Member
+        |> Ash.Query.sort([:last_name, :first_name])
+        |> Ash.Query.limit(300)
 
-        assign(socket, available_members: available)
+      do_load_add_member_candidates(socket, query, actor)
+    else
+      query =
+        Mv.Membership.Member
+        |> Ash.Query.filter(expr(id not in ^exclude_ids))
+        |> Ash.Query.sort([:last_name, :first_name])
+        |> Ash.Query.limit(300)
+
+      do_load_add_member_candidates(socket, query, actor)
+    end
+  end
+
+  defp do_load_add_member_candidates(socket, query, actor) do
+    case Ash.read(query, actor: actor, domain: Mv.Membership) do
+      {:ok, candidates} ->
+        socket
+        |> assign(:add_member_candidates, candidates)
+        |> assign(:available_members, Enum.take(candidates, 10))
 
       {:error, error} ->
-        Logger.warning("Failed to load available members for group: #{inspect(error)}")
+        Logger.warning("Failed to load add-member candidates: #{inspect(error)}")
 
         socket
-        |> put_flash(:error, gettext("Could not load member search. Please try again."))
+        |> put_flash(:error, gettext("Could not load member list. Please try again."))
+        |> assign(:add_member_candidates, [])
         |> assign(:available_members, [])
     end
   end
 
-  defp available_members_base_query(query) do
-    search_query = if query && String.trim(query) != "", do: String.trim(query), else: nil
+  # Filter preloaded candidates by query string (name/email). No DB read. R2.
+  defp filter_candidates_in_memory(candidates, query) when is_list(candidates) do
+    q = if is_binary(query), do: String.trim(query) |> String.downcase(), else: ""
 
-    if search_query do
-      Mv.Membership.Member
-      |> Ash.Query.for_read(:search, %{query: search_query})
+    if q == "" do
+      candidates |> Enum.take(10)
     else
-      Mv.Membership.Member
-      |> Ash.Query.new()
+      candidates
+      |> Enum.filter(fn m ->
+        name = MemberHelpers.display_name(m) |> String.downcase()
+        email = (m.email || "") |> String.downcase()
+        String.contains?(name, q) or String.contains?(email, q)
+      end)
+      |> Enum.take(10)
     end
   end
 
+  defp filter_candidates_in_memory(_, _), do: []
+
   defp group_member_ids_set(group) do
     members = group.members || []
     members |> Enum.map(& &1.id) |> MapSet.new()
@@ -740,6 +784,7 @@ defmodule MvWeb.GroupLive.Show do
      |> assign(:show_add_member_input, false)
      |> assign(:member_search_query, "")
      |> assign(:available_members, [])
+     |> assign(:add_member_candidates, [])
      |> assign(:selected_member_ids, [])
      |> assign(:selected_members, [])
      |> assign(:show_member_dropdown, false)