Merge branch 'attributed-to-suggestions' into 'develop'

Ensure owner of tracks/albums/artists can approve suggestions

See merge request funkwhale/funkwhale!833

api/funkwhale_api/music/mutations.py

@@ -11,7 +11,12 @@ def can_suggest(obj, actor):
 
 
 def can_approve(obj, actor):
-    return obj.is_local and actor.user and actor.user.get_permissions()["library"]
+    if not obj.is_local or not actor.user:
+        return False
+
+    return (
+        actor.id is not None and actor.id == obj.attributed_to_id
+    ) or actor.user.get_permissions()["library"]
 
 
 class TagMutation(mutations.UpdateMutationSerializer):

api/funkwhale_api/music/serializers.py

@@ -19,6 +19,16 @@ from . import filters, models, tasks
 cover_field = VersatileImageFieldSerializer(allow_null=True, sizes="square")
 
 
+def serialize_attributed_to(self, obj):
+    # Import at runtime to avoid a circular import issue
+    from funkwhale_api.federation import serializers as federation_serializers
+
+    if not obj.attributed_to_id:
+        return
+
+    return federation_serializers.APIActorSerializer(obj.attributed_to).data
+
+
 class LicenseSerializer(serializers.Serializer):
     id = serializers.SerializerMethodField()
     url = serializers.URLField()
@@ -68,6 +78,7 @@ class ArtistAlbumSerializer(serializers.ModelSerializer):
 class ArtistWithAlbumsSerializer(serializers.ModelSerializer):
     albums = ArtistAlbumSerializer(many=True, read_only=True)
     tags = serializers.SerializerMethodField()
+    attributed_to = serializers.SerializerMethodField()
 
     class Meta:
         model = models.Artist
@@ -80,12 +91,15 @@ class ArtistWithAlbumsSerializer(serializers.ModelSerializer):
             "albums",
             "is_local",
             "tags",
+            "attributed_to",
         )
 
     def get_tags(self, obj):
         tagged_items = getattr(obj, "_prefetched_tagged_items", [])
         return [ti.tag.name for ti in tagged_items]
 
+    get_attributed_to = serialize_attributed_to
+
 
 class ArtistSimpleSerializer(serializers.ModelSerializer):
     class Meta:
@@ -139,6 +153,7 @@ class AlbumSerializer(serializers.ModelSerializer):
     cover = cover_field
     is_playable = serializers.SerializerMethodField()
     tags = serializers.SerializerMethodField()
+    attributed_to = serializers.SerializerMethodField()
 
     class Meta:
         model = models.Album
@@ -155,8 +170,11 @@ class AlbumSerializer(serializers.ModelSerializer):
             "is_playable",
             "is_local",
             "tags",
+            "attributed_to",
         )
 
+    get_attributed_to = serialize_attributed_to
+
     def get_tracks(self, o):
         ordered_tracks = o.tracks.all()
         return AlbumTrackSerializer(ordered_tracks, many=True).data
@@ -213,6 +231,7 @@ class TrackSerializer(serializers.ModelSerializer):
     uploads = serializers.SerializerMethodField()
     listen_url = serializers.SerializerMethodField()
     tags = serializers.SerializerMethodField()
+    attributed_to = serializers.SerializerMethodField()
 
     class Meta:
         model = models.Track
@@ -232,8 +251,11 @@ class TrackSerializer(serializers.ModelSerializer):
             "license",
             "is_local",
             "tags",
+            "attributed_to",
         )
 
+    get_attributed_to = serialize_attributed_to
+
     def get_listen_url(self, obj):
         return obj.listen_url
 

api/funkwhale_api/music/views.py

@@ -60,7 +60,7 @@ def get_libraries(filter_uploads):
 
 
 class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet):
-    queryset = models.Artist.objects.all()
+    queryset = models.Artist.objects.all().select_related("attributed_to")
     serializer_class = serializers.ArtistWithAlbumsSerializer
     permission_classes = [oauth_permissions.ScopePermission]
     required_scope = "libraries"
@@ -92,7 +92,9 @@ class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelV
 
 class AlbumViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet):
     queryset = (
-        models.Album.objects.all().order_by("artist", "release_date").select_related()
+        models.Album.objects.all()
+        .order_by("artist", "release_date")
+        .select_related("artist", "attributed_to")
     )
     serializer_class = serializers.AlbumSerializer
     permission_classes = [oauth_permissions.ScopePermission]
