Resolve "Track position don't take care about disc number"

api/funkwhale_api/federation/serializers.py

@@ -729,6 +729,7 @@ class AlbumSerializer(MusicEntitySerializer):
 
 class TrackSerializer(MusicEntitySerializer):
     position = serializers.IntegerField(min_value=0, allow_null=True, required=False)
+    disc = serializers.IntegerField(min_value=1, allow_null=True, required=False)
     artists = serializers.ListField(child=ArtistSerializer(), min_length=1)
     album = AlbumSerializer()
     license = serializers.URLField(allow_null=True, required=False)
@@ -742,6 +743,7 @@ class TrackSerializer(MusicEntitySerializer):
             "published": instance.creation_date.isoformat(),
             "musicbrainzId": str(instance.mbid) if instance.mbid else None,
             "position": instance.position,
+            "disc": instance.disc_number,
             "license": instance.local_license["identifiers"][0]
             if instance.local_license
             else None,

api/funkwhale_api/music/metadata.py

@@ -92,7 +92,7 @@ def get_mp3_recording_id(f, k):
         raise TagNotFound(k)
 
 
-def convert_track_number(v):
+def convert_position(v):
     try:
         return int(v)
     except ValueError:
@@ -156,8 +156,9 @@ CONF = {
         "fields": {
             "track_number": {
                 "field": "TRACKNUMBER",
-                "to_application": convert_track_number,
+                "to_application": convert_position,
             },
+            "disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
             "title": {},
             "artist": {},
             "album_artist": {
@@ -179,8 +180,9 @@ CONF = {
         "fields": {
             "track_number": {
                 "field": "TRACKNUMBER",
-                "to_application": convert_track_number,
+                "to_application": convert_position,
             },
+            "disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
             "title": {},
             "artist": {},
             "album_artist": {
@@ -202,8 +204,9 @@ CONF = {
         "fields": {
             "track_number": {
                 "field": "TRACKNUMBER",
-                "to_application": convert_track_number,
+                "to_application": convert_position,
             },
+            "disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
             "title": {},
             "artist": {},
             "album_artist": {"field": "albumartist"},
@@ -222,7 +225,8 @@ CONF = {
         "getter": get_id3_tag,
         "clean_pictures": clean_id3_pictures,
         "fields": {
-            "track_number": {"field": "TRCK", "to_application": convert_track_number},
+            "track_number": {"field": "TRCK", "to_application": convert_position},
+            "disc_number": {"field": "TPOS", "to_application": convert_position},
             "title": {"field": "TIT2"},
             "artist": {"field": "TPE1"},
             "album_artist": {"field": "TPE2"},
@@ -246,8 +250,9 @@ CONF = {
         "fields": {
             "track_number": {
                 "field": "tracknumber",
-                "to_application": convert_track_number,
+                "to_application": convert_position,
             },
+            "disc_number": {"field": "discnumber", "to_application": convert_position},
             "title": {},
             "artist": {},
             "album_artist": {"field": "albumartist"},
@@ -267,6 +272,7 @@ CONF = {
 
 ALL_FIELDS = [
     "track_number",
+    "disc_number",
     "title",
     "artist",
     "album_artist",

api/funkwhale_api/music/migrations/0036_track_disc_number.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.0.9 on 2018-12-04 15:10
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('music', '0035_auto_20181203_1515'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='track',
+            name='disc_number',
+            field=models.PositiveIntegerField(blank=True, null=True),
+        ),
+    ]

api/funkwhale_api/music/models.py

@@ -440,6 +440,12 @@ class TrackQuerySet(models.QuerySet):
             models.Prefetch("uploads", queryset=uploads, to_attr="playable_uploads")
         )
 
+    def order_for_album(self):
+        """
+        Order by disc number then position
+        """
+        return self.order_by("disc_number", "position", "title")
+
 
 def get_artist(release_list):
     return Artist.get_or_create_from_api(
@@ -450,6 +456,7 @@ def get_artist(release_list):
 class Track(APIModelMixin):
     title = models.CharField(max_length=255)
     artist = models.ForeignKey(Artist, related_name="tracks", on_delete=models.CASCADE)
+    disc_number = models.PositiveIntegerField(null=True, blank=True)
     position = models.PositiveIntegerField(null=True, blank=True)
     album = models.ForeignKey(
         Album, related_name="tracks", null=True, blank=True, on_delete=models.CASCADE
@@ -485,7 +492,7 @@ class Track(APIModelMixin):
     tags = TaggableManager(blank=True)
 
     class Meta:
-        ordering = ["album", "position"]
+        ordering = ["album", "disc_number", "position"]
 
     def __str__(self):
         return self.title

api/funkwhale_api/music/serializers.py

@@ -88,6 +88,7 @@ class AlbumTrackSerializer(serializers.ModelSerializer):
             "artist",
             "creation_date",
             "position",
+            "disc_number",
             "uploads",
             "listen_url",
             "duration",
@@ -130,10 +131,7 @@ class AlbumSerializer(serializers.ModelSerializer):
         )
 
     def get_tracks(self, o):
-        ordered_tracks = sorted(
-            o.tracks.all(),
-            key=lambda v: (v.position, v.title) if v.position else (99999, v.title),
-        )
+        ordered_tracks = o.tracks.all()
         return AlbumTrackSerializer(ordered_tracks, many=True).data
 
     def get_is_playable(self, obj):
@@ -193,6 +191,7 @@ class TrackSerializer(serializers.ModelSerializer):
             "artist",
             "creation_date",
             "position",
+            "disc_number",
             "lyrics",
             "uploads",
             "listen_url",

api/funkwhale_api/music/tasks.py

@@ -274,6 +274,7 @@ def federation_audio_track_to_metadata(payload):
         "title": payload["name"],
         "album": payload["album"]["name"],
         "track_number": payload["position"],
+        "disc_number": payload.get("disc"),
         "artist": payload["artists"][0]["name"],
         "album_artist": payload["album"]["artists"][0]["name"],
         "date": payload["album"].get("released"),
@@ -497,6 +498,7 @@ def get_track_from_import_metadata(data):
         "mbid": track_mbid,
         "artist": artist,
         "position": track_number,
+        "disc_number": data.get("disc_number"),
         "fid": track_fid,
         "from_activity_id": from_activity_id,
         "license": licenses.match(data.get("license"), data.get("copyright")),

api/funkwhale_api/music/views.py

@@ -93,8 +93,10 @@ class AlbumViewSet(viewsets.ReadOnlyModelViewSet):
 
     def get_queryset(self):
         queryset = super().get_queryset()
-        tracks = models.Track.objects.select_related("artist").with_playable_uploads(
-            utils.get_actor_from_request(self.request)
+        tracks = (
+            models.Track.objects.select_related("artist")
+            .with_playable_uploads(utils.get_actor_from_request(self.request))
+            .order_for_album()
         )
         qs = queryset.prefetch_related(Prefetch("tracks", queryset=tracks))
         return qs.distinct()