Skip to content

Commit

Permalink
save call to lookup() when creating playlist
Browse files Browse the repository at this point in the history
  • Loading branch information
girst committed Jan 2, 2021
1 parent 191f2a8 commit c34a7b0
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 16 deletions.
18 changes: 15 additions & 3 deletions mopidy_spotify/playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,21 @@ def refresh(self):
self._loaded = True

def create(self, name):
logger.info(f"Creating Spotify playlist '{name}'")
uri = web.create_playlist(self._backend._web_client, name)
return self.lookup(uri) if uri else None
if not name:
return None
web_playlist = web.create_playlist(self._backend._web_client, name)
if web_playlist is None:
logger.error(f"Failed to create Spotify playlist '{name}'")
return
logger.info(f"Created Spotify playlist '{name}'")
return translator.to_playlist(
web_playlist,
username=self._backend._web_client.user_id,
bitrate=self._backend._bitrate,
# Note: we are not filtering out (currently) unplayable tracks;
# otherwise they would be removed when editing the playlist.
check_playable=False,
)

def delete(self, uri):
logger.info(f"Deleting Spotify playlist {uri!r}")
Expand Down
2 changes: 1 addition & 1 deletion mopidy_spotify/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ def create_playlist(web_client, name, public=False):
url = f"users/{web_client.user_id}/playlists"
response = web_client.post(url, json={"name": name, "public": public})
web_client.remove_from_cache("me/playlists")
return response["uri"] if response.status_ok else None
return response if response.status_ok else None


def delete_playlist(web_client, uri):
Expand Down
22 changes: 10 additions & 12 deletions tests/test_playlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from mopidy.models import Ref, Track

import spotify
from mopidy_spotify import playlists
from mopidy_spotify import playlists, web


@pytest.fixture
Expand Down Expand Up @@ -238,8 +238,11 @@ def request(method, path, *, json=None):
return mock.Mock(status_ok=rv)
elif re.fullmatch(r"users/(.*?)/playlists", path):
ok, playlist_id = _create_playlist(method, json)
rv = mock.MagicMock(status_ok=ok)
rv.__getitem__.return_value = playlist_id
rv = mock.MagicMock(status_ok=ok, spec=web.WebResponse)
rv.__getitem__.side_effect = web_playlists_map[
playlist_id
].__getitem__
rv.get = web_playlists_map[playlist_id].get
return rv
elif re.fullmatch(r"playlists/(.*?)/followers", path):
playlist_id = path_parts[1]
Expand Down Expand Up @@ -336,8 +339,7 @@ def test_playlist_save_failed1(provider, mopidy_track_factory, caplog):
assert retval is None
assert (
"Cannot modify Spotify playlist 'spotify:user:bob:playlist:baz' "
"owned by other user bob"
in caplog.text
"owned by other user bob" in caplog.text
)


Expand Down Expand Up @@ -367,17 +369,13 @@ def test_playlist_save_failed2(provider, mopidy_track_factory, caplog):
def test_playlist_save_failed3(provider, caplog):
playlist = provider.lookup("spotify:user:alice:playlist:foo")
tracks = list(playlist.tracks)
tracks.append(
Track(
name="non-spotify track",
uri="some:other:uri",
)
)
tracks.append(Track(name="non-spotify track", uri="some:other:uri"))
new_pl = playlist.replace(tracks=tracks)
retval = provider.save(new_pl)
assert retval is playlist
assert (
"Skipping adding non-Spotify tracks to Spotify playlist 'spotify:user:alice:playlist:foo'"
"Skipping adding non-Spotify tracks to Spotify playlist "
"'spotify:user:alice:playlist:foo'"
in caplog.text
)

Expand Down

0 comments on commit c34a7b0

Please sign in to comment.