From a8955f24e0b5f31b1399bbdacf9fd2f8d543b9c0 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Wed, 1 May 2024 20:05:36 -0400
Subject: [PATCH] Fix AlbumPlayCountMode. Closes #2984

---
 persistence/album_repository.go      |  8 +++-----
 persistence/album_repository_test.go | 28 ++++++++++++++++++++++------
 persistence/sql_annotations.go       | 12 ++++++++++--
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/persistence/album_repository.go b/persistence/album_repository.go
index 3ef3528ae8e..fa9f7f1d878 100644
--- a/persistence/album_repository.go
+++ b/persistence/album_repository.go
@@ -10,7 +10,6 @@ import (
 	. "github.com/Masterminds/squirrel"
 	"github.com/deluan/rest"
 	"github.com/navidrome/navidrome/conf"
-	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/pocketbase/dbx"
@@ -23,13 +22,12 @@ type albumRepository struct {
 
 type dbAlbum struct {
 	*model.Album `structs:",flatten"`
-	Discs        string `structs:"-" json:"discs"`
+	Discs        string  `structs:"-" json:"discs"`
+	PlayCount    float64 `structs:"-" json:"play_count"`
 }
 
 func (a *dbAlbum) PostScan() error {
-	if conf.Server.AlbumPlayCountMode == consts.AlbumPlayCountModeNormalized && a.Album.SongCount != 0 {
-		a.Album.PlayCount = int64(math.Round(float64(a.Album.PlayCount) / float64(a.Album.SongCount)))
-	}
+	a.Album.PlayCount = int64(math.Round(a.PlayCount))
 	if a.Discs != "" {
 		return json.Unmarshal([]byte(a.Discs), &a.Album.Discs)
 	}
diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go
index 04b311f5016..5cc278daa27 100644
--- a/persistence/album_repository_test.go
+++ b/persistence/album_repository_test.go
@@ -2,8 +2,10 @@ package persistence
 
 import (
 	"context"
+	"time"
 
 	"github.com/fatih/structs"
+	"github.com/google/uuid"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
@@ -96,9 +98,16 @@ var _ = Describe("AlbumRepository", func() {
 			DescribeTable("normalizes play count when AlbumPlayCountMode is absolute",
 				func(songCount, playCount, expected int) {
 					conf.Server.AlbumPlayCountMode = consts.AlbumPlayCountModeAbsolute
-					dba := dbAlbum{Album: &model.Album{ID: "1", Name: "name", SongCount: songCount, Annotations: model.Annotations{PlayCount: int64(playCount)}}}
-					Expect(dba.PostScan()).To(Succeed())
-					Expect(dba.Album.PlayCount).To(Equal(int64(expected)))
+
+					id := uuid.NewString()
+					Expect(repo.Put(&model.Album{ID: id, Name: "name", SongCount: songCount})).To(Succeed())
+					for i := 0; i < playCount; i++ {
+						Expect(repo.IncPlayCount(id, time.Now())).To(Succeed())
+					}
+
+					album, err := repo.Get(id)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(album.PlayCount).To(Equal(int64(expected)))
 				},
 				Entry("1 song, 0 plays", 1, 0, 0),
 				Entry("1 song, 4 plays", 1, 4, 4),
@@ -112,9 +121,16 @@ var _ = Describe("AlbumRepository", func() {
 			DescribeTable("normalizes play count when AlbumPlayCountMode is normalized",
 				func(songCount, playCount, expected int) {
 					conf.Server.AlbumPlayCountMode = consts.AlbumPlayCountModeNormalized
-					dba := dbAlbum{Album: &model.Album{ID: "1", Name: "name", SongCount: songCount, Annotations: model.Annotations{PlayCount: int64(playCount)}}}
-					Expect(dba.PostScan()).To(Succeed())
-					Expect(dba.Album.PlayCount).To(Equal(int64(expected)))
+
+					id := uuid.NewString()
+					Expect(repo.Put(&model.Album{ID: id, Name: "name", SongCount: songCount})).To(Succeed())
+					for i := 0; i < playCount; i++ {
+						Expect(repo.IncPlayCount(id, time.Now())).To(Succeed())
+					}
+
+					album, err := repo.Get(id)
+					Expect(err).ToNot(HaveOccurred())
+					Expect(album.PlayCount).To(Equal(int64(expected)))
 				},
 				Entry("1 song, 0 plays", 1, 0, 0),
 				Entry("1 song, 4 plays", 1, 4, 4),
diff --git a/persistence/sql_annotations.go b/persistence/sql_annotations.go
index 7b31d07a51a..a8eef3f2f16 100644
--- a/persistence/sql_annotations.go
+++ b/persistence/sql_annotations.go
@@ -7,6 +7,8 @@ import (
 
 	. "github.com/Masterminds/squirrel"
 	"github.com/google/uuid"
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 )
@@ -14,7 +16,7 @@ import (
 const annotationTable = "annotation"
 
 func (r sqlRepository) newSelectWithAnnotation(idField string, options ...model.QueryOptions) SelectBuilder {
-	return r.newSelect(options...).
+	query := r.newSelect(options...).
 		LeftJoin("annotation on ("+
 			"annotation.item_id = "+idField+
 			" AND annotation.item_type = '"+r.tableName+"'"+
@@ -22,10 +24,16 @@ func (r sqlRepository) newSelectWithAnnotation(idField string, options ...model.
 		Columns(
 			"coalesce(starred, 0) as starred",
 			"coalesce(rating, 0) as rating",
-			"coalesce(play_count, 0) as play_count",
 			"starred_at",
 			"play_date",
 		)
+	if conf.Server.AlbumPlayCountMode == consts.AlbumPlayCountModeNormalized && r.tableName == "album" {
+		query = query.Columns("coalesce(round(cast(play_count as float) / coalesce(song_count, 1), 1), 0) as play_count")
+	} else {
+		query = query.Columns("coalesce(play_count, 0) as play_count")
+	}
+
+	return query
 }
 
 func (r sqlRepository) annId(itemID ...string) And {