feat: hide missing artists from regular users and Subsonic API (#4092)

* Handle missing artists for non-admin users * feat(artist): enhance ArtistList with missing row styling and class management Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
2025-05-20 21:53:02 -04:00
parent 4733616d90
commit 453630d430
7 changed files with 140 additions and 9 deletions
@@ -116,6 +116,7 @@ func NewArtistRepository(ctx context.Context, db dbx.Builder) model.ArtistReposi
 		"name":    fullTextFilter(r.tableName),
 		"starred": booleanFilter,
 		"role":    roleFilter,
+		"missing": booleanFilter,
 	})
 	r.setSortMappings(map[string]string{
 		"name":        "order_artist_name",
@@ -202,7 +203,7 @@ func (r *artistRepository) getIndexKey(a model.Artist) string {
 }

 // TODO Cache the index (recalculate when there are changes to the DB)
-func (r *artistRepository) GetIndex(roles ...model.Role) (model.ArtistIndexes, error) {
+func (r *artistRepository) GetIndex(includeMissing bool, roles ...model.Role) (model.ArtistIndexes, error) {
 	options := model.QueryOptions{Sort: "name"}
 	if len(roles) > 0 {
 		roleFilters := slice.Map(roles, func(r model.Role) Sqlizer {
@@ -210,6 +211,13 @@ func (r *artistRepository) GetIndex(roles ...model.Role) (model.ArtistIndexes, e
 		})
 		options.Filters = And(roleFilters)
 	}
+	if !includeMissing {
+		if options.Filters == nil {
+			options.Filters = Eq{"artist.missing": false}
+		} else {
+			options.Filters = And{options.Filters, Eq{"artist.missing": false}}
+		}
+	}
 	artists, err := r.GetAll(options)
 	if err != nil {
 		return nil, err
@@ -236,6 +244,26 @@ func (r *artistRepository) purgeEmpty() error {
 	return nil
 }

+// markMissing sets the Missing flag based on album data.
+func (r *artistRepository) markMissing() (int64, error) {
+	q := Expr(`
+                update artist
+                set missing = not exists (
+                        select 1 from album_artists aa
+                        join album a on aa.album_id = a.id
+                        where aa.artist_id = artist.id and a.missing = false
+                )
+        `)
+	c, err := r.executeSQL(q)
+	if err != nil {
+		return 0, fmt.Errorf("marking missing artists: %w", err)
+	}
+	if c > 0 {
+		log.Debug(r.ctx, "Marked missing artists", "totalUpdated", c)
+	}
+	return c, nil
+}
+
 // RefreshPlayCounts updates the play count and last play date annotations for all artists, based
 // on the media files associated with them.
 func (r *artistRepository) RefreshPlayCounts() (int64, error) {
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"

+	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
 	"github.com/navidrome/navidrome/log"
@@ -94,7 +95,7 @@ var _ = Describe("ArtistRepository", func() {
 				er := repo.Put(&artistBeatles)
 				Expect(er).To(BeNil())

-				idx, err := repo.GetIndex()
+				idx, err := repo.GetIndex(false)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(idx).To(HaveLen(2))
 				Expect(idx[0].ID).To(Equal("F"))
@@ -112,7 +113,7 @@ var _ = Describe("ArtistRepository", func() {

 			// BFR Empty SortArtistName is not saved in the DB anymore
 			XIt("returns the index when PreferSortTags is true and SortArtistName is empty", func() {
-				idx, err := repo.GetIndex()
+				idx, err := repo.GetIndex(false)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(idx).To(HaveLen(2))
 				Expect(idx[0].ID).To(Equal("B"))
@@ -134,7 +135,7 @@ var _ = Describe("ArtistRepository", func() {
 				er := repo.Put(&artistBeatles)
 				Expect(er).To(BeNil())

-				idx, err := repo.GetIndex()
+				idx, err := repo.GetIndex(false)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(idx).To(HaveLen(2))
 				Expect(idx[0].ID).To(Equal("B"))
@@ -151,7 +152,7 @@ var _ = Describe("ArtistRepository", func() {
 			})

 			It("returns the index when SortArtistName is empty", func() {
-				idx, err := repo.GetIndex()
+				idx, err := repo.GetIndex(false)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(idx).To(HaveLen(2))
 				Expect(idx[0].ID).To(Equal("B"))
@@ -233,5 +234,91 @@ var _ = Describe("ArtistRepository", func() {
 				Expect(m).ToNot(HaveKey("mbz_artist_id"))
 			})
 		})
+
+		Describe("Missing artist visibility", func() {
+			var raw *artistRepository
+			var missing model.Artist
+
+			insertMissing := func() {
+				missing = model.Artist{ID: "m1", Name: "Missing", OrderArtistName: "missing"}
+				Expect(repo.Put(&missing)).To(Succeed())
+				raw = repo.(*artistRepository)
+				_, err := raw.executeSQL(squirrel.Update(raw.tableName).Set("missing", true).Where(squirrel.Eq{"id": missing.ID}))
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			removeMissing := func() {
+				if raw != nil {
+					_, _ = raw.executeSQL(squirrel.Delete(raw.tableName).Where(squirrel.Eq{"id": missing.ID}))
+				}
+			}
+
+			Context("regular user", func() {
+				BeforeEach(func() {
+					ctx := log.NewContext(context.TODO())
+					ctx = request.WithUser(ctx, model.User{ID: "u1"})
+					repo = NewArtistRepository(ctx, GetDBXBuilder())
+					insertMissing()
+				})
+
+				AfterEach(func() { removeMissing() })
+
+				It("does not return missing artist in GetAll", func() {
+					artists, err := repo.GetAll(model.QueryOptions{Filters: squirrel.Eq{"artist.missing": false}})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(artists).To(HaveLen(2))
+				})
+
+				It("does not return missing artist in Search", func() {
+					res, err := repo.Search("missing", 0, 10, false)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(res).To(BeEmpty())
+				})
+
+				It("does not return missing artist in GetIndex", func() {
+					idx, err := repo.GetIndex(false)
+					Expect(err).ToNot(HaveOccurred())
+					// Only 2 artists should be present
+					total := 0
+					for _, ix := range idx {
+						total += len(ix.Artists)
+					}
+					Expect(total).To(Equal(2))
+				})
+			})
+
+			Context("admin user", func() {
+				BeforeEach(func() {
+					ctx := log.NewContext(context.TODO())
+					ctx = request.WithUser(ctx, model.User{ID: "admin", IsAdmin: true})
+					repo = NewArtistRepository(ctx, GetDBXBuilder())
+					insertMissing()
+				})
+
+				AfterEach(func() { removeMissing() })
+
+				It("returns missing artist in GetAll", func() {
+					artists, err := repo.GetAll()
+					Expect(err).ToNot(HaveOccurred())
+					Expect(artists).To(HaveLen(3))
+				})
+
+				It("returns missing artist in Search", func() {
+					res, err := repo.Search("missing", 0, 10, true)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(res).To(HaveLen(1))
+				})
+
+				It("returns missing artist in GetIndex when included", func() {
+					idx, err := repo.GetIndex(true)
+					Expect(err).ToNot(HaveOccurred())
+					total := 0
+					for _, ix := range idx {
+						total += len(ix.Artists)
+					}
+					Expect(total).To(Equal(3))
+				})
+			})
+		})
 	})
 })
@@ -170,6 +170,7 @@ func (s *SQLStore) GC(ctx context.Context) error {
 	err := chain.RunSequentially(
 		trace(ctx, "purge empty albums", func() error { return s.Album(ctx).(*albumRepository).purgeEmpty() }),
 		trace(ctx, "purge empty artists", func() error { return s.Artist(ctx).(*artistRepository).purgeEmpty() }),
+		trace(ctx, "mark missing artists", func() error { _, err := s.Artist(ctx).(*artistRepository).markMissing(); return err }),
 		trace(ctx, "purge empty folders", func() error { return s.Folder(ctx).(*folderRepository).purgeEmpty() }),
 		trace(ctx, "clean album annotations", func() error { return s.Album(ctx).(*albumRepository).cleanAnnotations() }),
 		trace(ctx, "clean artist annotations", func() error { return s.Artist(ctx).(*artistRepository).cleanAnnotations() }),