spl0k / supysonic

Supysonic is a Python implementation of the Subsonic server API.
https://supysonic.readthedocs.io
GNU Affero General Public License v3.0
264 stars 58 forks source link

Fix listing album tracks without track number #247

Open vithyze opened 1 year ago

vithyze commented 1 year ago

This fixes an issue when listing "albums" that are just folders with tracks that don't have a disk or track number, so the listing becomes out of order.

codecov[bot] commented 1 year ago

Codecov Report

Merging #247 (57ee391) into master (8e2adf8) will decrease coverage by 1.75%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
- Coverage   86.41%   84.66%   -1.75%     
==========================================
  Files          46       46              
  Lines        3776     3776              
==========================================
- Hits         3263     3197      -66     
- Misses        513      579      +66     
Impacted Files Coverage Δ
supysonic/db.py 92.63% <100.00%> (ø)
supysonic/lastfm.py 34.84% <0.00%> (-33.34%) :arrow_down:
supysonic/api/media.py 75.59% <0.00%> (-10.51%) :arrow_down:
supysonic/api/search.py 92.20% <0.00%> (-7.80%) :arrow_down:
supysonic/api/playlists.py 90.54% <0.00%> (-6.76%) :arrow_down:
supysonic/api/formatters.py 92.22% <0.00%> (-1.12%) :arrow_down:
supysonic/api/browse.py 93.40% <0.00%> (-1.10%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

spl0k commented 1 year ago

Hello.

Tracks that don't have a track or disc number in their metadata just use 1 for these fields instead. Your PR doesn't touch this behaviour and even if it did I fail to see how this fixes anything. You're basically trying to remove 01s from a sort key. This might even cause other sorting issues for poorly tagged files if you mix in the same folder some with a track number but no disc number with files without track number but a disc number. Actually the sorting doesn't even work because you have an invalid syntax in Track.sort_key(). One way you can have an out of order listing is if you mix in the same folder tracks for which their album and/or artist metadata don't match, and your PR won't change anything about that.

The Subsonic API schema requires the track and discNumber fields to be an integer type. If a track doesn't have a track or disc number you're passing an empty string. This is why the tests are failing.

If you ever need to modify the database schema, please provide migrations for each supported engine. You only changed SQL files that are only executed when the application is run for the first time. Existing installations need migrations to bring their schema up to date without having to delete their whole database.

vithyze commented 1 year ago

Hi, I made these changes some time ago so I don't really remember but I have some folders with tracks named just Artist - Title with the same genre so DSub displays the folder as an album, and I remember it made the listing inverted (as z-a) and the reason I found was the track number, making it null made the trick and I didn't have problems with normal albums. But I might do a proper testing later.