improve ActivityLog.list_shot_records

lib/cannery/activity_log.ex

@@ -8,38 +8,49 @@ defmodule Cannery.ActivityLog do
   alias Cannery.{Accounts.User, ActivityLog.ShotRecord, Repo}
   alias Ecto.{Multi, Queryable}
 
+  @type list_shot_records_option ::
+          {:search, String.t() | nil}
+          | {:class, Type.class() | :all | nil}
+          | {:pack_id, Pack.id() | nil}
+  @type list_shot_records_options :: [list_shot_records_option()]
+
   @doc """
   Returns the list of shot_records.
 
   ## Examples
 
-      iex> list_shot_records(:all, %User{id: 123})
+      iex> list_shot_records(%User{id: 123})
       [%ShotRecord{}, ...]
 
-      iex> list_shot_records("cool", :all, %User{id: 123})
+      iex> list_shot_records(%User{id: 123}, search: "cool")
       [%ShotRecord{notes: "My cool shot record"}, ...]
 
-      iex> list_shot_records("cool", :rifle, %User{id: 123})
+      iex> list_shot_records(%User{id: 123}, search: "cool", class: :rifle)
       [%ShotRecord{notes: "Shot some rifle rounds"}, ...]
 
+      iex> list_shot_records(%User{id: 123}, pack_id: 456)
+      [%ShotRecord{pack_id: 456}, ...]
+
   """
-  @spec list_shot_records(Type.class() | :all, User.t()) :: [ShotRecord.t()]
+  @spec list_shot_records(User.t()) :: [ShotRecord.t()]
-  @spec list_shot_records(search :: nil | String.t(), Type.class() | :all, User.t()) ::
+  @spec list_shot_records(User.t(), list_shot_records_options()) :: [ShotRecord.t()]
-          [ShotRecord.t()]
+  def list_shot_records(%User{id: user_id}, opts \\ []) do
-  def list_shot_records(search \\ nil, type, %User{id: user_id}) do
     from(sr in ShotRecord,
       as: :sr,
       left_join: p in Pack,
       as: :p,
       on: sr.pack_id == p.id,
-      left_join: at in Type,
+      on: p.user_id == ^user_id,
-      as: :at,
+      left_join: t in Type,
-      on: p.type_id == at.id,
+      as: :t,
+      on: p.type_id == t.id,
+      on: t.user_id == ^user_id,
       where: sr.user_id == ^user_id,
       distinct: sr.id
     )
-    |> list_shot_records_search(search)
+    |> list_shot_records_search(Keyword.get(opts, :search))
-    |> list_shot_records_filter_type(type)
+    |> list_shot_records_class(Keyword.get(opts, :class))
+    |> list_shot_records_pack_id(Keyword.get(opts, :pack_id))
     |> Repo.all()
   end
 
@@ -52,7 +63,7 @@ defmodule Cannery.ActivityLog do
 
     query
     |> where(
-      [sr: sr, p: p, at: at],
+      [sr: sr, p: p, t: t],
       fragment(
         "? @@ websearch_to_tsquery('english', ?)",
         sr.search,
@@ -65,7 +76,7 @@ defmodule Cannery.ActivityLog do
         ) or
         fragment(
           "? @@ websearch_to_tsquery('english', ?)",
-          at.search,
+          t.search,
           ^trimmed_search
         )
     )
@@ -79,18 +90,17 @@ defmodule Cannery.ActivityLog do
     })
   end
 
-  @spec list_shot_records_filter_type(Queryable.t(), Type.class() | :all) ::
+  @spec list_shot_records_class(Queryable.t(), Type.class() | :all | nil) :: Queryable.t()
-          Queryable.t()
+  defp list_shot_records_class(query, class) when class in [:rifle, :pistol, :shotgun],
-  defp list_shot_records_filter_type(query, :rifle),
+    do: query |> where([t: t], t.class == ^class)
-    do: query |> where([at: at], at.class == :rifle)
 
-  defp list_shot_records_filter_type(query, :pistol),
+  defp list_shot_records_class(query, _all), do: query
-    do: query |> where([at: at], at.class == :pistol)
 
-  defp list_shot_records_filter_type(query, :shotgun),
+  @spec list_shot_records_pack_id(Queryable.t(), Pack.id() | nil) :: Queryable.t()
-    do: query |> where([at: at], at.class == :shotgun)
+  defp list_shot_records_pack_id(query, pack_id) when pack_id |> is_binary(),
+    do: query |> where([sr: sr], sr.pack_id == ^pack_id)
 
