Merge lp:~manishsinha/software-properties/fix-874766-updates-tab-failed-auth into lp:software-properties

Status: Merged
Merged at revision: 833
Proposed branch: lp:~manishsinha/software-properties/fix-874766-updates-tab-failed-auth
Merge into: lp:software-properties
Diff against target: 233 lines (+70/-49)
1 file modified
softwareproperties/gtk/SoftwarePropertiesGtk.py (+70/-49)
To merge this branch: bzr merge lp:~manishsinha/software-properties/fix-874766-updates-tab-failed-auth
Reviewer Review Type Date Requested Status
Michael Vogt (community) Approve
Review via email: mp+148924@code.launchpad.net

Description of the change

Fixes LP #874766 where after a failed authentication on Updates tab didn't revert the older values in ComboBox

To post a comment you must log in.
834. By Manish Sinha (मनीष सिन्हा)

Fixed the authentication issue in Security Update ComboBox

835. By Manish Sinha (मनीष सिन्हा)

Fixed the issue for release policy too

836. By Manish Sinha (मनीष सिन्हा)

Put back the line removed by mistake

The line was
if e._dbus_error_name == 'com.ubuntu.SoftwareProperties.PermissionDeniedByPolicy':

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks a lot for this branch (and sorry for my slow reply).

This looks mostly good, one question I have is if the iteration over handlers could be simplified
with something like:

            self.block_handlers()
            self.set_security_update_level()
            self.unblock_handlers()
instead of the current:
  for (combo, combo_id) in self.handlers:
  ...
approach.

I like that you extracted the security updates setting into its own method now btw :)

Revision history for this message
Manish Sinha (मनीष सिन्हा) (manishsinha) wrote :

The self.block_handlers() method might end up blocking everything.

The other option to get around it is that instead of self.handlers being a list, it is a dict so that we don't need to iterate over it to find the correct event_id

837. By Manish Sinha (मनीष सिन्हा)

Changed the self.handlers from a list to a dict and updated all the relevant sections dealing with self.handlers

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for the update, the diff looks good!

One small note, instead of:
+ for widget in self.handlers:
you could write:
+ for handler in self.handlers.values():
But the way you wrote it is fine of course, I just wanted to mention it.

