details: Confirm with user before throwing away changes on page change

We've had many users be confused by changes in the details pages not
taking immediate effect, or just forgetting to hit apply.

Now by default we prompt users if switching pages if there are unapplied
changes. This can be disabled like other prompts in Edit->Prefs or with
a 'Don't warn me again' chkbox in the dialog.

src/virt-manager.schemas.in

@@ -351,6 +351,19 @@
       </locale>
     </schema>
 
+    <schema>
+      <key>/schemas/apps/::PACKAGE::/confirm/unapplied_dev</key>
+      <applyto>/apps/::PACKAGE::/confirm/unapplied_dev</applyto>
+      <owner>::PACKAGE::</owner>
+      <type>bool</type>
+      <default>1</default>
+
+      <locale name="C">
+        <short>Confirm about unapplied device changes</short>
+        <long>Whether we ask the user to apply or discard unapplied device changes</long>
+      </locale>
+    </schema>
+
     <schema>
       <key>/schemas/apps/::PACKAGE::/manager_window_height</key>
       <applyto>/apps/::PACKAGE::/manager_window_height</applyto>

src/virtManager/config.py

@@ -386,6 +386,8 @@ class vmmConfig(object):
         return self.conf.get_bool(self.conf_dir + "/confirm/removedev")
     def get_confirm_interface(self):
         return self.conf.get_bool(self.conf_dir + "/confirm/interface_power")
+    def get_confirm_unapplied(self):
+        return self.conf.get_bool(self.conf_dir + "/confirm/unapplied_dev")
 
 
     def set_confirm_forcepoweroff(self, val):
@@ -398,6 +400,8 @@ class vmmConfig(object):
         self.conf.set_bool(self.conf_dir + "/confirm/removedev", val)
     def set_confirm_interface(self, val):
         self.conf.set_bool(self.conf_dir + "/confirm/interface_power", val)
+    def set_confirm_unapplied(self, val):
+        self.conf.set_bool(self.conf_dir + "/confirm/unapplied_dev", val)
 
     def on_confirm_forcepoweroff_changed(self, cb):
         return self.conf.notify_add(self.conf_dir + "/confirm/forcepoweroff", cb)
@@ -409,6 +413,8 @@ class vmmConfig(object):
         return self.conf.notify_add(self.conf_dir + "/confirm/removedev", cb)
     def on_confirm_interface_changed(self, cb):
         return self.conf.notify_add(self.conf_dir + "/confirm/interface_power", cb)
+    def on_confirm_unapplied_changed(self, cb):
+        return self.conf.notify_add(self.conf_dir + "/confirm/unapplied_dev", cb)
 
 
     # System tray visibility

src/virtManager/details.py

@@ -307,6 +307,7 @@ class vmmDetails(vmmGObjectUI):
         w, h = self.vm.get_details_window_size()
         self.topwin.set_default_size(w or 800, h or 600)
 
+        self.oldhwrow = None
         self.addhwmenu = None
         self.keycombo_menu = None
         self.init_menus()
@@ -458,7 +459,7 @@ class vmmDetails(vmmGObjectUI):
         self.vm.connect("config-changed", self.refresh_vm_state)
         self.vm.connect("resources-sampled", self.refresh_resources)
         self.widget("hw-list").get_selection().connect("changed",
-                                                       self.hw_selected)
+                                                       self.hw_changed)
         self.widget("config-boot-list").get_selection().connect(
                                             "changed",
                                             self.config_bootdev_selected)
@@ -477,6 +478,8 @@ class vmmDetails(vmmGObjectUI):
         self.close()
 
         try:
+            self.oldhwrow = None
+
             if self.addhw:
                 self.addhw.cleanup()
                 self.addhw = None
@@ -1084,26 +1087,29 @@ class vmmDetails(vmmGObjectUI):
         else:
             self.widget("toolbar-box").hide()
 
-    def get_boot_selection(self):
+    def get_selected_row(self, widget):
-        widget = self.widget("config-boot-list")
         selection = widget.get_selection()
         model, treepath = selection.get_selected()
         if treepath == None:
             return None
         return model[treepath]
 
+    def get_boot_selection(self):
+        return self.get_selected_row(self.widget("config-boot-list"))
+
     def set_hw_selection(self, page):
         hwlist = self.widget("hw-list")
         selection = hwlist.get_selection()
         selection.select_path(str(page))
 
+    def get_hw_row(self):
+        return self.get_selected_row(self.widget("hw-list"))
+
     def get_hw_selection(self, field):
-        vmlist = self.widget("hw-list")
+        row = self.get_hw_row()
-        selection = vmlist.get_selection()
+        if not row:
-        active = selection.get_selected()
-        if active[1] == None:
             return None
