openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/bug-1386710 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1386710 in OpenLP: "Saving setting gives traceback"
  https://bugs.launchpad.net/openlp/+bug/1386710

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/bug-1386710/+merge/239903

[fix 1386710] Fix traceback after settings form is saved

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1386710 (revision 2432)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/720/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/663/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/607/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/547/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/156/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/361/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/235/

-- 
https://code.launchpad.net/~raoul-snyman/openlp/bug-1386710/+merge/239903
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1386710 into lp:openlp.

=== modified file 'openlp/core/ui/settingsform.py'
--- openlp/core/ui/settingsform.py	2014-10-25 20:26:19 +0000
+++ openlp/core/ui/settingsform.py	2014-10-28 19:48:53 +0000
@@ -96,9 +96,21 @@
         Process the form saving the settings
         """
         log.debug('Processing settings exit')
-        for tabIndex in range(self.stacked_layout.count()):
-            self.stacked_layout.widget(tabIndex).save()
-        # if the display of image background are changing we need to regenerate the image cache
+        # We add all the forms into the stacked layout, even if the plugin is inactive,
+        # but we don't add the item to the list on the side if the plugin is inactive,
+        # so loop through the list items, and then find the tab for that item.
+        for item_index in range(self.setting_list_widget.count()):
+            # Get the list item
+            list_item = self.setting_list_widget.item(item_index)
+            if not list_item:
+                continue
+            # Now figure out if there's a tab for it, and save the tab.
+            plugin_name = list_item.data(QtCore.Qt.UserRole)
+            for tab_index in range(self.stacked_layout.count()):
+                tab_widget = self.stacked_layout.widget(tab_index)
+                if tab_widget.tab_title == plugin_name:
+                    tab_widget.save()
+        # if the image background has been changed we need to regenerate the image cache
         if 'images_config_updated' in self.processes or 'config_screen_changed' in self.processes:
             self.register_post_process('images_regenerate')
         # Now lets process all the post save handlers

=== modified file 'tests/functional/openlp_core_ui/test_settingsform.py'
--- tests/functional/openlp_core_ui/test_settingsform.py	2014-10-25 20:26:19 +0000
+++ tests/functional/openlp_core_ui/test_settingsform.py	2014-10-28 19:48:53 +0000
@@ -29,6 +29,7 @@
 """
 Package to test the openlp.core.ui.settingsform package.
 """
+from PyQt4 import QtGui
 from unittest import TestCase
 
 from openlp.core.common import Registry
@@ -83,3 +84,31 @@
             # THEN: Stuff should happen
             mocked_add_widget.assert_called_with(general_tab)
             self.assertEqual(0, mocked_add_item.call_count, 'addItem should not have been called')
+
+    def accept_with_inactive_plugins_test(self):
+        """
+        Test that the accept() method works correctly when some of the plugins are inactive
+        """
+        # GIVEN: A visible general tab and an invisible theme tab in a Settings Form
+        settings_form = SettingsForm(None)
+        general_tab = QtGui.QWidget(None)
+        general_tab.tab_title = 'mock-general'
+        general_tab.tab_title_visible = 'Mock General'
+        general_tab.icon_path = ':/icon/openlp-logo-16x16.png'
+        mocked_general_save = MagicMock()
+        general_tab.save = mocked_general_save
+        settings_form.insert_tab(general_tab, is_visible=True)
+        themes_tab = QtGui.QWidget(None)
+        themes_tab.tab_title = 'mock-themes'
+        themes_tab.tab_title_visible = 'Mock Themes'
+        themes_tab.icon_path = ':/icon/openlp-logo-16x16.png'
+        mocked_theme_save = MagicMock()
+        themes_tab.save = mocked_theme_save
+        settings_form.insert_tab(themes_tab, is_visible=False)
+
+        # WHEN: The accept() method is called
+        settings_form.accept()
+
+        # THEN: The general tab's save() method should have been called, but not the themes tab
+        mocked_general_save.assert_called_with()
+        self.assertEqual(0, mocked_theme_save.call_count, 'The Themes tab\'s save() should not have been called')