fix: associate artists with merged items

internal/db/opts.go

@@ -96,10 +96,12 @@ type UpdateArtistOpts struct {
 }
 
 type UpdateAlbumOpts struct {
-	ID            int32
+	ID                   int32
-	MusicBrainzID uuid.UUID
+	MusicBrainzID        uuid.UUID
-	Image         uuid.UUID
+	Image                uuid.UUID
-	ImageSrc      string
+	ImageSrc             string
+	VariousArtistsUpdate bool
+	VariousArtistsValue  bool
 }
 
 type UpdateUserOpts struct {

internal/db/psql/album.go

@@ -200,6 +200,16 @@ func (d *Psql) UpdateAlbum(ctx context.Context, opts db.UpdateAlbumOpts) error {
 			return err
 		}
 	}
+	if opts.VariousArtistsUpdate {
+		l.Debug().Msgf("Updating release with ID %d with image %s", opts.ID, opts.Image)
+		err := qtx.UpdateReleaseVariousArtists(ctx, repository.UpdateReleaseVariousArtistsParams{
+			ID:             opts.ID,
+			VariousArtists: opts.VariousArtistsValue,
+		})
+		if err != nil {
+			return err
+		}
+	}
 	return tx.Commit(ctx)
 }
 

internal/db/psql/album_test.go

@@ -116,10 +116,12 @@ func TestUpdateAlbum(t *testing.T) {
 	newMbzID := uuid.New()
 	imgid := uuid.New()
 	err = store.UpdateAlbum(ctx, db.UpdateAlbumOpts{
-		ID:            rg.ID,
+		ID:                   rg.ID,
-		MusicBrainzID: newMbzID,
+		MusicBrainzID:        newMbzID,
-		Image:         imgid,
+		Image:                imgid,
-		ImageSrc:      catalog.ImageSourceUserUpload,
+		ImageSrc:             catalog.ImageSourceUserUpload,
+		VariousArtistsUpdate: true,
+		VariousArtistsValue:  true,
 	})
 	require.NoError(t, err)
 
@@ -127,6 +129,7 @@ func TestUpdateAlbum(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, newMbzID, *result.MbzID)
 	assert.Equal(t, imgid, *result.Image)
+	assert.True(t, result.VariousArtists)
 
 	truncateTestData(t)
 }

internal/db/psql/merge.go

@@ -19,12 +19,36 @@ func (d *Psql) MergeTracks(ctx context.Context, fromId, toId int32) error {
 	}
 	defer tx.Rollback(ctx)
 	qtx := d.q.WithTx(tx)
+	from, err := qtx.GetTrack(ctx, fromId)
+	if err != nil {
+		return fmt.Errorf("MergeTracks: GetTrack: %w", err)
+	}
+	to, err := qtx.GetTrack(ctx, toId)
+	if err != nil {
+		return fmt.Errorf("MergeTracks: GetTrack: %w", err)
+	}
 	err = qtx.UpdateTrackIdForListens(ctx, repository.UpdateTrackIdForListensParams{
 		TrackID:   fromId,
 		TrackID_2: toId,
 	})
 	if err != nil {
-		return fmt.Errorf("MergeTracks: %w", err)
+		return fmt.Errorf("MergeTracks: UpdateTrackIdForListens: %w", err)
+	}
+	if from.ReleaseID != to.ReleaseID {
+		// tracks are from different releases, track artist should be associated with to.release
+		artists, err := qtx.GetTrackArtists(ctx, fromId)
+		if err != nil {
+			return fmt.Errorf("MergeTracks: GetTrackArtists: %w", err)
+		}
+		for _, artist := range artists {
+			err = qtx.AssociateArtistToRelease(ctx, repository.AssociateArtistToReleaseParams{
+				ArtistID:  artist.ID,
+				ReleaseID: to.ReleaseID,
+			})
+			if err != nil {
+				return fmt.Errorf("MergeTracks: AssociateArtistToRelease: %w", err)
+			}
+		}
 	}
 	err = qtx.CleanOrphanedEntries(ctx)
 	if err != nil {
@@ -44,6 +68,12 @@ func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32, replaceImage
 	}
 	defer tx.Rollback(ctx)
 	qtx := d.q.WithTx(tx)
