mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-18 07:30:29 +00:00
fix(persistence): recompute the cached average rating inside ReassignAnnotation
Moving annotation rows always changes the new item's rating population, so the recompute belongs with the move rather than at each call site. Covers the album reassign in the remap and the two scanner sites, and replaces the explicit call ReassignReferences was making. Album was the worse case: rate an album, move its files, and 'missing fix' handed the rating to an album still caching an average of 0.
This commit is contained in:
parent
1d592af243
commit
385bab35fe
@ -119,7 +119,8 @@ func (s *maintenanceService) RemapMissingFile(ctx context.Context, missingID, ta
|
||||
return err
|
||||
}
|
||||
|
||||
// Stats are refreshed synchronously, unlike deleteMissing, so the CLI sees them before it exits
|
||||
// Stats are refreshed synchronously, unlike deleteMissing, so the CLI sees them before it exits.
|
||||
// album/artist play count aggregates are not recalculated here; they are refreshed by the next scan.
|
||||
if _, err := s.ds.Artist(ctx).RefreshStats(true); err != nil {
|
||||
log.Error(ctx, "Error refreshing artist stats after remapping missing file", err)
|
||||
}
|
||||
|
||||
@ -404,10 +404,6 @@ func (r *mediaFileRepository) ReassignReferences(prevID, newID string) error {
|
||||
if err := r.ReassignAnnotation(prevID, newID); err != nil {
|
||||
return fmt.Errorf("reassigning annotations: %w", err)
|
||||
}
|
||||
// The merged rows change the rating population, so the cached average no longer matches
|
||||
if err := r.updateAvgRating(newID); err != nil {
|
||||
return fmt.Errorf("recomputing average rating: %w", err)
|
||||
}
|
||||
if err := r.reassignBookmark(prevID, newID); err != nil {
|
||||
return fmt.Errorf("reassigning bookmarks: %w", err)
|
||||
}
|
||||
|
||||
@ -188,8 +188,11 @@ func (r sqlRepository) ReassignAnnotation(prevID string, newID string) error {
|
||||
// OR IGNORE keeps newID's own row where a user annotated both, instead of aborting the whole statement
|
||||
upd := Expr("update or ignore "+annotationTable+" set item_id = ? where item_type = ? and item_id = ?",
|
||||
newID, r.tableName, prevID)
|
||||
_, err := r.executeSQL(upd)
|
||||
return err
|
||||
if _, err := r.executeSQL(upd); err != nil {
|
||||
return err
|
||||
}
|
||||
// The moved rows change newID's rating population, so its cached average no longer matches
|
||||
return r.updateAvgRating(newID)
|
||||
}
|
||||
|
||||
func (r sqlRepository) cleanAnnotations() error {
|
||||
|
||||
@ -55,6 +55,16 @@ var _ = Describe("Annotation Filters", func() {
|
||||
Expect(got.Rating).To(Equal(4))
|
||||
})
|
||||
|
||||
It("recomputes the new item's cached average rating", func() {
|
||||
Expect(albumRepo.SetRating(4, prev.ID)).To(Succeed())
|
||||
|
||||
Expect(albumRepo.ReassignAnnotation(prev.ID, next.ID)).To(Succeed())
|
||||
|
||||
got, err := albumRepo.Get(next.ID)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(got.AverageRating).To(Equal(4.0))
|
||||
})
|
||||
|
||||
It("keeps the new item's annotation when both exist", func() {
|
||||
Expect(albumRepo.SetRating(4, prev.ID)).To(Succeed())
|
||||
Expect(albumRepo.SetRating(2, next.ID)).To(Succeed())
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user