From f29dd1f0abc91ad8c2842f32fa635b1e54c1338b Mon Sep 17 00:00:00 2001 From: Claudio Ortolina Date: Wed, 5 Feb 2025 10:12:27 +0000 Subject: [PATCH] Fetch 100 releases when importing a record The release group contains only 25 releases, which breaks tracking scrobble status. Albums like "Toto IV" or "Hounds of Love" end up looking like they're not part of the collections because the individual releases are not stored. Still error prone, because this change doesn't paginate to more than 100 records, but that will be addressed in a future commit. --- lib/music_brainz/api_behaviour.ex | 2 ++ lib/music_brainz/api_impl.ex | 8 ++++++++ lib/music_library/records.ex | 19 ++++++++++++++++--- test/music_library/records_test.exs | 12 ++++++++++++ .../live/collection_live/index_test.exs | 4 ++++ .../live/stats_live/index_test.exs | 4 ++++ 6 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/music_brainz/api_behaviour.ex b/lib/music_brainz/api_behaviour.ex index ca27fb46..64a7738d 100644 --- a/lib/music_brainz/api_behaviour.ex +++ b/lib/music_brainz/api_behaviour.ex @@ -4,6 +4,8 @@ defmodule MusicBrainz.APIBehaviour do @callback get_release_group(musicbrainz_id, config) :: {:ok, map()} | {:error, String.t()} + @callback get_releases(musicbrainz_id, config) :: {:ok, map()} | {:error, String.t()} + @callback get_release(musicbrainz_id, config) :: {:ok, map()} | {:error, String.t()} @callback search_release_group(String.t(), Keyword.t(), config) :: diff --git a/lib/music_brainz/api_impl.ex b/lib/music_brainz/api_impl.ex index d20b33dc..03ea5233 100644 --- a/lib/music_brainz/api_impl.ex +++ b/lib/music_brainz/api_impl.ex @@ -287,6 +287,14 @@ defmodule MusicBrainz.APIImpl do json_get(url, config) end + @impl true + def get_releases(release_group_id, config) do + url = + "https://musicbrainz.org/ws/2/release?fmt=json&limit=100&release-group=#{release_group_id}" + + json_get(url, config) + end + @doc """ Uses the [search](https://musicbrainz.org/doc/MusicBrainz_API/Search#Release_Group) endpoint with a search query string. diff --git a/lib/music_library/records.ex b/lib/music_library/records.ex index 5197d2b0..00477a11 100644 --- a/lib/music_library/records.ex +++ b/lib/music_library/records.ex @@ -144,9 +144,10 @@ defmodule MusicLibrary.Records do purchased_at = Keyword.get(opts, :purchased_at), {:ok, release_group} <- music_brainz_config().api.get_release_group(musicbrainz_id, music_brainz_config()), + {:ok, release_group_with_releases} <- merge_releases(musicbrainz_id, release_group), {:ok, cover_data} <- get_cover_art_or_default(musicbrainz_id), record_attrs = - build_record_attrs(release_group, %{ + build_record_attrs(release_group_with_releases, %{ "cover_data" => cover_data, "format" => format, "purchased_at" => purchased_at @@ -217,13 +218,25 @@ defmodule MusicLibrary.Records do music_brainz_config().api.get_release_group( record.musicbrainz_id, music_brainz_config() - ) do + ), + {:ok, data_with_releases} <- merge_releases(record.musicbrainz_id, data) do record - |> Record.add_musicbrainz_data(data) + |> Record.add_musicbrainz_data(data_with_releases) |> Repo.update() end end + # TODO: paginate and merge as needed + defp merge_releases(musicbrainz_id, musicbrainz_data) do + with {:ok, data} <- + music_brainz_config().api.get_releases( + musicbrainz_id, + music_brainz_config() + ) do + {:ok, Map.put(musicbrainz_data, "releases", data["releases"])} + end + end + defp build_record_attrs(release_group, attrs) do release_group |> Record.attrs_from_release_group() diff --git a/test/music_library/records_test.exs b/test/music_library/records_test.exs index ed6b817a..9556ef05 100644 --- a/test/music_library/records_test.exs +++ b/test/music_library/records_test.exs @@ -66,6 +66,10 @@ defmodule MusicLibrary.RecordsTest do {:ok, release_group(:lockdown_trilogy)} end) + expect(APIBehaviourMock, :get_releases, fn ^release_group_id, _config -> + {:ok, %{"releases" => release_group(:lockdown_trilogy)["releases"]}} + end) + {:ok, updated_record} = Records.refresh_musicbrainz_data(record) assert record.release_ids !== updated_record.release_ids @@ -203,6 +207,10 @@ defmodule MusicLibrary.RecordsTest do {:ok, release_group} end) + expect(APIBehaviourMock, :get_releases, fn ^release_group_id, _config -> + {:ok, %{"releases" => release_group["releases"]}} + end) + cover_data = File.read!(marbles_cover_fixture()) expect(APIBehaviourMock, :get_cover_art, fn {:musicbrainz_id, ^release_group_id}, _config -> @@ -256,6 +264,10 @@ defmodule MusicLibrary.RecordsTest do {:ok, release_group} end) + expect(APIBehaviourMock, :get_releases, fn ^release_group_id, _config -> + {:ok, %{"releases" => release_group["releases"]}} + end) + cover_data = File.read!(marbles_cover_fixture()) expect(APIBehaviourMock, :get_cover_art, fn {:musicbrainz_id, ^release_group_id}, _config -> diff --git a/test/music_library_web/live/collection_live/index_test.exs b/test/music_library_web/live/collection_live/index_test.exs index 84468a72..0fdf8bc5 100644 --- a/test/music_library_web/live/collection_live/index_test.exs +++ b/test/music_library_web/live/collection_live/index_test.exs @@ -271,6 +271,10 @@ defmodule MusicLibraryWeb.CollectionLive.IndexTest do {:ok, release_group} end) + expect(APIBehaviourMock, :get_releases, fn ^first_result_id, _config -> + {:ok, %{"releases" => release_group["releases"]}} + end) + cover_data = File.read!(marbles_cover_fixture()) expect(APIBehaviourMock, :get_cover_art, fn {:musicbrainz_id, ^first_result_id}, _config -> diff --git a/test/music_library_web/live/stats_live/index_test.exs b/test/music_library_web/live/stats_live/index_test.exs index 5e8613d8..54417cc7 100644 --- a/test/music_library_web/live/stats_live/index_test.exs +++ b/test/music_library_web/live/stats_live/index_test.exs @@ -193,6 +193,10 @@ defmodule MusicLibraryWeb.StatsLive.IndexTest do {:ok, release_group} end) + expect(APIBehaviourMock, :get_releases, fn ^release_group_id, _config -> + {:ok, %{"releases" => release_group["releases"]}} + end) + # Doesn't matter if we use a different cover cover_data = File.read!(marbles_cover_fixture())