Ubuntu Software Center

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

add-to-launcher-after-auth-lp972710
Merge into trunk

Proposed by Gary Lasker on 2012-04-18

Status:	Merged
Merged at revision:	2985
Proposed branch:	lp://staging/~gary-lasker/software-center/add-to-launcher-after-auth-lp972710
Merge into:	lp://staging/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://staging/~gary-lasker/software-center/add-to-launcher-after-auth-lp972710
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Vogt (community)		2012-04-18	Approve on 2012-04-19
Review via email: mp+102605@code.staging.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!

Revision history for this message

Michael Vogt (mvo) wrote on 2012-04-19:

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

Revision history for this message

Michael Vogt (mvo) wrote on 2012-04-19:

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.

Revision history for this message

Michael Vogt (mvo) wrote on 2012-04-19:

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.

Revision history for this message

Gary Lasker (gary-lasker) wrote on 2012-04-19:

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!