Commit 8b0ce6ad authored by Agate's avatar Agate 💬

Merge branch '1102-album-tracks' into 'develop'

Resolve "Remove tracks from /api/v1/albums API"

Closes #1102

See merge request funkwhale/funkwhale!1159
parents fd25d28a 55f4fde0
......@@ -336,6 +336,7 @@ class ManageBaseArtistSerializer(serializers.ModelSerializer):
class ManageBaseAlbumSerializer(serializers.ModelSerializer):
cover = music_serializers.cover_field
domain = serializers.CharField(source="domain_name")
tracks_count = serializers.SerializerMethodField()
class Meta:
model = music_models.Album
......@@ -349,8 +350,12 @@ class ManageBaseAlbumSerializer(serializers.ModelSerializer):
"cover",
"domain",
"is_local",
"tracks_count",
]
def get_tracks_count(self, o):
return getattr(o, "_tracks_count", None)
class ManageNestedTrackSerializer(serializers.ModelSerializer):
domain = serializers.CharField(source="domain_name")
......@@ -428,7 +433,6 @@ class ManageNestedArtistSerializer(ManageBaseArtistSerializer):
class ManageAlbumSerializer(
music_serializers.OptionalDescriptionMixin, ManageBaseAlbumSerializer
):
tracks = ManageNestedTrackSerializer(many=True)
attributed_to = ManageBaseActorSerializer()
artist = ManageNestedArtistSerializer()
tags = serializers.SerializerMethodField()
......@@ -437,7 +441,6 @@ class ManageAlbumSerializer(
model = music_models.Album
fields = ManageBaseAlbumSerializer.Meta.fields + [
"artist",
"tracks",
"attributed_to",
"tags",
]
......
......@@ -128,7 +128,7 @@ class ManageAlbumViewSet(
music_models.Album.objects.all()
.order_by("-id")
.select_related("attributed_to", "artist", "attachment_cover")
.prefetch_related("tracks", music_views.TAG_PREFETCH)
.with_tracks_count()
)
serializer_class = serializers.ManageAlbumSerializer
filterset_class = filters.ManageAlbumFilterSet
......
......@@ -319,10 +319,6 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet):
else:
return self.exclude(pk__in=matches)
def with_prefetched_tracks_and_playable_uploads(self, actor):
tracks = Track.objects.with_playable_uploads(actor)
return self.prefetch_related(models.Prefetch("tracks", queryset=tracks))
class Album(APIModelMixin):
title = models.CharField(max_length=MAX_LENGTHS["ALBUM_TITLE"])
......
......@@ -187,35 +187,12 @@ def serialize_artist_simple(artist):
return data
def serialize_album_track(track):
return {
"id": track.id,
"fid": track.fid,
"mbid": str(track.mbid),
"title": track.title,
"artist": serialize_artist_simple(track.artist),
"album": track.album_id,
"creation_date": DATETIME_FIELD.to_representation(track.creation_date),
"position": track.position,
"disc_number": track.disc_number,
"uploads": [
serialize_upload(u) for u in getattr(track, "playable_uploads", [])
],
"listen_url": track.listen_url,
"duration": getattr(track, "duration", None),
"copyright": track.copyright,
"license": track.license_id,
"is_local": track.is_local,
}
class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer):
# XXX: remove in 1.0, it's expensive and can work with a filter/api call
tracks = serializers.SerializerMethodField()
artist = serializers.SerializerMethodField()
cover = cover_field
is_playable = serializers.SerializerMethodField()
tags = serializers.SerializerMethodField()
tracks_count = serializers.SerializerMethodField()
attributed_to = serializers.SerializerMethodField()
id = serializers.IntegerField()
fid = serializers.URLField()
......@@ -232,9 +209,8 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer):
def get_artist(self, o):
return serialize_artist_simple(o.artist)
def get_tracks(self, o):
ordered_tracks = o.tracks.all()
return [serialize_album_track(track) for track in ordered_tracks]
def get_tracks_count(self, o):
return getattr(o, "_tracks_count", None)
def get_is_playable(self, obj):
try:
......
......@@ -181,6 +181,7 @@ class AlbumViewSet(
queryset = (
models.Album.objects.all()
.order_by("-creation_date")
.with_tracks_count()
.prefetch_related("artist", "attributed_to", "attachment_cover")
)
serializer_class = serializers.AlbumSerializer
......@@ -217,14 +218,7 @@ class AlbumViewSet(
queryset = queryset.exclude(artist__channel=None).filter(
artist__attributed_to=self.request.user.actor
)
tracks = (
models.Track.objects.prefetch_related("artist")
.with_playable_uploads(utils.get_actor_from_request(self.request))
.order_for_album()
)
qs = queryset.prefetch_related(
Prefetch("tracks", queryset=tracks), TAG_PREFETCH
)
qs = queryset.prefetch_related(TAG_PREFETCH)
return qs
libraries = action(methods=["get"], detail=True)(
......
......@@ -373,7 +373,7 @@ def test_manage_nested_artist_serializer(factories, now, to_api_date):
def test_manage_album_serializer(factories, now, to_api_date):
album = factories["music.Album"](attributed=True, with_cover=True)
track = factories["music.Track"](album=album)
setattr(album, "_tracks_count", 42)
expected = {
"id": album.id,
"domain": album.domain_name,
......@@ -385,11 +385,11 @@ def test_manage_album_serializer(factories, now, to_api_date):
"release_date": album.release_date.isoformat(),
"cover": common_serializers.AttachmentSerializer(album.attachment_cover).data,
"artist": serializers.ManageNestedArtistSerializer(album.artist).data,
"tracks": [serializers.ManageNestedTrackSerializer(track).data],
"attributed_to": serializers.ManageBaseActorSerializer(
album.attributed_to
).data,
"tags": [],
"tracks_count": 42,
}
s = serializers.ManageAlbumSerializer(album)
......
......@@ -115,34 +115,6 @@ def test_artist_with_albums_serializer_channel(factories, to_api_date):
assert serializer.data == expected
def test_album_track_serializer(factories, to_api_date):
upload = factories["music.Upload"](
track__license="cc-by-4.0", track__copyright="test", track__disc_number=2
)
track = upload.track
setattr(track, "playable_uploads", [upload])
expected = {
"id": track.id,
"fid": track.fid,
"artist": serializers.serialize_artist_simple(track.artist),
"album": track.album.id,
"mbid": str(track.mbid),
"title": track.title,
"position": track.position,
"disc_number": track.disc_number,
"uploads": [serializers.serialize_upload(upload)],
"creation_date": to_api_date(track.creation_date),
"listen_url": track.listen_url,
"duration": None,
"license": track.license.code,
"copyright": track.copyright,
"is_local": track.is_local,
}
data = serializers.serialize_album_track(track)
assert data == expected
def test_upload_serializer(factories, to_api_date):
upload = factories["music.Upload"]()
......@@ -200,7 +172,7 @@ def test_album_serializer(factories, to_api_date):
track1 = factories["music.Track"](
position=2, album__attributed_to=actor, album__with_cover=True
)
track2 = factories["music.Track"](position=1, album=track1.album)
factories["music.Track"](position=1, album=track1.album)
album = track1.album
expected = {
"id": album.id,
......@@ -212,12 +184,14 @@ def test_album_serializer(factories, to_api_date):
"is_playable": False,
"cover": common_serializers.AttachmentSerializer(album.attachment_cover).data,
"release_date": to_api_date(album.release_date),
"tracks": [serializers.serialize_album_track(t) for t in [track2, track1]],
"tracks_count": 2,
"is_local": album.is_local,
"tags": [],
"attributed_to": federation_serializers.APIActorSerializer(actor).data,
}
serializer = serializers.AlbumSerializer(album)
serializer = serializers.AlbumSerializer(
album.__class__.objects.with_tracks_count().get(pk=album.pk)
)
assert serializer.data == expected
......
......@@ -57,7 +57,7 @@ def test_album_list_serializer(api_request, factories, logged_in_api_client):
).track
album = track.album
request = api_request.get("/")
qs = album.__class__.objects.with_prefetched_tracks_and_playable_uploads(None)
qs = album.__class__.objects.with_tracks_count()
serializer = serializers.AlbumSerializer(
qs, many=True, context={"request": request}
)
......
Do not include tracks in album API representation (#1102)
\ No newline at end of file
......@@ -31,3 +31,11 @@ Now, it returns all the accessible libraries (including ones from other users an
If you are consuming the API via a third-party client and need to retrieve your libraries,
use the ``scope`` parameter, like this: ``GET /api/v1/libraries?scope=me``
API breaking change in ``/api/v1/albums``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
To increase performance, querying ``/api/v1/albums`` doesn't return album tracks anymore. This caused
some performance issues, especially as some albums and series have dozens or even hundreds of tracks.
If you want to retrieve tracks for an album, you can query ``/api/v1/tracks/?album=<albumid>``.
\ No newline at end of file
......@@ -198,10 +198,9 @@ Album:
- $ref: "#/BaseAlbum"
- type: "object"
properties:
tracks:
type: "array"
items:
$ref: "#/AlbumTrack"
tracks_count:
type: "integer"
format: "int64"
ArtistAlbum:
type: "object"
......
......@@ -15,8 +15,8 @@
<div class="description">
<translate translate-context="Content/Channel/Paragraph"
translate-plural="%{ count } episodes"
:translate-n="serie.tracks.length"
:translate-params="{count: serie.tracks.length}">
:translate-n="serie.tracks_count"
:translate-params="{count: serie.tracks_count}">
%{ count } episode
</translate>
</div>
......
......@@ -20,7 +20,7 @@
</div>
</div>
<div class="extra content">
<translate translate-context="*/*/*" :translate-params="{count: album.tracks.length}" :translate-n="album.tracks.length" translate-plural="%{ count } tracks">%{ count } track</translate>
<translate translate-context="*/*/*" :translate-params="{count: album.tracks_count}" :translate-n="album.tracks_count" translate-plural="%{ count } tracks">%{ count } track</translate>
<play-button class="right floated basic icon" :dropdown-only="true" :is-playable="album.is_playable" :dropdown-icon-classes="['ellipsis', 'horizontal', 'large really discrete']" :album="album"></play-button>
</div>
</div>
......
......@@ -9,7 +9,7 @@
<translate translate-context="*/*/*">None</translate>
</option>
<option v-for="album in albums" :key="album.id" :value="album.id">
{{ album.title }} (<translate translate-context="*/*/*" :translate-params="{count: album.tracks.length}" :translate-n="album.tracks.length" translate-plural="%{ count } tracks">%{ count } track</translate>)
{{ album.title }} (<translate translate-context="*/*/*" :translate-params="{count: album.tracks_count}" :translate-n="album.tracks_count" translate-plural="%{ count } tracks">%{ count } track</translate>)
</option>
</select>
</div>
......
......@@ -195,7 +195,7 @@ export default {
},
computed: {
totalTracks () {
return this.object.tracks.length
return this.object.tracks_count
},
isChannel () {
return this.object.artist.channel
......
......@@ -67,7 +67,7 @@
</span>
</td>
<td>
{{ scope.obj.tracks.length }}
{{ scope.obj.tracks_count }}
</td>
<td>
<human-date v-if="scope.obj.release_date" :date="scope.obj.release_date"></human-date>
......
......@@ -43,7 +43,6 @@ export default {
}.bind(this),
onHidden: function () {
this.$emit('update:show', false)
this.focusTrap.pause()
}.bind(this),
onVisible: function () {
this.focusTrap.activate()
......@@ -59,11 +58,15 @@ export default {
this.initModal()
this.$emit('show')
this.control.modal('show')
this.focusTrap.activate()
this.focusTrap.unpause()
} else {
if (this.control) {
this.$emit('hide')
this.control.modal('hide')
this.control.remove()
this.focusTrap.deactivate()
this.focusTrap.pause()
}
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment