Merge lp:~gary-lasker/software-center/add-to-launcher-after-auth-lp972710 into lp:software-center

[Gary Lasker]

This branch fixes bug 972710 where cancelling out of the authorization dialog for an application install leaves a (broken) icon in the Unity launcher. To fix this we now maintain a simple queue of pending installs, and we do not fire the add-to-launcher dbus signal to Unity until we get the very first transactions-changed event that corresponds to each package in the queue. In this way, the add-to-launcher animation does not occur until *after* the user has entered their authorization.

To test:

1. Choose any application and click to install (either from the list view or the details view)
2. When the auth window appears, simply cancel it.
3. Verify that the corresponding application icon is NOT found in the Unity launcher.

4. Click the install button again, but this time complete the authorizations step.
5. Verify that, within a few seconds, the icon animation occurs and the application icon is added to the Unity launcher (and shows the installation progress there).

Please also test with multiple simultaneous installs. Each will add the icon as soon as the corresponding transaction begins. The queue is maintained and cleared as each transaction completes (either finishes or is otherwise stopped).

The Unity launcher unit tests continue to work.

Many thanks for your review!

[Michael Vogt (mvo)]

Thanks, this looks fine and works great.

One tiny question/nitpick, I noticed that:
-class UnityLauncherInfo(object):
+class UnityLauncherInfo(GObject.GObject):

and also for TransactionDetails and UnityLauncher. It seems like we don't use properties/signals
or other gobject features so I was wondering about the change? Or am I simply overlooking something?

review: Approve

[Michael Vogt (mvo)]

And one more detail, in:
    def on_transactions_changed(self, *args):
I would prefer to have
    def on_transactions_changed(self, backend, pending_transactions):
here to make the code easier to read.

[Michael Vogt (mvo)]

Thanks for correcting my "explictly" typo along the way :)

I merged this branch with some small tweaks (" def on_transactions_changed(self, backend, pending_transactions):" some comments added to the code. Please re-merge and double check that it makes sense. I left the GObject for now, its a minor thing but feedback on it is still welcome.

[Gary Lasker (gary-lasker)]

Hi Michael, thanks a lot, the merge looks good. Definitely agreed on the "pending_transactions" and the GObject is not necessary, so I'll fix that now. Thanks again!

=== modified file 'softwarecenter/backend/unitylauncher.py'
--- softwarecenter/backend/unitylauncher.py	2012-03-19 13:35:47 +0000
+++ softwarecenter/backend/unitylauncher.py	2012-04-18 23:53:20 +0000
@@ -16,13 +16,14 @@
 # this program; if not, write to the Free Software Foundation, Inc.,
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
+from gi.repository import GObject
 import dbus
 import logging
 
 LOG = logging.getLogger(__name__)
 
 
-class UnityLauncherInfo(object):
+class UnityLauncherInfo(GObject.GObject):
     """ Simple class to keep track of application details needed for
         Unity launcher integration
     """
@@ -45,7 +46,22 @@
         self.trans_id = trans_id
 
 
-class UnityLauncher(object):
+class TransactionDetails(GObject.GObject):
+    """ Simple class to keep track of aptdaemon transaction details for
+        use with the Unity launcher integration
+    """
+    def __init__(self,
+                 pkgname,
+                 appname,
+                 trans_id,
+                 trans_type):
+        self.pkgname = pkgname
+        self.appname = appname
+        self.trans_id = trans_id
+        self.trans_type = trans_type
+
+
+class UnityLauncher(GObject.GObject):
     """ Implements the integration between Software Center and the Unity
         launcher
     """

=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
--- softwarecenter/ui/gtk3/panes/availablepane.py	2012-04-13 11:30:48 +0000
+++ softwarecenter/ui/gtk3/panes/availablepane.py	2012-04-18 23:53:20 +0000
@@ -49,7 +49,8 @@
 from softwarecenter.ui.gtk3.session.viewmanager import get_viewmanager
 from softwarecenter.ui.gtk3.session.appmanager import get_appmanager
 from softwarecenter.backend.unitylauncher import (UnityLauncher,
-                                                  UnityLauncherInfo)
+                                                  UnityLauncherInfo,
+                                                  TransactionDetails)
 
 LOG = logging.getLogger(__name__)
 