-        return active[0].get_value(active[1], field)
+        return row[field]
 
     def force_get_hw_pagetype(self, page=None):
         if page:
@@ -1116,7 +1122,56 @@ class vmmDetails(vmmGObjectUI):
 
         return page
 
-    def hw_selected(self, ignore1=None, page=None, selected=True):
+    def compare_hw_rows(self, row1, row2):
+        if row1 == row2:
+            return True
+        if not row1 or not row2:
+            return False
+
+        for idx in range(len(row1)):
+            if row1[idx] != row2[idx]:
+                return False
+        return True
+
+    def has_unapplied_changes(self, row):
+        if not row:
+            return False
+
+        if not self.widget("config-apply").get_property("sensitive"):
+            return False
+
+        if util.chkbox_helper(self,
+            self.config.get_confirm_unapplied,
+            self.config.set_confirm_unapplied,
+            text1=(_("There are unapplied changes. Would you like to apply "
+                     "them now?")),
+            chktext=_("Don't warn me again."),
+            alwaysrecord=True):
+            return False
+
+        return not self.config_apply(row=row)
+
+    def hw_changed(self, ignore):
+        newrow = self.get_hw_row()
+        oldrow = self.oldhwrow
+        model = self.widget("hw-list").get_model()
+
+        if self.compare_hw_rows(newrow, oldrow):
+            return
+
+        if self.has_unapplied_changes(oldrow):
+            # Unapplied changes, and syncing them failed
+            pageidx = 0
+            for idx in range(len(model)):
+                if self.compare_hw_rows(model[idx], oldrow):
+                    pageidx = idx
+                    break
+            self.set_hw_selection(pageidx)
+        else:
+            self.oldhwrow = newrow
+            self.hw_selected()
+
+    def hw_selected(self, page=None):
         pagetype = self.force_get_hw_pagetype(page)
 
         self.widget("config-remove").set_sensitive(True)
@@ -1165,8 +1220,7 @@ class vmmDetails(vmmGObjectUI):
             return
 
         rem = pagetype in remove_pages
-        if selected:
+        self.disable_apply()
-            self.disable_apply()
         self.widget("config-remove").set_property("visible", rem)
 
         self.widget("hw-panel").set_current_page(pagetype)
@@ -1184,6 +1238,11 @@ class vmmDetails(vmmGObjectUI):
             is_details = True
 
         pages = self.widget("details-pages")
+        if pages.get_current_page() == PAGE_DETAILS:
+            if self.has_unapplied_changes(self.get_hw_row()):
+                self.sync_details_console_view(True)
+                return
+
         if is_details:
             pages.set_current_page(PAGE_DETAILS)
         else:
@@ -1732,13 +1791,20 @@ class vmmDetails(vmmGObjectUI):
     # Details/Hardware config changes (apply button) #
     ##################################################
 
-    def config_cancel(self, ignore):
+    def config_cancel(self, ignore=None):
         # Remove current changes and deactive 'apply' button
         self.hw_selected()
 
-    def config_apply(self, ignore):
+    def config_apply(self, ignore=None, row=None):
-        pagetype = self.get_hw_selection(HW_LIST_COL_TYPE)
+        pagetype = None
-        devobj = self.get_hw_selection(HW_LIST_COL_DEVICE)
+        devobj = None
+
+        if not row:
+            row = self.get_hw_row()
+        if row:
+            pagetype = row[HW_LIST_COL_TYPE]
+            devobj = row[HW_LIST_COL_DEVICE]
+
         key = devobj
         ret = False
 
@@ -1768,10 +1834,11 @@ class vmmDetails(vmmGObjectUI):
             else:
                 ret = False
         except Exception, e:
-            self.err.show_err(_("Error apply changes: %s") % e)
+            return self.err.show_err(_("Error apply changes: %s") % e)
 
         if ret is not False:
             self.disable_apply()
+        return True
 
     def get_text(self, widgetname, strip=True):
         ret = self.widget(widgetname).get_text()

src/virtManager/preferences.py

@@ -46,6 +46,7 @@ class vmmPreferences(vmmGObjectUI):
         self.add_gconf_handle(self.config.on_confirm_pause_changed(self.refresh_confirm_pause))
         self.add_gconf_handle(self.config.on_confirm_removedev_changed(self.refresh_confirm_removedev))
         self.add_gconf_handle(self.config.on_confirm_interface_changed(self.refresh_confirm_interface))
+        self.add_gconf_handle(self.config.on_confirm_unapplied_changed(self.refresh_confirm_unapplied))
 
         self.refresh_view_system_tray()
         self.refresh_update_interval()
