From 4f7dc105b0414cd202fc7e560d51b8abc27ad7ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 6 Nov 2025 16:50:54 -0500 Subject: [PATCH] fix(ui): correct track ordering when sorting playlists by album (#4657) * fix(deps): update wazero dependencies to resolve issues Signed-off-by: Deluan * fix(deps): update wazero dependency to latest version Signed-off-by: Deluan * fix: correct track ordering when sorting playlists by album Fixed issue #3177 where tracks within multi-disc albums were displayed out of order when sorting playlists by album. The playlist track repository was using an incomplete sort mapping that only sorted by album name and artist, missing the critical disc_number and track_number fields. Changed the album sort mapping in playlist_track_repository from: order_album_name, order_album_artist_name to: order_album_name, order_album_artist_name, disc_number, track_number, order_artist_name, title This now matches the sorting used in the media file repository, ensuring tracks are sorted by: 1. Album name (groups by album) 2. Album artist (handles compilations) 3. Disc number (multi-disc album discs in order) 4. Track number (tracks within disc in order) 5. Artist name and title (edge cases with missing metadata) Added comprehensive tests with a multi-disc test album to verify correct sorting behavior. * chore: sync go.mod and go.sum with master * chore: align playlist album sort order with mediafile_repository (use album_id) * fix: clean up test playlist to prevent state leakage in randomized test runs --------- Signed-off-by: Deluan --- persistence/album_repository_test.go | 2 ++ persistence/mediafile_repository_test.go | 2 +- persistence/persistence_suite_test.go | 13 +++++++++- persistence/playlist_repository_test.go | 33 ++++++++++++++++++++++++ persistence/playlist_track_repository.go | 2 +- 5 files changed, 49 insertions(+), 3 deletions(-) diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 4be89bcb..a062b439 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -55,6 +55,7 @@ var _ = Describe("AlbumRepository", func() { It("returns all records sorted", func() { Expect(GetAll(model.QueryOptions{Sort: "name"})).To(Equal(model.Albums{ albumAbbeyRoad, + albumMultiDisc, albumRadioactivity, albumSgtPeppers, })) @@ -64,6 +65,7 @@ var _ = Describe("AlbumRepository", func() { Expect(GetAll(model.QueryOptions{Sort: "name", Order: "desc"})).To(Equal(model.Albums{ albumSgtPeppers, albumRadioactivity, + albumMultiDisc, albumAbbeyRoad, })) }) diff --git a/persistence/mediafile_repository_test.go b/persistence/mediafile_repository_test.go index 002b8249..ab926c00 100644 --- a/persistence/mediafile_repository_test.go +++ b/persistence/mediafile_repository_test.go @@ -38,7 +38,7 @@ var _ = Describe("MediaRepository", func() { }) It("counts the number of mediafiles in the DB", func() { - Expect(mr.CountAll()).To(Equal(int64(6))) + Expect(mr.CountAll()).To(Equal(int64(10))) }) It("returns songs ordered by lyrics with a specific title/artist", func() { diff --git a/persistence/persistence_suite_test.go b/persistence/persistence_suite_test.go index 1007d84f..f3cb4f3d 100644 --- a/persistence/persistence_suite_test.go +++ b/persistence/persistence_suite_test.go @@ -69,10 +69,12 @@ var ( albumSgtPeppers = al(model.Album{ID: "101", Name: "Sgt Peppers", AlbumArtist: "The Beatles", OrderAlbumName: "sgt peppers", AlbumArtistID: "3", EmbedArtPath: p("/beatles/1/sgt/a day.mp3"), SongCount: 1, MaxYear: 1967}) albumAbbeyRoad = al(model.Album{ID: "102", Name: "Abbey Road", AlbumArtist: "The Beatles", OrderAlbumName: "abbey road", AlbumArtistID: "3", EmbedArtPath: p("/beatles/1/come together.mp3"), SongCount: 1, MaxYear: 1969}) albumRadioactivity = al(model.Album{ID: "103", Name: "Radioactivity", AlbumArtist: "Kraftwerk", OrderAlbumName: "radioactivity", AlbumArtistID: "2", EmbedArtPath: p("/kraft/radio/radio.mp3"), SongCount: 2}) + albumMultiDisc = al(model.Album{ID: "104", Name: "Multi Disc Album", AlbumArtist: "Test Artist", OrderAlbumName: "multi disc album", AlbumArtistID: "1", EmbedArtPath: p("/test/multi/disc1/track1.mp3"), SongCount: 4}) testAlbums = model.Albums{ albumSgtPeppers, albumAbbeyRoad, albumRadioactivity, + albumMultiDisc, } ) @@ -94,13 +96,22 @@ var ( Lyrics: `[{"lang":"xxx","line":[{"value":"This is a set of lyrics"}],"synced":false}]`, }) songAntenna2 = mf(model.MediaFile{ID: "1006", Title: "Antenna", ArtistID: "2", Artist: "Kraftwerk", AlbumID: "103"}) - testSongs = model.MediaFiles{ + // Multi-disc album tracks (intentionally out of order to test sorting) + songDisc2Track11 = mf(model.MediaFile{ID: "2001", Title: "Disc 2 Track 11", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 2, TrackNumber: 11, Path: p("/test/multi/disc2/track11.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc1Track01 = mf(model.MediaFile{ID: "2002", Title: "Disc 1 Track 1", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 1, TrackNumber: 1, Path: p("/test/multi/disc1/track1.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc2Track01 = mf(model.MediaFile{ID: "2003", Title: "Disc 2 Track 1", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 2, TrackNumber: 1, Path: p("/test/multi/disc2/track1.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + songDisc1Track02 = mf(model.MediaFile{ID: "2004", Title: "Disc 1 Track 2", ArtistID: "1", Artist: "Test Artist", AlbumID: "104", Album: "Multi Disc Album", DiscNumber: 1, TrackNumber: 2, Path: p("/test/multi/disc1/track2.mp3"), OrderAlbumName: "multi disc album", OrderArtistName: "test artist"}) + testSongs = model.MediaFiles{ songDayInALife, songComeTogether, songRadioactivity, songAntenna, songAntennaWithLyrics, songAntenna2, + songDisc2Track11, + songDisc1Track01, + songDisc2Track01, + songDisc1Track02, } ) diff --git a/persistence/playlist_repository_test.go b/persistence/playlist_repository_test.go index 15ae438d..7fad93b1 100644 --- a/persistence/playlist_repository_test.go +++ b/persistence/playlist_repository_test.go @@ -219,4 +219,37 @@ var _ = Describe("PlaylistRepository", func() { }) }) }) + + Describe("Playlist Track Sorting", func() { + var testPlaylistID string + + AfterEach(func() { + if testPlaylistID != "" { + Expect(repo.Delete(testPlaylistID)).To(BeNil()) + testPlaylistID = "" + } + }) + + It("sorts tracks correctly by album (disc and track number)", func() { + By("creating a playlist with multi-disc album tracks in arbitrary order") + newPls := model.Playlist{Name: "Multi-Disc Test", OwnerID: "userid"} + // Add tracks in intentionally scrambled order + newPls.AddMediaFilesByID([]string{"2001", "2002", "2003", "2004"}) + Expect(repo.Put(&newPls)).To(Succeed()) + testPlaylistID = newPls.ID + + By("retrieving tracks sorted by album") + tracksRepo := repo.Tracks(newPls.ID, false) + tracks, err := tracksRepo.GetAll(model.QueryOptions{Sort: "album", Order: "asc"}) + Expect(err).ToNot(HaveOccurred()) + + By("verifying tracks are sorted by disc number then track number") + Expect(tracks).To(HaveLen(4)) + // Expected order: Disc 1 Track 1, Disc 1 Track 2, Disc 2 Track 1, Disc 2 Track 11 + Expect(tracks[0].MediaFileID).To(Equal("2002")) // Disc 1, Track 1 + Expect(tracks[1].MediaFileID).To(Equal("2004")) // Disc 1, Track 2 + Expect(tracks[2].MediaFileID).To(Equal("2003")) // Disc 2, Track 1 + Expect(tracks[3].MediaFileID).To(Equal("2001")) // Disc 2, Track 11 + }) + }) }) diff --git a/persistence/playlist_track_repository.go b/persistence/playlist_track_repository.go index 01eec0d0..b3f9e0c0 100644 --- a/persistence/playlist_track_repository.go +++ b/persistence/playlist_track_repository.go @@ -55,7 +55,7 @@ func (r *playlistRepository) Tracks(playlistId string, refreshSmartPlaylist bool "id": "playlist_tracks.id", "artist": "order_artist_name", "album_artist": "order_album_artist_name", - "album": "order_album_name, order_album_artist_name", + "album": "order_album_name, album_id, disc_number, track_number, order_artist_name, title", "title": "order_title", // To make sure these fields will be whitelisted "duration": "duration",