diff --git a/backlog/tasks/ml-153 - Move-MusicBrainz-data-transformations-out-of-Record-schema.md b/backlog/tasks/ml-153 - Move-MusicBrainz-data-transformations-out-of-Record-schema.md index 9b95e61e..09597eb5 100644 --- a/backlog/tasks/ml-153 - Move-MusicBrainz-data-transformations-out-of-Record-schema.md +++ b/backlog/tasks/ml-153 - Move-MusicBrainz-data-transformations-out-of-Record-schema.md @@ -1,9 +1,10 @@ --- id: ML-153 title: Move MusicBrainz data transformations out of Record schema -status: To Do +status: Done assignee: [] created_date: '2026-04-30 10:48' +updated_date: '2026-04-30 12:20' labels: - refactor - records @@ -36,9 +37,21 @@ Update callers in: ## Acceptance Criteria -- [ ] #1 `parse_artists/1`, `parse_subtype/2`, `parse_secondary_types/1`, and `attrs_from_release_group/1` are moved to `MusicBrainz` modules -- [ ] #2 `Record` schema retains only struct introspection and presentation helpers -- [ ] #3 All callers are updated to use the new locations -- [ ] #4 Full test suite passes -- [ ] #5 `@moduledoc` on moved functions explains their purpose +- [x] #1 `parse_artists/1`, `parse_subtype/2`, `parse_secondary_types/1`, and `attrs_from_release_group/1` are moved to `MusicBrainz` modules +- [x] #2 `Record` schema retains only struct introspection and presentation helpers +- [x] #3 All callers are updated to use the new locations +- [x] #4 Full test suite passes +- [x] #5 `@moduledoc` on moved functions explains their purpose + +## Implementation Notes + + +`parse_artist_credits/1` and `parse_record_type/2` moved to `MusicBrainz.ReleaseGroup` — these are pure MusicBrainz data parsing functions with no Record schema knowledge. `attrs_from_release_group/1` stays on `Record` because it maps MusicBrainz response fields to Record changeset keys (an implicit Record schema dependency). The Record schema now delegates parsing to `ReleaseGroup.parse_artist_credits/1` and `ReleaseGroup.parse_record_type/2`, keeping the coupling boundary clean: MusicBrainz modules parse MusicBrainz data; the Record schema owns its own field mapping. + + +## Final Summary + + +Moved `parse_artist_credits/1` and `parse_record_type/2` to `MusicBrainz.ReleaseGroup`. `attrs_from_release_group/1` kept on `Record` to avoid reverse dependency — it now delegates parsing to the new MusicBrainz functions. `Record.update_artists/1` uses `ReleaseGroup.parse_artist_credits/1`. All 884 tests pass. + diff --git a/lib/music_brainz/release_group.ex b/lib/music_brainz/release_group.ex index 6c01667b..e535c9f4 100644 --- a/lib/music_brainz/release_group.ex +++ b/lib/music_brainz/release_group.ex @@ -1,5 +1,10 @@ defmodule MusicBrainz.ReleaseGroup do - @moduledoc false + @moduledoc """ + Helper functions for working with MusicBrainz release group API responses. + + Provides extraction and transformation utilities for release group data, + including artist credit parsing and type classification. + """ alias MusicBrainz.ReleaseGroupSearchResult @@ -45,6 +50,51 @@ defmodule MusicBrainz.ReleaseGroup do def parse_type("Single"), do: :single def parse_type(_), do: :other + @doc """ + Parses artist credits from a MusicBrainz release group API response. + + Extracts the `artist-credit` array and maps each artist into a map + suitable for use as an embedded artist in the application `Record` schema. + + Returns a list of maps with keys: `:name`, `:musicbrainz_id`, `:sort_name`, + `:disambiguation`, and `:joinphrase`. + """ + @spec parse_artist_credits(map()) :: [map()] + def parse_artist_credits(musicbrainz_data) do + musicbrainz_data + |> get_in(["artist-credit", Access.all()]) + |> Enum.map(fn artist_credit -> + %{ + name: artist_credit["artist"]["name"], + musicbrainz_id: artist_credit["artist"]["id"], + sort_name: artist_credit["artist"]["sort-name"], + disambiguation: artist_credit["artist"]["disambiguation"], + joinphrase: artist_credit["joinphrase"] + } + end) + end + + @doc """ + Determines the application record type from MusicBrainz release group type fields. + + Maps the `primary-type` and `secondary-types` fields from a MusicBrainz + release group API response to the application's record type enum values + (`:album`, `:ep`, `:live`, `:compilation`, `:single`, `:other`). + """ + @spec parse_record_type(String.t() | nil, [String.t()] | nil) :: atom() + def parse_record_type("Album", secondary_types) when is_list(secondary_types) do + cond do + "Live" in secondary_types -> :live + "Compilation" in secondary_types -> :compilation + true -> :album + end + end + + def parse_record_type("Album", _secondary_types), do: :album + def parse_record_type("EP", _secondary_types), do: :ep + def parse_record_type("Single", _secondary_types), do: :single + def parse_record_type(_primary_type, _secondary_types), do: :other + defp get_release_groups(release_group) do release_group |> Map.get("relations", []) diff --git a/lib/music_library/records/record.ex b/lib/music_library/records/record.ex index f3ac9285..0d6b22dc 100644 --- a/lib/music_library/records/record.ex +++ b/lib/music_library/records/record.ex @@ -207,7 +207,7 @@ defmodule MusicLibrary.Records.Record do changeset musicbrainz_data -> - put_change(changeset, :artists, parse_artists(musicbrainz_data)) + put_change(changeset, :artists, ReleaseGroup.parse_artist_credits(musicbrainz_data)) end end @@ -244,8 +244,7 @@ defmodule MusicLibrary.Records.Record do @spec attrs_from_release_group(map()) :: map() def attrs_from_release_group(release_group) do musicbrainz_id = release_group["id"] - - artists_attrs = parse_artists(release_group) + artists_attrs = ReleaseGroup.parse_artist_credits(release_group) %{ "musicbrainz_id" => musicbrainz_id, @@ -253,40 +252,17 @@ defmodule MusicLibrary.Records.Record do "title" => release_group["title"], "artists" => artists_attrs, "release_date" => release_group["first-release-date"], - "type" => parse_subtype(release_group["primary-type"], release_group["secondary-types"]), + "type" => + ReleaseGroup.parse_record_type( + release_group["primary-type"], + release_group["secondary-types"] + ), "genres" => Enum.map(release_group["genres"], fn g -> g["name"] end), "release_ids" => Enum.map(release_group["releases"], fn r -> r["id"] end), "cover_url" => "https://coverartarchive.org/release-group/#{musicbrainz_id}/front" } end - defp parse_artists(musicbrainz_data) do - musicbrainz_data - |> get_in(["artist-credit", Access.all()]) - |> Enum.map(fn artist_credit -> - %{ - name: artist_credit["artist"]["name"], - musicbrainz_id: artist_credit["artist"]["id"], - sort_name: artist_credit["artist"]["sort-name"], - disambiguation: artist_credit["artist"]["disambiguation"], - joinphrase: artist_credit["joinphrase"] - } - end) - end - - defp parse_subtype("Album", secondary_types), do: parse_secondary_types(secondary_types) - defp parse_subtype("EP", _secondary_types), do: :ep - defp parse_subtype("Single", _secondary_types), do: :single - defp parse_subtype(_primary_type, _secondary_types), do: :other - - defp parse_secondary_types(secondary_types) do - cond do - "Live" in secondary_types -> :live - "Compilation" in secondary_types -> :compilation - true -> :album - end - end - @doc """ Format a release date in a conventional format.