fix(ui): correct track ordering when sorting playlists by album (#4657)
* fix(deps): update wazero dependencies to resolve issues Signed-off-by: Deluan <deluan@navidrome.org> * fix(deps): update wazero dependency to latest version Signed-off-by: Deluan <deluan@navidrome.org> * 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 <deluan@navidrome.org>
This commit is contained in:
@@ -55,6 +55,7 @@ var _ = Describe("AlbumRepository", func() {
|
|||||||
It("returns all records sorted", func() {
|
It("returns all records sorted", func() {
|
||||||
Expect(GetAll(model.QueryOptions{Sort: "name"})).To(Equal(model.Albums{
|
Expect(GetAll(model.QueryOptions{Sort: "name"})).To(Equal(model.Albums{
|
||||||
albumAbbeyRoad,
|
albumAbbeyRoad,
|
||||||
|
albumMultiDisc,
|
||||||
albumRadioactivity,
|
albumRadioactivity,
|
||||||
albumSgtPeppers,
|
albumSgtPeppers,
|
||||||
}))
|
}))
|
||||||
@@ -64,6 +65,7 @@ var _ = Describe("AlbumRepository", func() {
|
|||||||
Expect(GetAll(model.QueryOptions{Sort: "name", Order: "desc"})).To(Equal(model.Albums{
|
Expect(GetAll(model.QueryOptions{Sort: "name", Order: "desc"})).To(Equal(model.Albums{
|
||||||
albumSgtPeppers,
|
albumSgtPeppers,
|
||||||
albumRadioactivity,
|
albumRadioactivity,
|
||||||
|
albumMultiDisc,
|
||||||
albumAbbeyRoad,
|
albumAbbeyRoad,
|
||||||
}))
|
}))
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -38,7 +38,7 @@ var _ = Describe("MediaRepository", func() {
|
|||||||
})
|
})
|
||||||
|
|
||||||
It("counts the number of mediafiles in the DB", 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() {
|
It("returns songs ordered by lyrics with a specific title/artist", func() {
|
||||||
|
|||||||
@@ -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})
|
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})
|
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})
|
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{
|
testAlbums = model.Albums{
|
||||||
albumSgtPeppers,
|
albumSgtPeppers,
|
||||||
albumAbbeyRoad,
|
albumAbbeyRoad,
|
||||||
albumRadioactivity,
|
albumRadioactivity,
|
||||||
|
albumMultiDisc,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -94,13 +96,22 @@ var (
|
|||||||
Lyrics: `[{"lang":"xxx","line":[{"value":"This is a set of lyrics"}],"synced":false}]`,
|
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"})
|
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,
|
songDayInALife,
|
||||||
songComeTogether,
|
songComeTogether,
|
||||||
songRadioactivity,
|
songRadioactivity,
|
||||||
songAntenna,
|
songAntenna,
|
||||||
songAntennaWithLyrics,
|
songAntennaWithLyrics,
|
||||||
songAntenna2,
|
songAntenna2,
|
||||||
|
songDisc2Track11,
|
||||||
|
songDisc1Track01,
|
||||||
|
songDisc2Track01,
|
||||||
|
songDisc1Track02,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -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
|
||||||
|
})
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -55,7 +55,7 @@ func (r *playlistRepository) Tracks(playlistId string, refreshSmartPlaylist bool
|
|||||||
"id": "playlist_tracks.id",
|
"id": "playlist_tracks.id",
|
||||||
"artist": "order_artist_name",
|
"artist": "order_artist_name",
|
||||||
"album_artist": "order_album_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",
|
"title": "order_title",
|
||||||
// To make sure these fields will be whitelisted
|
// To make sure these fields will be whitelisted
|
||||||
"duration": "duration",
|
"duration": "duration",
|
||||||
|
|||||||
Reference in New Issue
Block a user