From a94d2eebf42cf2a4c87585e0ec3bf1e61f573742 Mon Sep 17 00:00:00 2001 From: shibao Date: Sun, 4 Jun 2023 20:53:57 -0400 Subject: [PATCH] improve Ammo.get_packs_count --- CHANGELOG.md | 2 +- lib/cannery/ammo.ex | 115 ++++++------------ .../core_components/container_card.html.heex | 4 +- .../controllers/export_controller.ex | 2 +- lib/cannery_web/live/container_live/show.ex | 4 +- lib/cannery_web/live/pack_live/index.ex | 2 +- lib/cannery_web/live/type_live/show.ex | 6 +- priv/gettext/de/LC_MESSAGES/errors.po | 2 +- priv/gettext/en/LC_MESSAGES/errors.po | 2 +- priv/gettext/errors.pot | 2 +- priv/gettext/es/LC_MESSAGES/errors.po | 2 +- priv/gettext/fr/LC_MESSAGES/errors.po | 2 +- priv/gettext/ga/LC_MESSAGES/errors.po | 2 +- test/cannery/ammo_test.exs | 52 ++++---- .../controllers/export_controller_test.exs | 7 +- 15 files changed, 82 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be9d5e1b..79c7ddc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -# v0.10.0 +# v0.9.4 - Code quality fixes - Fix error/404 pages not rendering properly diff --git a/lib/cannery/ammo.ex b/lib/cannery/ammo.ex index 1fc901a4..b3696a09 100644 --- a/lib/cannery/ammo.ex +++ b/lib/cannery/ammo.ex @@ -512,68 +512,66 @@ defmodule Cannery.Ammo do defp list_packs_staged(query, _nil), do: query + @type get_packs_count_option :: + {:container_id, Container.id() | nil} + | {:type_id, Type.id() | nil} + | {:show_used, :only_used | boolean() | nil} + @type get_packs_count_options :: [get_packs_count_option()] + @doc """ Returns a count of packs. ## Examples - iex> get_packs_count!(%User{id: 123}) + iex> get_packs_count(%User{id: 123}) 3 - iex> get_packs_count!(%User{id: 123}, true) + iex> get_packs_count(%User{id: 123}, show_used: true) 4 + iex> get_packs_count(%User{id: 123}, container_id: 456) + 1 + + iex> get_packs_count(%User{id: 123}, type_id: 456) + 2 + """ - @spec get_packs_count!(User.t()) :: integer() - @spec get_packs_count!(User.t(), show_used :: boolean()) :: integer() - def get_packs_count!(%User{id: user_id}, show_used \\ false) do + @spec get_packs_count(User.t()) :: integer() + @spec get_packs_count(User.t(), get_packs_count_options()) :: integer() + def get_packs_count(%User{id: user_id}, opts \\ []) do from(p in Pack, as: :p, where: p.user_id == ^user_id, select: count(p.id), distinct: true ) - |> get_packs_count_show_used(show_used) + |> get_packs_count_show_used(Keyword.get(opts, :show_used)) + |> get_packs_count_container_id(Keyword.get(opts, :container_id)) + |> get_packs_count_type_id(Keyword.get(opts, :type_id)) |> Repo.one() || 0 end - @spec get_packs_count_show_used(Queryable.t(), show_used :: boolean()) :: Queryable.t() - defp get_packs_count_show_used(query, false), + @spec get_packs_count_show_used(Queryable.t(), show_used :: :only_used | boolean() | nil) :: + Queryable.t() + defp get_packs_count_show_used(query, true), do: query + + defp get_packs_count_show_used(query, :only_used), + do: query |> where([p: p], p.count == 0) + + defp get_packs_count_show_used(query, _false), do: query |> where([p: p], p.count > 0) - defp get_packs_count_show_used(query, _true), do: query + @spec get_packs_count_type_id(Queryable.t(), Type.id() | nil) :: Queryable.t() + defp get_packs_count_type_id(query, type_id) when type_id |> is_binary(), + do: query |> where([p: p], p.type_id == ^type_id) - @doc """ - Returns the count of packs for a type. + defp get_packs_count_type_id(query, _nil), do: query - ## Examples + @spec get_packs_count_container_id(Queryable.t(), Container.id() | nil) :: Queryable.t() + defp get_packs_count_container_id(query, container_id) when container_id |> is_binary(), + do: query |> where([p: p], p.container_id == ^container_id) - iex> get_packs_count_for_type( - ...> %Type{id: 123, user_id: 456}, - ...> %User{id: 456} - ...> ) - 3 - - iex> get_packs_count_for_type( - ...> %Type{id: 123, user_id: 456}, - ...> %User{id: 456}, - ...> true - ...> ) - 5 - - """ - @spec get_packs_count_for_type(Type.t(), User.t()) :: non_neg_integer() - @spec get_packs_count_for_type(Type.t(), User.t(), show_used :: boolean()) :: - non_neg_integer() - def get_packs_count_for_type( - %Type{id: type_id} = type, - user, - show_used \\ false - ) do - [type] - |> get_packs_count_for_types(user, show_used) - |> Map.get(type_id, 0) - end + defp get_packs_count_container_id(query, _nil), do: query @doc """ Returns the count of packs for multiple types. @@ -623,25 +621,6 @@ defmodule Cannery.Ammo do query |> where([p: p], not (p.count == 0)) end - @doc """ - Returns the count of used packs for a type. - - ## Examples - - iex> get_used_packs_count_for_type( - ...> %Type{id: 123, user_id: 456}, - ...> %User{id: 456} - ...> ) - 3 - - """ - @spec get_used_packs_count_for_type(Type.t(), User.t()) :: non_neg_integer() - def get_used_packs_count_for_type(%Type{id: type_id} = type, user) do - [type] - |> get_used_packs_count_for_types(user) - |> Map.get(type_id, 0) - end - @doc """ Returns the count of used packs for multiple types. @@ -672,28 +651,6 @@ defmodule Cannery.Ammo do |> Map.new() end - @doc """ - Returns number of ammo packs in a container. - - ## Examples - - iex> get_packs_count_for_container( - ...> %Container{id: 123, user_id: 456}, - ...> %User{id: 456} - ...> ) - 3 - - """ - @spec get_packs_count_for_container!(Container.t(), User.t()) :: non_neg_integer() - def get_packs_count_for_container!( - %Container{id: container_id} = container, - %User{} = user - ) do - [container] - |> get_packs_count_for_containers(user) - |> Map.get(container_id, 0) - end - @doc """ Returns number of ammo packs in multiple containers. diff --git a/lib/cannery_web/components/core_components/container_card.html.heex b/lib/cannery_web/components/core_components/container_card.html.heex index a4fd4877..a202f8a3 100644 --- a/lib/cannery_web/components/core_components/container_card.html.heex +++ b/lib/cannery_web/components/core_components/container_card.html.heex @@ -27,10 +27,10 @@ <%= @container.location %> - <%= if @container |> Ammo.get_packs_count_for_container!(@current_user) != 0 do %> + <%= if Ammo.get_packs_count(@current_user, container_id: @container.id) != 0 do %> <%= gettext("Packs:") %> - <%= @container |> Ammo.get_packs_count_for_container!(@current_user) %> + <%= Ammo.get_packs_count(@current_user, container_id: @container.id) %> diff --git a/lib/cannery_web/controllers/export_controller.ex b/lib/cannery_web/controllers/export_controller.ex index e596a04a..02d9285d 100644 --- a/lib/cannery_web/controllers/export_controller.ex +++ b/lib/cannery_web/controllers/export_controller.ex @@ -52,7 +52,7 @@ defmodule CanneryWeb.ExportController do containers = Containers.list_containers(current_user) |> Enum.map(fn container -> - pack_count = container |> Ammo.get_packs_count_for_container!(current_user) + pack_count = Ammo.get_packs_count(current_user, container_id: container.id) round_count = container |> Ammo.get_round_count_for_container!(current_user) container diff --git a/lib/cannery_web/live/container_live/show.ex b/lib/cannery_web/live/container_live/show.ex index 19572920..9bfb8b0d 100644 --- a/lib/cannery_web/live/container_live/show.ex +++ b/lib/cannery_web/live/container_live/show.ex @@ -122,8 +122,8 @@ defmodule CanneryWeb.ContainerLive.Show do socket |> assign( container: container, - round_count: Ammo.get_round_count_for_container!(container, current_user), - packs_count: Ammo.get_packs_count_for_container!(container, current_user), + round_count: container |> Ammo.get_round_count_for_container!(current_user), + packs_count: Ammo.get_packs_count(current_user, container_id: container.id), packs: packs, original_counts: original_counts, cprs: cprs, diff --git a/lib/cannery_web/live/pack_live/index.ex b/lib/cannery_web/live/pack_live/index.ex index 96a8eb51..bb902c23 100644 --- a/lib/cannery_web/live/pack_live/index.ex +++ b/lib/cannery_web/live/pack_live/index.ex @@ -148,7 +148,7 @@ defmodule CanneryWeb.PackLive.Index do ) do # get total number of packs to determine whether to display onboarding # prompts - packs_count = Ammo.get_packs_count!(current_user, true) + packs_count = Ammo.get_packs_count(current_user, show_used: true) packs = Ammo.list_packs(current_user, search: search, class: class, show_used: show_used) types_count = Ammo.get_types_count!(current_user) containers_count = Containers.get_containers_count!(current_user) diff --git a/lib/cannery_web/live/type_live/show.ex b/lib/cannery_web/live/type_live/show.ex index d2b27fe1..ab5199ed 100644 --- a/lib/cannery_web/live/type_live/show.ex +++ b/lib/cannery_web/live/type_live/show.ex @@ -66,8 +66,8 @@ defmodule CanneryWeb.TypeLive.Show do if show_used do [ packs |> Ammo.get_original_counts(current_user), - type |> Ammo.get_used_packs_count_for_type(current_user), - type |> Ammo.get_packs_count_for_type(current_user, true), + Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used), + Ammo.get_packs_count(current_user, type_id: type.id, show_used: true), type |> ActivityLog.get_used_count_for_type(current_user), type |> Ammo.get_historical_count_for_type(current_user) ] @@ -99,7 +99,7 @@ defmodule CanneryWeb.TypeLive.Show do original_counts: original_counts, used_rounds: used_rounds, historical_round_count: historical_round_count, - packs_count: type |> Ammo.get_packs_count_for_type(current_user), + packs_count: Ammo.get_packs_count(current_user, type_id: type.id), used_packs_count: used_packs_count, historical_packs_count: historical_packs_count, fields_to_display: fields_to_display(type), diff --git a/priv/gettext/de/LC_MESSAGES/errors.po b/priv/gettext/de/LC_MESSAGES/errors.po index 0a244233..09b502ca 100644 --- a/priv/gettext/de/LC_MESSAGES/errors.po +++ b/priv/gettext/de/LC_MESSAGES/errors.po @@ -170,7 +170,7 @@ msgstr "" "Ungültige Nummer an Kopien. Muss zwischen 1 and %{max} liegen. War " "%{multiplier}" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "" diff --git a/priv/gettext/en/LC_MESSAGES/errors.po b/priv/gettext/en/LC_MESSAGES/errors.po index 01525152..6f244595 100644 --- a/priv/gettext/en/LC_MESSAGES/errors.po +++ b/priv/gettext/en/LC_MESSAGES/errors.po @@ -153,7 +153,7 @@ msgstr "" msgid "Invalid number of copies, must be between 1 and %{max}. Was %{multiplier}" msgstr "" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "" diff --git a/priv/gettext/errors.pot b/priv/gettext/errors.pot index 8218e8fc..b776eb92 100644 --- a/priv/gettext/errors.pot +++ b/priv/gettext/errors.pot @@ -152,7 +152,7 @@ msgstr "" msgid "Invalid number of copies, must be between 1 and %{max}. Was %{multiplier}" msgstr "" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "" diff --git a/priv/gettext/es/LC_MESSAGES/errors.po b/priv/gettext/es/LC_MESSAGES/errors.po index eee68490..2e189277 100644 --- a/priv/gettext/es/LC_MESSAGES/errors.po +++ b/priv/gettext/es/LC_MESSAGES/errors.po @@ -168,7 +168,7 @@ msgstr "No se ha podido procesar el número de copias" msgid "Invalid number of copies, must be between 1 and %{max}. Was %{multiplier}" msgstr "Número inválido de copias, debe ser entre 1 y %{max}. Fue %{multiplier" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "Multiplicador inválido" diff --git a/priv/gettext/fr/LC_MESSAGES/errors.po b/priv/gettext/fr/LC_MESSAGES/errors.po index 4d250d20..aa705342 100644 --- a/priv/gettext/fr/LC_MESSAGES/errors.po +++ b/priv/gettext/fr/LC_MESSAGES/errors.po @@ -169,7 +169,7 @@ msgstr "Impossible d'analyser le nombre de copies" msgid "Invalid number of copies, must be between 1 and %{max}. Was %{multiplier}" msgstr "Nombre de copies invalide, doit être 1 et %{max}. Été %{multiplier}" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "Multiplicateur invalide" diff --git a/priv/gettext/ga/LC_MESSAGES/errors.po b/priv/gettext/ga/LC_MESSAGES/errors.po index 8c822dc1..a05cf375 100644 --- a/priv/gettext/ga/LC_MESSAGES/errors.po +++ b/priv/gettext/ga/LC_MESSAGES/errors.po @@ -168,7 +168,7 @@ msgstr "" msgid "Invalid number of copies, must be between 1 and %{max}. Was %{multiplier}" msgstr "" -#: lib/cannery/ammo.ex:1033 +#: lib/cannery/ammo.ex:990 #, elixir-autogen, elixir-format msgid "Invalid multiplier" msgstr "" diff --git a/test/cannery/ammo_test.exs b/test/cannery/ammo_test.exs index c4c11cd4..73ab7ae8 100644 --- a/test/cannery/ammo_test.exs +++ b/test/cannery/ammo_test.exs @@ -471,23 +471,23 @@ defmodule Cannery.AmmoTest do assert %{^another_type_id => 1} = historical_counts end - test "get_used_packs_count_for_type/2 gets accurate total ammo count for type", + test "get_packs_count/2 gets accurate total ammo count for type with show_used", %{type: type, current_user: current_user, container: container} do - assert 0 = Ammo.get_used_packs_count_for_type(type, current_user) + assert 0 = Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used) {1, [first_pack]} = pack_fixture(%{count: 1}, type, container, current_user) - assert 0 = Ammo.get_used_packs_count_for_type(type, current_user) + assert 0 = Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used) {1, [pack]} = pack_fixture(%{count: 50}, type, container, current_user) - assert 0 = Ammo.get_used_packs_count_for_type(type, current_user) + assert 0 = Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used) shot_record_fixture(%{count: 50}, current_user, pack) - assert 1 = Ammo.get_used_packs_count_for_type(type, current_user) + assert 1 = Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used) shot_record_fixture(%{count: 1}, current_user, first_pack) - assert 2 = Ammo.get_used_packs_count_for_type(type, current_user) + assert 2 = Ammo.get_packs_count(current_user, type_id: type.id, show_used: :only_used) end test "get_used_packs_count_for_types/2 gets accurate total ammo counts for types", @@ -540,21 +540,21 @@ defmodule Cannery.AmmoTest do assert %{^another_type_id => 1} = used_counts end - test "get_packs_count_for_container!/2 gets accurate ammo count for container", + test "get_packs_count/2 gets accurate ammo count for container by container_id", %{type: type, current_user: current_user, container: container} do {1, [first_pack]} = pack_fixture(%{count: 5}, type, container, current_user) - assert 1 = Ammo.get_packs_count_for_container!(container, current_user) + assert 1 = Ammo.get_packs_count(current_user, container_id: container.id) {25, _packs} = pack_fixture(%{count: 5}, 25, type, container, current_user) - assert 26 = Ammo.get_packs_count_for_container!(container, current_user) + assert 26 = Ammo.get_packs_count(current_user, container_id: container.id) shot_record_fixture(%{count: 1}, current_user, first_pack) - assert 26 = Ammo.get_packs_count_for_container!(container, current_user) + assert 26 = Ammo.get_packs_count(current_user, container_id: container.id) shot_record_fixture(%{count: 4}, current_user, first_pack) - assert 25 = Ammo.get_packs_count_for_container!(container, current_user) + assert 25 = Ammo.get_packs_count(current_user, container_id: container.id) end test "get_packs_count_for_containers/2 gets accurate ammo count for containers", %{ @@ -607,14 +607,14 @@ defmodule Cannery.AmmoTest do %{type: type, current_user: current_user, container: container} do {1, [first_pack]} = pack_fixture(%{count: 5}, type, container, current_user) - assert 5 = Ammo.get_round_count_for_container!(container, current_user) + assert 5 = container |> Ammo.get_round_count_for_container!(current_user) {25, _packs} = pack_fixture(%{count: 5}, 25, type, container, current_user) - assert 130 = Ammo.get_round_count_for_container!(container, current_user) + assert 130 = container |> Ammo.get_round_count_for_container!(current_user) shot_record_fixture(%{count: 5}, current_user, first_pack) - assert 125 = Ammo.get_round_count_for_container!(container, current_user) + assert 125 = container |> Ammo.get_round_count_for_container!(current_user) end test "get_round_count_for_containers/2 gets accurate total round count for containers", @@ -692,19 +692,19 @@ defmodule Cannery.AmmoTest do ] end - test "get_packs_count!/2 returns the correct amount of ammo", + test "get_packs_count/2 returns the correct amount of ammo", %{type: type, container: container, current_user: current_user} do - assert Ammo.get_packs_count!(current_user) == 1 + assert Ammo.get_packs_count(current_user) == 1 pack_fixture(type, container, current_user) - assert Ammo.get_packs_count!(current_user) == 2 + assert Ammo.get_packs_count(current_user) == 2 pack_fixture(type, container, current_user) - assert Ammo.get_packs_count!(current_user) == 3 + assert Ammo.get_packs_count(current_user) == 3 other_user = user_fixture() - assert Ammo.get_packs_count!(other_user) == 0 - assert Ammo.get_packs_count!(other_user, true) == 0 + assert Ammo.get_packs_count(other_user) == 0 + assert Ammo.get_packs_count(other_user, show_used: true) == 0 other_type = type_fixture(other_user) other_container = container_fixture(other_user) @@ -712,8 +712,8 @@ defmodule Cannery.AmmoTest do {1, [another_pack]} = pack_fixture(%{count: 30}, other_type, other_container, other_user) shot_record_fixture(%{count: 30}, other_user, another_pack) - assert Ammo.get_packs_count!(other_user) == 0 - assert Ammo.get_packs_count!(other_user, true) == 1 + assert Ammo.get_packs_count(other_user) == 0 + assert Ammo.get_packs_count(other_user, show_used: true) == 1 end test "list_packs/2 returns all packs for a type" do @@ -874,18 +874,18 @@ defmodule Cannery.AmmoTest do assert pistol_pack in packs end - test "get_packs_count_for_type/2 returns count of packs for a type", %{ + test "get_packs_count/2 with type_id returns count of packs for a type", %{ type: type, container: container, current_user: current_user } do - assert 1 = Ammo.get_packs_count_for_type(type, current_user) + assert 1 = Ammo.get_packs_count(current_user, type_id: type.id) another_type = type_fixture(current_user) - assert 0 = Ammo.get_packs_count_for_type(another_type, current_user) + assert 0 = Ammo.get_packs_count(current_user, type_id: another_type.id) {5, _packs} = pack_fixture(%{}, 5, type, container, current_user) - assert 6 = Ammo.get_packs_count_for_type(type, current_user) + assert 6 = Ammo.get_packs_count(current_user, type_id: type.id) end test "get_packs_count_for_types/2 returns counts of packs for types", %{ diff --git a/test/cannery_web/controllers/export_controller_test.exs b/test/cannery_web/controllers/export_controller_test.exs index a2132ace..de89320c 100644 --- a/test/cannery_web/controllers/export_controller_test.exs +++ b/test/cannery_web/controllers/export_controller_test.exs @@ -93,8 +93,9 @@ defmodule CanneryWeb.ExportControllerTest do "average_cost" => type |> Ammo.get_average_cost_for_type(current_user), "round_count" => type |> Ammo.get_round_count_for_type(current_user), "used_count" => type |> ActivityLog.get_used_count_for_type(current_user), - "pack_count" => type |> Ammo.get_packs_count_for_type(current_user), - "total_pack_count" => type |> Ammo.get_packs_count_for_type(current_user, true) + "pack_count" => Ammo.get_packs_count(current_user, type_id: type.id), + "total_pack_count" => + Ammo.get_packs_count(current_user, type_id: type.id, show_used: true) } ideal_container = %{ @@ -111,7 +112,7 @@ defmodule CanneryWeb.ExportControllerTest do } ], "type" => container.type, - "pack_count" => container |> Ammo.get_packs_count_for_container!(current_user), + "pack_count" => Ammo.get_packs_count(current_user, container_id: container.id), "round_count" => container |> Ammo.get_round_count_for_container!(current_user) }