From 7e55446b3eac69fae33f3859ae3714da2a573b58 Mon Sep 17 00:00:00 2001 From: shibao Date: Mon, 5 Jun 2023 21:47:03 -0400 Subject: [PATCH] improve ActivityLog.list_shot_records --- lib/cannery/activity_log.ex | 68 +++++++++---------- .../controllers/export_controller.ex | 2 +- lib/cannery_web/live/pack_live/show.ex | 2 +- lib/cannery_web/live/range_live/index.ex | 2 +- test/cannery/activity_log_test.exs | 16 ++--- 5 files changed, 44 insertions(+), 46 deletions(-) diff --git a/lib/cannery/activity_log.ex b/lib/cannery/activity_log.ex index 6abc09b9..c91253a9 100644 --- a/lib/cannery/activity_log.ex +++ b/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(search :: nil | String.t(), Type.class() | :all, User.t()) :: - [ShotRecord.t()] - def list_shot_records(search \\ nil, type, %User{id: user_id}) do + @spec list_shot_records(User.t()) :: [ShotRecord.t()] + @spec list_shot_records(User.t(), list_shot_records_options()) :: [ShotRecord.t()] + def list_shot_records(%User{id: user_id}, opts \\ []) 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, - as: :at, - on: p.type_id == at.id, + on: p.user_id == ^user_id, + left_join: t in Type, + 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_filter_type(type) + |> list_shot_records_search(Keyword.get(opts, :search)) + |> 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) :: - Queryable.t() - defp list_shot_records_filter_type(query, :rifle), - do: query |> where([at: at], at.class == :rifle) + @spec list_shot_records_class(Queryable.t(), Type.class() | :all | nil) :: Queryable.t() + defp list_shot_records_class(query, class) when class in [:rifle, :pistol, :shotgun], + do: query |> where([t: t], t.class == ^class) - defp list_shot_records_filter_type(query, :pistol), - do: query |> where([at: at], at.class == :pistol) + defp list_shot_records_class(query, _all), do: query - defp list_shot_records_filter_type(query, :shotgun), - do: query |> where([at: at], at.class == :shotgun) + @spec list_shot_records_pack_id(Queryable.t(), Pack.id() | nil) :: Queryable.t() + 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. diff --git a/lib/cannery_web/controllers/export_controller.ex b/lib/cannery_web/controllers/export_controller.ex index eea39c2e..7ec9d4ad 100644 --- a/lib/cannery_web/controllers/export_controller.ex +++ b/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) diff --git a/lib/cannery_web/live/pack_live/show.ex b/lib/cannery_web/live/pack_live/show.ex index 1e22cf6d..af837f54 100644 --- a/lib/cannery_web/live/pack_live/show.ex +++ b/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 diff --git a/lib/cannery_web/live/range_live/index.ex b/lib/cannery_web/live/range_live/index.ex index 29cbed77..a2d2661e 100644 --- a/lib/cannery_web/live/range_live/index.ex +++ b/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) diff --git a/test/cannery/activity_log_test.exs b/test/cannery/activity_log_test.exs index 9b13416d..f95705ba 100644 --- a/test/cannery/activity_log_test.exs +++ b/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 [^shotgun_shot_record] = ActivityLog.list_shot_records(:shotgun, current_user) - assert [^pistol_shot_record] = ActivityLog.list_shot_records(:pistol, current_user) + assert [^rifle_shot_record] = ActivityLog.list_shot_records(current_user, class: :rifle) + assert [^shotgun_shot_record] = ActivityLog.list_shot_records(current_user, class: :shotgun) + 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