New "index" field on playlist tracks, and .insert() metod to manage playlists

api/funkwhale_api/playlists/admin.py

@@ -12,6 +12,6 @@ class PlaylistAdmin(admin.ModelAdmin):
 
 @admin.register(models.PlaylistTrack)
 class PlaylistTrackAdmin(admin.ModelAdmin):
-    list_display = ['playlist', 'track', 'position', ]
+    list_display = ['playlist', 'track', 'index']
     search_fields = ['track__name', 'playlist__name']
     list_select_related = True

api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py

+# Generated by Django 2.0.3 on 2018-03-19 12:14
+
+from django.db import migrations, models
+import django.utils.timezone
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('playlists', '0002_auto_20180316_2217'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='playlisttrack',
+            options={'ordering': ('-playlist', 'index')},
+        ),
+        migrations.AddField(
+            model_name='playlisttrack',
+            name='creation_date',
+            field=models.DateTimeField(default=django.utils.timezone.now),
+        ),
+        migrations.AddField(
+            model_name='playlisttrack',
+            name='index',
+            field=models.PositiveIntegerField(null=True),
+        ),
+        migrations.RemoveField(
+            model_name='playlisttrack',
+            name='lft',
+        ),
+        migrations.RemoveField(
+            model_name='playlisttrack',
+            name='position',
+        ),
+        migrations.RemoveField(
+            model_name='playlisttrack',
+            name='previous',
+        ),
+        migrations.RemoveField(
+            model_name='playlisttrack',
+            name='rght',
+        ),
+        migrations.RemoveField(
+            model_name='playlisttrack',
+            name='tree_id',
+        ),
+        migrations.AlterUniqueTogether(
+            name='playlisttrack',
+            unique_together={('playlist', 'index')},
+        ),
+    ]

api/funkwhale_api/playlists/models.py

+from django import forms
 from django.db import models
+from django.db import transaction
 from django.utils import timezone
 
 from funkwhale_api.common import fields
@@ -14,14 +16,59 @@ class Playlist(models.Model):
     def __str__(self):
         return self.name
 
-    def add_track(self, track, previous=None):
-        plt = PlaylistTrack(previous=previous, track=track, playlist=self)
-        plt.save()
+    @transaction.atomic
+    def insert(self, plt, index=None):
+        """
+        Given a PlaylistTrack, insert it at the correct index in the playlist,
+        and update other tracks index if necessary.
+        """
+        old_index = plt.index
+        move = old_index is not None
+        if index is not None and index == old_index:
+            # moving at same position, just skip
+            return index
 
-        return plt
+        existing = self.playlist_tracks.select_for_update()
+        if move:
+            existing = existing.exclude(pk=plt.pk)
+        total = existing.filter(index__isnull=False).count()
 
+        if index is None:
+            # we simply increment the last track index by 1
+            index = total
 
