From bf382863d2c1cf73ae434606ce4cc39beaa6f3f1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 10 Nov 2014 09:48:22 +0100 Subject: [PATCH 1/7] Fix for bug1390987 Fixes: https://launchpad.net/bugs/1390987 --- openlp/core/ui/servicemanager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 48d4de34a..3c53cf035 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -1427,9 +1427,10 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ServiceManage self.drop_position = -1 self.set_modified() - def make_preview(self): + def make_preview(self, field=None): """ Send the current item to the Preview slide controller + :param field: """ self.application.set_busy_cursor() item, child = self.find_service_item() From 0a500eeb8dbb012777e0b2cbe5e36b35eaa96dfd Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 10 Nov 2014 10:25:31 +0100 Subject: [PATCH 2/7] Fix for WA import with escaped characters, partial fix for bug 1390986 Fixes: https://launchpad.net/bugs/1390986 --- openlp/plugins/songs/lib/importers/worshipassistant.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openlp/plugins/songs/lib/importers/worshipassistant.py b/openlp/plugins/songs/lib/importers/worshipassistant.py index 6ddc71159..6af038c95 100644 --- a/openlp/plugins/songs/lib/importers/worshipassistant.py +++ b/openlp/plugins/songs/lib/importers/worshipassistant.py @@ -93,7 +93,7 @@ class WorshipAssistantImport(SongImport): details = chardet.detect(detect_content) detect_file.close() songs_file = open(self.import_source, 'r', encoding=details['encoding']) - songs_reader = csv.DictReader(songs_file) + songs_reader = csv.DictReader(songs_file, escapechar='\\') try: records = list(songs_reader) except csv.Error as e: @@ -119,7 +119,6 @@ class WorshipAssistantImport(SongImport): self.title = record['TITLE'] if record['AUTHOR'] != EMPTY_STR: self.parse_author(record['AUTHOR']) - print(record['AUTHOR']) if record['COPYRIGHT'] != EMPTY_STR: self.add_copyright(record['COPYRIGHT']) if record['CCLINR'] != EMPTY_STR: From 6b538995f4c668832aca567141ce72cf6a7c90d0 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 10 Nov 2014 11:24:44 +0100 Subject: [PATCH 3/7] Fix for theme copy/creation on windows, fixes bug 1390917 Fixes: https://launchpad.net/bugs/1390917 --- openlp/core/ui/thememanager.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 03890a028..89e269f24 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -37,7 +37,7 @@ from xml.etree.ElementTree import ElementTree, XML from PyQt4 import QtCore, QtGui from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ - check_directory_exists, UiStrings, translate + check_directory_exists, UiStrings, translate, is_win from openlp.core.lib import FileDialog, ImageSource, OpenLPToolbar, get_text_file_string, build_icon, \ check_item_selected, create_thumb, validate_thumb from openlp.core.lib.theme import ThemeXML, BackgroundType @@ -662,8 +662,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ThemeManager, R out_file.close() if image_from and os.path.abspath(image_from) != os.path.abspath(image_to): try: - encoding = get_filesystem_encoding() - shutil.copyfile(str(image_from).encode(encoding), str(image_to).encode(encoding)) + # Windows is always unicode, so no need to encode filenames + if is_win(): + shutil.copyfile(image_from, image_to) + else: + encoding = get_filesystem_encoding() + shutil.copyfile(image_from.encode(encoding), image_to.encode(encoding)) except IOError as xxx_todo_changeme: shutil.Error = xxx_todo_changeme self.log_exception('Failed to save theme image') From 61e42a9782ff852f17bb04f247a1ea5d480e2eda Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 10 Nov 2014 11:51:03 +0100 Subject: [PATCH 4/7] When querying values from Setting, return None if key does not exists. Fixes bug 1387278 Fixes: https://launchpad.net/bugs/1387278 --- openlp/core/common/settings.py | 12 ++++++++---- tests/functional/openlp_core_common/test_settings.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index abbf09ec3..624179826 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -431,10 +431,14 @@ class Settings(QtCore.QSettings): :param key: The key to return the value from. """ # if group() is not empty the group has not been specified together with the key. - if self.group(): - default_value = Settings.__default_settings__[self.group() + '/' + key] - else: - default_value = Settings.__default_settings__[key] + try: + if self.group(): + default_value = Settings.__default_settings__[self.group() + '/' + key] + else: + default_value = Settings.__default_settings__[key] + except KeyError: + log.warning('Key "%s" was not found in settings, returning None!' % key) + return None setting = super(Settings, self).value(key, default_value) return self._convert_value(setting, default_value) diff --git a/tests/functional/openlp_core_common/test_settings.py b/tests/functional/openlp_core_common/test_settings.py index ea4bcf849..bcd901a81 100644 --- a/tests/functional/openlp_core_common/test_settings.py +++ b/tests/functional/openlp_core_common/test_settings.py @@ -115,3 +115,15 @@ class TestSettings(TestCase, TestMixin): # THEN the new value is returned when re-read self.assertEqual('very short', Settings().value('test/extend'), 'The saved value should be returned') + + def settings_nonexisting_test(self): + """ + Test the Settings on query for non-existing value + """ + # GIVEN: A new Settings setup + + # WHEN reading a setting that doesn't exists + does_not_exist_value = Settings().value('core/does not exists') + + # THEN None should be returned + self.assertEqual(does_not_exist_value, None, 'The value should be None') From 9b517cd5693b3ba81372c2de769ec7adc37d4ac1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 11 Nov 2014 11:38:24 +0100 Subject: [PATCH 5/7] Improved support for verseorder in WA import --- .../songs/lib/importers/worshipassistant.py | 41 ++++++++++++++----- .../songs/test_worshipassistantimport.py | 2 + .../would_you_be_free.json | 2 +- .../would_you_be_free2.csv | 31 ++++++++++++++ 4 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 tests/resources/worshipassistantsongs/would_you_be_free2.csv diff --git a/openlp/plugins/songs/lib/importers/worshipassistant.py b/openlp/plugins/songs/lib/importers/worshipassistant.py index 6af038c95..965e7d857 100644 --- a/openlp/plugins/songs/lib/importers/worshipassistant.py +++ b/openlp/plugins/songs/lib/importers/worshipassistant.py @@ -104,6 +104,8 @@ class WorshipAssistantImport(SongImport): num_records = len(records) log.info('%s records found in CSV file' % num_records) self.import_wizard.progress_bar.setMaximum(num_records) + # Create regex to strip html tags + re_html_strip = re.compile(r'<[^>]+>') for index, record in enumerate(records, 1): if self.stop_import_flag: return @@ -124,7 +126,7 @@ class WorshipAssistantImport(SongImport): if record['CCLINR'] != EMPTY_STR: self.ccli_number = record['CCLINR'] if record['ROADMAP'] != EMPTY_STR: - verse_order_list = record['ROADMAP'].split(',') + verse_order_list = [x.strip() for x in record['ROADMAP'].split(',')] lyrics = record['LYRICS2'] except UnicodeDecodeError as e: self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d' % index), @@ -135,8 +137,15 @@ class WorshipAssistantImport(SongImport): 'File not valid WorshipAssistant CSV format.'), 'TypeError: %s' % e) return verse = '' + used_verses = [] for line in lyrics.splitlines(): if line.startswith('['): # verse marker + # Add previous verse + if verse: + # remove trailing linebreak, part of the WA syntax + self.add_verse(verse[:-1], verse_id) + used_verses.append(verse_id) + verse = '' # drop the square brackets right_bracket = line.find(']') content = line[1:right_bracket].lower() @@ -151,19 +160,31 @@ class WorshipAssistantImport(SongImport): verse_index = VerseType.from_loose_input(verse_tag) if verse_tag else 0 verse_tag = VerseType.tags[verse_index] # Update verse order when the verse name has changed - if content != verse_tag + verse_num: + verse_id = verse_tag + verse_num + # Make sure we've not choosen an id already used + while verse_id in verse_order_list and content in verse_order_list: + verse_num = str(int(verse_num) + 1) + verse_id = verse_tag + verse_num + if content != verse_id: for i in range(len(verse_order_list)): if verse_order_list[i].lower() == content.lower(): - verse_order_list[i] = verse_tag + verse_num - elif line and not line.isspace(): - verse += line + '\n' - elif verse: - self.add_verse(verse, verse_tag+verse_num) - verse = '' + verse_order_list[i] = verse_id + else: + # add line text to verse. Strip out html + verse += re_html_strip.sub('', line) + '\n' if verse: - self.add_verse(verse, verse_tag+verse_num) + # remove trailing linebreak, part of the WA syntax + if verse.endswith('\n\n'): + verse = verse[:-1] + self.add_verse(verse, verse_id) + used_verses.append(verse_id) if verse_order_list: - self.verse_order_list = verse_order_list + # Use the verse order in the import, but remove entries that doesn't have a text + cleaned_verse_order_list = [] + for verse in verse_order_list: + if verse in used_verses: + cleaned_verse_order_list.append(verse) + self.verse_order_list = cleaned_verse_order_list if not self.finish(): self.log_error(translate('SongsPlugin.WorshipAssistantImport', 'Record %d') % index + (': "' + self.title + '"' if self.title else '')) diff --git a/tests/functional/openlp_plugins/songs/test_worshipassistantimport.py b/tests/functional/openlp_plugins/songs/test_worshipassistantimport.py index 63ead5b30..346f9c83d 100644 --- a/tests/functional/openlp_plugins/songs/test_worshipassistantimport.py +++ b/tests/functional/openlp_plugins/songs/test_worshipassistantimport.py @@ -54,3 +54,5 @@ class TestWorshipAssistantFileImport(SongImportTestHelper): self.load_external_result_data(os.path.join(TEST_PATH, 'du_herr.json'))) self.file_import(os.path.join(TEST_PATH, 'would_you_be_free.csv'), self.load_external_result_data(os.path.join(TEST_PATH, 'would_you_be_free.json'))) + self.file_import(os.path.join(TEST_PATH, 'would_you_be_free2.csv'), + self.load_external_result_data(os.path.join(TEST_PATH, 'would_you_be_free.json'))) diff --git a/tests/resources/worshipassistantsongs/would_you_be_free.json b/tests/resources/worshipassistantsongs/would_you_be_free.json index 96bc06a59..2c0a39973 100644 --- a/tests/resources/worshipassistantsongs/would_you_be_free.json +++ b/tests/resources/worshipassistantsongs/would_you_be_free.json @@ -8,7 +8,7 @@ "copyright": "Public Domain", "verses": [ [ - "Would you be free from your burden of sin? \nThere's power in the blood, power in the blood \nWould you o'er evil a victory win? \nThere's wonderful power in the blood \n", + "Would you be free from your burden of sin? \nThere's power in the blood, power in the blood \nWould you o'er evil a victory win? \nThere's wonderful power in the blood \n ", "v1" ], [ diff --git a/tests/resources/worshipassistantsongs/would_you_be_free2.csv b/tests/resources/worshipassistantsongs/would_you_be_free2.csv new file mode 100644 index 000000000..ca4da9998 --- /dev/null +++ b/tests/resources/worshipassistantsongs/would_you_be_free2.csv @@ -0,0 +1,31 @@ +SONGNR,TITLE,AUTHOR,COPYRIGHT,FIRSTLINE,PRIKEY,ALTKEY,TEMPO,FOCUS,THEME,SCRIPTURE,ACTIVE,SONGBOOK,TIMESIG,INTRODUCED,LASTUSED,TIMESUSED,CCLINR,USER1,USER2,USER3,USER4,USER5,ROADMAP,FILELINK1,OVERMAP,FILELINK2,LYRICS,INFO,LYRICS2,Background +"7","Would You Be Free","Jones, Lewis E.","Public Domain","Would you be free from your burden of sin?","G","","Moderate","Only To Others","","","N","Y","","1899-12-30","1899-12-30","","","","","","","","1, C, 1","","","",".G C G + Would you be free from your burden of sin? +. D D7 G + There's power in the blood, power in the blood +. C G + Would you o'er evil a victory win? +. D D7 G + There's wonderful power in the blood + +.G C G + There is power, power, wonder working power +.D G + In the blood of the Lamb +. C G + There is power, power, wonder working power +. D G + In the precious blood of the Lamb +","","[1] +Would you be free from your burden of sin? +There's power in the blood, power in the blood +Would you o'er evil a victory win? +There's wonderful power in the blood + +[C] +There is power, power, wonder working power +In the blood of the Lamb +There is power, power, wonder working power +In the precious blood of the Lamb + +[x]","" From 2efdc97311e5c6df57aa8443389efddcfea6f6f3 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 11 Nov 2014 12:34:47 +0100 Subject: [PATCH 6/7] Fix traceback when saving service while in debug mode --- openlp/core/ui/servicemanager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index 3c53cf035..e8b201cf6 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -494,7 +494,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtGui.QWidget, Ui_ServiceManage service.append({'openlp_core': core}) return service - def save_file(self): + def save_file(self, field=None): """ Save the current service file. From 378132d82f299c72a8cc02ae300d727d055d0960 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 13 Nov 2014 20:15:43 +0000 Subject: [PATCH 7/7] Moved Settings KeyError handling to the export. --- openlp/core/common/settings.py | 12 ++++-------- openlp/core/ui/mainwindow.py | 9 ++++++++- tests/functional/openlp_core_common/test_settings.py | 10 +++++----- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index 624179826..abbf09ec3 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -431,14 +431,10 @@ class Settings(QtCore.QSettings): :param key: The key to return the value from. """ # if group() is not empty the group has not been specified together with the key. - try: - if self.group(): - default_value = Settings.__default_settings__[self.group() + '/' + key] - else: - default_value = Settings.__default_settings__[key] - except KeyError: - log.warning('Key "%s" was not found in settings, returning None!' % key) - return None + if self.group(): + default_value = Settings.__default_settings__[self.group() + '/' + key] + else: + default_value = Settings.__default_settings__[key] setting = super(Settings, self).value(key, default_value) return self._convert_value(setting, default_value) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 33e1f6bc7..35d05620a 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -978,7 +978,14 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties): # FIXME: We are conflicting with the standard "General" section. if 'eneral' in section_key: section_key = section_key.lower() - key_value = settings.value(section_key) + try: + key_value = settings.value(section_key) + except KeyError: + QtGui.QMessageBox.critical(self, translate('OpenLP.MainWindow', 'Export setting error'), + translate('OpenLP.MainWindow', 'The key "%s" does not have a default value ' + 'so it will be skipped in this export.') % section_key, + QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Ok)) + key_value = None if key_value is not None: export_settings.setValue(section_key, key_value) export_settings.sync() diff --git a/tests/functional/openlp_core_common/test_settings.py b/tests/functional/openlp_core_common/test_settings.py index bcd901a81..e3ce6990d 100644 --- a/tests/functional/openlp_core_common/test_settings.py +++ b/tests/functional/openlp_core_common/test_settings.py @@ -121,9 +121,9 @@ class TestSettings(TestCase, TestMixin): Test the Settings on query for non-existing value """ # GIVEN: A new Settings setup + with self.assertRaises(KeyError) as cm: + # WHEN reading a setting that doesn't exists + does_not_exist_value = Settings().value('core/does not exists') - # WHEN reading a setting that doesn't exists - does_not_exist_value = Settings().value('core/does not exists') - - # THEN None should be returned - self.assertEqual(does_not_exist_value, None, 'The value should be None') + # THEN: An exception with the non-existing key should be thrown + self.assertEqual(str(cm.exception), "'core/does not exists'", 'We should get an exception')