fix(playlists): allow toggling auto-import and avoid unnecessary artwork reloads (#5421)

* fix(playlists): allow toggling auto-import (sync) via REST API

The updatePlaylistEntity handler was not applying the sync field from
incoming requests, causing the auto-import toggle in the UI to have no
effect. Apply the sync value for file-backed playlists only.

* fix(playlists): enhance update logic for playlist metadata and sync toggle

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(playlists): address code review feedback

- Add pointer equality short-circuit in rulesEqual before reflect.DeepEqual
- Guard against empty ID in Put's partial-update path
- Only apply Sync when it actually differs from current value, preventing
  zero-value overwrites from partial payloads

* fix(playlists): remove unused parameters from Update method

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

core/playlists/rest_adapter.go

@@ -3,9 +3,11 @@ package playlists
 import (
 	"context"
 	"errors"
+	"reflect"
 
 	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/model"
+	"github.com/navidrome/navidrome/model/criteria"
 	"github.com/navidrome/navidrome/model/request"
 )
 
@@ -32,8 +34,8 @@ func (r *playlistRepositoryWrapper) Save(entity any) (string, error) {
 	return r.service.savePlaylist(r.ctx, entity.(*model.Playlist))
 }
 
-func (r *playlistRepositoryWrapper) Update(id string, entity any, cols ...string) error {
+func (r *playlistRepositoryWrapper) Update(id string, entity any, _ ...string) error {
-	return r.service.updatePlaylistEntity(r.ctx, id, entity.(*model.Playlist), cols...)
+	return r.service.updatePlaylistEntity(r.ctx, id, entity.(*model.Playlist))
 }
 
 func (r *playlistRepositoryWrapper) Delete(id string) error {
@@ -77,7 +79,7 @@ func (s *playlists) savePlaylist(ctx context.Context, pls *model.Playlist) (stri
 
 // updatePlaylistEntity updates playlist metadata with permission checks.
 // Used by the REST API wrapper.
-func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity *model.Playlist, cols ...string) error {
+func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity *model.Playlist) error {
 	current, err := s.checkWritable(ctx, id)
 	if err != nil {
 		switch {
@@ -93,11 +95,45 @@ func (s *playlists) updatePlaylistEntity(ctx context.Context, id string, entity
 	if !usr.IsAdmin && entity.OwnerID != "" && entity.OwnerID != current.OwnerID {
 		return rest.ErrPermissionDenied
 	}
-	// Apply ownership change (admin only)
+
-	if entity.OwnerID != "" {
+	contentChanged := entity.Name != current.Name ||
-		current.OwnerID = entity.OwnerID
+		entity.Comment != current.Comment ||
+		(entity.OwnerID != "" && entity.OwnerID != current.OwnerID) ||
+		!rulesEqual(current.Rules, entity.Rules)
+
+	if contentChanged {
+		if entity.OwnerID != "" {
+			current.OwnerID = entity.OwnerID
+		}
+		current.Rules = entity.Rules
+		if current.Path != "" && current.Sync != entity.Sync {
+			current.Sync = entity.Sync
+		}
+		return s.updateMetadata(ctx, s.ds, current, &entity.Name, &entity.Comment, &entity.Public)
 	}
-	// Apply smart playlist rules update
+
-	current.Rules = entity.Rules
+	// Only sync/public changed — skip updatedAt so cover art URLs stay stable
-	return s.updateMetadata(ctx, s.ds, current, &entity.Name, &entity.Comment, &entity.Public)
+	var cols []string
+	if current.Path != "" && current.Sync != entity.Sync {
+		current.Sync = entity.Sync
+		cols = append(cols, "sync")
+	}
+	if current.Public != entity.Public {
+		current.Public = entity.Public
+		cols = append(cols, "public")
+	}
+	if len(cols) == 0 {
+		return nil
+	}
+	return s.ds.Playlist(ctx).Put(current, cols...)
+}
+
+func rulesEqual(a, b *criteria.Criteria) bool {
+	if a == b {
+		return true
+	}
+	if a == nil || b == nil {
+		return false
+	}
+	return reflect.DeepEqual(a, b)
 }

core/playlists/rest_adapter_test.go

@@ -142,6 +142,76 @@ var _ = Describe("REST Adapter", func() {
 				Expect(mockPlsRepo.Last.Rules).To(Equal(newRules))
 			})
 
+			It("allows toggling sync for file-backed playlists", func() {
+				originalTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+				mockPlsRepo.Data["file-pls"] = &model.Playlist{
+					ID:        "file-pls",
+					Name:      "File Playlist",
+					OwnerID:   "user-1",
+					Path:      "/music/playlist.m3u",
+					Sync:      true,
+					UpdatedAt: originalTime,
+				}
+				ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
+				repo = ps.NewRepository(ctx).(rest.Persistable)
+				pls := &model.Playlist{Name: "File Playlist", Sync: false}
+				err := repo.Update("file-pls", pls)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(mockPlsRepo.Last.Sync).To(BeFalse())
+				Expect(mockPlsRepo.Last.UpdatedAt).To(Equal(originalTime))
+			})
+
+			It("does not allow setting sync on non-file-backed playlists", func() {
+				mockPlsRepo.Data["manual-pls"] = &model.Playlist{
+					ID:      "manual-pls",
+					Name:    "Manual Playlist",
+					OwnerID: "user-1",
+					Path:    "",
+					Sync:    false,
+				}
+				ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
+				repo = ps.NewRepository(ctx).(rest.Persistable)
+				pls := &model.Playlist{Name: "Manual Playlist", Sync: true}
+				err := repo.Update("manual-pls", pls)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(mockPlsRepo.Last).To(BeNil())
+			})
+
+			It("does not bump updatedAt when only public changes", func() {
+				originalTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
+				mockPlsRepo.Data["pls-pub"] = &model.Playlist{
+					ID:        "pls-pub",
+					Name:      "My Playlist",
+					OwnerID:   "user-1",
+					Public:    false,
+					UpdatedAt: originalTime,
+				}
+				ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
+				repo = ps.NewRepository(ctx).(rest.Persistable)
+				pls := &model.Playlist{Name: "My Playlist", Public: true}
+				err := repo.Update("pls-pub", pls)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(mockPlsRepo.Last.Public).To(BeTrue())
+				Expect(mockPlsRepo.Last.UpdatedAt).To(Equal(originalTime))
+			})
+
+			It("bumps updatedAt when name changes along with sync", func() {
+				mockPlsRepo.Data["file-pls2"] = &model.Playlist{
+					ID:      "file-pls2",
+					Name:    "Old Name",
+					OwnerID: "user-1",
+					Path:    "/music/playlist.m3u",
+					Sync:    true,
+				}
+				ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
+				repo = ps.NewRepository(ctx).(rest.Persistable)
+				pls := &model.Playlist{Name: "New Name", Sync: false}
+				err := repo.Update("file-pls2", pls)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(mockPlsRepo.Last.Name).To(Equal("New Name"))
+				Expect(mockPlsRepo.Last.Sync).To(BeFalse())
+			})
+
 			It("returns rest.ErrNotFound when playlist doesn't exist", func() {
 				ctx = request.WithUser(ctx, model.User{ID: "user-1", IsAdmin: false})
 				repo = ps.NewRepository(ctx).(rest.Persistable)

model/playlist.go

@@ -123,7 +123,7 @@ type PlaylistRepository interface {
 	ResourceRepository
 	CountAll(options ...QueryOptions) (int64, error)
 	Exists(id string) (bool, error)
-	Put(pls *Playlist) error
+	Put(pls *Playlist, cols ...string) error
 	Get(id string) (*Playlist, error)
 	GetWithTracks(id string, refreshSmartPlaylist, includeMissing bool) (*Playlist, error)
 	GetAll(options ...QueryOptions) (Playlists, error)

persistence/playlist_repository.go

@@ -97,8 +97,15 @@ func (r *playlistRepository) Delete(id string) error {
 	return r.delete(And{Eq{"id": id}, r.userFilter()})
 }
 
-func (r *playlistRepository) Put(p *model.Playlist) error {
+func (r *playlistRepository) Put(p *model.Playlist, cols ...string) error {
 	pls := dbPlaylist{Playlist: *p}
+	if len(cols) > 0 {
+		if pls.ID == "" {
+			return errors.New("playlist id is required for partial update")
+		}
+		_, err := r.put(pls.ID, pls, cols...)
+		return err
+	}
 	if pls.ID == "" {
 		pls.CreatedAt = time.Now()
 	}

tests/mock_playlist_repo.go

@@ -45,7 +45,7 @@ func (m *MockPlaylistRepo) GetWithTracks(id string, _, _ bool) (*model.Playlist,
 	return m.Get(id)
 }
 
-func (m *MockPlaylistRepo) Put(pls *model.Playlist) error {
+func (m *MockPlaylistRepo) Put(pls *model.Playlist, _ ...string) error {
 	if m.Err {
 		return errors.New("error")
 	}