LB-1214, LB-1215: Add playlist cover #3149
Conversation
|
Hello @anshg1214! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2025-01-26 08:01:53 UTC |
This is necessory to fetch the new cover art.
frontend/js/src/utils/APIService.ts
Outdated
| @@ -1822,4 +1822,21 @@ export default class APIService { | |||
| await this.checkStatus(response); | |||
| return response.json(); | |||
| }; | |||
|
|
|||
| getPlaylistCoverArt = async ( | |||
There was a problem hiding this comment.
This is now unused, with the server endpoint removed.
I think the endpoint could actually be useful. In particular, I'm thinking about the grid view on the playlists page, where we don't have all the playlist metadata but we could show each playlist's cover art.
Also thinking about atom feeds where we sometimes need a URL to the SVG rather than SVG content.
There was a problem hiding this comment.
So, LMK do a thing. I'll remove the frontend function to call the server api, and bring back the server endpoint.
There was a problem hiding this comment.
Currently I am getting an error about image size when I try the endpoint on test:
"image size must be between 128 and 1024, inclusive."
URL was POST https://test-api.listenbrainz.org/1/art/playlist/5bb057eb-b38a-4a8d-8218-c5004f052fb2/250/1/
On a side note: why is it a POST endpoint rather than GET?
My envisioned used would be a simple <img src="https://api.listenbrainz.org/1/art/playlist/${playlist.identifier}/3/1/"> or something along those lines in the PlaylistCard component to get the image directly, but that would require a GET endpoint.
Might also be useful if I could pass a request arg for image size, so i can get a 250px thumbnail.
What was your use case?
There was a problem hiding this comment.
So the endpoint's structure is /playlist/<uuid:playlist_mbid>/<int:dimension>/<int:layout>. The dimension and layout are based on those defined in the GRID_TILE_DESIGNS.
Since, playlists can be private, we cannot remove authentication from the request, so therefore we cannot add it as a src in the image HTML Tag.
There was a problem hiding this comment.
Ah bummer, I forgot about that. Yeah, I guess we fetch the SVG content with a fetch call then 👍
Still, the image_size issue was not related to the dimension or layout params, not sure what is happening there. Did you successfully test a POST request with auth on the test-api.lb art endpoint?
There was a problem hiding this comment.
There was a problem hiding this comment.
It is indeed working. 🎉
Not sure what I did wrong before ¯_(ツ)_/¯
|
@MonkeyDo Do you know why the tests are failing on this PR? |
No idea, sorry :/ |
| sorted_options = sorted( | ||
| options, | ||
| key=lambda x: (-x["dimension"], x["layout"]) | ||
| ) | ||
| selected_cover_art = sorted_options[0] |
There was a problem hiding this comment.
This could always be a randomly selected option, to add some spice?
Just an idea, not sure if it's a good one.
There was a problem hiding this comment.
I'm not sure on it. Let's keep it static for now.
Co-authored-by: Monkey Do <[email protected]>
LB-1214, LB-1215