Normalize top artist and top album counts

- Make sure joins don't discard tracks
- Make sure we group by names as MB ids can be empty

lib/music_library/scrobble_activity.ex

@@ -352,26 +352,21 @@ defmodule MusicLibrary.ScrobbleActivity do
   @doc """
   Gets the top artists by scrobble count across all time.
   Returns a list of maps with artist information and play counts.
-
-  For a lot of artists, Last.fm doesn't have the correct artist musicbrainz_id
-  (often it's a ""), which makes this query not work as expected at the JOIN
-  level. To fix that, we need to update the data via Scrobble Rules.
   """
   def get_top_artists(opts) do
     limit = Keyword.get(opts, :limit, 10)
 
     query =
       from t in Track,
-        join: ai in Artists.ArtistInfo,
+        left_join: ai in Artists.ArtistInfo,
         on: ai.id == fragment("json_extract(?, '$.musicbrainz_id')", t.artist),
         group_by: [
-          fragment("json_extract(artist, '$.name')"),
-          fragment("json_extract(artist, '$.musicbrainz_id')")
+          fragment("json_extract(artist, '$.name')")
         ],
         select: %{
           name: fragment("json_extract(artist, '$.name')"),
-          musicbrainz_id: fragment("json_extract(artist, '$.musicbrainz_id')"),
-          image_hash: ai.image_data_hash,
+          musicbrainz_id: max(fragment("json_extract(artist, '$.musicbrainz_id')")),
+          image_hash: max(ai.image_data_hash),
           play_count: count(t.scrobbled_at_uts, :distinct)
         },
         order_by: [desc: count(t.scrobbled_at_uts, :distinct)],
@@ -383,10 +378,6 @@ defmodule MusicLibrary.ScrobbleActivity do
   @doc """
   Gets the top artists by scrobble count for the given number of days.
   Returns a list of maps with artist information and play counts.
-
-  For a lot of artists, Last.fm doesn't have the correct artist musicbrainz_id
-  (often it's a ""), which makes this query not work as expected at the JOIN
-  level. To fix that, we need to update the data via Scrobble Rules.
   """
   def get_top_artists_by_days(days, opts) do
     limit = Keyword.get(opts, :limit, 10)
@@ -402,17 +393,16 @@ defmodule MusicLibrary.ScrobbleActivity do
 
     query =
       from t in Track,
-        join: ai in Artists.ArtistInfo,
+        left_join: ai in Artists.ArtistInfo,
         on: ai.id == fragment("json_extract(?, '$.musicbrainz_id')", t.artist),
         where: t.scrobbled_at_uts >= ^cutoff_timestamp,
         group_by: [
-          fragment("json_extract(artist, '$.name')"),
-          fragment("json_extract(artist, '$.musicbrainz_id')")
+          fragment("json_extract(artist, '$.name')")
         ],
         select: %{
           name: fragment("json_extract(artist, '$.name')"),
-          musicbrainz_id: fragment("json_extract(artist, '$.musicbrainz_id')"),
-          image_hash: ai.image_data_hash,
+          musicbrainz_id: max(fragment("json_extract(artist, '$.musicbrainz_id')")),
+          image_hash: max(ai.image_data_hash),
           play_count: count(t.scrobbled_at_uts, :distinct)
         },
         order_by: [desc: count(t.scrobbled_at_uts, :distinct)],

test/music_library/scrobble_activity_test.exs

@@ -271,6 +271,132 @@ defmodule MusicLibrary.ScrobbleActivityTest do
     end
   end
 
+  describe "get_top_artists/1" do
+    test "counts tracks with missing artist_infos records" do
+      artist_mbid = Ecto.UUID.generate()
+
+      # Create an artist_info record for the known musicbrainz_id
+      MusicLibrary.Repo.insert!(%MusicLibrary.Artists.ArtistInfo{
+        id: artist_mbid,
+        musicbrainz_data: %{"name" => "Thin Lizzy"}
+      })
+
+      now = System.system_time(:second)
+
+      # Two tracks with a matching artist_infos record
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: artist_mbid,
+        title: "The Boys Are Back in Town",
+        scrobbled_at_uts: now - 100
+      })
+
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: artist_mbid,
+        title: "Jailbreak",
+        scrobbled_at_uts: now - 200
+      })
+
+      # One track with an empty musicbrainz_id (no matching artist_infos)
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: "",
+        title: "Rosalie / Cowgirl's Song",
+        scrobbled_at_uts: now - 300
+      })
+
+      results = ScrobbleActivity.get_top_artists(limit: 10)
+
+      # All 3 tracks should be counted as a single artist entry
+      assert [thin_lizzy] = Enum.filter(results, fn r -> r.name == "Thin Lizzy" end)
+      assert thin_lizzy.play_count == 3
+      # MAX picks the real musicbrainz_id over the empty string
+      assert thin_lizzy.musicbrainz_id == artist_mbid
+    end
+
+    test "returns image_hash from artist_infos when available" do
+      artist_mbid = Ecto.UUID.generate()
+
+      MusicLibrary.Repo.insert!(%MusicLibrary.Artists.ArtistInfo{
+        id: artist_mbid,
+        musicbrainz_data: %{"name" => "Test Artist"},
+        image_data_hash: "abc123"
+      })
+
+      track_fixture(%{
+        artist_name: "Test Artist",
+        artist_musicbrainz_id: artist_mbid,
+        title: "Track 1",
+        scrobbled_at_uts: System.system_time(:second) - 100
+      })
+
+      [result] = ScrobbleActivity.get_top_artists(limit: 10)
+
+      assert result.image_hash == "abc123"
+    end
+
+    test "returns nil image_hash for tracks without artist_infos" do
+      track_fixture(%{
+        artist_name: "Unknown Artist",
+        artist_musicbrainz_id: "",
+        title: "Track 1",
+        scrobbled_at_uts: System.system_time(:second) - 100
+      })
+
+      [result] = ScrobbleActivity.get_top_artists(limit: 10)
+
+      assert result.image_hash == nil
+    end
+  end
+
+  describe "get_top_artists_by_days/2" do
+    test "counts tracks with missing artist_infos records within date range" do
+      artist_mbid = Ecto.UUID.generate()
+
+      MusicLibrary.Repo.insert!(%MusicLibrary.Artists.ArtistInfo{
+        id: artist_mbid,
+        musicbrainz_data: %{"name" => "Thin Lizzy"}
+      })
+
+      now = DateTime.utc_now()
+      now_unix = DateTime.to_unix(now)
+
+      # Two tracks with matching artist_infos
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: artist_mbid,
+        title: "The Boys Are Back in Town",
+        scrobbled_at_uts: now_unix - 100
+      })
+
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: artist_mbid,
+        title: "Jailbreak",
+        scrobbled_at_uts: now_unix - 200
+      })
+
+      # One track with empty musicbrainz_id
+      track_fixture(%{
+        artist_name: "Thin Lizzy",
+        artist_musicbrainz_id: "",
+        title: "Rosalie / Cowgirl's Song",
+        scrobbled_at_uts: now_unix - 300
+      })
+
+      results =
+        ScrobbleActivity.get_top_artists_by_days(7,
+          limit: 10,
+          current_time: now,
+          timezone: "Etc/UTC"
+        )
+
+      assert [thin_lizzy] = Enum.filter(results, fn r -> r.name == "Thin Lizzy" end)
+      assert thin_lizzy.play_count == 3
+    end
+  end
+
   describe "integration with existing functions" do
     test "scrobble_count returns correct count" do
       initial_count = ScrobbleActivity.scrobble_count()