openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

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

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1386896 in OpenLP: "OpenLP fails to open when VLC is not installed on Mac OS X"
  https://bugs.launchpad.net/openlp/+bug/1386896

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

Two fixes:
  - Fix bug #1386896 by catching the OSError on Mac OS X and just ignoring it
  - Found a potential bug where the settings form was canceled, with one or more inactive plugins
  - Wrote a test for the above settings form bug

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1386896 (revision 2435)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/725/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/668/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/612/
[SUCCESS] http://ci.openlp.org/job/Branch-04a-Windows_Functional_Tests/552/
[SUCCESS] http://ci.openlp.org/job/Branch-04b-Windows_Interface_Tests/161/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/366/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/240/
-- 
https://code.launchpad.net/~raoul-snyman/openlp/bug-1386896/+merge/240187
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1386896 into lp:openlp.

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2014-09-02 20:17:56 +0000
+++ openlp/core/ui/media/vlcplayer.py	2014-10-30 23:19:46 +0000
@@ -33,7 +33,6 @@
 from distutils.version import LooseVersion
 import logging
 import os
-import sys
 import threading
 
 from PyQt4 import QtGui
@@ -55,6 +54,8 @@
     if is_win():
         if not isinstance(e, WindowsError) and e.winerror != 126:
             raise
+    elif is_macosx():
+        pass
     else:
         raise
 

=== modified file 'openlp/core/ui/settingsform.py'
--- openlp/core/ui/settingsform.py	2014-10-28 20:48:20 +0000
+++ openlp/core/ui/settingsform.py	2014-10-30 23:19:46 +0000
@@ -57,6 +57,11 @@
         self.processes = []
         self.setupUi(self)
         self.setting_list_widget.currentRowChanged.connect(self.list_item_changed)
+        self.general_tab = None
+        self.themes_tab = None
+        self.projector_tab = None
+        self.advanced_tab = None
+        self.player_tab = None
 
     def exec_(self):
         """
@@ -125,8 +130,18 @@
         Process the form saving the settings
         """
         self.processes = []
-        for tabIndex in range(self.stacked_layout.count()):
-            self.stacked_layout.widget(tabIndex).cancel()
+        # Same as accept(), we need to loop over the visible tabs, and skip the inactive ones
+        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.cancel()
         return QtGui.QDialog.reject(self)
 
     def bootstrap_post_set_up(self):

=== modified file 'tests/functional/openlp_core_ui/test_settingsform.py'
--- tests/functional/openlp_core_ui/test_settingsform.py	2014-10-28 20:27:09 +0000
+++ tests/functional/openlp_core_ui/test_settingsform.py	2014-10-30 23:19:46 +0000
@@ -130,3 +130,31 @@
 
             # THEN: The rest of the method should not have been called
             self.assertEqual(0, mocked_count.call_count, 'The count method of the stacked layout should not be called')
+
+    def reject_with_inactive_items_test(self):
+        """
+        Test that the reject() 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_cancel = MagicMock()
+        general_tab.cancel = mocked_general_cancel
+        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_cancel = MagicMock()
+        themes_tab.cancel = mocked_theme_cancel
+        settings_form.insert_tab(themes_tab, is_visible=False)
+
+        # WHEN: The reject() method is called
+        settings_form.reject()
+
+        # THEN: The general tab's cancel() method should have been called, but not the themes tab
+        mocked_general_cancel.assert_called_with()
+        self.assertEqual(0, mocked_theme_cancel.call_count, 'The Themes tab\'s cancel() should not have been called')
\ No newline at end of file