+
+	fromArtists, err := qtx.GetReleaseArtists(ctx, fromId)
+	if err != nil {
+		return fmt.Errorf("MergeTracks: GetReleaseArtists: %w", err)
+	}
+
 	err = qtx.UpdateReleaseForAll(ctx, repository.UpdateReleaseForAllParams{
 		ReleaseID:   fromId,
 		ReleaseID_2: toId,
@@ -65,10 +95,21 @@ func (d *Psql) MergeAlbums(ctx context.Context, fromId, toId int32, replaceImage
 			return fmt.Errorf("MergeAlbums: %w", err)
 		}
 	}
+
+	for _, artist := range fromArtists {
+		err = qtx.AssociateArtistToRelease(ctx, repository.AssociateArtistToReleaseParams{
+			ArtistID:  artist.ID,
+			ReleaseID: toId,
+		})
+		if err != nil {
+			return fmt.Errorf("MergeAlbums: AssociateArtistToRelease: %w", err)
+		}
+	}
+
 	err = qtx.CleanOrphanedEntries(ctx)
 	if err != nil {
 		l.Err(err).Msg("Failed to clean orphaned entries")
-		return err
+		return fmt.Errorf("MergeAlbums: CleanOrphanedEntries: %w", err)
 	}
 	return tx.Commit(ctx)
 }

internal/db/psql/merge_test.go

@@ -27,44 +27,52 @@ func setupTestDataForMerge(t *testing.T) {
 	err = store.Exec(context.Background(),
 		`INSERT INTO releases (musicbrainz_id, image, image_source) 
 			VALUES ('11111111-1111-1111-1111-111111111111', '20000000-0000-0000-0000-000000000000', 'source.com'),
-				   ('22222222-2222-2222-2222-222222222222', NULL, NULL)`)
+				   ('22222222-2222-2222-2222-222222222222', NULL, NULL),
+				   (NULL, NULL, NULL)`)
 	require.NoError(t, err)
 
 	err = store.Exec(context.Background(),
 		`INSERT INTO release_aliases (release_id, alias, source, is_primary) 
 			VALUES (1, 'Album One', 'Testing', true),
-				   (2, 'Album Two', 'Testing', true)`)
+				   (2, 'Album Two', 'Testing', true),
+				   (3, 'Album Three', 'Testing', true)`)
 	require.NoError(t, err)
 
 	// Insert tracks
 	err = store.Exec(context.Background(),
 		`INSERT INTO tracks (musicbrainz_id, release_id) 
 			VALUES ('33333333-3333-3333-3333-333333333333', 1),
-				   ('44444444-4444-4444-4444-444444444444', 2)`)
+				   ('44444444-4444-4444-4444-444444444444', 2),
+				   ('55555555-5555-5555-5555-555555555555', 1),
+				   (NULL, 3)`)
 	require.NoError(t, err)
 
 	err = store.Exec(context.Background(),
 		`INSERT INTO track_aliases (track_id, alias, source, is_primary) 
 			VALUES (1, 'Track One', 'Testing', true),
-				   (2, 'Track Two', 'Testing', true)`)
+				   (2, 'Track Two', 'Testing', true),
+				   (3, 'Track Three', 'Testing', true),
+				   (4, 'Track Four', 'Testing', true)`)
 	require.NoError(t, err)
 
 	// Associate artists with albums and tracks
 	err = store.Exec(context.Background(),
 		`INSERT INTO artist_releases (artist_id, release_id) 
-			VALUES (1, 1), (2, 2)`)
+			VALUES (1, 1), (2, 2), (1, 3)`)
 	require.NoError(t, err)
 
 	err = store.Exec(context.Background(),
 		`INSERT INTO artist_tracks (artist_id, track_id) 
