MirrorMirror
/
funkwhale
mirror of https://dev.funkwhale.audio/funkwhale/funkwhale.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'playlist-tracks' into 'develop' 

Updated playlist management API

See merge request funkwhale/funkwhale!1180
This commit is contained in:
Agate 2020-07-27 15:31:50 +02:00
parent a1865cf9d8 f6458fd75a
commit c42257b938
10 changed files with 157 additions and 247 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
api/config/api_urls.py
View File

@ -26,9 +26,6 @@ router.register(r"subscriptions", audio_views.SubscriptionsViewSet, "subscriptio
router.register(r"albums", views.AlbumViewSet, "albums")
router.register(r"licenses", views.LicenseViewSet, "licenses")
router.register(r"playlists", playlists_views.PlaylistViewSet, "playlists")
router.register(
r"playlist-tracks", playlists_views.PlaylistTrackViewSet, "playlist-tracks"
)
router.register(r"mutations", common_views.MutationViewSet, "mutations")
router.register(r"attachments", common_views.AttachmentViewSet, "attachments")
v1_patterns = router.urls

9
api/funkwhale_api/playlists/models.py
View File

@ -203,6 +203,15 @@ class PlaylistTrackQuerySet(models.QuerySet):
else:
return self.exclude(track__pk__in=tracks).distinct()
def by_index(self, index):
plts = self.order_by("index").values_list("id", flat=True)
try:
plt_id = plts[index]
except IndexError:
raise PlaylistTrack.DoesNotExist
return PlaylistTrack.objects.get(pk=plt_id)
class PlaylistTrack(models.Model):
track = models.ForeignKey(

55
api/funkwhale_api/playlists/serializers.py
View File

@ -1,7 +1,5 @@
from django.db import transaction
from rest_framework import serializers
from funkwhale_api.common import preferences
from funkwhale_api.federation import serializers as federation_serializers
from funkwhale_api.music.models import Track
from funkwhale_api.music.serializers import TrackSerializer
@ -16,64 +14,13 @@ class PlaylistTrackSerializer(serializers.ModelSerializer):
class Meta:
model = models.PlaylistTrack
fields = ("id", "track", "playlist", "index", "creation_date")
fields = ("track", "index", "creation_date")
def get_track(self, o):
track = o._prefetched_track if hasattr(o, "_prefetched_track") else o.track
return TrackSerializer(track).data
class PlaylistTrackWriteSerializer(serializers.ModelSerializer):
index = serializers.IntegerField(required=False, min_value=0, allow_null=True)
allow_duplicates = serializers.BooleanField(required=False)
class Meta:
model = models.PlaylistTrack
fields = ("id", "track", "playlist", "index", "allow_duplicates")
def validate_playlist(self, value):
if self.context.get("request"):
# validate proper ownership on the playlist
if self.context["request"].user != value.user:
raise serializers.ValidationError(
"You do not have the permission to edit this playlist"
)
existing = value.playlist_tracks.count()
max_tracks = preferences.get("playlists__max_tracks")
if existing >= max_tracks:
raise serializers.ValidationError(
"Playlist has reached the maximum of {} tracks".format(max_tracks)
)
return value
@transaction.atomic
def create(self, validated_data):
index = validated_data.pop("index", None)
allow_duplicates = validated_data.pop("allow_duplicates", True)
instance = super().create(validated_data)
instance.playlist.insert(instance, index, allow_duplicates)
return instance
@transaction.atomic
def update(self, instance, validated_data):
update_index = "index" in validated_data
index = validated_data.pop("index", None)
allow_duplicates = validated_data.pop("allow_duplicates", True)
super().update(instance, validated_data)
if update_index:
instance.playlist.insert(instance, index, allow_duplicates)
return instance
def get_unique_together_validators(self):
"""
We explicitely disable unique together validation here
because it collides with our internal logic
"""
return []
class PlaylistSerializer(serializers.ModelSerializer):
tracks_count = serializers.SerializerMethodField(read_only=True)
duration = serializers.SerializerMethodField(read_only=True)

67
api/funkwhale_api/playlists/views.py
View File

@ -93,40 +93,43 @@ class PlaylistViewSet(
),
)
@action(methods=["post", "delete"], detail=True)
@transaction.atomic
def remove(self, request, *args, **kwargs):
playlist = self.get_object()
try:
index = int(request.data["index"])
assert index >= 0
except (KeyError, ValueError, AssertionError, TypeError):
return Response(status=400)
class PlaylistTrackViewSet(
mixins.RetrieveModelMixin,
mixins.CreateModelMixin,
mixins.UpdateModelMixin,
mixins.DestroyModelMixin,
mixins.ListModelMixin,
viewsets.GenericViewSet,
):
try:
plt = playlist.playlist_tracks.by_index(index)
except models.PlaylistTrack.DoesNotExist:
return Response(status=404)
plt.delete(update_indexes=True)
serializer_class = serializers.PlaylistTrackSerializer
queryset = models.PlaylistTrack.objects.all()
permission_classes = [
oauth_permissions.ScopePermission,
permissions.OwnerPermission,
]
required_scope = "playlists"
anonymous_policy = "setting"
owner_field = "playlist.user"
owner_checks = ["write"]
return Response(status=204)
def get_serializer_class(self):
if self.request.method in ["PUT", "PATCH", "DELETE", "POST"]:
return serializers.PlaylistTrackWriteSerializer
return self.serializer_class
@action(methods=["post"], detail=True)
@transaction.atomic
def move(self, request, *args, **kwargs):
playlist = self.get_object()
try:
from_index = int(request.data["from"])
assert from_index >= 0
except (KeyError, ValueError, AssertionError, TypeError):
return Response({"detail": "invalid from index"}, status=400)
def get_queryset(self):
return self.queryset.filter(
fields.privacy_level_query(
self.request.user,
lookup_field="playlist__privacy_level",
user_field="playlist__user",
)
).for_nested_serialization(music_utils.get_actor_from_request(self.request))
try:
to_index = int(request.data["to"])
assert to_index >= 0
except (KeyError, ValueError, AssertionError, TypeError):
return Response({"detail": "invalid to index"}, status=400)
def perform_destroy(self, instance):
instance.delete(update_indexes=True)
try:
plt = playlist.playlist_tracks.by_index(from_index)
except models.PlaylistTrack.DoesNotExist:
return Response(status=404)
playlist.insert(plt, to_index)
return Response(status=204)

90
api/tests/playlists/test_serializers.py
View File

@ -1,96 +1,8 @@
from funkwhale_api.federation import serializers as federation_serializers
from funkwhale_api.playlists import models, serializers
from funkwhale_api.playlists import serializers
from funkwhale_api.users import serializers as users_serializers
def test_cannot_max_500_tracks_per_playlist(factories, preferences):
preferences["playlists__max_tracks"] = 2
playlist = factories["playlists.Playlist"]()
factories["playlists.PlaylistTrack"].create_batch(size=2, playlist=playlist)
track = factories["music.Track"]()
serializer = serializers.PlaylistTrackWriteSerializer(
data={"playlist": playlist.pk, "track": track.pk}
)
assert serializer.is_valid() is False
assert "playlist" in serializer.errors
def test_create_insert_is_called_when_index_is_None(factories, mocker):
insert = mocker.spy(models.Playlist, "insert")
playlist = factories["playlists.Playlist"]()
track = factories["music.Track"]()
serializer = serializers.PlaylistTrackWriteSerializer(
data={"playlist": playlist.pk, "track": track.pk, "index": None}
)
assert serializer.is_valid() is True
plt = serializer.save()
insert.assert_called_once_with(playlist, plt, None, True)
assert plt.index == 0
def test_create_insert_is_called_when_index_is_provided(factories, mocker):
playlist = factories["playlists.Playlist"]()
first = factories["playlists.PlaylistTrack"](playlist=playlist, index=0)
insert = mocker.spy(models.Playlist, "insert")
factories["playlists.Playlist"]()
track = factories["music.Track"]()
serializer = serializers.PlaylistTrackWriteSerializer(
data={"playlist": playlist.pk, "track": track.pk, "index": 0}
)
assert serializer.is_valid() is True
plt = serializer.save()
first.refresh_from_db()
insert.assert_called_once_with(playlist, plt, 0, True)
assert plt.index == 0
assert first.index == 1
def test_update_insert_is_called_when_index_is_provided(factories, mocker):
playlist = factories["playlists.Playlist"]()
first = factories["playlists.PlaylistTrack"](playlist=playlist, index=0)
second = factories["playlists.PlaylistTrack"](playlist=playlist, index=1)
insert = mocker.spy(models.Playlist, "insert")
factories["playlists.Playlist"]()
factories["music.Track"]()
serializer = serializers.PlaylistTrackWriteSerializer(
second, data={"playlist": playlist.pk, "track": second.track.pk, "index": 0}
)
assert serializer.is_valid() is True
plt = serializer.save()
first.refresh_from_db()
insert.assert_called_once_with(playlist, plt, 0, True)
assert plt.index == 0
assert first.index == 1
def test_update_insert_is_called_with_duplicate_override_when_duplicates_allowed(
factories, mocker
):
playlist = factories["playlists.Playlist"]()
plt = factories["playlists.PlaylistTrack"](playlist=playlist, index=0)
insert = mocker.spy(models.Playlist, "insert")
factories["playlists.Playlist"]()
factories["music.Track"]()
serializer = serializers.PlaylistTrackWriteSerializer(
plt,
data={
"playlist": playlist.pk,
"track": plt.track.pk,
"index": 0,
"allow_duplicates": True,
},
)
assert serializer.is_valid() is True
plt = serializer.save()
insert.assert_called_once_with(playlist, plt, 0, True)
def test_playlist_serializer_include_covers(factories, api_request):
playlist = factories["playlists.Playlist"]()
t1 = factories["music.Track"](album__with_cover=True)

105
api/tests/playlists/test_views.py
View File

@ -1,7 +1,7 @@
import pytest
from django.urls import reverse
from funkwhale_api.playlists import models, serializers
from funkwhale_api.playlists import models
def test_can_create_playlist_via_api(logged_in_api_client):
@ -60,21 +60,8 @@ def test_playlist_inherits_user_privacy(logged_in_api_client):
assert playlist.privacy_level == user.privacy_level
def test_can_add_playlist_track_via_api(factories, logged_in_api_client):
tracks = factories["music.Track"].create_batch(5)
playlist = factories["playlists.Playlist"](user=logged_in_api_client.user)
url = reverse("api:v1:playlist-tracks-list")
data = {"playlist": playlist.pk, "track": tracks[0].pk}
response = logged_in_api_client.post(url, data)
assert response.status_code == 201
plts = logged_in_api_client.user.playlists.latest("id").playlist_tracks.all()
assert plts.first().track == tracks[0]
@pytest.mark.parametrize(
"name,method",
[("api:v1:playlist-tracks-list", "post"), ("api:v1:playlists-list", "post")],
"name,method", [("api:v1:playlists-list", "post")],
)
def test_url_requires_login(name, method, factories, api_client):
url = reverse(name)
@ -87,26 +74,30 @@ def test_url_requires_login(name, method, factories, api_client):
def test_only_can_add_track_on_own_playlist_via_api(factories, logged_in_api_client):
track = factories["music.Track"]()
playlist = factories["playlists.Playlist"]()
url = reverse("api:v1:playlist-tracks-list")
data = {"playlist": playlist.pk, "track": track.pk}
url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk})
data = {"tracks": [track.pk]}
response = logged_in_api_client.post(url, data)
assert response.status_code == 400
response = logged_in_api_client.post(url, data, format="json")
assert response.status_code == 404
assert playlist.playlist_tracks.count() == 0
def test_deleting_plt_updates_indexes(mocker, factories, logged_in_api_client):
remove = mocker.spy(models.Playlist, "remove")
factories["music.Track"]()
plt = factories["playlists.PlaylistTrack"](
index=0, playlist__user=logged_in_api_client.user
)
url = reverse("api:v1:playlist-tracks-detail", kwargs={"pk": plt.pk})
playlist = factories["playlists.Playlist"](user=logged_in_api_client.user)
plt0 = factories["playlists.PlaylistTrack"](index=0, playlist=playlist)
plt1 = factories["playlists.PlaylistTrack"](index=1, playlist=playlist)
url = reverse("api:v1:playlists-remove", kwargs={"pk": playlist.pk})
response = logged_in_api_client.delete(url)
response = logged_in_api_client.delete(url, {"index": 0})
assert response.status_code == 204
remove.assert_called_once_with(plt.playlist, 0)
remove.assert_called_once_with(plt0.playlist, 0)
with pytest.raises(plt0.DoesNotExist):
plt0.refresh_from_db()
plt1.refresh_from_db()
assert plt1.index == 0
@pytest.mark.parametrize("level", ["instance", "me", "followers"])
@ -130,38 +121,6 @@ def test_only_owner_can_edit_playlist(method, factories, logged_in_api_client):
assert response.status_code == 404
@pytest.mark.parametrize("method", ["PUT", "PATCH", "DELETE"])
def test_only_owner_can_edit_playlist_track(method, factories, logged_in_api_client):
plt = factories["playlists.PlaylistTrack"]()
url = reverse("api:v1:playlist-tracks-detail", kwargs={"pk": plt.pk})
response = getattr(logged_in_api_client, method.lower())(url)
assert response.status_code == 404
@pytest.mark.parametrize("level", ["instance", "me", "followers"])
def test_playlist_track_privacy_respected_in_list_anon(
level, factories, api_client, preferences
):
preferences["common__api_authentication_required"] = False
factories["playlists.PlaylistTrack"](playlist__privacy_level=level)
url = reverse("api:v1:playlist-tracks-list")
response = api_client.get(url)
assert response.data["count"] == 0
@pytest.mark.parametrize("level", ["instance", "me", "followers"])
def test_can_list_tracks_from_playlist(level, factories, logged_in_api_client):
plt = factories["playlists.PlaylistTrack"](playlist__user=logged_in_api_client.user)
url = reverse("api:v1:playlists-tracks", kwargs={"pk": plt.playlist.pk})
response = logged_in_api_client.get(url)
serialized_plt = serializers.PlaylistTrackSerializer(plt).data
assert response.data["count"] == 1
assert response.data["results"][0] == serialized_plt
def test_can_add_multiple_tracks_at_once_via_api(
factories, mocker, logged_in_api_client
):
@ -176,10 +135,24 @@ def test_can_add_multiple_tracks_at_once_via_api(
assert playlist.playlist_tracks.count() == len(track_ids)
for plt in playlist.playlist_tracks.order_by("index"):
assert response.data["results"][plt.index]["id"] == plt.id
assert response.data["results"][plt.index]["index"] == plt.index
assert plt.track == tracks[plt.index]
def test_honor_max_playlist_size(factories, mocker, logged_in_api_client, preferences):
preferences["playlists__max_tracks"] = 3
playlist = factories["playlists.Playlist"](user=logged_in_api_client.user)
tracks = factories["music.Track"].create_batch(
size=preferences["playlists__max_tracks"] + 1
)
track_ids = [t.id for t in tracks]
mocker.spy(playlist, "insert_many")
url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk})
response = logged_in_api_client.post(url, {"tracks": track_ids})
assert response.status_code == 400
def test_can_clear_playlist_from_api(factories, mocker, logged_in_api_client):
playlist = factories["playlists.Playlist"](user=logged_in_api_client.user)
factories["playlists.PlaylistTrack"].create_batch(size=5, playlist=playlist)
@ -199,3 +172,19 @@ def test_update_playlist_from_api(factories, mocker, logged_in_api_client):
assert response.status_code == 200
assert response.data["user"]["username"] == playlist.user.username
def test_move_plt_updates_indexes(mocker, factories, logged_in_api_client):
playlist = factories["playlists.Playlist"](user=logged_in_api_client.user)
plt0 = factories["playlists.PlaylistTrack"](index=0, playlist=playlist)
plt1 = factories["playlists.PlaylistTrack"](index=1, playlist=playlist)
url = reverse("api:v1:playlists-move", kwargs={"pk": playlist.pk})
response = logged_in_api_client.post(url, {"from": 1, "to": 0})
assert response.status_code == 204
plt0.refresh_from_db()
plt1.refresh_from_db()
assert plt0.index == 1
assert plt1.index == 0

1
api/tests/users/oauth/test_api_permissions.py
View File

@ -22,7 +22,6 @@ from funkwhale_api.users.oauth import scopes
("api:v1:listen-detail", {"uuid": uuid.uuid4()}, "read:libraries", "get"),
("api:v1:uploads-list", {}, "read:libraries", "get"),
("api:v1:playlists-list", {}, "read:playlists", "get"),
("api:v1:playlist-tracks-list", {}, "read:playlists", "get"),
("api:v1:favorites:tracks-list", {}, "read:favorites", "get"),
("api:v1:history:listenings-list", {}, "read:listenings", "get"),
("api:v1:radios:radios-list", {}, "read:radios", "get"),

58
docs/swagger.yml
View File

@ -1,5 +1,4 @@
# Undocumented endpoints:
# /api/v1/playlist-tracks
# /api/v1/radios
# /api/v1/history
@ -1626,6 +1625,63 @@ paths:
type: "array"
items:
$ref: "./api/definitions.yml#/PlaylistTrack"
/api/v1/playlists/{id}/move:
parameters:
- name: id
in: path
required: true
schema:
type: "integer"
format: "int64"
post:
tags:
- "Content curation"
summary: Move a track to another index within its playlist
requestBody:
required: true
content:
application/json:
schema:
type: object
properties:
from:
type: "integer"
format: "int64"
description: Current index of the track
to:
type: "integer"
format: "int64"
description: New index of the track
responses:
204:
/api/v1/playlists/{id}/remove:
parameters:
- name: id
in: path
required: true
schema:
type: "integer"
format: "int64"
post:
tags:
- "Content curation"
summary: Remove a track from its playlist
requestBody:
required: true
content:
application/json:
schema:
type: object
properties:
index:
type: "integer"
format: "int64"
description: Index of the track to remove
responses:
204:
/api/v1/playlists/{id}/clear:
parameters:
- name: id