-  defp list_shot_records_filter_type(query, _all), do: query
+  defp list_shot_records_pack_id(query, _all), do: query
 
   @doc """
   Returns a count of shot records.
@@ -111,18 +121,6 @@ defmodule Cannery.ActivityLog do
     ) || 0
   end
 
-  @spec list_shot_records_for_pack(Pack.t(), User.t()) :: [ShotRecord.t()]
-  def list_shot_records_for_pack(
-        %Pack{id: pack_id, user_id: user_id},
-        %User{id: user_id}
-      ) do
-    Repo.all(
-      from sr in ShotRecord,
-        where: sr.pack_id == ^pack_id,
-        where: sr.user_id == ^user_id
-    )
-  end
-
   @doc """
   Gets a single shot_record.
 

lib/cannery_web/controllers/export_controller.ex

@@ -54,7 +54,7 @@ defmodule CanneryWeb.ExportController do
         })
       end)
 
-    shot_records = ActivityLog.list_shot_records(:all, current_user)
+    shot_records = ActivityLog.list_shot_records(current_user)
 
     containers =
       Containers.list_containers(current_user)

lib/cannery_web/live/pack_live/show.ex

@@ -92,7 +92,7 @@ defmodule CanneryWeb.PackLive.Show do
       %{label: gettext("Actions"), key: :actions, sortable: false}
     ]
 
-    shot_records = ActivityLog.list_shot_records_for_pack(pack, current_user)
+    shot_records = ActivityLog.list_shot_records(current_user, pack_id: pack.id)
 
     rows =
       shot_records

lib/cannery_web/live/range_live/index.ex

@@ -120,7 +120,7 @@ defmodule CanneryWeb.RangeLive.Index do
   defp display_shot_records(
          %{assigns: %{class: class, search: search, current_user: current_user}} = socket
        ) do
-    shot_records = ActivityLog.list_shot_records(search, class, current_user)
+    shot_records = ActivityLog.list_shot_records(current_user, search: search, class: class)
     packs = Ammo.list_packs(current_user, staged: true)
     chart_data = shot_records |> get_chart_data_for_shot_record()
     original_counts = packs |> Ammo.get_original_counts(current_user)

test/cannery/activity_log_test.exs

@@ -407,17 +407,17 @@ defmodule Cannery.ActivityLogTest do
       {1, [pistol_pack]} = pack_fixture(pistol_type, container, current_user)
       pistol_shot_record = shot_record_fixture(current_user, pistol_pack)
 
-      assert [^rifle_shot_record] = ActivityLog.list_shot_records(:rifle, current_user)
+      assert [^rifle_shot_record] = ActivityLog.list_shot_records(current_user, class: :rifle)
-      assert [^shotgun_shot_record] = ActivityLog.list_shot_records(:shotgun, current_user)
+      assert [^shotgun_shot_record] = ActivityLog.list_shot_records(current_user, class: :shotgun)
-      assert [^pistol_shot_record] = ActivityLog.list_shot_records(:pistol, current_user)
+      assert [^pistol_shot_record] = ActivityLog.list_shot_records(current_user, class: :pistol)
 
-      shot_records = ActivityLog.list_shot_records(:all, current_user)
+      shot_records = ActivityLog.list_shot_records(current_user, class: :all)
       assert Enum.count(shot_records) == 3
       assert rifle_shot_record in shot_records
       assert shotgun_shot_record in shot_records
       assert pistol_shot_record in shot_records
 
-      shot_records = ActivityLog.list_shot_records(nil, current_user)
+      shot_records = ActivityLog.list_shot_records(current_user, class: nil)
       assert Enum.count(shot_records) == 3
       assert rifle_shot_record in shot_records
       assert shotgun_shot_record in shot_records
@@ -451,13 +451,13 @@ defmodule Cannery.ActivityLogTest do
       _shouldnt_return = shot_record_fixture(another_user, another_pack)
 
       # notes
-      assert ActivityLog.list_shot_records("amazing", :all, current_user) == [shot_record_a]
+      assert ActivityLog.list_shot_records(current_user, search: "amazing") == [shot_record_a]
 
       # pack attributes
-      assert ActivityLog.list_shot_records("stupendous", :all, current_user) == [shot_record_b]
+      assert ActivityLog.list_shot_records(current_user, search: "stupendous") == [shot_record_b]
 
       # type attributes
-      assert ActivityLog.list_shot_records("fabulous", :all, current_user) == [shot_record_c]
+      assert ActivityLog.list_shot_records(current_user, search: "fabulous") == [shot_record_c]
     end
   end
 end