From 34373d020c14dedd194c349d09231af9b268c959 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 28 Sep 2018 16:45:28 +0200 Subject: [PATCH] Fixed unplayable playlists --- api/funkwhale_api/playlists/models.py | 34 +++++++++++++++++++++ api/funkwhale_api/playlists/serializers.py | 17 ++++++++++- api/funkwhale_api/playlists/views.py | 8 +++-- api/tests/playlists/test_models.py | 35 ++++++++++++++++++++++ api/tests/playlists/test_views.py | 10 +++++++ front/src/components/playlists/Card.vue | 4 +-- 6 files changed, 102 insertions(+), 6 deletions(-) diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index 3616c8821..510620334 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -38,6 +38,23 @@ class PlaylistQuerySet(models.QuerySet): ) return self.prefetch_related(plt_prefetch) + def annotate_playable_by_actor(self, actor): + plts = ( + PlaylistTrack.objects.playable_by(actor) + .filter(playlist=models.OuterRef("id")) + .order_by("id") + .values("id")[:1] + ) + subquery = models.Subquery(plts) + return self.annotate(is_playable_by_actor=subquery) + + def playable_by(self, actor, include=True): + plts = PlaylistTrack.objects.playable_by(actor, include) + if include: + return self.filter(playlist_tracks__in=plts) + else: + return self.exclude(playlist_tracks__in=plts) + class Playlist(models.Model): name = models.CharField(max_length=50) @@ -139,6 +156,23 @@ class PlaylistTrackQuerySet(models.QuerySet): ) ) + def annotate_playable_by_actor(self, actor): + tracks = ( + music_models.Track.objects.playable_by(actor) + .filter(pk=models.OuterRef("track")) + .order_by("id") + .values("id")[:1] + ) + subquery = models.Subquery(tracks) + return self.annotate(is_playable_by_actor=subquery) + + def playable_by(self, actor, include=True): + tracks = music_models.Track.objects.playable_by(actor, include) + if include: + return self.filter(track__pk__in=tracks) + else: + return self.exclude(track__pk__in=tracks) + class PlaylistTrack(models.Model): track = models.ForeignKey( diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index a60a34938..40a1c62b5 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -11,10 +11,17 @@ from . import models class PlaylistTrackSerializer(serializers.ModelSerializer): track = TrackSerializer() + is_playable = serializers.SerializerMethodField() class Meta: model = models.PlaylistTrack - fields = ("id", "track", "playlist", "index", "creation_date") + fields = ("id", "track", "playlist", "index", "creation_date", "is_playable") + + def get_is_playable(self, obj): + try: + return bool(obj.is_playable_by_actor) + except AttributeError: + return None class PlaylistTrackWriteSerializer(serializers.ModelSerializer): @@ -68,6 +75,7 @@ class PlaylistSerializer(serializers.ModelSerializer): duration = serializers.SerializerMethodField(read_only=True) album_covers = serializers.SerializerMethodField(read_only=True) user = UserBasicSerializer(read_only=True) + is_playable = serializers.SerializerMethodField() class Meta: model = models.Playlist @@ -81,9 +89,16 @@ class PlaylistSerializer(serializers.ModelSerializer): "tracks_count", "album_covers", "duration", + "is_playable", ) read_only_fields = ["id", "modification_date", "creation_date"] + def get_is_playable(self, obj): + try: + return bool(obj.is_playable_by_actor) + except AttributeError: + return None + def get_tracks_count(self, obj): try: return obj.tracks_count diff --git a/api/funkwhale_api/playlists/views.py b/api/funkwhale_api/playlists/views.py index 8db076a86..0c90335a9 100644 --- a/api/funkwhale_api/playlists/views.py +++ b/api/funkwhale_api/playlists/views.py @@ -6,7 +6,7 @@ from rest_framework.permissions import IsAuthenticatedOrReadOnly from rest_framework.response import Response from funkwhale_api.common import fields, permissions - +from funkwhale_api.music import utils as music_utils from . import filters, models, serializers @@ -74,7 +74,9 @@ class PlaylistViewSet( return Response(status=204) def get_queryset(self): - return self.queryset.filter(fields.privacy_level_query(self.request.user)) + return self.queryset.filter( + fields.privacy_level_query(self.request.user) + ).annotate_playable_by_actor(music_utils.get_actor_from_request(self.request)) def perform_create(self, serializer): return serializer.save( @@ -116,7 +118,7 @@ class PlaylistTrackViewSet( lookup_field="playlist__privacy_level", user_field="playlist__user", ) - ) + ).annotate_playable_by_actor(music_utils.get_actor_from_request(self.request)) def perform_destroy(self, instance): instance.delete(update_indexes=True) diff --git a/api/tests/playlists/test_models.py b/api/tests/playlists/test_models.py index 25c40d557..46c14d11c 100644 --- a/api/tests/playlists/test_models.py +++ b/api/tests/playlists/test_models.py @@ -122,3 +122,38 @@ def test_insert_many_honor_max_tracks(preferences, factories): track = factories["music.Track"]() with pytest.raises(exceptions.ValidationError): playlist.insert_many([track, track, track]) + + +@pytest.mark.parametrize( + "privacy_level,expected", [("me", False), ("instance", False), ("everyone", True)] +) +def test_playlist_track_playable_by_anonymous(privacy_level, expected, factories): + plt = factories["playlists.PlaylistTrack"]() + track = plt.track + factories["music.Upload"]( + track=track, library__privacy_level=privacy_level, import_status="finished" + ) + queryset = plt.__class__.objects.playable_by(None).annotate_playable_by_actor(None) + match = plt in list(queryset) + assert match is expected + if expected: + assert bool(queryset.first().is_playable_by_actor) is expected + + +@pytest.mark.parametrize( + "privacy_level,expected", [("me", False), ("instance", False), ("everyone", True)] +) +def test_playlist_playable_by_anonymous(privacy_level, expected, factories): + plt = factories["playlists.PlaylistTrack"]() + playlist = plt.playlist + track = plt.track + factories["music.Upload"]( + track=track, library__privacy_level=privacy_level, import_status="finished" + ) + queryset = playlist.__class__.objects.playable_by(None).annotate_playable_by_actor( + None + ) + match = playlist in list(queryset) + assert match is expected + if expected: + assert bool(queryset.first().is_playable_by_actor) is expected diff --git a/api/tests/playlists/test_views.py b/api/tests/playlists/test_views.py index e7b47c7a2..1c2b0f19e 100644 --- a/api/tests/playlists/test_views.py +++ b/api/tests/playlists/test_views.py @@ -25,6 +25,16 @@ def test_serializer_includes_tracks_count(factories, logged_in_api_client): assert response.data["tracks_count"] == 1 +def test_serializer_includes_is_playable(factories, logged_in_api_client): + playlist = factories["playlists.Playlist"]() + factories["playlists.PlaylistTrack"](playlist=playlist) + + url = reverse("api:v1:playlists-detail", kwargs={"pk": playlist.pk}) + response = logged_in_api_client.get(url) + + assert response.data["is_playable"] is False + + def test_playlist_inherits_user_privacy(logged_in_api_client): url = reverse("api:v1:playlists-list") user = logged_in_api_client.user diff --git a/front/src/components/playlists/Card.vue b/front/src/components/playlists/Card.vue index 956a543a6..344eb5fbf 100644 --- a/front/src/components/playlists/Card.vue +++ b/front/src/components/playlists/Card.vue @@ -5,8 +5,8 @@
- - + +
{{ playlist.name | truncate(30) }}