Merge lp:~mterry/update-manager/toggle-fix into lp:update-manager

[Michael Terry]

-Refactors the toggle-an-item logic from two code paths down to one code path
-Introduce a convenience method is_selected() to cover a common question asked about a package
-Fixes bug 1129191 by dropping "auto_fix=False, auto_inst=False" when marking a package from install. These flags were inherited from the old "toggle_by_origin" method and seem to have been an optimization because we knew that all affected packages were from same PPA/origin and wouldn't need a fixer check. But we can no longer make that same assumption. Running the fixer makes the bug go away.

[Martin Pitt (pitti)]

I can reproduce the bug and confirm that this branch fixes it, so I merged it. Thanks! However, see the bug for more details, I can't upload it just yet.

review: Approve

=== modified file 'UpdateManager/Core/UpdateList.py'
--- UpdateManager/Core/UpdateList.py	2013-01-23 15:46:52 +0000
+++ UpdateManager/Core/UpdateList.py	2013-02-18 17:55:25 +0000
@@ -50,6 +50,9 @@
         self.name = name
         self.pkg = pkg
 
+    def is_selected(self):
+        return self.pkg.marked_install or self.pkg.marked_upgrade
+
 
 class UpdateGroup(UpdateItem):
     def __init__(self, pkg, name, icon):
@@ -101,13 +104,13 @@
 
     def packages_are_selected(self):
         for item in self.items:
-            if item.pkg.marked_install or item.pkg.marked_upgrade:
+            if item.is_selected():
                 return True
         return False
 
     def selection_is_inconsistent(self):
         pkgs_installing = [item for item in self.items
-            if item.pkg.marked_install or item.pkg.marked_upgrade]
+            if item.is_selected()]
         return (len(pkgs_installing) > 0 and
                 len(pkgs_installing) < len(self.items))
 

=== modified file 'UpdateManager/UpdatesAvailable.py'
--- UpdateManager/UpdatesAvailable.py	2013-01-29 21:14:59 +0000
+++ UpdateManager/UpdatesAvailable.py	2013-02-18 17:55:25 +0000
@@ -420,7 +420,7 @@
             for group in data.groups:
                 for item in group.items:
                     pkg = item.pkg
-                    this_install = pkg.marked_install or pkg.marked_upgrade
+                    this_install = item.is_selected()
                     if saw_install is not None and saw_install != this_install:
                         inconsistent = True
                         break
@@ -658,8 +658,7 @@
             data = model.get_value(iter, LIST_UPDATE_DATA)
             active = False
             if data.item:
-                active = data.item.pkg.marked_install or \
-                         data.item.pkg.marked_upgrade
+                active = data.item.is_selected()
             elif data.group:
                 active = data.group.packages_are_selected()
             elif data.groups:
@@ -871,32 +870,13 @@
         data = self.store.get_value(iter, LIST_UPDATE_DATA)
         # make sure that we don't allow to toggle deactivated updates
         # this is needed for the call by the row activation callback
-        if data.groups or data.group:
-            if data.groups:
-                self.toggle_from_groups(data.groups)
-            elif data.group:
-                self.toggle_from_groups([data.group])
-            self.updates_changed()
-            self.treeview_update.queue_draw()
-            return
-        if (data.item is None or data.item.pkg is None or
-                data.item.pkg.name in self.list.held_back):
-            return False
-        self.setBusy(True)
-        # update the cache
-        if data.item.pkg.marked_install or data.item.pkg.marked_upgrade:
-            data.item.pkg.mark_keep()
-            if self.cache._depcache.broken_count:
-                Fix = apt_pkg.ProblemResolver(self.cache._depcache)
-                Fix.resolve_by_keep()
+        if data.groups:
+            self.toggle_from_items([item for group in data.groups
+                                    for item in group.items])
+        elif data.group:
+            self.toggle_from_items(data.group.items)
         else:
-            try:
-                data.item.pkg.mark_install()
-            except SystemError:
-                pass
-        self.updates_changed()
-        self.treeview_update.queue_draw()
-        self.setBusy(False)
+            self.toggle_from_items([data.item])
 
     def on_treeview_update_row_activated(self, treeview, path, column, *args):
         """
@@ -905,18 +885,20 @@
         """
         self.on_update_toggled(None, path)
 
-    def toggle_from_groups(self, groups):
+    def toggle_from_items(self, items):
         self.setBusy(True)
         actiongroup = apt_pkg.ActionGroup(self.cache._depcache)
 
-        # Deselect all updates if any in group are selected
-        keep_packages = any([g.packages_are_selected() for g in groups])
-        for group in groups:
-            for item in group.items:
+        # Deselect all updates if any are selected
+        keep_packages = any([item.is_selected() for item in items])
+        for item in items:
+            try:
                 if keep_packages:
                     item.pkg.mark_keep()
                 elif not item.pkg.name in self.list.held_back:
-                    item.pkg.mark_install(auto_fix=False, auto_inst=False)
+                    item.pkg.mark_install()
+            except SystemError:
+                pass
 
         # check if we left breakage
         if self.cache._depcache.broken_count: