From f11a22cf5db103c23d14ec534ed3ee268b02d47b Mon Sep 17 00:00:00 2001 From: Petitminion Date: Tue, 18 Mar 2025 13:17:55 +0100 Subject: [PATCH] tests minor issues --- api/funkwhale_api/federation/serializers.py | 5 +++- .../0061_migrate_libraries_to_playlist.py | 2 +- api/funkwhale_api/playlists/renderers.py | 2 +- api/tests/common/test_models.py | 2 +- api/tests/playlists/test_serializers.py | 3 +- api/tests/playlists/test_urls_v2.py | 12 ++++---- api/tests/playlists/test_views.py | 30 +++++++++---------- 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 7dd8984d4..a093c4f4b 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -944,6 +944,9 @@ def get_additional_fields(data): v = data.get(field, UNSET) if v == UNSET: continue + # in some cases we use the serializer context to pass objects instances, we don't want to add them + if not isinstance(v, str) or isinstance(v, dict): + continue additional_fields[field] = v return additional_fields @@ -2434,7 +2437,7 @@ class PlaylistSerializer(jsonld.JsonLdSerializer): if not actor.is_local: ap_to_fw_data["privacy_level"] = ( contexts.AS.Public - if validated_data["privacy_level"] == "everyone" + if validated_data.get("privacy_level", "") == "everyone" else "" ) diff --git a/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py b/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py index d3443eea8..b2028da99 100644 --- a/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py +++ b/api/funkwhale_api/music/migrations/0061_migrate_libraries_to_playlist.py @@ -144,7 +144,7 @@ def check_succefull_migration(apps, schema_editor): not_build_in_libs = len(actor.playlists.all()) + len( actor.libraries.filter(channel__isnull=False) ) - if len(actor.libraries.all()) - 2 != not_build_in_libs: + if len(actor.libraries.all()) - 3 != not_build_in_libs: raise Exception( "Incoherent library database state, check for errors in log and share them to the funkwhale team. Migration was abordted to prevent data loss" ) diff --git a/api/funkwhale_api/playlists/renderers.py b/api/funkwhale_api/playlists/renderers.py index 0abe6ed8b..65013496c 100644 --- a/api/funkwhale_api/playlists/renderers.py +++ b/api/funkwhale_api/playlists/renderers.py @@ -15,7 +15,7 @@ class PlaylistXspfRenderer(renderers.BaseRenderer): if isinstance(data, bytes): return data - fw_playlist = Playlist.objects.get(id=data["id"]) + fw_playlist = Playlist.objects.get(uuid=data["uuid"]) plt_tracks = fw_playlist.playlist_tracks.prefetch_related("track") top = Element("playlist", version="1", xmlns="http://xspf.org/ns/0/") title_xspf = SubElement(top, "title") diff --git a/api/tests/common/test_models.py b/api/tests/common/test_models.py index aee98d4f1..574845184 100644 --- a/api/tests/common/test_models.py +++ b/api/tests/common/test_models.py @@ -22,7 +22,7 @@ def test_mutation_fid_is_populated(factories, model, factory_args, namespace): ("music.Artist", "/library/artists/{obj.pk}"), ("music.Album", "/library/albums/{obj.pk}"), ("music.Track", "/library/tracks/{obj.pk}"), - ("playlists.Playlist", "/library/playlists/{obj.pk}"), + ("playlists.Playlist", "/library/playlists/{obj.uuid}"), ], ) def test_get_absolute_url(factory_name, factories, expected): diff --git a/api/tests/playlists/test_serializers.py b/api/tests/playlists/test_serializers.py index 3c365f59d..397ee7f72 100644 --- a/api/tests/playlists/test_serializers.py +++ b/api/tests/playlists/test_serializers.py @@ -75,7 +75,8 @@ def test_playlist_serializer(factories, to_api_date): actor = playlist.actor expected = { - "id": playlist.pk, + "uuid": playlist.uuid, + "fid": playlist.fid, "name": playlist.name, "privacy_level": playlist.privacy_level, "is_playable": False, diff --git a/api/tests/playlists/test_urls_v2.py b/api/tests/playlists/test_urls_v2.py index 53355df0b..5647d6ba1 100644 --- a/api/tests/playlists/test_urls_v2.py +++ b/api/tests/playlists/test_urls_v2.py @@ -24,7 +24,7 @@ def test_can_get_playlists_octet_stream(factories, logged_in_api_client): factories["playlists.PlaylistTrack"](playlist=pl) factories["playlists.PlaylistTrack"](playlist=pl) - url = reverse("api:v2:playlists-detail", kwargs={"pk": pl.pk}) + url = reverse("api:v2:playlists-detail", kwargs={"uuid": pl.uuid}) headers = {"Accept": "application/octet-stream"} response = logged_in_api_client.get(url, headers=headers) el = etree.fromstring(response.content) @@ -36,7 +36,7 @@ def test_can_get_playlists_octet_stream(factories, logged_in_api_client): def test_can_get_playlists_json(factories, logged_in_api_client): logged_in_api_client.user.create_actor() pl = factories["playlists.Playlist"]() - url = reverse("api:v2:playlists-detail", kwargs={"pk": pl.pk}) + url = reverse("api:v2:playlists-detail", kwargs={"uuid": pl.uuid}) response = logged_in_api_client.get(url, format="json") assert response.status_code == 200 assert response.data["name"] == pl.name @@ -105,7 +105,7 @@ def test_can_patch_playlists_octet_stream(factories, logged_in_api_client): track = factories["music.Track"]( title="Opinel 12", artist_credit__artist=artist, album=album ) - url = reverse("api:v2:playlists-detail", kwargs={"pk": pl.pk}) + url = reverse("api:v2:playlists-detail", kwargs={"uuid": pl.uuid}) data = open("./tests/playlists/test.xspf", "rb").read() response = logged_in_api_client.patch(url, data=data, format="xspf") pl.refresh_from_db() @@ -118,7 +118,7 @@ def test_can_get_playlists_track(factories, logged_in_api_client): logged_in_api_client.user.create_actor() pl = factories["playlists.Playlist"]() plt = factories["playlists.PlaylistTrack"](playlist=pl) - url = reverse("api:v2:playlists-tracks", kwargs={"pk": pl.pk}) + url = reverse("api:v2:playlists-tracks", kwargs={"uuid": pl.uuid}) response = logged_in_api_client.get(url) data = json.loads(response.content.decode("utf-8")) assert response.status_code == 200 @@ -130,7 +130,7 @@ def test_can_get_playlists_releases(factories, logged_in_api_client): logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"]() plt = factories["playlists.PlaylistTrack"](playlist=playlist) - url = reverse("api:v2:playlists-albums", kwargs={"pk": playlist.pk}) + url = reverse("api:v2:playlists-albums", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.get(url) data = json.loads(response.content) assert response.status_code == 200 @@ -141,7 +141,7 @@ def test_can_get_playlists_artists(factories, logged_in_api_client): logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"]() plt = factories["playlists.PlaylistTrack"](playlist=playlist) - url = reverse("api:v2:playlists-artists", kwargs={"pk": playlist.pk}) + url = reverse("api:v2:playlists-artists", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.get(url) data = json.loads(response.content) assert response.status_code == 200 diff --git a/api/tests/playlists/test_views.py b/api/tests/playlists/test_views.py index 4237fe37c..ded58f453 100644 --- a/api/tests/playlists/test_views.py +++ b/api/tests/playlists/test_views.py @@ -21,7 +21,7 @@ def test_serializer_includes_tracks_count(factories, logged_in_api_client): actor = logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"](actor=actor) factories["playlists.PlaylistTrack"](playlist=playlist) - url = reverse("api:v1:playlists-detail", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-detail", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.get(url, content_type="application/json") assert response.data["tracks_count"] == 1 @@ -34,7 +34,7 @@ def test_serializer_includes_tracks_count_986(factories, logged_in_api_client): factories["music.Upload"].create_batch( 3, track=plt.track, library__privacy_level="everyone", import_status="finished" ) - url = reverse("api:v1:playlists-detail", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-detail", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.get(url, content_type="application/json") assert response.data["tracks_count"] == 1 @@ -45,7 +45,7 @@ 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}) + url = reverse("api:v1:playlists-detail", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.get(url, content_type="application/json") assert response.data["is_playable"] is False @@ -81,7 +81,7 @@ def test_only_can_add_track_on_own_playlist_via_api(factories, logged_in_api_cli logged_in_api_client.user.create_actor() track = factories["music.Track"]() playlist = factories["playlists.Playlist"]() - url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-add", kwargs={"uuid": playlist.uuid}) data = {"tracks": [track.pk]} response = logged_in_api_client.post(url, data, content_type="application/json") @@ -96,7 +96,7 @@ def test_deleting_plt_updates_indexes(mocker, factories, logged_in_api_client): playlist = factories["playlists.Playlist"](actor=actor) 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}) + url = reverse("api:v1:playlists-remove", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.delete(url, {"index": 0}) @@ -125,7 +125,7 @@ def test_deleting_plt_updates_pl_lib(mocker, factories, logged_in_api_client): ) track_ids = [t.id for t in tracks] - url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-add", kwargs={"uuid": playlist.uuid}) logged_in_api_client.post(url, {"tracks": track_ids}) assert not_user_upload not in playlist.library.uploads.all() @@ -133,7 +133,7 @@ def test_deleting_plt_updates_pl_lib(mocker, factories, logged_in_api_client): for upload in playlist.library.uploads.all(): assert upload.tracks.filter(id=plt.track.id).exists() - url = reverse("api:v1:playlists-remove", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-remove", kwargs={"uuid": playlist.uuid}) logged_in_api_client.delete(url, {"index": 0}) playlist.library.refresh_from_db() @@ -158,7 +158,7 @@ def test_playlist_privacy_respected_in_list_anon( def test_only_owner_can_edit_playlist(method, factories, logged_in_api_client): logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"]() - url = reverse("api:v1:playlists-detail", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-detail", kwargs={"uuid": playlist.uuid}) response = getattr(logged_in_api_client, method.lower())(url) assert response.status_code == 404 @@ -172,7 +172,7 @@ def test_can_add_multiple_tracks_at_once_via_api( tracks = factories["music.Track"].create_batch(size=5) track_ids = [t.id for t in tracks] mocker.spy(playlist, "insert_many") - url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-add", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.post(url, {"tracks": track_ids}) assert response.status_code == 201 @@ -207,7 +207,7 @@ def test_add_multiple_tracks_at_once_update_pl_library( track_ids.append(not_user_track.id) track_ids.append(track_already_in_playlist.id) mocker.spy(playlist, "insert_many") - url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-add", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.post(url, {"tracks": track_ids}) assert response.status_code == 201 @@ -229,7 +229,7 @@ def test_honor_max_playlist_size(factories, mocker, logged_in_api_client, prefer ) track_ids = [t.id for t in tracks] mocker.spy(playlist, "insert_many") - url = reverse("api:v1:playlists-add", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-add", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.post(url, {"tracks": track_ids}) assert response.status_code == 400 @@ -239,7 +239,7 @@ def test_can_clear_playlist_from_api(factories, mocker, logged_in_api_client): actor = logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"](actor=actor) factories["playlists.PlaylistTrack"].create_batch(size=5, playlist=playlist) - url = reverse("api:v1:playlists-clear", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-clear", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.delete(url) assert response.status_code == 204 @@ -255,7 +255,7 @@ def test_clear_playlist_from_api_remove_pl_lib_uploads( for upload in playlist.library.uploads.all(): assert upload.playlist_libraries.filter(playlist=playlist).exists() assert upload.playlist_libraries.get(playlist=playlist).actor == actor - url = reverse("api:v1:playlists-clear", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-clear", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.delete(url) assert response.status_code == 204 @@ -266,7 +266,7 @@ def test_update_playlist_from_api(factories, mocker, logged_in_api_client): actor = logged_in_api_client.user.create_actor() playlist = factories["playlists.Playlist"](actor=actor) factories["playlists.PlaylistTrack"].create_batch(size=5, playlist=playlist) - url = reverse("api:v1:playlists-detail", kwargs={"pk": playlist.pk}) + url = reverse("api:v1:playlists-detail", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.patch(url, {"name": "test"}) playlist.refresh_from_db() @@ -279,7 +279,7 @@ def test_move_plt_updates_indexes(mocker, factories, logged_in_api_client): playlist = factories["playlists.Playlist"](actor=actor) 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}) + url = reverse("api:v1:playlists-move", kwargs={"uuid": playlist.uuid}) response = logged_in_api_client.post(url, {"from": 1, "to": 0})