Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix listing album tracks without track number #247

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vithyze
Copy link
Contributor

@vithyze vithyze commented Mar 5, 2023

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
Copy link

codecov bot commented Mar 5, 2023

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%) ⬇️
supysonic/api/media.py 75.59% <0.00%> (-10.51%) ⬇️
supysonic/api/search.py 92.20% <0.00%> (-7.80%) ⬇️
supysonic/api/playlists.py 90.54% <0.00%> (-6.76%) ⬇️
supysonic/api/formatters.py 92.22% <0.00%> (-1.12%) ⬇️
supysonic/api/browse.py 93.40% <0.00%> (-1.10%) ⬇️

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

@spl0k
Copy link
Owner

spl0k commented Mar 5, 2023

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
Copy link
Contributor Author

vithyze commented Mar 5, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants