Merge pull request #2389 from zas/collections_cleanup

Collections cleanup

picard/collection.py

@@ -36,77 +36,87 @@ user_collections = {}
 
 
 class Collection(QtCore.QObject):
-    COLLECTION_ADD = 1
-    COLLECTION_REMOVE = 2
 
-    def __init__(self, collection_id, name, size):
+    def __init__(self, collection_id):
         self.id = collection_id
-        self.name = name
-        self.pending = set()
-        self.size = int(size)
+        self.name = ''
+        self.size = 0
+        self.pending_releases = set()
         self.releases = set()
-        mb_api = self.tagger.mb_api
-        self.api_action = {
-            self.COLLECTION_ADD: mb_api.put_to_collection,
-            self.COLLECTION_REMOVE: mb_api.delete_from_collection,
-        }
+
+    @property
+    def size(self):
+        return self._size
+
+    @size.setter
+    def size(self, value):
+        self._size = int(value)
 
     def __repr__(self):
         return '<Collection %s (%s)>' % (self.name, self.id)
 
-    def _modify(self, kind, ids, callback):
-        ids -= self.pending
-        if ids:
-            self.pending |= ids
-            when_done = partial(self._finished, kind, ids, callback)
-            self.api_action[kind](self.id, list(ids), when_done)
+    def _modify(self, api_method, success_handler, releases, callback):
+        releases -= self.pending_releases
+        if releases:
+            self.pending_releases |= releases
+            when_done = partial(self._finished, success_handler, releases, callback)
+            api_method(self.id, list(releases), when_done)
 
-    def add_releases(self, ids, callback):
-        self._modify(self.COLLECTION_ADD, ids, callback)
+    def add_releases(self, releases, callback):
+        api_method = self.tagger.mb_api.put_to_collection
+        self._modify(api_method, self._success_add, releases, callback)
 
-    def remove_releases(self, ids, callback):
-        self._modify(self.COLLECTION_REMOVE, ids, callback)
+    def remove_releases(self, releases, callback):
+        api_method = self.tagger.mb_api.delete_from_collection
+        self._modify(api_method, self._success_remove, releases, callback)
 
-    def _finished(self, kind, ids, callback, document, reply, error):
-        self.pending -= ids
-        statusbar = self.tagger.window.set_statusbar_message
+    def _finished(self, success_handler, releases, callback, document, reply, error):
+        self.pending_releases -= releases
         if not error:
-            count = len(ids)
-            if kind == self.COLLECTION_ADD:
-                self.releases |= ids
-                self.size += count
-                status_msg = ngettext(
-                    'Added %(count)i release to collection "%(name)s"',
-                    'Added %(count)i releases to collection "%(name)s"',
-                    count)
-                debug_msg = 'Added %(count)i releases to collection "%(name)s"'
-            else:
-                self.releases -= ids
-                self.size -= count
-                status_msg = ngettext(
-                    'Removed %(count)i release from collection "%(name)s"',
-                    'Removed %(count)i releases from collection "%(name)s"',
-                    count)
-                debug_msg = 'Removed %(count)i releases from collection "%(name)s"'
-            callback()
-            mparms = {'count': count, 'name': self.name}
-            log.debug(debug_msg % mparms)
-            statusbar(status_msg, mparms, translate=None, echo=None)
+            success_handler(releases, callback)
         else:
-            statusbar(
-                N_("Error while modifying collections: %(error)s"),
-                {'error': reply.errorString()},
-                echo=log.error
-            )
+            self._error(reply)
+
+    def _error(self, reply):
+        self.tagger.window.set_statusbar_message(
+            N_("Error while modifying collections: %(error)s"),
+            {'error': reply.errorString()},
+            echo=log.error
+        )
+
+    def _success_add(self, releases, callback):
+        count = len(releases)
+        self.releases |= releases
+        self.size += count
+        status_msg = ngettext(
+            'Added %(count)i release to collection "%(name)s"',
+            'Added %(count)i releases to collection "%(name)s"',
+            count)
+        debug_msg = 'Added %(count)i release(s) to collection "%(name)s"'
+        self._success(count, callback, status_msg, debug_msg)
+
+    def _success_remove(self, releases, callback):
+        count = len(releases)
+        self.releases -= releases
+        self.size -= count
+        status_msg = ngettext(
+            'Removed %(count)i release from collection "%(name)s"',
+            'Removed %(count)i releases from collection "%(name)s"',
+            count)
+        debug_msg = 'Removed %(count)i release(s) from collection "%(name)s"'
+        self._success(count, callback, status_msg, debug_msg)
+
+    def _success(self, count, callback, status_msg, debug_msg):
+        callback()
+        mparms = {'count': count, 'name': self.name}
+        log.debug(debug_msg % mparms)
+        self.tagger.window.set_statusbar_message(status_msg, mparms, translate=None, echo=None)
 
 
-def get_user_collection(collection_id, name, size, refresh=False):
+def get_user_collection(collection_id):
     collection = user_collections.get(collection_id)
     if collection is None:
-        collection = user_collections[collection_id] = Collection(collection_id, name, size)
-    elif refresh:
-        collection.name = name
-        collection.size = size
+        collection = user_collections[collection_id] = Collection(collection_id)
     return collection
 
 
@@ -129,10 +139,10 @@ def load_user_collections(callback=None):
                 if node['entity-type'] != 'release':
                     continue
                 col_id = node['id']
-                col_name = node['name']
-                col_size = node['release-count']
                 new_collections.add(col_id)
-                get_user_collection(col_id, col_name, col_size, refresh=True)
+                collection = get_user_collection(col_id)
+                collection.name = node['name']
+                collection.size = node['release-count']
 
             # remove collections which aren't returned by the web service anymore
             old_collections = set(user_collections) - new_collections
@@ -158,9 +168,9 @@ def add_release_to_user_collections(release_node):
         username = config.persist['oauth_username'].lower()
         for node in release_node['collections']:
             if node['editor'].lower() == username:
-                col_id = node['id']
-                col_name = node['name']
-                col_size = node['release-count']
-                collection = get_user_collection(col_id, col_name, col_size)
+                collection = get_user_collection(node['id'])
+                collection.name = node['name']
+                collection.size = node['release-count']
+
                 collection.releases.add(release_id)
                 log.debug("Adding release %r to %r", release_id, collection)

picard/ui/collectionmenu.py

@@ -40,7 +40,7 @@ class CollectionMenu(QtWidgets.QMenu):
 
     def __init__(self, albums, *args):
         super().__init__(*args)
-        self.ids = set(a.id for a in albums)
+        self.releases = set(a.id for a in albums)
         self._ignore_update = False
         self.update_collections()
 
@@ -145,8 +145,8 @@ class CollectionCheckBox(QtWidgets.QCheckBox):
         self.collection = collection
         super().__init__(self.label(), parent)
 
-        releases = collection.releases & menu.ids
-        if len(releases) == len(menu.ids):
+        releases = collection.releases & menu.releases
+        if len(releases) == len(menu.releases):
             self.setCheckState(QtCore.Qt.CheckState.Checked)
         elif not releases:
             self.setCheckState(QtCore.Qt.CheckState.Unchecked)
@@ -154,15 +154,15 @@ class CollectionCheckBox(QtWidgets.QCheckBox):
             self.setCheckState(QtCore.Qt.CheckState.PartiallyChecked)
 
     def nextCheckState(self):
-        ids = self.menu.ids
-        if ids & self.collection.pending:
+        releases = self.menu.releases
+        if releases & self.collection.pending_releases:
             return
-        diff = ids - self.collection.releases
+        diff = releases - self.collection.releases
         if diff:
             self.collection.add_releases(diff, self.updateText)
             self.setCheckState(QtCore.Qt.CheckState.Checked)
         else:
-            self.collection.remove_releases(ids & self.collection.releases, self.updateText)
+            self.collection.remove_releases(releases & self.collection.releases, self.updateText)
             self.setCheckState(QtCore.Qt.CheckState.Unchecked)
 
     def updateText(self):