@@ -188,7 +190,11 @@ class TrackViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi
     A simple ViewSet for viewing and editing accounts.
     """
 
-    queryset = models.Track.objects.all().for_nested_serialization()
+    queryset = (
+        models.Track.objects.all()
+        .for_nested_serialization()
+        .select_related("attributed_to")
+    )
     serializer_class = serializers.TrackSerializer
     permission_classes = [oauth_permissions.ScopePermission]
     required_scope = "libraries"

api/tests/music/test_mutations.py

@@ -2,6 +2,8 @@ import datetime
 import pytest
 
 from funkwhale_api.music import licenses
+from funkwhale_api.music import mutations
+
 from funkwhale_api.tags import models as tags_models
 
 
@@ -140,3 +142,37 @@ def test_mutation_set_tags(factory_name, factories, now, mocker):
         {"type": "Update", "object": {"type": obj_type}},
         context={obj_type.lower(): obj},
     )
+
+
+@pytest.mark.parametrize("is_local, expected", [(True, True), (False, False)])
+def test_perm_checkers_can_suggest(factories, is_local, expected):
+    obj = factories["music.Track"](local=is_local)
+    assert mutations.can_suggest(obj, actor=None) is expected
+
+
+@pytest.mark.parametrize(
+    "is_local, permission_library, actor_is_attributed, expected",
+    [
+        # Not local object, so local users can't edit
+        (False, False, False, False),
+        (False, True, False, False),
+        # Local but no specific conditions met for permission
+        (True, False, False, False),
+        # Local and attributed_to -> ok
+        (True, False, True, True),
+        # Local and library permission -> ok
+        (True, True, False, True),
+    ],
+)
+def test_perm_checkers_can_approve(
+    factories, is_local, permission_library, actor_is_attributed, expected
+):
+    actor = factories["users.User"](
+        permission_library=permission_library
+    ).create_actor()
+    obj_kwargs = {"local": is_local}
+    if actor_is_attributed:
+        obj_kwargs["attributed_to"] = actor
+    obj = factories["music.Track"](**obj_kwargs)
+
+    assert mutations.can_approve(obj, actor=actor) is expected

api/tests/music/test_serializers.py

@@ -1,5 +1,6 @@
 import pytest
 
+from funkwhale_api.federation import serializers as federation_serializers
 from funkwhale_api.music import licenses
 from funkwhale_api.music import models
 from funkwhale_api.music import serializers
@@ -56,7 +57,8 @@ def test_artist_album_serializer(factories, to_api_date):
 
 
 def test_artist_with_albums_serializer(factories, to_api_date):
-    track = factories["music.Track"]()
+    actor = factories["federation.Actor"]()
+    track = factories["music.Track"](album__artist__attributed_to=actor)
     artist = track.artist
     artist = artist.__class__.objects.with_albums().get(pk=artist.pk)
     album = list(artist.albums.all())[0]
@@ -70,6 +72,7 @@ def test_artist_with_albums_serializer(factories, to_api_date):
         "creation_date": to_api_date(artist.creation_date),
         "albums": [serializers.ArtistAlbumSerializer(album).data],
         "tags": [],
+        "attributed_to": federation_serializers.APIActorSerializer(actor).data,
     }
     serializer = serializers.ArtistWithAlbumsSerializer(artist)
     assert serializer.data == expected
@@ -156,7 +159,8 @@ def test_upload_owner_serializer(factories, to_api_date):
 
 
 def test_album_serializer(factories, to_api_date):
-    track1 = factories["music.Track"](position=2)
+    actor = factories["federation.Actor"]()
+    track1 = factories["music.Track"](position=2, album__attributed_to=actor)
     track2 = factories["music.Track"](position=1, album=track1.album)
     album = track1.album
     expected = {
@@ -177,6 +181,7 @@ def test_album_serializer(factories, to_api_date):
         "tracks": serializers.AlbumTrackSerializer([track2, track1], many=True).data,
         "is_local": album.is_local,
         "tags": [],
+        "attributed_to": federation_serializers.APIActorSerializer(actor).data,
     }
     serializer = serializers.AlbumSerializer(album)
 
@@ -184,8 +189,12 @@ def test_album_serializer(factories, to_api_date):
 
 
 def test_track_serializer(factories, to_api_date):
+    actor = factories["federation.Actor"]()
     upload = factories["music.Upload"](
-        track__license="cc-by-4.0", track__copyright="test", track__disc_number=2
+        track__license="cc-by-4.0",
+        track__copyright="test",
+        track__disc_number=2,
+        track__attributed_to=actor,
     )
     track = upload.track
     setattr(track, "playable_uploads", [upload])
@@ -205,6 +214,7 @@ def test_track_serializer(factories, to_api_date):
         "copyright": upload.track.copyright,
         "is_local": upload.track.is_local,
         "tags": [],
+        "attributed_to": federation_serializers.APIActorSerializer(actor).data,
     }
     serializer = serializers.TrackSerializer(track)
     assert serializer.data == expected