From 3a67eae46d1a1726089c03e9e9b90f8c6b74d34f Mon Sep 17 00:00:00 2001
From: Rahul Ahuja <rah.ahuja@gmail.com>
Date: Tue, 22 Jan 2019 10:41:18 -0800
Subject: [PATCH] Use track attrs directly, better naming/docstrings

---
 beetsplug/spotify.py | 52 +++++++++++++++++++++++++++++---------------
 test/test_spotify.py |  8 +++----
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/beetsplug/spotify.py b/beetsplug/spotify.py
index 31d28c842..cc21f8f05 100644
--- a/beetsplug/spotify.py
+++ b/beetsplug/spotify.py
@@ -261,7 +261,7 @@ class SpotifyPlugin(BeetsPlugin):
         for i, track_data in enumerate(album_data['tracks']['items']):
             if track_data['disc_number'] == track.medium:
                 medium_total += 1
-                if track_data['id'] == spotify_id:
+                if track_data['id'] == track.track_id:
                     track.index = i + 1
         track.medium_total = medium_total
         return track
@@ -419,8 +419,8 @@ class SpotifyPlugin(BeetsPlugin):
         def queries(lib, opts, args):
             success = self._parse_opts(opts)
             if success:
-                results = self._query_spotify(lib, ui.decargs(args))
-                self._output_results(results)
+                results = self._match_library_tracks(lib, ui.decargs(args))
+                self._output_match_results(results)
 
         spotify_cmd = ui.Subcommand(
             'spotify', help=u'build a Spotify playlist'
@@ -458,11 +458,23 @@ class SpotifyPlugin(BeetsPlugin):
         self.opts = opts
         return True
 
-    def _query_spotify(self, lib, keywords):
+    def _match_library_tracks(self, library, keywords):
+        """
+        Get a list of simplified track objects dicts for library tracks
+        matching the specified ``keywords``.
+        
+        :param library: beets library object to query.
+        :type library: beets.library.Library
+        :param keywords: Query to match library items against.
+        :type keywords: str
+        :return: List of simplified track object dicts for library items
+            matching the specified query.
+        :rtype: list[dict]
+        """
         results = []
         failures = []
 
-        items = lib.items(keywords)
+        items = library.items(keywords)
 
         if not items:
             self._log.debug(
@@ -509,9 +521,9 @@ class SpotifyPlugin(BeetsPlugin):
             region_filter = self.config['region_filter'].get()
             if region_filter:
                 response_data_tracks = [
-                    x
-                    for x in response_data_tracks
-                    if region_filter in x['available_markets']
+                    track_data
+                    for track_data in response_data_tracks
+                    if region_filter in track_data['available_markets']
                 ]
 
             if (
@@ -552,16 +564,22 @@ class SpotifyPlugin(BeetsPlugin):
 
         return results
 
-    def _output_results(self, results):
-        if results:
-            ids = [x['id'] for x in results]
-            if self.config['mode'].get() == "open":
-                self._log.info(u'Attempting to open Spotify with playlist')
-                spotify_url = self.playlist_partial + ",".join(ids)
-                webbrowser.open(spotify_url)
+    def _output_match_results(self, results):
+        """
+        Open a playlist or print Spotify URLs for the provided track object dicts.
 
+        :param results: List of simplified track object dicts
+            (https://developer.spotify.com/documentation/web-api/reference/object-model/#track-object-simplified)
+        :type results: list[dict]
+        """
+        if results:
+            spotify_ids = [track_data['id'] for track_data in results]
+            if self.config['mode'].get() == 'open':
+                self._log.info(u'Attempting to open Spotify with playlist')
+                spotify_url = self.playlist_partial + ",".join(spotify_ids)
+                webbrowser.open(spotify_url)
             else:
-                for item in ids:
-                    print(self.open_track_url + item)
+                for spotify_id in spotify_ids:
+                    print(self.open_track_url + spotify_id)
         else:
             self._log.warning(u'No Spotify tracks found from beets query')
diff --git a/test/test_spotify.py b/test/test_spotify.py
index 06499e8d7..5c19cb0eb 100644
--- a/test/test_spotify.py
+++ b/test/test_spotify.py
@@ -59,7 +59,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper):
         self.assertEqual(True, self.spotify._parse_opts(opts))
 
     def test_empty_query(self):
-        self.assertEqual(None, self.spotify._query_spotify(self.lib, u"1=2"))
+        self.assertEqual(None, self.spotify._match_library_tracks(self.lib, u"1=2"))
 
     @responses.activate
     def test_missing_request(self):
@@ -84,7 +84,7 @@ class SpotifyPluginTest(_common.TestCase, TestHelper):
             length=10,
         )
         item.add(self.lib)
-        self.assertEqual([], self.spotify._query_spotify(self.lib, u""))
+        self.assertEqual([], self.spotify._match_library_tracks(self.lib, u""))
 
         params = _params(responses.calls[0].request.url)
         query = params['q'][0]
@@ -116,10 +116,10 @@ class SpotifyPluginTest(_common.TestCase, TestHelper):
             length=10,
         )
         item.add(self.lib)
-        results = self.spotify._query_spotify(self.lib, u"Happy")
+        results = self.spotify._match_library_tracks(self.lib, u"Happy")
         self.assertEqual(1, len(results))
         self.assertEqual(u"6NPVjNh8Jhru9xOmyQigds", results[0]['id'])
-        self.spotify._output_results(results)
+        self.spotify._output_match_results(results)
 
         params = _params(responses.calls[0].request.url)
         query = params['q'][0]