ML-153: move MusicBrainz data transforms from Record

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
 <!-- AC:BEGIN -->
-- [ ] #1 `parse_artists/1`, `parse_subtype/2`, `parse_secondary_types/1`, and `attrs_from_release_group/1` are moved to `MusicBrainz` modules
+- [x] #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
+- [x] #2 `Record` schema retains only struct introspection and presentation helpers
-- [ ] #3 All callers are updated to use the new locations
+- [x] #3 All callers are updated to use the new locations
-- [ ] #4 Full test suite passes
+- [x] #4 Full test suite passes
-- [ ] #5 `@moduledoc` on moved functions explains their purpose
+- [x] #5 `@moduledoc` on moved functions explains their purpose
 <!-- AC:END -->
+
+## Implementation Notes
+
+<!-- SECTION:NOTES:BEGIN -->
+`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.
+<!-- SECTION:NOTES:END -->
+
+## Final Summary
+
+<!-- SECTION:FINAL_SUMMARY:BEGIN -->
+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.
+<!-- SECTION:FINAL_SUMMARY:END -->

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", [])

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 = ReleaseGroup.parse_artist_credits(release_group)
-    artists_attrs = parse_artists(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.