Merge lp://qastaging/~mikemc/ubuntuone-control-panel/fix-sync-status into lp://qastaging/ubuntuone-control-panel

Proposed by Mike McCracken
Status: Merged
Approved by: Alejandro J. Cura
Approved revision: 359
Merged at revision: 356
Proposed branch: lp://qastaging/~mikemc/ubuntuone-control-panel/fix-sync-status
Merge into: lp://qastaging/ubuntuone-control-panel
Diff against target: 313 lines (+92/-56)
9 files modified
ubuntuone/controlpanel/backend.py (+21/-17)
ubuntuone/controlpanel/dbus_service.py (+1/-2)
ubuntuone/controlpanel/dbustests/test_dbus_service.py (+12/-25)
ubuntuone/controlpanel/gui/qt/filesyncstatus.py (+1/-1)
ubuntuone/controlpanel/gui/qt/systray.py (+1/-1)
ubuntuone/controlpanel/gui/qt/tests/__init__.py (+1/-0)
ubuntuone/controlpanel/gui/qt/tests/test_filesyncstatus.py (+4/-3)
ubuntuone/controlpanel/gui/qt/tests/test_systray.py (+4/-4)
ubuntuone/controlpanel/tests/test_backend.py (+47/-3)
To merge this branch: bzr merge lp://qastaging/~mikemc/ubuntuone-control-panel/fix-sync-status
Reviewer Review Type Date Requested Status
Alejandro J. Cura (community) Approve
Roberto Alsina (community) Approve
Review via email: mp+122974@code.qastaging.launchpad.net

Commit message

- Fix sync indicator in control panel main window. (LP: #1044197)

Description of the change

- Fix sync indicator in control panel main window. (LP: #1044197)

Added support in controlbackend for multiple status handlers.

Both the filsyncstatus.py widget in the main window and the menu item
in systray.py try to set the handler. This results in the
filesyncstatus widget no longer being updated, as the single variable
holding the callback in controlbackend is overwritten.

The issue does not appear on linux, because although the
controlbackend only has one variable, when it is changed, it wraps the
new callback and calls sd_client.set_status_changed_handler() with
that wrapper. On linux, the dbus implementation of the proxy that
sd_client uses *ADDS* the wrapper, while on the other platforms the
twisted implementation just overwrites the old handler.

This branch changes it so that we only ever set the sd_client callback
once, to a function in controlbackend that calls each of a list of
callbacks.

The filesyncstatus and systray widgets now call an
add_status_changed_handler function that adds to that list instead of
setting the single callback via a property.

Tests are included that use the new API, but would break if the new
API behaved the same as the old one.

IRL test shows this working on darwin. I had great difficulty getting
windows running from source, so I didn't do an IRL test on windows.

To post a comment you must log in.
Revision history for this message
Roberto Alsina (ralsina) wrote :

Looks good to me!

review: Approve
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Looks great. I requested on IRC a small removal, will approve when that gets pushed.

review: Needs Fixing
359. By Mike McCracken

remove unnecessary property for accessing status_changed_handlers

Revision history for this message
Alejandro J. Cura (alecu) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches