From b671d0ff7bc9b98466f360bded4694b21cc60b1c Mon Sep 17 00:00:00 2001
From: caiocotts <31974888+caiocotts@users.noreply.github.com>
Date: Sat, 24 Apr 2021 21:40:55 -0400
Subject: [PATCH] Better handling of album comments (#1013)
* Change album comment behaviour
* Don't check first item
* Fix previously imported album comments.
* Remove song comments if album comment is present
---
.../20210418232815_fix_album_comments.go | 66 +++++++++++++++++++
persistence/album_repository.go | 13 ++--
persistence/album_repository_test.go | 7 +-
ui/src/album/AlbumShow.js | 1 +
ui/src/album/AlbumSongs.js | 10 +++
ui/src/album/AlbumSongs.test.js | 24 +++++++
6 files changed, 114 insertions(+), 7 deletions(-)
create mode 100644 db/migration/20210418232815_fix_album_comments.go
create mode 100644 ui/src/album/AlbumSongs.test.js
diff --git a/db/migration/20210418232815_fix_album_comments.go b/db/migration/20210418232815_fix_album_comments.go
new file mode 100644
index 00000000..44a231f5
--- /dev/null
+++ b/db/migration/20210418232815_fix_album_comments.go
@@ -0,0 +1,66 @@
+package migrations
+
+import (
+ "database/sql"
+ "strings"
+
+ "github.com/navidrome/navidrome/log"
+ "github.com/pressly/goose"
+)
+
+func init() {
+ goose.AddMigration(upFixAlbumComments, downFixAlbumComments)
+}
+
+func upFixAlbumComments(tx *sql.Tx) error {
+ const zwsp = string('\u200b')
+ rows, err := tx.Query(`
+ SELECT album.id, group_concat(media_file.comment, '` + zwsp + `') FROM album, media_file WHERE media_file.album_id = album.id GROUP BY album.id;
+ `)
+ if err != nil {
+ return err
+ }
+ defer rows.Close()
+
+ stmt, err := tx.Prepare("UPDATE album SET comment = ? WHERE id = ?")
+ if err != nil {
+ return err
+ }
+ var id string
+ var comments sql.NullString
+
+ for rows.Next() {
+ err = rows.Scan(&id, &comments)
+ if err != nil {
+ return err
+ }
+ if !comments.Valid {
+ continue
+ }
+ comment := getComment(comments.String, zwsp)
+ _, err = stmt.Exec(comment, id)
+
+ if err != nil {
+ log.Error("Error setting album's comments", "albumId", id, err)
+ }
+ }
+ return rows.Err()
+}
+
+func downFixAlbumComments(tx *sql.Tx) error {
+ return nil
+}
+
+func getComment(comments string, separator string) string {
+ cs := strings.Split(comments, separator)
+ if len(cs) == 0 {
+ return ""
+ }
+ first := cs[0]
+ for _, c := range cs[1:] {
+ if first != c {
+ return ""
+ }
+ }
+ return first
+}
diff --git a/persistence/album_repository.go b/persistence/album_repository.go
index 01d37935..94af9b9e 100644
--- a/persistence/album_repository.go
+++ b/persistence/album_repository.go
@@ -263,15 +263,18 @@ func (r *albumRepository) refresh(ids ...string) error {
return err
}
-// Return the first non empty comment, if any
func getComment(comments string, separator string) string {
cs := strings.Split(comments, separator)
- for _, c := range cs {
- if c != "" {
- return c
+ if len(cs) == 0 {
+ return ""
+ }
+ first := cs[0]
+ for _, c := range cs[1:] {
+ if first != c {
+ return ""
}
}
- return ""
+ return first
}
func getMinYear(years string) int {
diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go
index 561551f6..ec0f4344 100644
--- a/persistence/album_repository_test.go
+++ b/persistence/album_repository_test.go
@@ -96,8 +96,11 @@ var _ = Describe("AlbumRepository", func() {
It("returns empty string if there are no comments", func() {
Expect(getComment("", "")).To(Equal(""))
})
- It("returns first occurrence of non-empty comment", func() {
- Expect(getComment(zwsp+zwsp+"first"+zwsp+"second", zwsp)).To(Equal("first"))
+ It("returns empty string if comments are different", func() {
+ Expect(getComment("first"+zwsp+"second", zwsp)).To(Equal(""))
+ })
+ It("returns comment if all comments are the same", func() {
+ Expect(getComment("first"+zwsp+"first", zwsp)).To(Equal("first"))
})
})
diff --git a/ui/src/album/AlbumShow.js b/ui/src/album/AlbumShow.js
index ee37d443..b27a5642 100644
--- a/ui/src/album/AlbumShow.js
+++ b/ui/src/album/AlbumShow.js
@@ -40,6 +40,7 @@ const AlbumShowLayout = (props) => {
}
diff --git a/ui/src/album/AlbumSongs.js b/ui/src/album/AlbumSongs.js
index 44f4df1a..b5bf6be3 100644
--- a/ui/src/album/AlbumSongs.js
+++ b/ui/src/album/AlbumSongs.js
@@ -157,7 +157,17 @@ const AlbumSongs = (props) => {
)
}
+export const removeAlbumCommentsFromSongs = ({ album, data }) => {
+ if (album?.comment && data) {
+ Object.values(data).forEach((song) => {
+ song.comment = ''
+ })
+ }
+}
+
const SanitizedAlbumSongs = (props) => {
+ removeAlbumCommentsFromSongs(props)
+
const { loaded, loading, total, ...rest } = useListContext(props)
return <>{loaded && }
}
diff --git a/ui/src/album/AlbumSongs.test.js b/ui/src/album/AlbumSongs.test.js
new file mode 100644
index 00000000..4007bf8f
--- /dev/null
+++ b/ui/src/album/AlbumSongs.test.js
@@ -0,0 +1,24 @@
+import { removeAlbumCommentsFromSongs } from './AlbumSongs'
+
+describe('removeAlbumCommentsFromSongs', () => {
+ const data = { 1: { comment: 'one' }, 2: { comment: 'two' } }
+ it('does not remove song comments if album does not have comment', () => {
+ const album = { comment: '' }
+ removeAlbumCommentsFromSongs({ album, data })
+ expect(data['1'].comment).toEqual('one')
+ expect(data['2'].comment).toEqual('two')
+ })
+
+ it('removes song comments if album has comment', () => {
+ const album = { comment: 'test' }
+ removeAlbumCommentsFromSongs({ album, data })
+ expect(data['1'].comment).toEqual('')
+ expect(data['2'].comment).toEqual('')
+ })
+
+ it('does not crash if album and data arr not available', () => {
+ expect(() => {
+ removeAlbumCommentsFromSongs({})
+ }).not.toThrow()
+ })
+})