diff --git a/api/funkwhale_api/playlists/admin.py b/api/funkwhale_api/playlists/admin.py index 1d58abf9bfb8327f60d81cface2063e7f40d7dfe..68e447f3842d61ea392b262db83a70e87d09cd89 100644 --- a/api/funkwhale_api/playlists/admin.py +++ b/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 diff --git a/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py b/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py new file mode 100644 index 0000000000000000000000000000000000000000..0284f8f2cf88f1b04e5a05334b73789fb9233e8c --- /dev/null +++ b/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py @@ -0,0 +1,52 @@ +# 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')}, + ), + ] diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index 32e9a13b23749410d698cd38dee5af03558d5c91..b400934722b282851e4a2056d074a87174b84d2c 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -1,4 +1,6 @@ +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') diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index 77470c4dea0b68528429516971ad53d451b723ff..55603ee31863987436bfa4ca8daef9f1ae090b7e 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/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() diff --git a/api/tests/playlists/test_models.py b/api/tests/playlists/test_models.py index 9ec2a4af88125739fc646261b8a4a342f1e43ef5..ab60cdb849082a005a63b677d5d13d9b48f4e267 100644 --- a/api/tests/playlists/test_models.py +++ b/api/tests/playlists/test_models.py @@ -1,20 +1,74 @@ +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)