-class PlaylistTrack(MPTTModel):
+        if index > total:
+            raise forms.ValidationError('Index is not continuous')
+
+        if index < 0:
+            raise forms.ValidationError('Index must be zero or positive')
+
+        if move:
+            # we remove the index temporarily, to avoid integrity errors
+            plt.index = None
+            plt.save(update_fields=['index'])
+
+        if move:
+            if index > old_index:
+                # new index is higher than current, we decrement previous tracks
+                to_update = existing.filter(
+                    index__gt=old_index, index__lte=index)
+                to_update.update(index=models.F('index') - 1)
+            if index < old_index:
+                # new index is lower than current, we increment next tracks
+                to_update = existing.filter(
+                    index__lt=old_index, index__gte=index)
+                to_update.update(index=models.F('index') + 1)
+        else:
+            to_update = existing.filter(index__gte=index)
+            to_update.update(index=models.F('index') + 1)
+
+        plt.index = index
+        plt.save(update_fields=['index'])
+        return index
+
+
+class PlaylistTrack(models.Model):
     track = models.ForeignKey(
         'music.Track',
         related_name='playlist_tracks',
@@ -29,6 +76,7 @@ class PlaylistTrack(MPTTModel):
     index = models.PositiveIntegerField(null=True)
     playlist = models.ForeignKey(
         Playlist, related_name='playlist_tracks', on_delete=models.CASCADE)
+    creation_date = models.DateTimeField(default=timezone.now)
 
     class Meta:
         ordering = ('-playlist', 'index')

api/funkwhale_api/playlists/serializers.py

@@ -19,7 +19,7 @@ class PlaylistTrackCreateSerializer(serializers.ModelSerializer):
 
     class Meta:
         model = models.PlaylistTrack
-        fields = ('id', 'track', 'playlist', 'position')
+        fields = ('id', 'track', 'playlist', 'index')
 
     def validate_playlist(self, value):
         existing = value.playlist_tracks.count()

api/tests/playlists/test_models.py

+import pytest
 
+from django import forms
 
-def test_can_create_playlist(factories):
-    tracks = factories['music.Track'].create_batch(5)
+
+def test_can_insert_plt(factories):
+    plt = factories['playlists.PlaylistTrack']()
+
+    assert plt.index is None
+
+    plt.playlist.insert(plt)
+    plt.refresh_from_db()
+
+    assert plt.index == 0
+
+
+def test_insert_use_last_idx_by_default(factories):
+    playlist = factories['playlists.Playlist']()
+    plts = factories['playlists.PlaylistTrack'].create_batch(
+        size=3, playlist=playlist)
+
+    for i, plt in enumerate(plts):
+        index = playlist.insert(plt)
+        plt.refresh_from_db()
+
+        assert index == i
+        assert plt.index == i
+
+def test_can_insert_at_index(factories):
+    playlist = factories['playlists.Playlist']()
+    first = factories['playlists.PlaylistTrack'](playlist=playlist)
+    playlist.insert(first)
+    new_first = factories['playlists.PlaylistTrack'](playlist=playlist)
+    index = playlist.insert(new_first, index=0)
+    first.refresh_from_db()
+    new_first.refresh_from_db()
+
+    assert index == 0
+    assert first.index == 1
+    assert new_first.index == 0
+
+
+def test_can_insert_and_move(factories):
     playlist = factories['playlists.Playlist']()
+    first = factories['playlists.PlaylistTrack'](playlist=playlist)
+    second = factories['playlists.PlaylistTrack'](playlist=playlist)
+    third = factories['playlists.PlaylistTrack'](playlist=playlist)
+    playlist.insert(first)
+    playlist.insert(second)
+    playlist.insert(third)
+
+    playlist.insert(second, index=0)
+
+    first.refresh_from_db()
+    second.refresh_from_db()
+    third.refresh_from_db()
+
+    assert third.index == 2
+    assert second.index == 0
+    assert first.index == 1
+
 
-    previous = None
-    for track in tracks:
-        previous = playlist.add_track(track, previous=previous)
+def test_cannot_insert_at_wrong_index(factories):
+    plt = factories['playlists.PlaylistTrack']()
+    new = factories['playlists.PlaylistTrack'](playlist=plt.playlist)
+    with pytest.raises(forms.ValidationError):
+        plt.playlist.insert(new, 2)
 
-    playlist_tracks = list(playlist.playlist_tracks.all())
 
-    previous = None
-    for idx, track in enumerate(tracks):
-        plt = playlist_tracks[idx]
-        assert plt.position == idx
-        assert plt.track == track
-        if previous:
-            assert playlist_tracks[idx + 1] == previous
-        assert plt.playlist == playlist
+def test_cannot_insert_at_negative_index(factories):
+    plt = factories['playlists.PlaylistTrack']()
+    new = factories['playlists.PlaylistTrack'](playlist=plt.playlist)
+    with pytest.raises(forms.ValidationError):
+        plt.playlist.insert(new, -1)