openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #26116
[Merge] lp:~phill-ridout/openlp/bug1419691 into lp:openlp
Phill has proposed merging lp:~phill-ridout/openlp/bug1419691 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1419691 in OpenLP: "Remove support for .theme files"
https://bugs.launchpad.net/openlp/+bug/1419691
For more details, see:
https://code.launchpad.net/~phill-ridout/openlp/bug1419691/+merge/249734
Fixes bug1419691 by checking the theme version number (OpenLP1 themes didn't have a version no.) and by removing the *.theme filter from the file dialog.
Also added a ValidationError exception class, to tidy up the unzip code slightly
Tests are still failing on crosswalk, however all tests (except crosswalk) are passing locally.
Add this to your merge proposal:
--------------------------------
lp:~phill-ridout/openlp/bug1419691 (revision 2499)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/945/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/871/
[FAILURE] http://ci.openlp.org/job/Branch-03-Interface-Tests/816/
Stopping after failure
Process finished with exit code 0
--
Your team OpenLP Core is requested to review the proposed merge of lp:~phill-ridout/openlp/bug1419691 into lp:openlp.
=== modified file 'openlp/core/common/historycombobox.py'
--- openlp/core/common/historycombobox.py 2015-01-18 13:39:21 +0000
+++ openlp/core/common/historycombobox.py 2015-02-13 23:22:27 +0000
@@ -20,7 +20,7 @@
# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
###############################################################################
"""
-The :mod:`~openlp.core.lib.historycombobox` module contains the HistoryComboBox widget
+The :mod:`~openlp.core.common.historycombobox` module contains the HistoryComboBox widget
"""
from PyQt4 import QtCore, QtGui
@@ -28,7 +28,7 @@
class HistoryComboBox(QtGui.QComboBox):
"""
- The :class:`~openlp.core.lib.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed`` signal
+ The :class:`~openlp.core.common.historycombobox.HistoryComboBox` widget emulates the QLineEdit ``returnPressed`` signal
for when the :kbd:`Enter` or :kbd:`Return` keys are pressed, and saves anything that is typed into the edit box into
its list.
"""
=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py 2015-01-18 13:39:21 +0000
+++ openlp/core/lib/__init__.py 2015-02-13 23:22:27 +0000
@@ -312,6 +312,7 @@
from .colorbutton import ColorButton
+from .exceptions import ValidationError
from .filedialog import FileDialog
from .screen import ScreenList
from .listwidgetwithdnd import ListWidgetWithDnD
=== added file 'openlp/core/lib/exceptions.py'
--- openlp/core/lib/exceptions.py 1970-01-01 00:00:00 +0000
+++ openlp/core/lib/exceptions.py 2015-02-13 23:22:27 +0000
@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2015 OpenLP Developers #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it #
+# under the terms of the GNU General Public License as published by the Free #
+# Software Foundation; version 2 of the License. #
+# #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
+# more details. #
+# #
+# You should have received a copy of the GNU General Public License along #
+# with this program; if not, write to the Free Software Foundation, Inc., 59 #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
+###############################################################################
+"""
+The :mod:`~openlp.core.lib.exceptions` module contains custom exceptions
+"""
+
+
+class ValidationError(Exception):
+ """
+ The :class:`~openlp.core.lib.exceptions.ValidationError` exception provides a custom exception for validating
+ import files.
+ """
+ pass
\ No newline at end of file
=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py 2015-01-18 13:39:21 +0000
+++ openlp/core/ui/thememanager.py 2015-02-13 23:22:27 +0000
@@ -31,7 +31,7 @@
from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \
check_directory_exists, UiStrings, translate, is_win
-from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, get_text_file_string, build_icon, \
+from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, ValidationError, get_text_file_string, build_icon, \
check_item_selected, create_thumb, validate_thumb
from openlp.core.lib.theme import ThemeXML, BackgroundType
from openlp.core.lib.ui import critical_error_message_box, create_widget_action
@@ -414,13 +414,13 @@
def on_import_theme(self, field=None):
"""
Opens a file dialog to select the theme file(s) to import before attempting to extract OpenLP themes from
- those files. This process will load both OpenLP version 1 and version 2 themes.
+ those files. This process will only load version 2 themes.
:param field:
"""
files = FileDialog.getOpenFileNames(self,
translate('OpenLP.ThemeManager', 'Select Theme Import File'),
Settings().value(self.settings_section + '/last directory import'),
- translate('OpenLP.ThemeManager', 'OpenLP Themes (*.theme *.otz)'))
+ translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)'))
self.log_info('New Themes %s' % str(files))
if not files:
return
@@ -535,7 +535,6 @@
:param directory:
"""
self.log_debug('Unzipping theme %s' % file_name)
- file_name = str(file_name)
theme_zip = None
out_file = None
file_xml = None
@@ -545,8 +544,12 @@
xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
if len(xml_file) != 1:
self.log_error('Theme contains "%s" XML files' % len(xml_file))
- raise Exception('validation')
+ raise ValidationError
xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot()
+ theme_version = xml_tree.get('version', default=None)
+ if not theme_version or float(theme_version) < 2.0:
+ self.log_error('Theme version is less than 2.0')
+ raise ValidationError
theme_name = xml_tree.find('name').text.strip()
theme_folder = os.path.join(directory, theme_name)
theme_exists = os.path.exists(theme_folder)
@@ -573,13 +576,10 @@
out_file.close()
except (IOError, zipfile.BadZipfile):
self.log_exception('Importing theme from zip failed %s' % file_name)
- raise Exception('validation')
- except Exception as info:
- if str(info) == 'validation':
- critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),
- translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
- else:
- raise
+ raise ValidationError
+ except ValidationError:
+ critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),
+ translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
finally:
# Close the files, to be able to continue creating the theme.
if theme_zip:
=== modified file 'tests/functional/openlp_core_ui/test_thememanager.py'
--- tests/functional/openlp_core_ui/test_thememanager.py 2015-01-18 13:39:21 +0000
+++ tests/functional/openlp_core_ui/test_thememanager.py 2015-02-13 23:22:27 +0000
@@ -27,14 +27,13 @@
import shutil
from unittest import TestCase
-from tests.functional import MagicMock
from tempfile import mkdtemp
from openlp.core.ui import ThemeManager
from openlp.core.common import Registry
from tests.utils.constants import TEST_RESOURCES_PATH
-from tests.functional import MagicMock, patch
+from tests.functional import ANY, MagicMock, patch
class TestThemeManager(TestCase):
@@ -152,3 +151,23 @@
self.assertTrue(os.path.exists(os.path.join(folder, 'Moss on tree', 'Moss on tree.xml')))
self.assertEqual(mocked_critical_error_message_box.call_count, 0, 'No errors should have happened')
shutil.rmtree(folder)
+
+ def unzip_theme_invalid_version_test(self):
+ """
+ Test that themes with invalid (< 2.0) or with no version attributes are rejected
+ """
+ # GIVEN: An instance of ThemeManager whilst mocking a theme that returns a theme with no version attribute
+ with patch('openlp.core.ui.thememanager.zipfile.ZipFile') as mocked_zip_file,\
+ patch('openlp.core.ui.thememanager.ElementTree.getroot') as mocked_getroot,\
+ patch('openlp.core.ui.thememanager.XML'),\
+ patch('openlp.core.ui.thememanager.critical_error_message_box') as mocked_critical_error_message_box:
+
+ mocked_zip_file.return_value = MagicMock(**{'namelist.return_value': [os.path.join('theme', 'theme.xml')]})
+ mocked_getroot.return_value = MagicMock(**{'get.return_value': None})
+ theme_manager = ThemeManager(None)
+
+ # WHEN: unzip_theme is called
+ theme_manager.unzip_theme('theme.file', 'folder')
+
+ # THEN: The critical_error_message_box should have been called
+ mocked_critical_error_message_box.assert_called_once(ANY, ANY)
