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

Proposed by Gary Lasker
Status: Merged
Merged at revision: 2985
Proposed branch: lp:~gary-lasker/software-center/add-to-launcher-after-auth-lp972710
Merge into: lp:software-center
Diff against target: 160 lines (+57/-25)
2 files modified
softwarecenter/backend/unitylauncher.py (+18/-2)
softwarecenter/ui/gtk3/panes/availablepane.py (+39/-23)
To merge this branch: bzr merge lp:~gary-lasker/software-center/add-to-launcher-after-auth-lp972710
Reviewer Review Type Date Requested Status
Michael Vogt Approve
Review via email: mp+102605@code.launchpad.net

Description of the change

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!

To post a comment you must log in.
Michael Vogt (mvo) wrote :

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) wrote :

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) wrote :

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) wrote :

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!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwarecenter/backend/unitylauncher.py'
2--- softwarecenter/backend/unitylauncher.py 2012-03-19 13:35:47 +0000
3+++ softwarecenter/backend/unitylauncher.py 2012-04-18 23:53:20 +0000
4@@ -16,13 +16,14 @@
5 # this program; if not, write to the Free Software Foundation, Inc.,
6 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
7
8+from gi.repository import GObject
9 import dbus
10 import logging
11
12 LOG = logging.getLogger(__name__)
13
14
15-class UnityLauncherInfo(object):
16+class UnityLauncherInfo(GObject.GObject):
17 """ Simple class to keep track of application details needed for
18 Unity launcher integration
19 """
20@@ -45,7 +46,22 @@
21 self.trans_id = trans_id
22
23
24-class UnityLauncher(object):
25+class TransactionDetails(GObject.GObject):
26+ """ Simple class to keep track of aptdaemon transaction details for
27+ use with the Unity launcher integration
28+ """
29+ def __init__(self,
30+ pkgname,
31+ appname,
32+ trans_id,
33+ trans_type):
34+ self.pkgname = pkgname
35+ self.appname = appname
36+ self.trans_id = trans_id
37+ self.trans_type = trans_type
38+
39+
40+class UnityLauncher(GObject.GObject):
41 """ Implements the integration between Software Center and the Unity
42 launcher
43 """
44
45=== modified file 'softwarecenter/ui/gtk3/panes/availablepane.py'
46--- softwarecenter/ui/gtk3/panes/availablepane.py 2012-04-13 11:30:48 +0000
47+++ softwarecenter/ui/gtk3/panes/availablepane.py 2012-04-18 23:53:20 +0000
48@@ -49,7 +49,8 @@
49 from softwarecenter.ui.gtk3.session.viewmanager import get_viewmanager
50 from softwarecenter.ui.gtk3.session.appmanager import get_appmanager
51 from softwarecenter.backend.unitylauncher import (UnityLauncher,
52- UnityLauncherInfo)
53+ UnityLauncherInfo,
54+ TransactionDetails)
55
56 LOG = logging.getLogger(__name__)
57
58@@ -108,7 +109,9 @@
59
60 # integrate with the Unity launcher
61 self.unity_launcher = UnityLauncher()
62-
63+ # keep track of applications that are queued to be added
64+ # to the Unity launcher
65+ self.unity_launcher_transaction_queue = {}
66 # flag to indicate whether applications should be added to the
67 # unity launcher when installed (this value is initialized by
68 # the config load in app.py)
69@@ -213,10 +216,14 @@
70 Gtk.Label(label=NavButtons.PURCHASE))
71
72 # install backend
73- self.backend.connect("transactions-changed",
74- self._on_transactions_changed)
75 self.backend.connect("transaction-started",
76 self.on_transaction_started)
77+ self.backend.connect("transactions-changed",
78+ self.on_transactions_changed)
79+ self.backend.connect("transaction-finished",
80+ self.on_transaction_complete)
81+ self.backend.connect("transaction-stopped",
82+ self.on_transaction_complete)
83
84 # now we are initialized
85 self.searchentry.set_sensitive(True)
86@@ -364,36 +371,44 @@
87 """ unset the current showing category, but keep e.g. the current
88 search
89 """
90-
91 self.state.category = None
92 self.state.subcategory = None
93
94- def _on_transactions_changed(self, *args):
95+ def on_transaction_started(self, backend, pkgname, appname, trans_id,
96+ trans_type):
97+ # we only care about installs for the launcher
98+ if trans_type == TransactionTypes.INSTALL:
99+ transaction_details = TransactionDetails(
100+ pkgname, appname, trans_id, trans_type)
101+ self.unity_launcher_transaction_queue[pkgname] = (
102+ transaction_details)
103+
104+ def on_transactions_changed(self, *args):
105 """internal helper that keeps the action bar up-to-date by
106 keeping track of the transaction-started signals
107 """
108 if self._is_custom_list_search(self.state.search_term):
109 self._update_action_bar()
110-
111- def on_transaction_started(self, backend, pkgname, appname, trans_id,
112- trans_type):
113- self._add_application_to_unity_launcher(
114- backend, pkgname, appname, trans_id, trans_type)
115-
116- def _add_application_to_unity_launcher(self, backend, pkgname,
117- appname, trans_id,
118- trans_type):
119+ for pkgname in args[1]:
120+ if pkgname in self.unity_launcher_transaction_queue:
121+ transaction_details = (
122+ self.unity_launcher_transaction_queue.pop(pkgname))
123+ self._add_application_to_unity_launcher(transaction_details)
124+
125+ def on_transaction_complete(self, backend, result):
126+ if result.pkgname in self.unity_launcher_transaction_queue:
127+ self.unity_launcher_transaction_queue.pop(result.pkgname)
128+
129+ def _add_application_to_unity_launcher(self, transaction_details):
130 if not self.add_to_launcher_enabled:
131 return
132- # mvo: use use softwarecenter.utils explictly so that we can monkey
133+ # mvo: use use softwarecenter.utils explicitly so that we can monkey
134 # patch it in the test
135 if not softwarecenter.utils.is_unity_running():
136 return
137- # we only care about installs
138- if not trans_type == TransactionTypes.INSTALL:
139- return
140
141- app = Application(pkgname=pkgname, appname=appname)
142+ app = Application(pkgname=transaction_details.pkgname,
143+ appname=transaction_details.appname)
144 appdetails = app.get_details(self.db)
145 # we only add items to the launcher that have a desktop file
146 if not appdetails.desktop_file:
147@@ -408,9 +423,10 @@
148 # now gather up the unity launcher info items and send the app to the
149 # launcher service
150 launcher_info = self._get_unity_launcher_info(app, appdetails,
151- trans_id)
152- self.unity_launcher.send_application_to_launcher(pkgname,
153- launcher_info)
154+ transaction_details.trans_id)
155+ self.unity_launcher.send_application_to_launcher(
156+ transaction_details.pkgname,
157+ launcher_info)
158
159 def _get_unity_launcher_info(self, app, appdetails, trans_id):
160 (icon_size, icon_x, icon_y) = (

Subscribers

People subscribed via source and target branches