@@ -108,7 +109,9 @@
 
         # integrate with the Unity launcher
         self.unity_launcher = UnityLauncher()
-
+        # keep track of applications that are queued to be added
+        # to the Unity launcher
+        self.unity_launcher_transaction_queue = {}
         # flag to indicate whether applications should be added to the
         # unity launcher when installed (this value is initialized by
         # the config load in app.py)
@@ -213,10 +216,14 @@
             Gtk.Label(label=NavButtons.PURCHASE))
 
         # install backend
-        self.backend.connect("transactions-changed",
-            self._on_transactions_changed)
         self.backend.connect("transaction-started",
             self.on_transaction_started)
+        self.backend.connect("transactions-changed",
+            self.on_transactions_changed)
+        self.backend.connect("transaction-finished",
+            self.on_transaction_complete)
+        self.backend.connect("transaction-stopped",
+            self.on_transaction_complete)
 
         # now we are initialized
         self.searchentry.set_sensitive(True)
@@ -364,36 +371,44 @@
         """ unset the current showing category, but keep e.g. the current
             search
         """
-
         self.state.category = None
         self.state.subcategory = None
 
-    def _on_transactions_changed(self, *args):
+    def on_transaction_started(self, backend, pkgname, appname, trans_id,
+                               trans_type):
+        # we only care about installs for the launcher
+        if trans_type == TransactionTypes.INSTALL:
+            transaction_details = TransactionDetails(
+                    pkgname, appname, trans_id, trans_type)
+            self.unity_launcher_transaction_queue[pkgname] = (
+                    transaction_details)
+
+    def on_transactions_changed(self, *args):
         """internal helper that keeps the action bar up-to-date by
            keeping track of the transaction-started signals
         """
         if self._is_custom_list_search(self.state.search_term):
             self._update_action_bar()
-
-    def on_transaction_started(self, backend, pkgname, appname, trans_id,
-                               trans_type):
-        self._add_application_to_unity_launcher(
-                backend, pkgname, appname, trans_id, trans_type)
-
-    def _add_application_to_unity_launcher(self, backend, pkgname,
-                                           appname, trans_id,
-                                           trans_type):
+        for pkgname in args[1]:
+            if pkgname in self.unity_launcher_transaction_queue:
+                transaction_details = (
+                    self.unity_launcher_transaction_queue.pop(pkgname))
+                self._add_application_to_unity_launcher(transaction_details)
+
+    def on_transaction_complete(self, backend, result):
+        if result.pkgname in self.unity_launcher_transaction_queue:
+            self.unity_launcher_transaction_queue.pop(result.pkgname)
+
+    def _add_application_to_unity_launcher(self, transaction_details):
         if not self.add_to_launcher_enabled:
             return
-        # mvo: use use softwarecenter.utils explictly so that we can monkey
+        # mvo: use use softwarecenter.utils explicitly so that we can monkey
         #      patch it in the test
         if not softwarecenter.utils.is_unity_running():
             return
-        # we only care about installs
-        if not trans_type == TransactionTypes.INSTALL:
-            return
 
-        app = Application(pkgname=pkgname, appname=appname)
+        app = Application(pkgname=transaction_details.pkgname,
+                          appname=transaction_details.appname)
         appdetails = app.get_details(self.db)
         # we only add items to the launcher that have a desktop file
         if not appdetails.desktop_file:
@@ -408,9 +423,10 @@
         # now gather up the unity launcher info items and send the app to the
         # launcher service
         launcher_info = self._get_unity_launcher_info(app, appdetails,
-            trans_id)
-        self.unity_launcher.send_application_to_launcher(pkgname,
-            launcher_info)
+                transaction_details.trans_id)
+        self.unity_launcher.send_application_to_launcher(
+                transaction_details.pkgname,
+                launcher_info)
 
     def _get_unity_launcher_info(self, app, appdetails, trans_id):
         (icon_size, icon_x, icon_y) = (