I will approve this now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'softwareproperties/gtk/SoftwarePropertiesGtk.py'
2--- softwareproperties/gtk/SoftwarePropertiesGtk.py 2013-01-30 11:22:52 +0000
3+++ softwareproperties/gtk/SoftwarePropertiesGtk.py 2013-02-21 04:01:21 +0000
4@@ -152,7 +152,7 @@
5 self.window_main.show()
6
7 # used to store the handlers of callbacks
8- self.handlers = []
9+ self.handlers = {}
10
11 self.apt_cache = apt.Cache()
12 self.apt_client = client.AptClient()
13@@ -204,8 +204,9 @@
14 " setup the widgets that allow configuring the release upgrades "
15 i = self.get_release_upgrades_policy()
16 self.combobox_release_upgrades.set_active(i)
17- self.combobox_release_upgrades.connect(
18- 'changed', self.on_combobox_release_upgrades_changed)
19+ self.handlers[self.combobox_release_upgrades] = \
20+ self.combobox_release_upgrades.connect("changed",
21+ self.on_combobox_release_upgrades_changed)
22
23 def init_auto_update(self):
24 """ Set up the widgets that allow to configure the update automation """
25@@ -236,15 +237,38 @@
26 self.combobox_update_interval.set_active(key)
27 break
28
29- self.handlers.append(
30- (self.combobox_update_interval,
31+ self.handlers[self.combobox_update_interval] = \
32 self.combobox_update_interval.connect("changed",
33- self.on_combobox_update_interval_changed)))
34+ self.on_combobox_update_interval_changed)
35
36 def show_auto_update_level(self):
37 """Represent the level of update automation in the user interface"""
38
39 # Security Updates
40+ self.set_security_update_level()
41+
42+ # Other Updates
43+ if self.settings:
44+ level_other = self.settings.get_int("regular-auto-launch-interval")
45+ model = self.combobox_other_updates.get_model()
46+ for (i, row) in enumerate(model):
47+ level = model.get_value(row.iter, 1)
48+ if level_other == level:
49+ self.combobox_other_updates.set_active(i)
50+ break
51+
52+ self.handlers[self.combobox_security_updates] = \
53+ self.combobox_security_updates.connect("changed",
54+ self.set_sec_update_automation_level)
55+
56+ self.handlers[self.combobox_other_updates] = \
57+ self.combobox_other_updates.connect("changed",
58+ self.set_other_update_automation_level)
59+
60+ def set_security_update_level(self):
61+ """Fetch the security level, Enable/Disable and set the value appropriately"""
62+
63+ # Security Updates
64 level_sec = self.get_update_automation_level()
65 if level_sec == None:
66 self.combobox_security_updates.set_sensitive(False)
67@@ -258,26 +282,7 @@
68 self.combobox_security_updates.set_active(1) # Download automatically
69 elif level_sec == softwareproperties.UPDATE_INST_SEC:
70 self.combobox_security_updates.set_active(2) # Download and install automatically
71-
72- # Other Updates
73- if self.settings:
74- level_other = self.settings.get_int("regular-auto-launch-interval")
75- model = self.combobox_other_updates.get_model()
76- for (i, row) in enumerate(model):
77- level = model.get_value(row.iter, 1)
78- if level_other == level:
79- self.combobox_other_updates.set_active(i)
80- break
81-
82- self.handlers.append(
83- (self.combobox_security_updates,
84- self.combobox_security_updates.connect("changed",
85- self.set_sec_update_automation_level)))
86-
87- self.handlers.append(
88- (self.combobox_other_updates,
89- self.combobox_other_updates.connect("changed",
90- self.set_other_update_automation_level)))
91+
92
93 def init_distro(self):
94 """Setup the user interface elements to represent the distro"""
95@@ -285,9 +290,9 @@
96 # TRANS: %s stands for the distribution name e.g. Debian or Ubuntu
97 self.label_dist_name.set_label(_("%s Software") % self.distro.id)
98
99- self.handlers.append((self.checkbutton_source_code,
100+ self.handlers[self.checkbutton_source_code] = \
101 self.checkbutton_source_code.connect("toggled",
102- self.on_checkbutton_source_code_toggled)))
103+ self.on_checkbutton_source_code_toggled)
104
105 # Setup the checkbuttons for the components
106 for checkbutton in self.vbox_dist_comps.get_children():
107@@ -301,10 +306,9 @@
108
109 checkbox.comp = comp
110 # setup the callback and show the checkbutton
111- self.handlers.append((checkbox,
112- checkbox.connect("toggled",
113+ self.handlers[checkbox] = checkbox.connect("toggled",
114 self.on_component_toggled,
115- comp.name)))
116+ comp.name)
117 self.vbox_dist_comps.add(checkbox)
118 checkbox.show()
119
120@@ -321,10 +325,9 @@
121 checkbox = Gtk.CheckButton(label="%s (%s)" % (template.description,
122 template.name))
123 checkbox.template = template
124- self.handlers.append((checkbox,
125- checkbox.connect("toggled",
126+ self.handlers[checkbox] = checkbox.connect("toggled",
127 self.on_checkbutton_child_toggled,
128- template)))
129+ template)
130 self.vbox_updates.add(checkbox)
131 checkbox.show()
132
133@@ -332,9 +335,8 @@
134 cell = Gtk.CellRendererText()
135 self.combobox_server.pack_start(cell, True)
136 self.combobox_server.add_attribute(cell, 'text', 0)
137- self.handlers.append((self.combobox_server,
138- self.combobox_server.connect("changed",
139- self.on_combobox_server_changed)))
140+ self.handlers[self.combobox_server] = self.combobox_server.connect("changed",
141+ self.on_combobox_server_changed)
142 server_store = Gtk.ListStore(GObject.TYPE_STRING,
143 GObject.TYPE_STRING,
144 GObject.TYPE_BOOLEAN)
145@@ -342,14 +344,14 @@
146 self.combobox_server.set_row_separator_func(self.is_row_separator, 2)
147
148 def block_handlers(self):
149- for (widget, handler) in self.handlers:
150- if widget.handler_is_connected(handler):
151- widget.handler_block(handler)
152+ for widget in self.handlers:
153+ if widget.handler_is_connected(self.handlers[widget]):
154+ widget.handler_block(self.handlers[widget])
155
156 def unblock_handlers(self):
157- for (widget, handler) in self.handlers:
158- if widget.handler_is_connected(handler):
159- widget.handler_unblock(handler)
160+ for widget in self.handlers:
161+ if widget.handler_is_connected(self.handlers[widget]):
162+ widget.handler_unblock(self.handlers[widget])
163
164 def show_distro(self):
165 """Fill the distro user interface with life"""
166@@ -460,6 +462,11 @@
167 except dbus.DBusException as e:
168 if e._dbus_error_name == 'com.ubuntu.SoftwareProperties.PermissionDeniedByPolicy':
169 logging.error("Authentication canceled, changes have not been saved")
170+
171+ combo_handler = self.handlers[self.combobox_security_updates]
172+ self.combobox_security_updates.handler_block(combo_handler)
173+ self.set_security_update_level()
174+ self.combobox_security_updates.handler_unblock(combo_handler)
175
176 def set_other_update_automation_level(self, widget):
177 """Set the other update automation level to the given value via gconf"""
178@@ -482,6 +489,13 @@
179 except dbus.DBusException as e:
180 if e._dbus_error_name == 'com.ubuntu.SoftwareProperties.PermissionDeniedByPolicy':
181 logging.error("Authentication canceled, changes have not been saved")
182+
183+ combo_handler = self.handlers[self.combobox_release_upgrades]
184+ self.combobox_release_upgrades.handler_block(combo_handler)
185+ i = self.get_release_upgrades_policy()
186+ self.combobox_release_upgrades.set_active(i)
187+ self.combobox_release_upgrades.handler_unblock(combo_handler)
188+
189
190 def on_combobox_server_changed(self, combobox):
191 """
192@@ -633,10 +647,9 @@
193 cell_toggle = Gtk.CellRendererToggle()
194 cell_toggle.set_property("xpad", 2)
195 cell_toggle.set_property("ypad", 2)
196- self.handlers.append([cell_toggle,
197- cell_toggle.connect('toggled',
198+ self.handlers[cell_toggle] = cell_toggle.connect('toggled',
199 self.on_isv_source_toggled,
200- self.cdrom_store)])
201+ self.cdrom_store)
202 col_active = Gtk.TreeViewColumn(_("Active"), cell_toggle,
203 active=COLUMN_ACTIVE)
204
205@@ -653,10 +666,9 @@
206 cell_toggle = Gtk.CellRendererToggle()
207 cell_toggle.set_property("xpad", 2)
208 cell_toggle.set_property("ypad", 2)
209- self.handlers.append([cell_toggle,
210- cell_toggle.connect('toggled',
211+ self.handlers[cell_toggle] = cell_toggle.connect('toggled',
212 self.on_isv_source_toggled,
213- self.source_store)])
214+ self.source_store)
215 col_active = Gtk.TreeViewColumn(_("Active"), cell_toggle,
216 active=COLUMN_ACTIVE)
217
218@@ -833,6 +845,15 @@
219 except dbus.DBusException as e:
220 if e._dbus_error_name == 'com.ubuntu.SoftwareProperties.PermissionDeniedByPolicy':
221 logging.error("Authentication canceled, changes have not been saved")
222+
223+ update_days = self.get_update_interval()
224+ combo_handler = self.handlers[self.combobox_update_interval]
225+ for key in self.combobox_interval_mapping:
226+ if self.combobox_interval_mapping[key] == update_days:
227+ self.combobox_update_interval.handler_block(combo_handler)
228+ self.combobox_update_interval.set_active(key)
229+ self.combobox_update_interval.handler_unblock(combo_handler)
230+ break
231
232 def on_add_clicked(self, widget):
233 """Show a dialog that allows to enter the apt line of a to be used repo"""