@@ -63,6 +64,7 @@ class vmmPreferences(vmmGObjectUI):
         self.refresh_confirm_pause()
         self.refresh_confirm_removedev()
         self.refresh_confirm_interface()
+        self.refresh_confirm_unapplied()
 
         self.window.signal_autoconnect({
             "on_prefs_system_tray_toggled" : self.change_view_system_tray,
@@ -82,6 +84,7 @@ class vmmPreferences(vmmGObjectUI):
             "on_prefs_confirm_pause_toggled": self.change_confirm_pause,
             "on_prefs_confirm_removedev_toggled": self.change_confirm_removedev,
             "on_prefs_confirm_interface_toggled": self.change_confirm_interface,
+            "on_prefs_confirm_unapplied_toggled": self.change_confirm_unapplied,
             "on_prefs_btn_keys_define_clicked": self.change_grab_keys,
             "on_prefs_graphics_type_changed": self.change_graphics_type,
             })
@@ -192,6 +195,10 @@ class vmmPreferences(vmmGObjectUI):
                                   ignore3=None, ignore4=None):
         self.widget("prefs-confirm-interface").set_active(
                                 self.config.get_confirm_interface())
+    def refresh_confirm_unapplied(self, ignore1=None, ignore2=None,
+                                  ignore3=None, ignore4=None):
+        self.widget("prefs-confirm-unapplied").set_active(
+                                self.config.get_confirm_unapplied())
 
     def grabkeys_get_string(self, keysyms):
         keystr = None
@@ -286,6 +293,8 @@ class vmmPreferences(vmmGObjectUI):
         self.config.set_confirm_removedev(src.get_active())
     def change_confirm_interface(self, src):
         self.config.set_confirm_interface(src.get_active())
+    def change_confirm_unapplied(self, src):
+        self.config.set_confirm_unapplied(src.get_active())
 
     def change_graphics_type(self, src):
         gtype = 'vnc'

src/virtManager/util.py

@@ -361,7 +361,8 @@ def pretty_bytes(val):
 
 xpath = virtinst.util.get_xml_path
 
-def chkbox_helper(src, getcb, setcb, text1, text2=None):
+def chkbox_helper(src, getcb, setcb, text1, text2=None, alwaysrecord=False,
+                  chktext=_("Don't ask me again")):
     """
     Helper to prompt user about proceeding with an operation
     Returns True if operation should be cancelled
@@ -373,14 +374,13 @@ def chkbox_helper(src, getcb, setcb, text1, text2=None):
         return False
 
     res = src.err.warn_chkbox(text1=text1, text2=text2,
-                              chktext=_("Don't ask me again."),
+                              chktext=chktext,
                               buttons=gtk.BUTTONS_YES_NO)
     response, skip_prompt = res
-    if not response:
+    if alwaysrecord or response:
-        return True
+        setcb(not skip_prompt)
 
-    setcb(not skip_prompt)
+    return not response
-    return False
 
 def get_list_selection(widget):
     selection = widget.get_selection()

src/vmm-preferences.glade

@@ -589,7 +589,7 @@ Spice</property>
                         <child>
                           <widget class="GtkTable" id="table6">
                             <property name="visible">True</property>
-                            <property name="n_rows">5</property>
+                            <property name="n_rows">6</property>
                             <property name="n_columns">2</property>
                             <property name="column_spacing">12</property>
                             <property name="row_spacing">6</property>
@@ -741,6 +741,33 @@ Spice</property>
                                 <property name="x_options">GTK_FILL</property>
                               </packing>
                             </child>
+                            <child>
+                              <widget class="GtkLabel" id="label5">
+                                <property name="visible">True</property>
+                                <property name="xalign">0</property>
+                                <property name="label" translatable="yes">Unapplied changes:</property>
+                              </widget>
+                              <packing>
+                                <property name="top_attach">5</property>
+                                <property name="bottom_attach">6</property>
+                                <property name="x_options">GTK_FILL</property>
+                              </packing>
+                            </child>
+                            <child>
+                              <widget class="GtkCheckButton" id="prefs-confirm-unapplied">
+                                <property name="visible">True</property>
+                                <property name="can_focus">True</property>
+                                <property name="receives_default">False</property>
+                                <property name="draw_indicator">True</property>
+                                <signal name="toggled" handler="on_prefs_confirm_unapplied_toggled"/>
+                              </widget>
+                              <packing>
+                                <property name="left_attach">1</property>
+                                <property name="right_attach">2</property>
+                                <property name="top_attach">5</property>
+                                <property name="bottom_attach">6</property>
+                              </packing>
+                            </child>
                           </widget>
                         </child>
                       </widget>