-			VALUES (1, 1), (2, 2)`)
+			VALUES (1, 1), (2, 2), (1, 3), (1, 4)`)
 	require.NoError(t, err)
 
 	// Insert listens
 	err = store.Exec(context.Background(),
 		`INSERT INTO listens (user_id, track_id, listened_at) 
 			VALUES (1, 1, NOW() - INTERVAL '1 day'),
-				   (1, 2, NOW() - INTERVAL '2 days')`)
+				   (1, 2, NOW() - INTERVAL '2 days'),
+				   (1, 3, NOW() - INTERVAL '3 days'),
+				   (1, 4, NOW() - INTERVAL '3 days')`)
 	require.NoError(t, err)
 }
 
@@ -82,6 +90,32 @@ func TestMergeTracks(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, 2, count, "expected all listens to be merged into Track 2")
 
+	// Verify artist is associated with album
+	exists, err := store.RowExists(ctx, `
+    SELECT EXISTS (
+      SELECT 1 FROM artist_releases
+      WHERE release_id = $1 AND artist_id = $2
+    )`, 2, 1)
+	require.NoError(t, err)
+	assert.True(t, exists, "expected old artist to be associated with album")
+
+	truncateTestData(t)
+}
+
+func TestMergeTracks_SameRelease(t *testing.T) {
+	ctx := context.Background()
+	setupTestDataForMerge(t)
+
+	// Merge Track 1 into Track 2
+	err := store.MergeTracks(ctx, 1, 3)
+	require.NoError(t, err)
+
+	// Verify listens are updated
+	var count int
+	count, err = store.Count(ctx, `SELECT COUNT(*) FROM listens WHERE track_id = 3`)
+	require.NoError(t, err)
+	assert.Equal(t, 2, count, "expected all listens to be merged into Track 3")
+
 	truncateTestData(t)
 }
 
@@ -101,7 +135,32 @@ func TestMergeAlbums(t *testing.T) {
 	// Verify tracks are updated
 	count, err = store.Count(ctx, `SELECT COUNT(*) FROM tracks WHERE release_id = 2`)
 	require.NoError(t, err)
-	assert.Equal(t, 2, count, "expected all tracks to be merged into Album 2")
+	assert.Equal(t, 3, count, "expected all tracks to be merged into Album 2")
+
+	// Verify artist is associated with primary album
+	exists, err := store.RowExists(ctx, `
+    SELECT EXISTS (
+      SELECT 1 FROM artist_releases
+      WHERE release_id = $1 AND artist_id = $2
+    )`, 2, 1)
+	require.NoError(t, err)
+	assert.True(t, exists, "expected old album artist to be associated with new album")
+
+	truncateTestData(t)
+}
+
+func TestMergeAlbums_SameArtists(t *testing.T) {
+	ctx := context.Background()
+	setupTestDataForMerge(t)
+
+	// Merge Album 1 into Album 3
+	err := store.MergeAlbums(ctx, 1, 3, false)
+	require.NoError(t, err)
+
+	// Verify tracks are updated
+	count, err := store.Count(ctx, `SELECT COUNT(*) FROM tracks WHERE release_id = 3`)
+	require.NoError(t, err)
+	assert.Equal(t, 3, count, "expected all tracks to be merged into Album 3")
 
 	truncateTestData(t)
 }
@@ -122,11 +181,93 @@ func TestMergeArtists(t *testing.T) {
 	// Verify artist associations are updated
 	count, err = store.Count(ctx, `SELECT COUNT(*) FROM artist_tracks WHERE artist_id = 2`)
 	require.NoError(t, err)
-	assert.Equal(t, 2, count, "expected all tracks to be associated with Artist 2")
+	assert.Equal(t, 4, count, "expected all tracks to be associated with Artist 2")
 
 	count, err = store.Count(ctx, `SELECT COUNT(*) FROM artist_releases WHERE artist_id = 2`)
 	require.NoError(t, err)
-	assert.Equal(t, 2, count, "expected all releases to be associated with Artist 2")
+	assert.Equal(t, 3, count, "expected all releases to be associated with Artist 2")
+
+	truncateTestData(t)
+}
 
+func TestMergeTracks_EnsureParentOfReleaseIsKept(t *testing.T) {
 	truncateTestData(t)
+
+	// Prepare test data with only album assigned to artist 1,
+	// and two tracks, both under album one, but under two different artists
+
+	// Insert artists
+	err := store.Exec(context.Background(),
+		`INSERT INTO artists (musicbrainz_id, image, image_source) 
+			VALUES ('00000000-0000-0000-0000-000000000001', '10000000-0000-0000-0000-000000000000', 'source.com'),
+				   ('00000000-0000-0000-0000-000000000002', NULL, NULL)`)
+	require.NoError(t, err)
+
+	err = store.Exec(context.Background(),
+		`INSERT INTO artist_aliases (artist_id, alias, source, is_primary) 
+			VALUES (1, 'Artist One', 'Testing', true),
+				   (2, 'Artist Two', 'Testing', true)`)
+	require.NoError(t, err)
+
+	// Insert albums
+	err = store.Exec(context.Background(),
+		`INSERT INTO releases (musicbrainz_id, image, image_source) 
+			VALUES ('11111111-1111-1111-1111-111111111111', '20000000-0000-0000-0000-000000000000', 'source.com')`)
+	require.NoError(t, err)
+
+	err = store.Exec(context.Background(),
+		`INSERT INTO release_aliases (release_id, alias, source, is_primary) 
+			VALUES (1, 'Album One', 'Testing', true)`)
+	require.NoError(t, err)
+
+	// Insert tracks
+	err = store.Exec(context.Background(),
+		`INSERT INTO tracks (musicbrainz_id, release_id) 
+			VALUES ('33333333-3333-3333-3333-333333333333', 1),
+				   ('44444444-4444-4444-4444-444444444444', 1)`)
+	require.NoError(t, err)
+
+	err = store.Exec(context.Background(),
+		`INSERT INTO track_aliases (track_id, alias, source, is_primary) 
+			VALUES (1, 'Track One', 'Testing', true),
+				   (2, 'Track Two', 'Testing', true)`)
+	require.NoError(t, err)
+
+	// Associate artists with albums and tracks
+	err = store.Exec(context.Background(),
+		`INSERT INTO artist_releases (artist_id, release_id) 
+			VALUES (1, 1)`)
+	require.NoError(t, err)
+
+	err = store.Exec(context.Background(),
+		`INSERT INTO artist_tracks (artist_id, track_id) 
+			VALUES (1, 1), (2, 2)`)
+	require.NoError(t, err)
+
+	// Insert listens
+	err = store.Exec(context.Background(),
+		`INSERT INTO listens (user_id, track_id, listened_at) 
+			VALUES (1, 1, NOW() - INTERVAL '1 day'),
+				   (1, 2, NOW() - INTERVAL '2 days')`)
+	require.NoError(t, err)
+
+	ctx := context.Background()
+	err = store.MergeTracks(ctx, 1, 2)
+	require.NoError(t, err)
+
+	exists, err := store.RowExists(ctx, `
+    SELECT EXISTS (
+      SELECT 1 FROM artists
+      WHERE id = $1
+    )`, 1)
+	require.NoError(t, err)
+	assert.True(t, exists, "expected artist associated with release to still exist")
+
+	exists, err = store.RowExists(ctx, `
+    SELECT EXISTS (
+      SELECT 1 FROM releases
+      WHERE id = $1
+    )`, 1)
+	require.NoError(t, err)
+	assert.True(t, exists, "expected release to still exist")
 }

internal/repository/release.sql.go

@@ -460,3 +460,18 @@ func (q *Queries) UpdateReleaseMbzID(ctx context.Context, arg UpdateReleaseMbzID
 	_, err := q.db.Exec(ctx, updateReleaseMbzID, arg.ID, arg.MusicBrainzID)
 	return err
 }
+
+const updateReleaseVariousArtists = `-- name: UpdateReleaseVariousArtists :exec
+UPDATE releases SET various_artists = $2
+WHERE id = $1
+`
+
+type UpdateReleaseVariousArtistsParams struct {
+	ID             int32
+	VariousArtists bool
+}
+
+func (q *Queries) UpdateReleaseVariousArtists(ctx context.Context, arg UpdateReleaseVariousArtistsParams) error {
+	_, err := q.db.Exec(ctx, updateReleaseVariousArtists, arg.ID, arg.VariousArtists)
+	return err
+}