use strict context boundaries and remove all n+1 queries

lib/cannery_web/live/ammo_group_live/form_component.ex

@@ -44,7 +44,7 @@ defmodule CanneryWeb.AmmoGroupLive.FormComponent do
 
   @impl true
   def handle_event("validate", %{"ammo_group" => ammo_group_params}, socket) do
-    {:noreply, socket |> assign_changeset(ammo_group_params)}
+    {:noreply, socket |> assign_changeset(ammo_group_params, :validate)}
   end
 
   def handle_event(
@@ -56,6 +56,7 @@ defmodule CanneryWeb.AmmoGroupLive.FormComponent do
   end
 
   # HTML Helpers
+
   @spec container_options([Container.t()]) :: [{String.t(), Container.id()}]
   defp container_options(containers) do
     containers |> Enum.map(fn %{id: id, name: name} -> {name, id} end)
@@ -70,35 +71,28 @@ defmodule CanneryWeb.AmmoGroupLive.FormComponent do
 
   defp assign_changeset(
          %{assigns: %{action: action, ammo_group: ammo_group, current_user: user}} = socket,
-         ammo_group_params
+         ammo_group_params,
+         changeset_action \\ nil
        ) do
-    changeset_action =
-      cond do
-        action in [:new, :clone] -> :insert
-        action == :edit -> :update
+    default_action =
+      case action do
+        create when create in [:new, :clone] -> :insert
+        :edit -> :update
       end
 
     changeset =
-      cond do
-        action in [:new, :clone] ->
-          ammo_type =
-            if ammo_group_params |> Map.has_key?("ammo_type_id"),
-              do: ammo_group_params |> Map.get("ammo_type_id") |> Ammo.get_ammo_type!(user),
-              else: nil
-
-          container =
-            if ammo_group_params |> Map.has_key?("container_id"),
-              do: ammo_group_params |> Map.get("container_id") |> Containers.get_container!(user),
-              else: nil
-
+      case default_action do
+        :insert ->
+          ammo_type = maybe_get_ammo_type(ammo_group_params, user)
+          container = maybe_get_container(ammo_group_params, user)
           ammo_group |> AmmoGroup.create_changeset(ammo_type, container, user, ammo_group_params)
 
-        action == :edit ->
+        :update ->
           ammo_group |> AmmoGroup.update_changeset(ammo_group_params, user)
       end
 
     changeset =
-      case changeset |> Changeset.apply_action(changeset_action) do
+      case changeset |> Changeset.apply_action(changeset_action || default_action) do
         {:ok, _data} -> changeset
         {:error, changeset} -> changeset
       end
@@ -106,6 +100,20 @@ defmodule CanneryWeb.AmmoGroupLive.FormComponent do
     socket |> assign(:changeset, changeset)
   end
 
+  defp maybe_get_container(%{"container_id" => container_id}, user)
+       when is_binary(container_id) do
+    container_id |> Containers.get_container!(user)
+  end
+
+  defp maybe_get_container(_params_not_found, _user), do: nil
+
+  defp maybe_get_ammo_type(%{"ammo_type_id" => ammo_type_id}, user)
+       when is_binary(ammo_type_id) do
+    ammo_type_id |> Ammo.get_ammo_type!(user)
+  end
+
+  defp maybe_get_ammo_type(_params_not_found, _user), do: nil
+
   defp save_ammo_group(
          %{assigns: %{ammo_group: ammo_group, current_user: current_user, return_to: return_to}} =
            socket,
@@ -146,27 +154,26 @@ defmodule CanneryWeb.AmmoGroupLive.FormComponent do
               multiplier: multiplier
             )
 
-          {:error, changeset} =
-            changeset
-            |> Changeset.add_error(:multiplier, error_msg)
-            |> Changeset.apply_action(:insert)
-
-          socket |> assign(:changeset, changeset)
+          save_multiplier_error(socket, changeset, error_msg)
 
         :error ->
           error_msg = dgettext("errors", "Could not parse number of copies")
-
-          {:error, changeset} =
-            changeset
-            |> Changeset.add_error(:multiplier, error_msg)
-            |> Changeset.apply_action(:insert)
-
-          socket |> assign(:changeset, changeset)
+          save_multiplier_error(socket, changeset, error_msg)
       end
 
     {:noreply, socket}
   end
 
+  @spec save_multiplier_error(Socket.t(), Changeset.t(), String.t()) :: Socket.t()
+  defp save_multiplier_error(socket, changeset, error_msg) do
+    {:error, changeset} =
+      changeset
+      |> Changeset.add_error(:multiplier, error_msg)
+      |> Changeset.apply_action(:insert)
+
+    socket |> assign(:changeset, changeset)
+  end
+
   defp create_multiple(
          %{assigns: %{current_user: current_user, return_to: return_to}} = socket,
          ammo_group_params,

lib/cannery_web/live/ammo_group_live/index.html.heex

@@ -104,7 +104,7 @@
           </.link>
         </div>
       </:range>
-      <:container :let={%{container: %{name: container_name} = container} = ammo_group}>
+      <:container :let={{ammo_group, %{name: container_name} = container}}>
         <div class="min-w-20 py-2 px-4 h-full flex flew-wrap justify-center items-center">
           <.link
             navigate={Routes.container_show_path(Endpoint, :show, container)}
@@ -117,18 +117,18 @@
             patch={Routes.ammo_group_index_path(Endpoint, :move, ammo_group)}
             class="mx-2 my-1 text-sm btn btn-primary"
           >
-            <%= gettext("Move ammo") %>
+            <%= dgettext("actions", "Move ammo") %>
           </.link>
         </div>
       </:container>
-      <:actions :let={ammo_group}>
+      <:actions :let={%{count: ammo_group_count} = ammo_group}>
         <div class="py-2 px-4 h-full space-x-4 flex justify-center items-center">
           <.link
             navigate={Routes.ammo_group_show_path(Endpoint, :show, ammo_group)}
             class="text-primary-600 link"
             aria-label={
-              gettext("View ammo group of %{ammo_group_count} bullets",
-                ammo_group_count: ammo_group.count
+              dgettext("actions", "View ammo group of %{ammo_group_count} bullets",
+                ammo_group_count: ammo_group_count
               )
             }
           >
@@ -139,8 +139,8 @@
             patch={Routes.ammo_group_index_path(Endpoint, :edit, ammo_group)}
             class="text-primary-600 link"
             aria-label={
-              gettext("Edit ammo group of %{ammo_group_count} bullets",
-                ammo_group_count: ammo_group.count
+              dgettext("actions", "Edit ammo group of %{ammo_group_count} bullets",
+                ammo_group_count: ammo_group_count
               )
             }
           >
@@ -151,8 +151,8 @@
             patch={Routes.ammo_group_index_path(Endpoint, :clone, ammo_group)}
             class="text-primary-600 link"
             aria-label={
-              gettext("Clone ammo group of %{ammo_group_count} bullets",
-                ammo_group_count: ammo_group.count
+              dgettext("actions", "Clone ammo group of %{ammo_group_count} bullets",
+                ammo_group_count: ammo_group_count
               )
             }
           >
@@ -166,8 +166,8 @@
             phx-value-id={ammo_group.id}
             data-confirm={dgettext("prompts", "Are you sure you want to delete this ammo?")}
             aria-label={
-              gettext("Delete ammo group of %{ammo_group_count} bullets",
-                ammo_group_count: ammo_group.count
+              dgettext("actions", "Delete ammo group of %{ammo_group_count} bullets",
+                ammo_group_count: ammo_group_count
               )
             }
           >
@@ -179,8 +179,8 @@
   <% end %>
 </div>
 
-<%= cond do %>
-  <% @live_action in [:new, :edit, :clone] -> %>
+<%= case @live_action do %>
+  <% create when create in [:new, :edit, :clone] -> %>
     <.modal return_to={Routes.ammo_group_index_path(Endpoint, :index)}>
       <.live_component
         module={CanneryWeb.AmmoGroupLive.FormComponent}
@@ -192,7 +192,7 @@
         current_user={@current_user}
       />
     </.modal>
-  <% @live_action == :add_shot_group -> %>
+  <% :add_shot_group -> %>
     <.modal return_to={Routes.ammo_group_index_path(Endpoint, :index)}>
       <.live_component
         module={CanneryWeb.Components.AddShotGroupComponent}
@@ -204,7 +204,7 @@
         current_user={@current_user}
       />
     </.modal>
-  <% @live_action == :move -> %>
+  <% :move -> %>
     <.modal return_to={Routes.ammo_group_index_path(Endpoint, :index)}>
       <.live_component
         module={CanneryWeb.Components.MoveAmmoGroupComponent}
@@ -216,6 +216,5 @@
         current_user={@current_user}
       />
     </.modal>
-  <% true -> %>
-    <%= nil %>
+  <% _ -> %>
 <% end %>

lib/cannery_web/live/ammo_group_live/show.ex

@@ -4,7 +4,9 @@ defmodule CanneryWeb.AmmoGroupLive.Show do
   """
 
   use CanneryWeb, :live_view
-  alias Cannery.{ActivityLog, ActivityLog.ShotGroup, Ammo, Ammo.AmmoGroup, Repo}
+  alias Cannery.{ActivityLog, ActivityLog.ShotGroup}
+  alias Cannery.{Ammo, Ammo.AmmoGroup}
+  alias Cannery.Containers
   alias CanneryWeb.Endpoint
   alias Phoenix.LiveView.Socket
 
@@ -81,30 +83,45 @@ defmodule CanneryWeb.AmmoGroupLive.Show do
   end
 
   @spec display_ammo_group(Socket.t(), AmmoGroup.t() | AmmoGroup.id()) :: Socket.t()
-  defp display_ammo_group(socket, %AmmoGroup{} = ammo_group) do
-    ammo_group = ammo_group |> Repo.preload([:container, :ammo_type, :shot_groups], force: true)
-
+  defp display_ammo_group(
+         %{assigns: %{current_user: current_user}} = socket,
+         %AmmoGroup{container_id: container_id} = ammo_group
+       ) do
     columns = [
       %{label: gettext("Rounds shot"), key: :count},
       %{label: gettext("Notes"), key: :notes},
-      %{label: gettext("Date"), key: :date},
+      %{label: gettext("Date"), key: :date, type: Date},
       %{label: nil, key: :actions, sortable: false}
     ]
 
+    shot_groups = ActivityLog.list_shot_groups_for_ammo_group(ammo_group, current_user)
+
     rows =
-      ammo_group.shot_groups
+      shot_groups
       |> Enum.map(fn shot_group ->
         ammo_group |> get_table_row_for_shot_group(shot_group, columns)
       end)
 
-    socket |> assign(ammo_group: ammo_group, columns: columns, rows: rows)
+    socket
+    |> assign(
+      ammo_group: ammo_group,
+      original_count: Ammo.get_original_count(ammo_group, current_user),
+      percentage_remaining: Ammo.get_percentage_remaining(ammo_group, current_user),
+      container: container_id && Containers.get_container!(container_id, current_user),
+      shot_groups: shot_groups,
+      columns: columns,
+      rows: rows
+    )
   end
 
   defp display_ammo_group(%{assigns: %{current_user: current_user}} = socket, id),
     do: display_ammo_group(socket, Ammo.get_ammo_group!(id, current_user))
 
+  @spec display_currency(float()) :: String.t()
+  defp display_currency(float), do: :erlang.float_to_binary(float, decimals: 2)
+
   @spec get_table_row_for_shot_group(AmmoGroup.t(), ShotGroup.t(), [map()]) :: map()
-  defp get_table_row_for_shot_group(ammo_group, %{date: date} = shot_group, columns) do
+  defp get_table_row_for_shot_group(ammo_group, %{id: id, date: date} = shot_group, columns) do
     assigns = %{ammo_group: ammo_group, shot_group: shot_group}
 
     columns
@@ -112,11 +129,11 @@ defmodule CanneryWeb.AmmoGroupLive.Show do
       value =
         case key do
           :date ->
-            assigns = %{date: date}
+            assigns = %{id: id, date: date}
 
             {date,
              ~H"""
-             <.date date={@date} />
+             <.date id={"#{@id}-date"} date={@date} />
              """}
 
           :actions ->

lib/cannery_web/live/ammo_group_live/show.html.heex

@@ -11,12 +11,12 @@
 
     <span class="rounded-lg title text-lg">
       <%= gettext("Original count:") %>
-      <%= Ammo.get_original_count(@ammo_group) %>
+      <%= @original_count %>
     </span>
 
     <span class="rounded-lg title text-lg">
       <%= gettext("Percentage left:") %>
-      <%= gettext("%{percentage}%", percentage: @ammo_group |> Ammo.get_percentage_remaining()) %>
+      <%= gettext("%{percentage}%", percentage: @percentage_remaining) %>
     </span>
 
     <%= if @ammo_group.notes do %>
@@ -28,23 +28,19 @@
 
     <span class="rounded-lg title text-lg">
       <%= gettext("Purchased on:") %>
-      <.date date={@ammo_group.purchased_on} />
+      <.date id={"#{@ammo_group.id}-purchased-on"} date={@ammo_group.purchased_on} />
     </span>
 
     <%= if @ammo_group.price_paid do %>
       <span class="rounded-lg title text-lg">
         <%= gettext("Original cost:") %>
-        <%= gettext("$%{amount}",
-          amount: @ammo_group.price_paid |> :erlang.float_to_binary(decimals: 2)
-        ) %>
+        <%= gettext("$%{amount}", amount: display_currency(@ammo_group.price_paid)) %>
       </span>
 
       <span class="rounded-lg title text-lg">
         <%= gettext("Current value:") %>
         <%= gettext("$%{amount}",
-          amount:
-            (@ammo_group.price_paid * Ammo.get_percentage_remaining(@ammo_group) / 100)
-            |> :erlang.float_to_binary(decimals: 2)
+          amount: display_currency(@ammo_group.price_paid * @percentage_remaining / 100)
         ) %>
       </span>
     <% end %>
@@ -77,7 +73,7 @@
         phx-click="delete"
         data-confirm={dgettext("prompts", "Are you sure you want to delete this ammo?")}
         aria-label={
-          gettext("Delete ammo group of %{ammo_group_count} bullets",
+          dgettext("actions", "Delete ammo group of %{ammo_group_count} bullets",
             ammo_group_count: @ammo_group.count
           )
         }
@@ -97,7 +93,7 @@
         patch={Routes.ammo_group_show_path(Endpoint, :move, @ammo_group)}
         class="btn btn-primary"
       >
-        <%= dgettext("actions", "Move containers") %>
+        <%= dgettext("actions", "Move ammo") %>
       </.link>
 
       <.link
@@ -112,18 +108,18 @@
   <hr class="mb-4 w-full" />
 
   <div>
-    <%= if @ammo_group.container do %>
+    <%= if @container do %>
       <h1 class="mb-4 px-4 py-2 text-center rounded-lg title text-xl">
         <%= gettext("Stored in") %>
       </h1>
 
-      <.container_card container={@ammo_group.container} />
+      <.container_card container={@container} current_user={@current_user} />
     <% else %>
       <%= gettext("This ammo is not in a container") %>
     <% end %>
   </div>
 
-  <%= unless @ammo_group.shot_groups |> Enum.empty?() do %>
+  <%= unless @shot_groups |> Enum.empty?() do %>
     <hr class="mb-4 w-full" />
 
     <h1 class="mb-4 px-4 py-2 text-center rounded-lg title text-xl">