Remove excessively defensive coding
This commit is contained in:
@@ -19,18 +19,28 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
|
|
||||||
"""
|
"""
|
||||||
def list_scrobble_rules(opts \\ []) do
|
def list_scrobble_rules(opts \\ []) do
|
||||||
query = from(r in ScrobbleRule, order_by: [desc: r.inserted_at])
|
query =
|
||||||
|
from r in ScrobbleRule,
|
||||||
|
order_by: [desc: r.inserted_at]
|
||||||
|
|
||||||
query =
|
query =
|
||||||
case Keyword.get(opts, :type) do
|
case Keyword.get(opts, :type) do
|
||||||
nil -> query
|
nil ->
|
||||||
type -> from(r in query, where: r.type == ^type)
|
query
|
||||||
|
|
||||||
|
type ->
|
||||||
|
from r in query,
|
||||||
|
where: r.type == ^type
|
||||||
end
|
end
|
||||||
|
|
||||||
query =
|
query =
|
||||||
case Keyword.get(opts, :enabled) do
|
case Keyword.get(opts, :enabled) do
|
||||||
nil -> query
|
nil ->
|
||||||
enabled -> from(r in query, where: r.enabled == ^enabled)
|
query
|
||||||
|
|
||||||
|
enabled ->
|
||||||
|
from r in query,
|
||||||
|
where: r.enabled == ^enabled
|
||||||
end
|
end
|
||||||
|
|
||||||
Repo.all(query)
|
Repo.all(query)
|
||||||
@@ -164,10 +174,6 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def apply_album_rule(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}. Expected 'album'"}
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Applies an artist rule to all matching scrobbled tracks.
|
Applies an artist rule to all matching scrobbled tracks.
|
||||||
|
|
||||||
@@ -202,10 +208,6 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def apply_artist_rule(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}. Expected 'artist'"}
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Applies a single rule based on its type.
|
Applies a single rule based on its type.
|
||||||
|
|
||||||
@@ -226,10 +228,6 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
apply_artist_rule(rule)
|
apply_artist_rule(rule)
|
||||||
end
|
end
|
||||||
|
|
||||||
def apply_rule(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}"}
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Applies all enabled rules.
|
Applies all enabled rules.
|
||||||
|
|
||||||
@@ -242,17 +240,12 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
def apply_all_rules do
|
def apply_all_rules do
|
||||||
rules = list_enabled_rules()
|
rules = list_enabled_rules()
|
||||||
|
|
||||||
results =
|
Enum.map(rules, fn rule ->
|
||||||
Enum.map(rules, fn rule ->
|
case apply_rule(rule) do
|
||||||
case apply_rule(rule) do
|
{:ok, count} -> {:ok, {rule.type, rule.match_value, count}}
|
||||||
{:ok, count} -> {:ok, {rule.type, rule.match_value, count}}
|
{:error, reason} -> {:error, {rule.type, rule.match_value, reason}}
|
||||||
{:error, reason} -> {:error, {rule.type, rule.match_value, reason}}
|
end
|
||||||
end
|
end)
|
||||||
end)
|
|
||||||
|
|
||||||
{:ok, results}
|
|
||||||
rescue
|
|
||||||
e -> {:error, "Failed to apply rules: #{Exception.message(e)}"}
|
|
||||||
end
|
end
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
@@ -274,10 +267,6 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
Repo.one(query) || 0
|
Repo.one(query) || 0
|
||||||
end
|
end
|
||||||
|
|
||||||
def count_album_matches(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}. Expected 'album'"}
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Counts how many tracks would be affected by an artist rule.
|
Counts how many tracks would be affected by an artist rule.
|
||||||
|
|
||||||
@@ -297,10 +286,6 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
Repo.one(query) || 0
|
Repo.one(query) || 0
|
||||||
end
|
end
|
||||||
|
|
||||||
def count_artist_matches(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}. Expected 'artist'"}
|
|
||||||
end
|
|
||||||
|
|
||||||
@doc """
|
@doc """
|
||||||
Counts how many tracks would be affected by a rule.
|
Counts how many tracks would be affected by a rule.
|
||||||
|
|
||||||
@@ -317,8 +302,4 @@ defmodule MusicLibrary.ScrobbleRules do
|
|||||||
def count_rule_matches(%ScrobbleRule{type: "artist"} = rule) do
|
def count_rule_matches(%ScrobbleRule{type: "artist"} = rule) do
|
||||||
count_artist_matches(rule)
|
count_artist_matches(rule)
|
||||||
end
|
end
|
||||||
|
|
||||||
def count_rule_matches(%ScrobbleRule{type: type}) do
|
|
||||||
{:error, "Invalid rule type: #{type}"}
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -16,14 +16,8 @@ defmodule MusicLibrary.ScrobbleRules.Worker do
|
|||||||
def perform(%Oban.Job{args: _}) do
|
def perform(%Oban.Job{args: _}) do
|
||||||
Logger.info("Starting scrobble rules application")
|
Logger.info("Starting scrobble rules application")
|
||||||
|
|
||||||
case ScrobbleRules.apply_all_rules() do
|
results = ScrobbleRules.apply_all_rules()
|
||||||
{:ok, results} ->
|
log_results(results)
|
||||||
log_results(results)
|
|
||||||
|
|
||||||
{:error, reason} ->
|
|
||||||
Logger.error("Failed to apply scrobble rules: #{inspect(reason)}")
|
|
||||||
{:error, reason}
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
defp log_results(results) do
|
defp log_results(results) do
|
||||||
|
|||||||
@@ -77,25 +77,20 @@ defmodule MusicLibraryWeb.ScrobbleRulesLive.Index do
|
|||||||
|
|
||||||
@impl true
|
@impl true
|
||||||
def handle_event("apply_all_rules", _params, socket) do
|
def handle_event("apply_all_rules", _params, socket) do
|
||||||
case ScrobbleRules.apply_all_rules() do
|
results = ScrobbleRules.apply_all_rules()
|
||||||
{:ok, results} ->
|
|
||||||
total_updated =
|
|
||||||
results
|
|
||||||
|> Enum.filter(fn {status, _} -> status == :ok end)
|
|
||||||
|> Enum.map(fn {:ok, {_, _, count}} -> count end)
|
|
||||||
|> Enum.sum()
|
|
||||||
|
|
||||||
message =
|
total_updated =
|
||||||
gettext("All rules applied successfully. Updated %{count} tracks total.",
|
results
|
||||||
count: total_updated
|
|> Enum.filter(fn {status, _} -> status == :ok end)
|
||||||
)
|
|> Enum.map(fn {:ok, {_, _, count}} -> count end)
|
||||||
|
|> Enum.sum()
|
||||||
|
|
||||||
{:noreply, put_flash(socket, :info, message)}
|
message =
|
||||||
|
gettext("All rules applied successfully. Updated %{count} tracks total.",
|
||||||
|
count: total_updated
|
||||||
|
)
|
||||||
|
|
||||||
{:error, reason} ->
|
{:noreply, put_flash(socket, :info, message)}
|
||||||
message = gettext("Error applying rules: %{reason}", reason: reason)
|
|
||||||
{:noreply, put_flash(socket, :error, message)}
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
defp rule_type_badge(type) do
|
defp rule_type_badge(type) do
|
||||||
|
|||||||
@@ -1034,11 +1034,6 @@ msgstr ""
|
|||||||
msgid "Error applying rule: %{reason}"
|
msgid "Error applying rule: %{reason}"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
#: lib/music_library_web/live/scrobble_rules_live/index.ex
|
|
||||||
#, elixir-autogen, elixir-format
|
|
||||||
msgid "Error applying rules: %{reason}"
|
|
||||||
msgstr ""
|
|
||||||
|
|
||||||
#: lib/music_library_web/live/scrobble_rules_live/index.html.heex
|
#: lib/music_library_web/live/scrobble_rules_live/index.html.heex
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Manage rules to fix data in scrobbled tracks"
|
msgid "Manage rules to fix data in scrobbled tracks"
|
||||||
|
|||||||
@@ -1034,11 +1034,6 @@ msgstr ""
|
|||||||
msgid "Error applying rule: %{reason}"
|
msgid "Error applying rule: %{reason}"
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
#: lib/music_library_web/live/scrobble_rules_live/index.ex
|
|
||||||
#, elixir-autogen, elixir-format
|
|
||||||
msgid "Error applying rules: %{reason}"
|
|
||||||
msgstr ""
|
|
||||||
|
|
||||||
#: lib/music_library_web/live/scrobble_rules_live/index.html.heex
|
#: lib/music_library_web/live/scrobble_rules_live/index.html.heex
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Manage rules to fix data in scrobbled tracks"
|
msgid "Manage rules to fix data in scrobbled tracks"
|
||||||
|
|||||||
@@ -188,16 +188,6 @@ defmodule MusicLibrary.ScrobbleRulesTest do
|
|||||||
assert updated_track.artist.musicbrainz_id == rule.target_musicbrainz_id
|
assert updated_track.artist.musicbrainz_id == rule.target_musicbrainz_id
|
||||||
end
|
end
|
||||||
|
|
||||||
test "apply_album_rule/1 with wrong type returns error" do
|
|
||||||
rule = scrobble_rule_fixture(@valid_artist_attrs)
|
|
||||||
assert {:error, _reason} = ScrobbleRules.apply_album_rule(rule)
|
|
||||||
end
|
|
||||||
|
|
||||||
test "apply_artist_rule/1 with wrong type returns error" do
|
|
||||||
rule = scrobble_rule_fixture(@valid_album_attrs)
|
|
||||||
assert {:error, _reason} = ScrobbleRules.apply_artist_rule(rule)
|
|
||||||
end
|
|
||||||
|
|
||||||
test "apply_rule/1 delegates to correct function based on type" do
|
test "apply_rule/1 delegates to correct function based on type" do
|
||||||
album_rule = scrobble_rule_fixture(@valid_album_attrs)
|
album_rule = scrobble_rule_fixture(@valid_album_attrs)
|
||||||
artist_rule = scrobble_rule_fixture(@valid_artist_attrs)
|
artist_rule = scrobble_rule_fixture(@valid_artist_attrs)
|
||||||
@@ -239,7 +229,7 @@ defmodule MusicLibrary.ScrobbleRulesTest do
|
|||||||
album: %{musicbrainz_id: "", title: "Different Album"}
|
album: %{musicbrainz_id: "", title: "Different Album"}
|
||||||
})
|
})
|
||||||
|
|
||||||
assert {:ok, results} = ScrobbleRules.apply_all_rules()
|
assert results = ScrobbleRules.apply_all_rules()
|
||||||
assert length(results) == 2
|
assert length(results) == 2
|
||||||
|
|
||||||
# Verify all results are successful
|
# Verify all results are successful
|
||||||
|
|||||||
Reference in New Issue
Block a user