guilhermesad / rspotify

A ruby wrapper for the Spotify Web API
MIT License
717 stars 290 forks source link

Playlist method calls don't fail gracefully when @owner != current_user #246

Open derekpovah opened 2 years ago

derekpovah commented 2 years ago

Related to #64, yet slightly different.

Doing something like this returns a NoMethodError:

playlist = current_user.playlists.first
playlist.user.id == current_user.id
=> false
playlist.add_tracks!([current_user.player.currently_playing])
NoMethodError: undefined method `[]' for nil:NilClass

        'Authorization' => "Bearer #{@@users_credentials[user_id]['token']}",
                                                                 ^^^^^^^^^

Here's one of the culprits: https://github.com/guilhermesad/rspotify/blob/4ba80db1c653227384e9be7c03ea54a33d04f88a/lib/rspotify/playlist.rb#L162

Shouldn't all calls to User.oauth_send be made with the credentials we are sure we have access to? Even if Spotify doesn't currently support adding tracks to playlists that you don't own (vote for my idea to change that, if you want), I think it's better to let the Spotify API return the 403 and let people deal with it outside of the gem.

justinthiele commented 8 months ago

@derekpovah Thanks for investigating this one! For my use case, I always want to the playlists to be created by the same user, so I ended up passing in an ENV variable with the static user id. Not an ideal solution but it works for the time being 👍

derekpovah commented 8 months ago

@justinthiele The create_playlist! method is on the User class, so I don't know if your issue is related to this.

The issue here is that when you make certain method calls on instances of a Playlist, RSpotify makes an assumption that you want to authenticate with the API as the owner of the playlist even if the playlist owner is not the user you have OAuth credentials for (the current_user in my case).

At the moment, it's not really make-or-break for me since all of the playlists I use this gem to update are owned by me. I still think this should be considered a bug since making assumptions about which user we want to authenticate as can lead to unexpected behavior.

justinthiele commented 8 months ago

Thanks for clarifying, @derekpovah. I made a few changes when I finally got my bug fixed, it's possible that this wasn't the root cause. I'll dig into that a bit more. Appreciate it!