[gnome-music/wip/jfelder/playlists-core-rewrite-prep-work: 5/11] playlistsview: Use the playlists managed by Playlists

[Jean Felder <jfelder src gnome org>]

commit a674ada2a7ebce9482d308d878321c30bd8a4c2a
Author: Jean Felder <jfelder src gnome org>
Date:   Mon May 13 16:47:51 2019 +0200

    playlistsview: Use the playlists managed by Playlists
    
    Instead of relying on grilo to populate the dialog with playlists, use
    the new API from Playlists class.
    
    It is based on some work done by Georges Basile Stavracas Neto.

 gnomemusic/grilo.py                    |   2 +-
 gnomemusic/playlists.py                |  72 +++++---------
 gnomemusic/views/playlistsview.py      | 169 ++++++++++++++-------------------
 gnomemusic/widgets/playlistcontrols.py |   4 +-
 4 files changed, 99 insertions(+), 148 deletions(-)
---
diff --git a/gnomemusic/grilo.py b/gnomemusic/grilo.py
index d089ecd8..9b26dbd4 100644
--- a/gnomemusic/grilo.py
+++ b/gnomemusic/grilo.py
@@ -327,7 +327,7 @@ class Grilo(GObject.GObject):
     @log
     def populate_playlist_songs(self, playlist, callback, count=-1):
         self.populate_items(
-            Query.playlist_songs(playlist.get_id()), 0, callback, count)
+            Query.playlist_songs(playlist.props.pl_id), 0, callback, count)
 
     @log
     def populate_custom_query(self, query, callback, count=-1, data=None):
diff --git a/gnomemusic/playlists.py b/gnomemusic/playlists.py
index 857e51f7..ecf7d9a8 100644
--- a/gnomemusic/playlists.py
+++ b/gnomemusic/playlists.py
@@ -162,13 +162,9 @@ class Playlists(GObject.GObject):
         'playlist-created': (
             GObject.SignalFlags.RUN_FIRST, None, (Grl.Media,)
         ),
-        'playlist-deleted': (
-            GObject.SignalFlags.RUN_FIRST, None, (str,)
-        ),
-        "playlist-updated": (GObject.SignalFlags.RUN_FIRST, None, (str,)),
-        'playlist-renamed': (
-            GObject.SignalFlags.RUN_FIRST, None, (Grl.Media,)
-        ),
+        "playlist-deleted": (GObject.SignalFlags.RUN_FIRST, None, (str,)),
+        "playlist-updated": (GObject.SignalFlags.RUN_FIRST, None, (Playlist,)),
+        "playlist-renamed": (GObject.SignalFlags.RUN_FIRST, None, (Playlist,)),
         'song-added-to-playlist': (
             GObject.SignalFlags.RUN_FIRST, None, (Playlist, Grl.Media)
         ),
@@ -202,7 +198,7 @@ class Playlists(GObject.GObject):
         }
         self._playlists_model = Gio.ListStore.new(Playlist)
 
-        self._pls_todelete = {}
+        self._pls_todelete = []
 
         self._loading_counter = len(self._smart_playlists)
         self._user_playlists_ready = False
@@ -457,38 +453,34 @@ class Playlists(GObject.GObject):
             update_callback, None)
 
     @log
-    def rename(self, item, new_name):
+    def rename(self, playlist, new_name):
         """Rename a playlist
 
-        :param item: playlist to rename
-        :param new_name: new playlist name
-        :type item: Grl.Media
-        :type new_name: str
-        :return: None
-        :rtype: None
+        :param Playlist item: playlist to rename
+        :param str new_name: new playlist name
         """
         def update_callback(conn, res, data):
             conn.update_finish(res)
-            self.emit('playlist-renamed', item)
+            self.emit("playlist-renamed", playlist)
 
+        query = Query.rename_playlist(playlist.props.pl_id, new_name)
         self._tracker.update_async(
-            Query.rename_playlist(item.get_id(), new_name), GLib.PRIORITY_LOW,
-            None, update_callback, None)
+            query, GLib.PRIORITY_LOW, None, update_callback, None)
 
     @log
-    def delete_playlist(self, item_id):
+    def delete_playlist(self, playlist):
         """Deletes a user playlist
 
-        :param str item_id: Playlist id to delete
+        :param Playlist playlist: Playlist to delete
         """
         def update_callback(conn, res, data):
             conn.update_finish(res)
-            self.emit("playlist-deleted", item_id)
+            self.emit("playlist-deleted", playlist.props.pl_id)
 
-        self._pls_todelete.pop(item_id)
+        self._pls_todelete.remove(playlist)
+        query = Query.delete_playlist(playlist.props.pl_id)
         self._tracker.update_async(
-            Query.delete_playlist(item_id), GLib.PRIORITY_LOW,
-            None, update_callback, None)
+            query, GLib.PRIORITY_LOW, None, update_callback, None)
 
     @log
     def add_to_playlist(self, playlist, items):
@@ -528,25 +520,24 @@ class Playlists(GObject.GObject):
         def update_callback(conn, res, data):
             conn.update_finish(res)
 
-        playlist_id = playlist.get_id()
         for item in items:
             item_id = item.get_id()
             self._tracker.update_async(
-                Query.remove_song_from_playlist(playlist_id, item_id),
+                Query.remove_song_from_playlist(playlist.props.pl_id, item_id),
                 GLib.PRIORITY_LOW, None, update_callback, item)
 
     @log
     def reorder_playlist(self, playlist, items, new_positions):
         """Change the order of songs on a playlist.
 
-        :param GlrMedia playlist: playlist to reorder
+        :param Playlist playlist: playlist to reorder
         :param list items: songs to reorder
         :param list new_positions: new songs positions
         """
         def update_callback(conn, res, data):
             conn.update_finish(res)
 
-        playlist_id = playlist.get_id()
+        playlist_id = playlist.props.pl_id
         for item, new_position in zip(items, new_positions):
             item_id = item.get_id()
             self._tracker.update_async(
@@ -565,7 +556,7 @@ class Playlists(GObject.GObject):
     @log
     def get_user_playlists(self):
         def user_playlists_filter(playlist):
-            return (playlist.props.pl_id not in self._pls_todelete.keys()
+            return (playlist not in self._pls_todelete
                     and not playlist.props.is_smart)
 
         model_filter = Dazzle.ListModelFilter.new(self._playlists_model)
@@ -610,32 +601,21 @@ class Playlists(GObject.GObject):
     def stage_playlist_for_deletion(self, playlist, index):
         """Adds a playlist to the list of playlists to delete
 
-        :param Grl.Media playlist: playlist to delete
+        :param Playlist playlist: playlist to delete
         :param int index: Playlist position in PlaylistView
         """
-        playlist_id = playlist.get_id()
-        self._pls_todelete[playlist_id] = {
-            "playlist": playlist,
-            "index": index
-        }
+        self._pls_todelete.append(playlist)
         self._playlists_model.remove(index)
 
     @log
     def undo_pending_deletion(self, playlist):
         """Undo pending playlist deletion
 
-        :param Grl.Media playlist: playlist to restore
-        :returns: playlist previous index
-        :rtype: int
+        :param Playlist playlist: playlist to restore
         """
-        playlist_id = playlist.get_id()
-        index = self._pls_todelete[playlist_id]["index"]
-        self._pls_todelete.pop(playlist_id)
-        playlist = Playlist(
-            pl_id=playlist_id, title=utils.get_media_title(playlist))
-        self._playlists_model.insert(index, playlist)
-
-        return index
+        self._pls_todelete.remove(playlist)
+        self._playlists_model.insert_sorted(
+            playlist, Playlist.compare_playlist_func)
 
     @GObject.Property(
         type=bool, default=False, flags=GObject.ParamFlags.READABLE)
diff --git a/gnomemusic/views/playlistsview.py b/gnomemusic/views/playlistsview.py
index bffd9b4f..7c1fb247 100644
--- a/gnomemusic/views/playlistsview.py
+++ b/gnomemusic/views/playlistsview.py
@@ -125,13 +125,19 @@ class PlaylistsView(BaseView):
         self.player.connect('song-validated', self._on_song_validated)
 
         self._playlists = Playlists.get_default()
-        self._playlists.connect("playlist-created", self._on_playlist_created)
+        self._playlists.connect("notify::ready", self._on_playlists_loading)
         self._playlists.connect("playlist-updated", self._on_playlist_update)
         self._playlists.connect(
             "song-added-to-playlist", self._on_song_added_to_playlist)
         self._playlists.connect(
             "activate-playlist", self._on_playlist_activation_request)
 
+        self._playlists_model = self._playlists.get_playlists()
+        self._sidebar.bind_model(
+            self._playlists_model, self._add_playlist_to_sidebar)
+        self._playlists_model.connect(
+            "items-changed", self._on_playlists_model_changed)
+
         self.show_all()
 
     @log
@@ -226,8 +232,9 @@ class PlaylistsView(BaseView):
             cell.set_property('text', utils.get_album_title(item))
 
     def _on_list_widget_icon_render(self, col, cell, model, _iter, data):
+        playlist_id = self._current_playlist.props.pl_id
         if not self.player.playing_playlist(
-                PlayerPlaylist.Type.PLAYLIST, self._current_playlist.get_id()):
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
             cell.set_visible(False)
             return
 
@@ -253,10 +260,11 @@ class PlaylistsView(BaseView):
         if self._current_playlist is None:
             return
 
+        playlist_id = self._current_playlist.props.pl_id
         if self._iter_to_clean:
             self._iter_to_clean_model[self._iter_to_clean][10] = False
         if not player.playing_playlist(
-                PlayerPlaylist.Type.PLAYLIST, self._current_playlist.get_id()):
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
             return False
 
         index = self.player.props.current_song_index
@@ -271,63 +279,38 @@ class PlaylistsView(BaseView):
         return False
 
     @log
-    def _add_playlist_item(
-            self, source, param, playlist, remaining=0,
-            select_playlist_id=None):
-        """Grilo.populate_playlists callback.
-
-        Add all playlists found by Grilo to sidebar
-
-        :param GrlTrackerSource source: tracker source
-        :param int param: param
-        :param GrlMedia playlist: playlist to add
-        :param int remaining: next playlist_id or zero if None
-        :param str select_playlist_id: playlist id to select on load
-        """
-        if not playlist:
-            self._window.notifications_popup.pop_loading()
-            if not self._sidebar.get_selected_row():
-                first_row = self._sidebar.get_row_at_index(0)
-                self._sidebar.select_row(first_row)
-                first_row.emit('activate')
-            return
-
-        select_playlist = (playlist.get_id() == select_playlist_id)
-        self._add_playlist_to_sidebar(playlist, None, select_playlist)
-
-    @log
-    def _add_playlist_to_sidebar(
-            self, playlist, index=None, select_playlist=False):
+    def _add_playlist_to_sidebar(self, playlist):
         """Add a playlist to sidebar
 
         :param GrlMedia playlist: playlist to add
         :param int index: position
         """
-        if index is None:
-            index = -1
-        if self._playlists.is_smart_playlist(playlist):
-            index = 0
-
-        title = utils.get_media_title(playlist)
         row = SidebarRow()
-        row.props.text = title
-        # FIXME: Passing the Grl.Media with the row object is ugly.
+        row.props.text = playlist.props.title
+        # FIXME: Passing the Playlist with the row object is ugly.
         row.playlist = playlist
 
-        self._sidebar.insert(row, index)
-        self._offset += 1
+        return row
 
-        if select_playlist:
-            self._sidebar.select_row(row)
-            row.emit('activate')
+    def _on_playlists_model_changed(self, model, position, removed, added):
+        # select the next row when a playlist is deleted
+        if removed == 0:
+            return
+
+        row_next = (self._sidebar.get_row_at_index(position)
+                    or self._sidebar.get_row_at_index(position - 1))
+        if row_next:
+            self._sidebar.select_row(row_next)
+            row_next.emit("activate")
 
     @log
     def _on_song_validated(self, player, index, status):
         if self._current_playlist is None:
             return
 
+        playlist_id = self._current_playlist.props.pl_id
         if not self.player.playing_playlist(
-                PlayerPlaylist.Type.PLAYLIST, self._current_playlist.get_id()):
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
             return
 
         iter_ = self.model.get_iter_from_string(str(index))
@@ -356,7 +339,7 @@ class PlaylistsView(BaseView):
             _iter = None
             if path:
                 _iter = self.model.get_iter(path)
-            playlist_id = self._current_playlist.get_id()
+            playlist_id = self._current_playlist.props.pl_id
             self.player.set_playlist(
                 PlayerPlaylist.Type.PLAYLIST, playlist_id, self.model, _iter)
             self.player.play()
@@ -415,8 +398,9 @@ class PlaylistsView(BaseView):
         last_pos = max(new_pos, prev_pos)
 
         # update player's playlist if necessary
+        playlist_id = self._current_playlist.props.pl_id
         if self.player.playing_playlist(
-                PlayerPlaylist.Type.PLAYLIST, self._current_playlist.get_id()):
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
             if new_pos < prev_pos:
                 prev_pos -= 1
             else:
@@ -471,17 +455,28 @@ class PlaylistsView(BaseView):
         self._create_notification(PlaylistNotification.Type.SONG, song_id)
 
     @log
-    def _on_playlist_update(self, playlists, playlist_id):
+    def _on_playlists_loading(self, klass, value):
+        if not self._playlists.props.ready:
+            self._window.notifications_popup.push_loading()
+        else:
+            self._window.notifications_popup.pop_loading()
+            first_row = self._sidebar.get_row_at_index(0)
+            self._sidebar.select_row(first_row)
+            first_row.emit("activate")
+
+    @log
+    def _on_playlist_update(self, playlists, playlist):
         """Refresh the displayed playlist if necessary
 
-        :param playlists: playlists
-        :param playlist_id: updated playlist's id
+        :param playlists: playlists object
+        :param Playlist playlist: updated playlist
         """
+        if not self._is_current_playlist(playlist):
+            return
+
+        self._star_handler.star_renderer_click = False
         for row in self._sidebar:
-            playlist = row.playlist
-            if (str(playlist_id) == playlist.get_id()
-                    and self._is_current_playlist(playlist)):
-                self._star_handler.star_renderer_click = False
+            if playlist == row.playlist:
                 self._on_playlist_activated(self._sidebar, row)
                 break
 
@@ -503,7 +498,7 @@ class PlaylistsView(BaseView):
 
         playlist_row = None
         for row in self._sidebar:
-            if row.playlist.get_id() == playlist_id:
+            if row.playlist.props.pl_id == playlist_id:
                 playlist_row = row
                 break
 
@@ -521,7 +516,7 @@ class PlaylistsView(BaseView):
     @log
     def remove_playlist(self):
         """Removes the current selected playlist"""
-        if self._playlists.is_smart_playlist(self._current_playlist):
+        if self._current_playlist.props.is_smart:
             return
         self._stage_playlist_for_deletion(None)
 
@@ -529,7 +524,7 @@ class PlaylistsView(BaseView):
     def _on_playlist_activated(self, sidebar, row, data=None):
         """Update view with content from selected playlist"""
         playlist = row.playlist
-        playlist_name = utils.get_media_title(playlist)
+        playlist_name = playlist.props.title
 
         if self.rename_active:
             self._pl_ctrls.disable_rename_playlist()
@@ -547,8 +542,7 @@ class PlaylistsView(BaseView):
         self._update_songs_count(0)
         grilo.populate_playlist_songs(playlist, self._add_song)
 
-        protected_pl = self._playlists.is_smart_playlist(
-            self._current_playlist)
+        protected_pl = self._current_playlist.props.is_smart
         self._playlist_delete_action.set_enabled(not protected_pl)
         self._playlist_rename_action.set_enabled(not protected_pl)
         self._remove_song_action.set_enabled(not protected_pl)
@@ -574,7 +568,7 @@ class PlaylistsView(BaseView):
                 first_iter = self.model.get_iter_first()
                 self.player.set_playlist(
                     PlayerPlaylist.Type.PLAYLIST,
-                    self._current_playlist.get_id(), self.model, first_iter)
+                    self._current_playlist.props.pl_id, self.model, first_iter)
                 self.player.play()
                 self._plays_songs_on_activation = False
 
@@ -607,7 +601,7 @@ class PlaylistsView(BaseView):
         if self._current_playlist is None:
             return False
 
-        return playlist.get_id() == self._current_playlist.get_id()
+        return playlist.props.pl_id == self._current_playlist.props.pl_id
 
     @log
     def _get_removal_notification_message(self, type_, data):
@@ -621,13 +615,12 @@ class PlaylistsView(BaseView):
 
         if type_ == PlaylistNotification.Type.PLAYLIST:
             pl_todelete = data
-            playlist_title = utils.get_media_title(pl_todelete)
-            msg = _("Playlist {} removed".format(playlist_title))
+            msg = _("Playlist {} removed".format(pl_todelete.props.title))
 
         else:
             song_id = data
             song_todelete = self._songs_todelete[song_id]
-            playlist_title = utils.get_media_title(song_todelete['playlist'])
+            playlist_title = song_todelete["playlist"].props.title
             song_title = utils.get_media_title(song_todelete['song'])
             msg = _("{} removed from {}".format(
                 song_title, playlist_title))
@@ -649,21 +642,14 @@ class PlaylistsView(BaseView):
         self.model.clear()
         selection = self._sidebar.get_selected_row()
         index = selection.get_index()
-        playlist_id = self._current_playlist.get_id()
+        playlist_id = selection.playlist.props.pl_id
         self._playlists.stage_playlist_for_deletion(selection.playlist, index)
-        row_next = (self._sidebar.get_row_at_index(index + 1)
-                    or self._sidebar.get_row_at_index(index - 1))
-        self._sidebar.remove(selection)
 
         if self.player.playing_playlist(
                 PlayerPlaylist.Type.PLAYLIST, playlist_id):
             self.player.stop()
             self._window.set_player_visible(False)
 
-        if row_next:
-            self._sidebar.select_row(row_next)
-            row_next.emit('activate')
-
         self._create_notification(
             PlaylistNotification.Type.PLAYLIST, selection.playlist)
 
@@ -674,8 +660,7 @@ class PlaylistsView(BaseView):
 
         if notification_type == PlaylistNotification.Type.PLAYLIST:
             pl_todelete = playlist_notification.data
-            index = self._playlists.undo_pending_deletion(pl_todelete)
-            self._add_playlist_to_sidebar(pl_todelete, index)
+            self._playlists.undo_pending_deletion(pl_todelete)
 
         else:
             song_id = playlist_notification.data
@@ -687,7 +672,7 @@ class PlaylistsView(BaseView):
             iter_ = self._add_song_to_model(
                 song_todelete['song'], self.model, song_todelete['index'])
 
-            playlist_id = self._current_playlist.get_id()
+            playlist_id = self._current_playlist.props.pl_id
             if not self.player.playing_playlist(
                     PlayerPlaylist.Type.PLAYLIST, playlist_id):
                 return
@@ -701,7 +686,7 @@ class PlaylistsView(BaseView):
 
         if notification_type == PlaylistNotification.Type.PLAYLIST:
             pl_todelete = playlist_notification.data
-            self._playlists.delete_playlist(pl_todelete.get_id())
+            self._playlists.delete_playlist(pl_todelete)
         else:
             song_id = playlist_notification.data
             song_todelete = self._songs_todelete[song_id]
@@ -726,38 +711,29 @@ class PlaylistsView(BaseView):
         selection.props.text = new_name
 
         pl_torename = selection.playlist
-        pl_torename.set_title(new_name)
+        pl_torename.props.title = new_name
         self._playlists.rename(pl_torename, new_name)
 
     @log
-    def _on_playlist_created(self, playlists, playlist):
-        """Adds new playlist to sidebar
-
-        If the sidebar has not been populated yet, it has no effect:
-        the playlist will be displayed once the playlists are loaded.
-        """
-        if not self._init:
+    def _on_song_added_to_playlist(self, playlists, playlist, item):
+        if not self._is_current_playlist(playlist):
             return
-        self._add_playlist_to_sidebar(playlist)
 
-    @log
-    def _on_song_added_to_playlist(self, playlists, playlist, item):
-        if (self._current_playlist
-                and playlist.props.pl_id != self._current_playlist.get_id()):
-            iter_ = self._add_song_to_model(item, self.model)
-            playlist_id = self._current_playlist.get_id()
-            if self.player.playing_playlist(
-                    PlayerPlaylist.Type.PLAYLIST, playlist_id):
-                path = self.model.get_path(iter_)
-                self.player.add_song(item, int(path.to_string()))
+        iter_ = self._add_song_to_model(item, self.model)
+        playlist_id = self._current_playlist.props.pl_id
+        if self.player.playing_playlist(
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
+            path = self.model.get_path(iter_)
+            self.player.add_song(item, int(path.to_string()))
 
     @log
     def _remove_song_from_playlist(self, playlist, item, index):
         if not self._is_current_playlist(playlist):
             return
 
+        playlist_id = self._current_playlist.props.pl_id
         if self.player.playing_playlist(
-                PlayerPlaylist.Type.PLAYLIST, self._current_playlist.get_id()):
+                PlayerPlaylist.Type.PLAYLIST, playlist_id):
             self.player.remove_song(index)
 
         iter_ = self.model.get_iter_from_string(str(index))
@@ -770,7 +746,4 @@ class PlaylistsView(BaseView):
         """Populate sidebar.
         Do not reload playlists already displayed.
         """
-        self._window.notifications_popup.push_loading()
-        grilo.populate_playlists(
-            self._offset, self._add_playlist_item, -1, data)
         self._init = True
diff --git a/gnomemusic/widgets/playlistcontrols.py b/gnomemusic/widgets/playlistcontrols.py
index a13a2ff4..014b2943 100644
--- a/gnomemusic/widgets/playlistcontrols.py
+++ b/gnomemusic/widgets/playlistcontrols.py
@@ -27,7 +27,6 @@ import gettext
 from gi.repository import Gdk, GObject, Gtk
 
 from gnomemusic import log
-import gnomemusic.utils as utils
 
 
 @Gtk.Template(resource_path='/org/gnome/Music/ui/PlaylistControls.ui')
@@ -99,8 +98,7 @@ class PlaylistControls(Gtk.Grid):
         :param Grl.Media pl_torename : The playlist to rename
         """
         self._name_stack.props.visible_child_name = "renaming_dialog"
-        self._set_rename_entry_text_and_focus(
-            utils.get_media_title(pl_torename))
+        self._set_rename_entry_text_and_focus(pl_torename.props.title)
 
     @log
     def disable_rename_playlist(self):