11
front/src/components/playlists/Editor.vue
View File

@ -61,7 +61,7 @@
<div class="table-wrapper">
<table class="ui compact very basic unstackable table">
<draggable v-model="plts" tag="tbody" @update="reorder">
<tr v-for="(plt, index) in plts" :key="plt.id">
<tr v-for="(plt, index) in plts" :key="`${index}-${plt.track.id}`">
<td class="left aligned">{{ plt.index + 1}}</td>
<td class="center aligned">
<img class="ui mini image" v-if="plt.track.album && plt.track.album.cover.original" v-lazy="$store.getters['instance/absoluteUrl'](plt.track.album.cover.small_square_crop)">
@ -125,20 +125,19 @@ export default {
let self = this
self.isLoading = true
let plt = this.plts[newIndex]
let url = 'playlist-tracks/' + plt.id + '/'
axios.patch(url, {index: newIndex}).then((response) => {
let url = `playlists/${this.playlist.id}/move`
axios.post(url, {from: oldIndex, to: newIndex}).then((response) => {
self.success()
}, error => {
self.errored(error.backendErrors)
})
},
removePlt (index) {
let plt = this.plts[index]
this.plts.splice(index, 1)
let self = this
self.isLoading = true
let url = 'playlist-tracks/' + plt.id + '/'
axios.delete(url).then((response) => {
let url = `playlists/${this.playlist.id}/remove`
axios.post(url, {index}).then((response) => {
self.success()
self.$store.dispatch('playlists/fetchOwn')
}, error => {

5
front/src/components/playlists/PlaylistModal.vue
View File

@ -139,14 +139,13 @@ export default {
addToPlaylist (playlistId, allowDuplicate) {
let self = this
let payload = {
track: this.track.id,
playlist: playlistId,
tracks: [this.track.id],
allow_duplicates: allowDuplicate
}
self.lastSelectedPlaylist = playlistId
return axios.post('playlist-tracks/', payload).then(response => {
return axios.post(`playlists/${playlistId}/add`, payload).then(response => {
logger.default.info('Successfully added track to playlist')
self.update(false)
self.$store.dispatch('playlists/fetchOwn')