Bug Fix: Handle null images for playlist #716
Conversation
|
sorry for not seeing this earlier, I'm usually busy with my job so I don't receive notifications from gh's PRs I'll look at it later today |
|
Also, try to target the maintenance branch as that's where current development is going |
|
I see. My apoligies, I will try to fix that. |
src/api/api_models.rs
Outdated
| &self.images[..] | ||
| match &self.images { | ||
| Some(x) => &x[..], | ||
| None => &def_image[..], |
There was a problem hiding this comment.
Can't we return an empty array here?
There was a problem hiding this comment.
No. Rust returns returns a value referencing data owned by the current function. I believe it has to do with garbage collection, but I do not know much rust so I just got around the issue by making a static constant outside the implementation.
| @@ -186,7 +186,7 @@ trait WithImages { | |||
| pub struct Playlist { | |||
| pub id: String, | |||
| pub name: String, | |||
| pub images: Vec<Image>, | |||
| pub images: Option<Vec<Image>>, | |||
There was a problem hiding this comment.
Does #[serde(default)] fix the issue?
There was a problem hiding this comment.
Surprisingly, no. I haven't investigated why but adding that to the images will still throw a null-errror.
|
hey @potatoes1286 I added some comments but I would be happy to merge this as-is. |
|
Any other concerns @Diegovsky or is it good to merge? |
|
@potatoes1286 please run |
|
I just want to make all the tests pass before merge. Looks fine by me. I apologise for the long review process. Thank you for your contribution :) |
|
@Diegovsky Formatted now 👍 Side note: renamed def_image -> EMPTY_IMAGE to match rust naming / more reflective naming |
|
Thank you for your work! |
Fixes #715, potentially #713
If a user has a playlist with no image (due to only local songs, no songs in playlist, etc), Spotify API returns a null for image and causes Spot to fail loading any playlist changes.
This turns images to an option and, if null, returns an empty image instead. It is a quick and dirty fix, but I do not know much Rust, so I've done what I could.
Thank you in advance.
@xou816