diff --git a/backlog/tasks/ml-170 - Expose-additional-data-points-for-api-v1-collection-records.md b/backlog/tasks/ml-170 - Expose-additional-data-points-for-api-v1-collection-records.md index ef755d0d..f3c5a80a 100644 --- a/backlog/tasks/ml-170 - Expose-additional-data-points-for-api-v1-collection-records.md +++ b/backlog/tasks/ml-170 - Expose-additional-data-points-for-api-v1-collection-records.md @@ -1,10 +1,10 @@ --- id: ML-170 title: Expose additional data points for api/v1/collection/* records -status: To Do +status: Done assignee: [] created_date: "2026-05-08 13:02" -updated_date: "2026-05-12 10:03" +updated_date: "2026-05-22 12:20" labels: - api - ready @@ -39,14 +39,14 @@ The API endpoints affected are: -- [ ] #1 `GET /api/v1/collection` (index) responses include `scrobble_count`, `last_listened_at`, `artist_country`, and `selected_release` for each record -- [ ] #2 `GET /api/v1/collection/latest` response includes all four new fields -- [ ] #3 `GET /api/v1/collection/random` response includes all four new fields -- [ ] #4 `GET /api/v1/collection/on_this_day` responses include all four new fields for each record -- [ ] #5 Records without scrobble data return `scrobble_count: 0` and `last_listened_at: null` -- [ ] #6 Records without artist info return `artist_country: null` -- [ ] #7 Records without a selected release return `selected_release: null` -- [ ] #8 All existing tests pass (backward compatible — no existing field removed or renamed) +- [x] #1 `GET /api/v1/collection` (index) responses include `scrobble_count`, `last_listened_at`, `artist_country`, and `selected_release` for each record +- [x] #2 `GET /api/v1/collection/latest` response includes all four new fields +- [x] #3 `GET /api/v1/collection/random` response includes all four new fields +- [x] #4 `GET /api/v1/collection/on_this_day` responses include all four new fields for each record +- [x] #5 Records without scrobble data return `scrobble_count: 0` and `last_listened_at: null` +- [x] #6 Records without artist info return `artist_country: null` +- [x] #7 Records without a selected release return `selected_release: null` +- [x] #8 All existing tests pass (backward compatible — no existing field removed or renamed) ## Implementation Plan @@ -334,3 +334,11 @@ The `release_summary` component in `record_components.ex` already derives a rele **Simplify Step 4**: instead of building a custom format map, call `ReleaseSearchResult.format(selected_release)` on the release returned by `Record.find_release/2`. Convert the result to a string with `to_string/1` for the API response. No new format parsing code needed. + +## Final Summary + + + +Added `MusicLibrary.Collection.Enrichment` module that batch-hydrates collection API results with scrobble stats, artist country, and selected release data using 3 fixed-count queries per request. Extended `CollectionJSON.record/1` with four new fields (`scrobble_count`, `last_listened_at`, `artist_country`, `selected_release`) using `Map.get/3` for nil-safety. Wired enrichment into all four endpoints (`index`, `latest`, `random`, `on_this_day`). 19 new unit tests + updated controller tests. Full test suite passes (1134 tests, no regressions). + + diff --git a/docs/architecture.md b/docs/architecture.md index 03a1f860..54155c68 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -97,7 +97,7 @@ Last.fm schemas (separate, not Ecto-persisted to main DB): | Context | Schemas | Responsibility | | ---------------------- | --------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `Records` | Record, RecordEmbedding, SearchIndex | CRUD, search, import from MusicBrainz, cover/genre/color management, PubSub notifications | -| `Collection` | Record (via SearchIndex) | Querying collected records (purchased_at != nil), stats, collected artist IDs, collection summary for AI chat | +| `Collection` | Record (via SearchIndex) | Querying collected records (purchased_at != nil), stats, collected artist IDs, collection summary for AI chat. Sub-module `Collection.Enrichment` batch-hydrates API results with scrobble stats, artist country, and selected release info. | | `Wishlist` | Record (via SearchIndex) | Querying wishlisted records (purchased_at is nil) | | `Artists` | ArtistInfo, ArtistRecord | Artist metadata from MusicBrainz/Discogs/Wikipedia/Last.fm, images, search | | `Assets` | Asset | Binary asset storage (covers, artist images), cache tracking, pruning unreferenced assets | @@ -118,40 +118,41 @@ Last.fm schemas (separate, not Ecto-persisted to main DB): ## Business Logic Modules -| Module | Purpose | -| ----------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `Records.SearchParser` | Parses search syntax: `artist:X`, `album:X`, `genre:"Y"`, `format:cd`, `type:album`, `purchase_year:2024`, `release_year:2024`, free text | -| `ListeningStats.SearchParser` | Parses scrobbled tracks search syntax: `record:X`, `album_mbid:X`, `artist_mbid:X`, `artist:X`, `album:X`, `track:X`, free text | -| `Records.Similarity` | Embedding generation and async enqueue (OpenAI, enriched with Last.fm tags, skips API call when text representation unchanged), artist-cascade regeneration when upstream metadata changes, cosine-distance search (sqlite-vec) | -| `Records.TracklistPdf` | Generates 120mm×120mm PDF tracklist from record + release data (Typst) | -| `Batch` | Generic batch runner: stream + transaction + error accumulation | -| `Records.Batch` | Batch operations: refresh all MusicBrainz data, generate all embeddings (uses `Batch`) | -| `Artists.Batch` | Batch refresh: MusicBrainz, Discogs, Wikipedia, Last.fm for all artists (uses `Batch`) | -| `Req.RateLimiter` | ETS-backed Req request step enforcing per-API minimum intervals between requests | -| `Req.RateLimiter.Clock` | Behaviour for time operations (allows test clock injection) | -| `Req.RateLimiter.SystemClock` | Real clock implementation using System.monotonic_time | -| `Assets.Cache` | ETS-based asset cache with TTL (7-day TTL, TTL-only invalidation since assets are content-addressable and immutable) | -| `Assets.Image` / `Assets.Transform` | Image processing via Vix (libvips) | -| `Colors.Extractor` | Behaviour for dominant color extraction (configurable, allows test stubbing) | -| `Colors.KMeansExtractor` | Color extraction via K-Means clustering (dominant_colors library), implements `Colors.Extractor` | -| `Chats.StreamProvider` | Behaviour for streaming AI chat (`stream_response/3` callback) | -| `Chats.RecordChat` | Chat implementation for records (OpenAI streaming, web search enabled) | -| `Chats.ArtistChat` | Chat implementation for artists (OpenAI streaming, uses Wikipedia/artist context) | -| `Chats.CollectionChat` | Chat implementation for the collection (OpenAI streaming via gpt-5.1, uses collection summary as context) | -| `Chats.Prompt` | Builds complete chat prompts by interpolating identity, content, and approach guidelines | -| `Country` | Country code (alpha-2, alpha-3, subdivision, IETF) to flag emoji conversion | -| `ErrorTracker.ErrorNotifier` | GenServer: attaches to ErrorTracker telemetry, skips muted errors, throttles repeated errors, dispatches email notifications | -| `ErrorTracker.ErrorNotifier.Email` | Builds and sends Swoosh error notification emails with stack trace formatting | -| `ErrorIgnorer` | ErrorTracker.Ignorer implementation: filters non-actionable errors (e.g., NoRouteError from bot scanners) | -| `MusicLibrary.ErrorResponse` | Behaviour for structured API error responses (`retryable?/1`, `retry_delay_seconds/1`) — per-API `ErrorResponse` modules implement this | -| `MusicLibrary.HttpError` | Default HTTP status → kind mapping (`:rate_limit`, `:server_error`, `:timeout`, `:auth_error`, `:not_found`, `:client_error`, `:unknown`) used as baseline by per-API `ErrorResponse` modules | -| `MusicLibrary.RetryDelay` | Parses and clamps provider retry/reset headers into Oban snooze delays for structured API errors | -| `MusicLibrary.Worker.ErrorHandler` | Translates per-API `ErrorResponse` structs into Oban tuples — `{:snooze, seconds}` for retryable, `{:cancel, reason}` for permanent | -| `MusicLibraryWeb.RecordsOnThisDayEmail` | Builds and sends daily "records on this day" email with cover images, anniversary styling | -| `MusicLibrary.Mailer` | Swoosh mailer (Mailgun in prod, local adapter in dev) | -| `FormatNumber` | Number formatting utility | -| `QueryReporter` | Dev-only Ecto telemetry reporter: captures executed SQL to a log file with interpolated params, source locations, and timing. Activated at runtime via `start/1` / `stop/0` | -| `MusicLibrary.Logger.SingleLineFormatter` | Production-only Logger.Formatter safety net: replaces embedded newlines (`\n`) with escaped `\\n` in all log messages, ensuring every physical log line is exactly one log event | +| Module | Purpose | +| ----------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Records.SearchParser` | Parses search syntax: `artist:X`, `album:X`, `genre:"Y"`, `format:cd`, `type:album`, `purchase_year:2024`, `release_year:2024`, free text | +| `ListeningStats.SearchParser` | Parses scrobbled tracks search syntax: `record:X`, `album_mbid:X`, `artist_mbid:X`, `artist:X`, `album:X`, `track:X`, free text | +| `Records.Similarity` | Embedding generation and async enqueue (OpenAI, enriched with Last.fm tags, skips API call when text representation unchanged), artist-cascade regeneration when upstream metadata changes, cosine-distance search (sqlite-vec) | +| `Records.TracklistPdf` | Generates 120mm×120mm PDF tracklist from record + release data (Typst) | +| `Batch` | Generic batch runner: stream + transaction + error accumulation | +| `Records.Batch` | Batch operations: refresh all MusicBrainz data, generate all embeddings (uses `Batch`) | +| `Artists.Batch` | Batch refresh: MusicBrainz, Discogs, Wikipedia, Last.fm for all artists (uses `Batch`) | +| `Collection.Enrichment` | Batch-hydrates collection API results with scrobble stats (from `scrobbled_tracks`), artist country (from `artist_infos`), and selected release details (from `records.musicbrainz_data`). Uses 3 fixed-count queries regardless of page size. | +| `Req.RateLimiter` | ETS-backed Req request step enforcing per-API minimum intervals between requests | +| `Req.RateLimiter.Clock` | Behaviour for time operations (allows test clock injection) | +| `Req.RateLimiter.SystemClock` | Real clock implementation using System.monotonic_time | +| `Assets.Cache` | ETS-based asset cache with TTL (7-day TTL, TTL-only invalidation since assets are content-addressable and immutable) | +| `Assets.Image` / `Assets.Transform` | Image processing via Vix (libvips) | +| `Colors.Extractor` | Behaviour for dominant color extraction (configurable, allows test stubbing) | +| `Colors.KMeansExtractor` | Color extraction via K-Means clustering (dominant_colors library), implements `Colors.Extractor` | +| `Chats.StreamProvider` | Behaviour for streaming AI chat (`stream_response/3` callback) | +| `Chats.RecordChat` | Chat implementation for records (OpenAI streaming, web search enabled) | +| `Chats.ArtistChat` | Chat implementation for artists (OpenAI streaming, uses Wikipedia/artist context) | +| `Chats.CollectionChat` | Chat implementation for the collection (OpenAI streaming via gpt-5.1, uses collection summary as context) | +| `Chats.Prompt` | Builds complete chat prompts by interpolating identity, content, and approach guidelines | +| `Country` | Country code (alpha-2, alpha-3, subdivision, IETF) to flag emoji conversion | +| `ErrorTracker.ErrorNotifier` | GenServer: attaches to ErrorTracker telemetry, skips muted errors, throttles repeated errors, dispatches email notifications | +| `ErrorTracker.ErrorNotifier.Email` | Builds and sends Swoosh error notification emails with stack trace formatting | +| `ErrorIgnorer` | ErrorTracker.Ignorer implementation: filters non-actionable errors (e.g., NoRouteError from bot scanners) | +| `MusicLibrary.ErrorResponse` | Behaviour for structured API error responses (`retryable?/1`, `retry_delay_seconds/1`) — per-API `ErrorResponse` modules implement this | +| `MusicLibrary.HttpError` | Default HTTP status → kind mapping (`:rate_limit`, `:server_error`, `:timeout`, `:auth_error`, `:not_found`, `:client_error`, `:unknown`) used as baseline by per-API `ErrorResponse` modules | +| `MusicLibrary.RetryDelay` | Parses and clamps provider retry/reset headers into Oban snooze delays for structured API errors | +| `MusicLibrary.Worker.ErrorHandler` | Translates per-API `ErrorResponse` structs into Oban tuples — `{:snooze, seconds}` for retryable, `{:cancel, reason}` for permanent | +| `MusicLibraryWeb.RecordsOnThisDayEmail` | Builds and sends daily "records on this day" email with cover images, anniversary styling | +| `MusicLibrary.Mailer` | Swoosh mailer (Mailgun in prod, local adapter in dev) | +| `FormatNumber` | Number formatting utility | +| `QueryReporter` | Dev-only Ecto telemetry reporter: captures executed SQL to a log file with interpolated params, source locations, and timing. Activated at runtime via `start/1` / `stop/0` | +| `MusicLibrary.Logger.SingleLineFormatter` | Production-only Logger.Formatter safety net: replaces embedded newlines (`\n`) with escaped `\\n` in all log messages, ensuring every physical log line is exactly one log event | --- diff --git a/lib/music_library/collection/enrichment.ex b/lib/music_library/collection/enrichment.ex new file mode 100644 index 00000000..d9829f6c --- /dev/null +++ b/lib/music_library/collection/enrichment.ex @@ -0,0 +1,259 @@ +defmodule MusicLibrary.Collection.Enrichment do + @moduledoc """ + Batch-hydrates SearchIndex (and Record struct) results with additional data + points: scrobble stats, artist country, and selected release info. + + All enrichment runs in fixed-count batch queries — 3 total, regardless of + result size. No N+1 risk and no title+artist fallback queries. + """ + + import Ecto.Query + + alias LastFm.Track + alias MusicBrainz.ReleaseSearchResult + alias MusicLibrary.Artists.ArtistInfo + alias MusicLibrary.Records.Record + alias MusicLibrary.Repo + + @doc """ + Enriches a list of records with scrobble stats, artist country, and selected + release information. + + Accepts any map or struct list where each element has: + - `:id` (binary_id or string) + - `:release_ids` (array of strings) + - `:artists` (embedded list with `.musicbrainz_id`) + - `:selected_release_id` (string or nil) + + Returns the same list with four additional fields on each element: + - `:scrobble_count` (integer, always present, defaults to 0) + - `:last_listened_at` (ISO8601 string or nil) + - `:artist_country` (%{name: ..., code: ...} or nil) + - `:selected_release` (map or nil) + """ + @spec enrich([map()]) :: [map()] + def enrich(records) do + records + |> enrich_scrobbles() + |> enrich_artist_country() + |> enrich_selected_release() + end + + @doc """ + Adds `:scrobble_count` and `:last_listened_at` to each record. + + Uses a single batch query against `scrobbled_tracks` keyed by + `json_extract(album, '$.musicbrainz_id')` (the release MusicBrainz ID). + For records with multiple release_ids (different editions of the same + release group), scrobble counts are summed and the most recent + `last_listened_at` across all releases is kept. + """ + @spec enrich_scrobbles([map()]) :: [map()] + def enrich_scrobbles(records) do + release_ids = + records + |> Enum.flat_map(&Map.get(&1, :release_ids, [])) + |> Enum.reject(&is_nil/1) + |> Enum.uniq() + + if release_ids == [] do + Enum.map(records, fn record -> + record + |> Map.put(:scrobble_count, 0) + |> Map.put(:last_listened_at, nil) + end) + else + lookup = build_scrobble_lookup(release_ids) + + Enum.map(records, fn record -> + enrich_one_record_scrobbles(record, lookup) + end) + end + end + + defp enrich_one_record_scrobbles(record, lookup) do + record_release_ids = Map.get(record, :release_ids, []) || [] + + {total_count, latest_uts} = + record_release_ids + |> Enum.reduce({0, nil}, fn release_id, {count_acc, uts_acc} -> + case Map.get(lookup, release_id) do + nil -> + {count_acc, uts_acc} + + %{scrobble_count: sc, last_listened_at: la} -> + new_count = count_acc + sc + new_uts = max_uts(uts_acc, la) + {new_count, new_uts} + end + end) + + record + |> Map.put(:scrobble_count, total_count) + |> Map.put(:last_listened_at, format_last_listened_at(latest_uts)) + end + + defp build_scrobble_lookup(release_ids) do + from(t in Track, + where: fragment("json_extract(?, '$.musicbrainz_id')", t.album) in ^release_ids, + group_by: fragment("json_extract(?, '$.musicbrainz_id')", t.album), + select: %{ + release_id: fragment("json_extract(?, '$.musicbrainz_id')", t.album), + scrobble_count: fragment("COUNT(DISTINCT ?)", t.scrobbled_at_uts), + last_listened_at: max(t.scrobbled_at_uts) + } + ) + |> Repo.all() + |> Map.new(fn row -> + {row.release_id, + %{scrobble_count: row.scrobble_count, last_listened_at: row.last_listened_at}} + end) + end + + defp max_uts(nil, nil), do: nil + defp max_uts(nil, val), do: val + defp max_uts(val, nil), do: val + defp max_uts(a, b), do: max(a, b) + + defp format_last_listened_at(nil), do: nil + + defp format_last_listened_at(uts) when is_integer(uts) do + uts + |> DateTime.from_unix!() + |> DateTime.to_iso8601() + end + + @doc """ + Adds `:artist_country` to each record, derived from the main artist. + + Extracts the first artist from the embedded `:artists` list, looks up + `artist_infos` by the artist's MusicBrainz ID, and extracts the country + via `ArtistInfo.country/1`. + + Returns `%{name: String, code: String}` or nil if no artist info exists. + """ + @spec enrich_artist_country([map()]) :: [map()] + def enrich_artist_country(records) do + artist_ids = + records + |> Enum.map(&main_artist_mbid/1) + |> Enum.reject(&is_nil/1) + |> Enum.uniq() + + if artist_ids == [] do + Enum.map(records, fn record -> + Map.put(record, :artist_country, nil) + end) + else + lookup = build_country_lookup(artist_ids) + + Enum.map(records, fn record -> + mbid = main_artist_mbid(record) + artist_country = if mbid, do: Map.get(lookup, mbid), else: nil + Map.put(record, :artist_country, artist_country) + end) + end + end + + defp main_artist_mbid(record) do + artists = Map.get(record, :artists, []) + + case artists do + [] -> + nil + + [first | _] -> + %{musicbrainz_id: mbid} = first + mbid + end + end + + defp build_country_lookup(artist_ids) do + from(ai in ArtistInfo, + where: ai.id in ^artist_ids, + select: %{id: ai.id, musicbrainz_data: ai.musicbrainz_data} + ) + |> Repo.all() + |> Map.new(fn row -> + {row.id, ArtistInfo.country(%ArtistInfo{musicbrainz_data: row.musicbrainz_data})} + end) + end + + @doc """ + Adds `:selected_release` to each record with details about the selected + release (edition). + + Looks up the `records` table by the record's `:id`, extracts the selected + release from `musicbrainz_data` using `Record.releases/1` and + `Record.find_release/2`, then exposes six fields: format, date, country, + catalog_number, packaging, disambiguation. + + Returns a map or nil if no selected release is available. + """ + @spec enrich_selected_release([map()]) :: [map()] + def enrich_selected_release(records) do + record_ids_with_selected = + records + |> Enum.filter(fn r -> + sr_id = Map.get(r, :selected_release_id) + sr_id != nil and sr_id != "" + end) + |> Enum.map(&Map.get(&1, :id)) + |> Enum.uniq() + + if record_ids_with_selected == [] do + Enum.map(records, fn record -> + Map.put(record, :selected_release, nil) + end) + else + lookup = build_selected_release_lookup(record_ids_with_selected) + + Enum.map(records, fn record -> + record + |> Map.put(:selected_release, Map.get(lookup, Map.get(record, :id))) + end) + end + end + + defp build_selected_release_lookup(record_ids) do + from(r in Record, + where: r.id in ^record_ids, + select: %{ + id: r.id, + musicbrainz_data: r.musicbrainz_data, + selected_release_id: r.selected_release_id + } + ) + |> Repo.all() + |> Map.new(fn row -> + selected_release = + extract_selected_release(row.musicbrainz_data, row.selected_release_id) + + {row.id, selected_release} + end) + end + + defp extract_selected_release(nil, _), do: nil + defp extract_selected_release(_, nil), do: nil + defp extract_selected_release(_, ""), do: nil + + defp extract_selected_release(musicbrainz_data, selected_release_id) do + # Build a minimal map with :musicbrainz_data for Record.releases/1 and Record.find_release/2 + temp_record = %{musicbrainz_data: musicbrainz_data} + + case Record.find_release(temp_record, selected_release_id) do + nil -> + nil + + release -> + %{ + format: to_string(ReleaseSearchResult.format(release)), + date: release.date, + country: release.country, + catalog_number: release.catalog_number, + packaging: release.packaging, + disambiguation: release.disambiguation + } + end + end +end diff --git a/lib/music_library_web/controllers/collection_controller.ex b/lib/music_library_web/controllers/collection_controller.ex index dce21840..42ab702e 100644 --- a/lib/music_library_web/controllers/collection_controller.ex +++ b/lib/music_library_web/controllers/collection_controller.ex @@ -3,17 +3,26 @@ defmodule MusicLibraryWeb.CollectionController do alias MusicBrainz alias MusicLibrary.Collection + alias MusicLibrary.Collection.Enrichment alias MusicLibrary.Records alias MusicLibrary.ScrobbleActivity def latest(conn, _params) do - latest_record = Collection.get_latest_record!() + latest_record = + Collection.get_latest_record!() + |> List.wrap() + |> Enrichment.enrich() + |> hd() render(conn, :show, record: latest_record) end def random(conn, _params) do - random_record = Collection.get_random_record!() + random_record = + Collection.get_random_record!() + |> List.wrap() + |> Enrichment.enrich() + |> hd() render(conn, :show, record: random_record) end @@ -25,7 +34,10 @@ defmodule MusicLibraryWeb.CollectionController do date_string -> Date.from_iso8601!(date_string) end - records_on_this_day = Collection.get_records_on_this_day(current_date) + records_on_this_day = + current_date + |> Collection.get_records_on_this_day() + |> Enrichment.enrich() render(conn, :on_this_day, records: records_on_this_day) end @@ -37,7 +49,10 @@ defmodule MusicLibraryWeb.CollectionController do total = Collection.search_records_count(query) - records = Collection.search_records(query, limit: limit, offset: offset) + records = + query + |> Collection.search_records(limit: limit, offset: offset) + |> Enrichment.enrich() render(conn, :index, total: total, limit: limit, offset: offset, records: records) end diff --git a/lib/music_library_web/controllers/collection_json.ex b/lib/music_library_web/controllers/collection_json.ex index 91e899c8..5a68d94d 100644 --- a/lib/music_library_web/controllers/collection_json.ex +++ b/lib/music_library_web/controllers/collection_json.ex @@ -38,7 +38,11 @@ defmodule MusicLibraryWeb.CollectionJSON do selected_release_id: record.selected_release_id, artists: Enum.map(record.artists, & &1.name), title: record.title, - covers: cover_urls(record.cover_hash) + covers: cover_urls(record.cover_hash), + scrobble_count: Map.get(record, :scrobble_count, 0), + last_listened_at: Map.get(record, :last_listened_at), + artist_country: Map.get(record, :artist_country), + selected_release: Map.get(record, :selected_release) } end diff --git a/test/music_library/collection/enrichment_test.exs b/test/music_library/collection/enrichment_test.exs new file mode 100644 index 00000000..1091efe8 --- /dev/null +++ b/test/music_library/collection/enrichment_test.exs @@ -0,0 +1,295 @@ +defmodule MusicLibrary.Collection.EnrichmentTest do + use MusicLibrary.DataCase + + alias MusicLibrary.Collection.Enrichment + alias MusicLibrary.Fixtures.Records + alias MusicLibrary.ScrobbledTracksFixtures + + describe "enrich_scrobbles/1" do + test "adds scrobble_count and last_listened_at when scrobbles exist" do + record = Records.record() + release_id = hd(record.release_ids) + + # Insert a scrobbled track referencing this release_id + _track = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: release_id, + album_title: record.title, + scrobbled_at_uts: 1_700_000_000 + }) + + _track2 = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: release_id, + album_title: record.title, + scrobbled_at_uts: 1_700_000_100 + }) + + [enriched] = Enrichment.enrich_scrobbles([record]) + + assert enriched.scrobble_count == 2 + assert enriched.last_listened_at != nil + assert String.contains?(enriched.last_listened_at, "T") + end + + test "returns zero and nil when no scrobbles match" do + record = Records.record() + + [enriched] = Enrichment.enrich_scrobbles([record]) + + assert enriched.scrobble_count == 0 + assert enriched.last_listened_at == nil + end + + test "returns zero and nil when release_ids is empty" do + record = Records.record(%{release_ids: []}) + + [enriched] = Enrichment.enrich_scrobbles([record]) + + assert enriched.scrobble_count == 0 + assert enriched.last_listened_at == nil + end + + test "returns zero and nil when release_ids is nil" do + record = Records.record(%{release_ids: nil}) + + [enriched] = Enrichment.enrich_scrobbles([record]) + + assert enriched.scrobble_count == 0 + assert enriched.last_listened_at == nil + end + + test "sums scrobbles across multiple release_ids for the same record" do + release_ids = ["release-a", "release-b"] + + record = Records.record(%{release_ids: release_ids, musicbrainz_data: nil}) + + _track = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: "release-a", + album_title: "Album A", + scrobbled_at_uts: 1_700_000_000 + }) + + _track2 = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: "release-b", + album_title: "Album B", + scrobbled_at_uts: 1_700_000_100 + }) + + [enriched] = Enrichment.enrich_scrobbles([record]) + + assert enriched.scrobble_count == 2 + end + + test "takes the max last_listened_at across release_ids" do + release_ids = ["release-a", "release-b"] + + record = Records.record(%{release_ids: release_ids, musicbrainz_data: nil}) + + earlier = 1_700_000_000 + later = 1_700_000_500 + + _track = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: "release-a", + album_title: "Album A", + scrobbled_at_uts: earlier + }) + + _track2 = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: "release-b", + album_title: "Album B", + scrobbled_at_uts: later + }) + + [enriched] = Enrichment.enrich_scrobbles([record]) + + expected_iso = later |> DateTime.from_unix!() |> DateTime.to_iso8601() + assert enriched.last_listened_at == expected_iso + end + + test "two records sharing a release group both get scrobble counts" do + release_ids = ["shared-release"] + record1 = Records.record(%{release_ids: release_ids, musicbrainz_data: nil}) + record2 = Records.record(%{release_ids: release_ids, musicbrainz_data: nil}) + + _track = + ScrobbledTracksFixtures.track_fixture(%{ + album_musicbrainz_id: "shared-release", + album_title: "Shared", + scrobbled_at_uts: 1_700_000_000 + }) + + [enriched1, enriched2] = Enrichment.enrich_scrobbles([record1, record2]) + + assert enriched1.scrobble_count == 1 + assert enriched2.scrobble_count == 1 + end + end + + describe "enrich_artist_country/1" do + test "adds artist_country when artist_info exists with country data" do + record = Records.record() + + main_artist = hd(record.artists) + artist_mbid = main_artist.musicbrainz_id + + # Create artist_info with area data (UK) + Records.artist_info(artist_mbid, %{ + musicbrainz_data: %{ + "name" => "Steven Wilson", + "area" => %{ + "name" => "United Kingdom", + "iso-3166-1-codes" => ["GB"] + } + } + }) + + [enriched] = Enrichment.enrich_artist_country([record]) + + assert enriched.artist_country == %{name: "United Kingdom", code: "GB"} + end + + test "returns nil when artist_info row does not exist" do + record = Records.record() + + [enriched] = Enrichment.enrich_artist_country([record]) + + assert enriched.artist_country == nil + end + + test "returns nil when artists list is empty" do + record = Records.record(%{artists: []}) + + [enriched] = Enrichment.enrich_artist_country([record]) + + assert enriched.artist_country == nil + end + + test "reuses lookup for multiple records with same main artist" do + artist_name = "Pink Floyd" + record1 = Records.record_with_artist(artist_name) + record2 = Records.record_with_artist(artist_name) + + artist_mbid = hd(record1.artists).musicbrainz_id + + # Only one artist_info created, shared by both records + Records.artist_info(artist_mbid, %{ + musicbrainz_data: %{ + "name" => "Pink Floyd", + "area" => %{ + "name" => "United Kingdom", + "iso-3166-1-codes" => ["GB"] + } + } + }) + + [enriched1, enriched2] = Enrichment.enrich_artist_country([record1, record2]) + + assert enriched1.artist_country == %{name: "United Kingdom", code: "GB"} + assert enriched2.artist_country == %{name: "United Kingdom", code: "GB"} + end + + test "returns nil for record whose artist lacks an artist_infos row" do + artist_name = "Unknown Artist" + record = Records.record_with_artist(artist_name) + + [enriched] = Enrichment.enrich_artist_country([record]) + + assert enriched.artist_country == nil + end + end + + describe "enrich_selected_release/1" do + test "adds selected_release with all six fields when matching release exists" do + record = Records.record() + + [enriched] = Enrichment.enrich_selected_release([record]) + + assert enriched.selected_release != nil + assert is_map(enriched.selected_release) + + sr = enriched.selected_release + assert Map.has_key?(sr, :format) + assert Map.has_key?(sr, :date) + assert Map.has_key?(sr, :country) + assert Map.has_key?(sr, :catalog_number) + assert Map.has_key?(sr, :packaging) + assert Map.has_key?(sr, :disambiguation) + end + + test "returns nil when selected_release_id is nil" do + record = Records.record(%{selected_release_id: nil}) + + [enriched] = Enrichment.enrich_selected_release([record]) + + assert enriched.selected_release == nil + end + + test "returns nil when selected_release_id is empty string" do + record = Records.record(%{selected_release_id: ""}) + + [enriched] = Enrichment.enrich_selected_release([record]) + + assert enriched.selected_release == nil + end + + test "returns nil when selected_release_id does not match any release" do + record = Records.record(%{selected_release_id: "nonexistent-release-id"}) + + [enriched] = Enrichment.enrich_selected_release([record]) + + assert enriched.selected_release == nil + end + + test "multiple records enriched independently" do + record1 = Records.record() + record2 = Records.record(%{selected_release_id: nil}) + + [enriched1, enriched2] = Enrichment.enrich_selected_release([record1, record2]) + + assert enriched1.selected_release != nil + assert enriched2.selected_release == nil + end + end + + describe "enrich/1" do + test "composes all three enrichments, adding all four fields" do + record = Records.record() + + main_artist = hd(record.artists) + artist_mbid = main_artist.musicbrainz_id + + Records.artist_info(artist_mbid, %{ + musicbrainz_data: %{ + "name" => "Steven Wilson", + "area" => %{ + "name" => "United Kingdom", + "iso-3166-1-codes" => ["GB"] + } + } + }) + + [enriched] = Enrichment.enrich([record]) + + # All four new fields are present + assert Map.has_key?(enriched, :scrobble_count) + assert Map.has_key?(enriched, :last_listened_at) + assert Map.has_key?(enriched, :artist_country) + assert Map.has_key?(enriched, :selected_release) + + # Original fields are preserved + assert enriched.id == record.id + assert enriched.title == record.title + assert enriched.musicbrainz_id == record.musicbrainz_id + assert enriched.artists == record.artists + end + + test "handles empty list gracefully" do + assert Enrichment.enrich([]) == [] + end + end +end diff --git a/test/music_library_web/controllers/collection_controller_test.exs b/test/music_library_web/controllers/collection_controller_test.exs index 542cd9e2..f70c4b64 100644 --- a/test/music_library_web/controllers/collection_controller_test.exs +++ b/test/music_library_web/controllers/collection_controller_test.exs @@ -327,6 +327,17 @@ defmodule MusicLibraryWeb.CollectionControllerTest do "large" => asset_url(record.cover_hash, 1000), "medium" => asset_url(record.cover_hash, 400), "small" => asset_url(record.cover_hash, 80) + }, + "scrobble_count" => 0, + "last_listened_at" => nil, + "artist_country" => nil, + "selected_release" => %{ + "catalog_number" => "", + "country" => "GB", + "date" => "2004-05-03", + "disambiguation" => "", + "format" => "multi", + "packaging" => "Jewel Case" } } end