diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py index ea70678ad..2355232aa 100644 --- a/api/funkwhale_api/common/permissions.py +++ b/api/funkwhale_api/common/permissions.py @@ -106,7 +106,7 @@ class PrivacyLevelPermission(BasePermission): elif privacy_level == "me" and obj_actor == request_actor: return True - elif request_actor in obj.user.actor.get_approved_followers(): + elif request_actor in obj_actor.get_approved_followers(): return True else: return False 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 184754fe3..97db650ff 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 @@ -1,7 +1,6 @@ # Generated by Django 4.2.9 on 2025-01-03 16:12 from django.db import migrations, models -from django.db import IntegrityError from funkwhale_api.federation import utils as federation_utils from django.urls import reverse @@ -19,7 +18,7 @@ def insert_tracks_to_playlist(apps, playlist, uploads): uuid=(new_uuid := uuid.uuid4()), fid=federation_utils.full_url( reverse( - f"federation:music:playlist-tracks-detail", + "federation:music:playlist-tracks-detail", kwargs={"uuid": new_uuid}, ) ), @@ -34,49 +33,36 @@ def insert_tracks_to_playlist(apps, playlist, uploads): def migrate_libraries_to_playlist(apps, schema_editor): Playlist = apps.get_model("playlists", "Playlist") Library = apps.get_model("music", "Library") - LibraryFollow = apps.get_model("federation", "LibraryFollow") - Follow = apps.get_model("federation", "Follow") - User = apps.get_model("users", "User") Actor = apps.get_model("federation", "Actor") - # library to playlist for library in Library.objects.all(): - playlist = Playlist.objects.create( - name=library.name, - actor=library.actor, - creation_date=library.creation_date, - privacy_level=library.privacy_level, - description=library.description, - uuid=(new_uuid := uuid.uuid4()), - fid=federation_utils.full_url( - reverse( - f"federation:music:playlists-detail", - kwargs={"uuid": new_uuid}, - ) - ), - ) - playlist.save() + try: + playlist = Playlist.objects.create( + name=library.name, + library=library, + actor=library.actor, + creation_date=library.creation_date, + privacy_level=library.privacy_level, + description=library.description, + uuid=(new_uuid := uuid.uuid4()), + fid=federation_utils.full_url( + reverse( + "federation:music:playlists-detail", + kwargs={"uuid": new_uuid}, + ) + ), + ) + playlist.save() - if library.uploads.all().exists(): - insert_tracks_to_playlist(apps, playlist, library.uploads.all()) + if library.uploads.all().exists(): + uploads = library.uploads.all() + insert_tracks_to_playlist(apps, playlist, uploads) + playlist.library.playlist_uploads.set(uploads) + except Exception as e: + print(f"An error occurred during library.playlist creation {e}") + continue - # library follows to user follow - for lib_follow in LibraryFollow.objects.filter(target=library): - try: - Follow.objects.create( - uuid=lib_follow.uuid, - target=library.actor, - actor=lib_follow.actor, - approved=lib_follow.approved, - creation_date=lib_follow.creation_date, - modification_date=lib_follow.modification_date, - ) - except IntegrityError: - pass - - LibraryFollow.objects.all().delete() - - # migrate uploads to new library + # migrate uploads to new built-in libraries for actor in Actor.objects.all(): privacy_levels = ["me", "instance", "everyone"] for privacy_level in privacy_levels: @@ -87,23 +73,30 @@ def migrate_libraries_to_playlist(apps, schema_editor): uuid=(new_uuid := uuid.uuid4()), fid=federation_utils.full_url( reverse( - f"federation:music:libraries-detail", + "federation:music:libraries-detail", kwargs={"uuid": new_uuid}, ) ), ) for library in actor.libraries.filter(privacy_level=privacy_level): library.uploads.all().update(library=build_in_lib) - if library.pk is not build_in_lib.pk: - library.delete() class Migration(migrations.Migration): dependencies = [ ("music", "0060_empty_for_test"), - ("playlists", "0008_playlist_library_drop"), + ("playlists", "0009_playlist_library"), ] operations = [ + migrations.AddField( + model_name="upload", + name="playlist_libraries", + field=models.ManyToManyField( + blank=True, + related_name="playlist_uploads", + to="music.library", + ), + ), migrations.RunPython( migrate_libraries_to_playlist, reverse_code=migrations.RunPython.noop ), diff --git a/api/funkwhale_api/music/migrations/0064_upload_library_playlist.py b/api/funkwhale_api/music/migrations/0064_upload_library_playlist.py deleted file mode 100644 index 43d321e47..000000000 --- a/api/funkwhale_api/music/migrations/0064_upload_library_playlist.py +++ /dev/null @@ -1,23 +0,0 @@ -# Generated by Django 5.1.6 on 2025-03-09 21:58 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("music", "0063_upload_third_party_provider"), - ] - - operations = [ - migrations.AddField( - model_name="upload", - name="playlist_libraries", - field=models.ManyToManyField( - blank=True, - null=True, - related_name="playlist_uploads", - to="music.library", - ), - ), - ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 4e6a07c1b..d67cbe43a 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -830,7 +830,6 @@ class Upload(models.Model): playlist_libraries = models.ManyToManyField( "library", null=True, - blank=True, related_name="playlist_uploads", ) diff --git a/api/funkwhale_api/playlists/migrations/0009_playlist_library.py b/api/funkwhale_api/playlists/migrations/0009_playlist_library.py index 32d8abe11..b04b69965 100644 --- a/api/funkwhale_api/playlists/migrations/0009_playlist_library.py +++ b/api/funkwhale_api/playlists/migrations/0009_playlist_library.py @@ -16,18 +16,21 @@ def create_playlist_libraries(apps, schema_editor): for playlist in Playlist.objects.all(): library = playlist.library if library is None: - library = Library.objects.create( - name=playlist.name, privacy_level="me", actor=playlist.actor - ) - library.save() - playlist.library = library - playlist.save() - add_uploads_to_pl_library(playlist, library) + try: + library = Library.objects.create( + name=playlist.name, privacy_level="me", actor=playlist.actor + ) + library.save() + playlist.library = library + playlist.save() + add_uploads_to_pl_library(playlist, library) + except Exception as e: + print(f"An error occurred during playlist.library creation {e}") + continue class Migration(migrations.Migration): dependencies = [ - ("music", "0063_upload_third_party_provider"), ("playlists", "0008_playlist_library_drop"), ] diff --git a/api/tests/common/test_permissions.py b/api/tests/common/test_permissions.py index ee76a9678..4428dade6 100644 --- a/api/tests/common/test_permissions.py +++ b/api/tests/common/test_permissions.py @@ -98,9 +98,10 @@ def test_privacylevel_permission_me( assert check is expected +# "me" expects true since the object can be private but share with followers @pytest.mark.parametrize( "privacy_level,expected", - [("me", False), ("followers", True), ("instance", False), ("everyone", True)], + [("me", True), ("followers", True), ("instance", False), ("everyone", True)], ) def test_privacylevel_permission_followers( factories, api_request, anonymous_user, privacy_level, expected, mocker diff --git a/api/tests/music/test_migrations.py b/api/tests/music/test_migrations.py index ae695b527..b7aef68df 100644 --- a/api/tests/music/test_migrations.py +++ b/api/tests/music/test_migrations.py @@ -107,8 +107,6 @@ def test_migrate_libraries_to_playlist(migrator): domain = Domain.objects.create() domain2 = Domain.objects.create(pk=2) actor = Actor.objects.create(name="Test Actor", domain=domain) - existing_urls = Actor.objects.values_list("fid", flat=True) - print(existing_urls) target_actor = Actor.objects.create( name="Test Actor 2", domain=domain2, @@ -152,9 +150,7 @@ def test_migrate_libraries_to_playlist(migrator): new_apps = migrator.loader.project_state([music_final_migration]).apps Playlist = new_apps.get_model("playlists", "Playlist") PlaylistTrack = new_apps.get_model("playlists", "PlaylistTrack") - Follow = new_apps.get_model("federation", "Follow") LibraryFollow = new_apps.get_model("federation", "LibraryFollow") - Follow = new_apps.get_model("federation", "Follow") # Assertions @@ -172,13 +168,17 @@ def test_migrate_libraries_to_playlist(migrator): for i, playlist_track in enumerate(playlist_tracks): assert playlist_track.track.pk == uploads[i].track.pk - # Verify User Follow creation - follow = Follow.objects.get(target__pk=target_actor.pk) + # Verify playlist.library Follow creation + follow = LibraryFollow.objects.get(target__pk=playlist.library.pk) assert follow.actor.pk == actor.pk assert follow.approved == library_follow.approved + assert follow.target == playlist.library - # Verify LibraryFollow deletion and library creation - assert LibraryFollow.objects.count() == 0 + # Verify uploads are migrated in lib.playlist_uploads + for upload in uploads: + assert upload.pk in [u.pk for u in playlist.library.playlist_uploads.all()] + assert upload.pk not in [u.pk for u in playlist.library.uploads.all()] + assert not playlist.library.uploads.all() # Test fail but works on real db I don't get why # no library are found in the new app diff --git a/docs/specs/playlist-library-federation/index.md b/docs/specs/playlist-library-federation/index.md index 78bef8ced..fe076f9bf 100644 --- a/docs/specs/playlist-library-federation/index.md +++ b/docs/specs/playlist-library-federation/index.md @@ -15,14 +15,15 @@ Users will be able to click on a "Request access to playlist audios files" butto #### Backend -- [x] ``PlaylistViewSet` `add` `clear` `remove` update the uploads.playlist_libraries relationships -- [x] ``PlaylistViewSet` `add` `clear` `remove` -> `schedule_scan` -> Update activity to remote -> playlist.library scan on remote +- [x] `PlaylistViewSet` `add` `clear` `remove` update the uploads.playlist_libraries relationships +- [x] `PlaylistViewSet` `add` `clear` `remove` -> `schedule_scan` -> Update activity to remote -> playlist.library scan on remote - [x] library and playlist scan delay are long (24h), force on ap update -- [ ] make sure only owned upload are added to the playlist.library -- [ ] update the "drop library" migrations to use the playlist.library instead of user follow +- [x] make sure only owned upload are added to the playlist.library +- [x] update the "drop library" migrations to use the playlist.library instead of user follow - [ ] make sure user get the new libraries created after library drop ### Follow up +- [ ] Finish library drop (delete libraries endpoints) - [ ] Playlist discovery : fetch federation endpoint for playlists - [ ] Playlist discovery : add the playlist to my playlist collection = follow request to playlist diff --git a/front/src/locales/en_US.json b/front/src/locales/en_US.json index 2ffb21065..0249b4956 100644 --- a/front/src/locales/en_US.json +++ b/front/src/locales/en_US.json @@ -3201,7 +3201,7 @@ }, "useErrorHandler": { "errorReportMessage": "To help us understand why it happened, please attach a detailed description of what you did that has triggered the error.", - "errorReportTitle": "An unexpected error occured.", + "errorReportTitle": "An unexpected error occurred.", "leaveFeedback": "Leave feedback", "unexpectedError": "An unexpected error occurred." },