From bf382863d2c1cf73ae434606ce4cc39beaa6f3f1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 10 Nov 2014 09:48:22 +0100 Subject: [PATCH 01/17] 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 02/17] 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 03/17] 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 04/17] 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 05/17] 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 06/17] 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 f609be65bceebdab81dcf95d90151ac199b2e971 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 12 Nov 2014 21:20:57 +0000 Subject: [PATCH 07/17] Make the settingswindow higher to make room for the new settings. Fixes: https://launchpad.net/bugs/1377283 --- openlp/core/ui/settingsdialog.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/settingsdialog.py b/openlp/core/ui/settingsdialog.py index ab5931584..61ee10e14 100644 --- a/openlp/core/ui/settingsdialog.py +++ b/openlp/core/ui/settingsdialog.py @@ -46,7 +46,7 @@ class Ui_SettingsDialog(object): """ settings_dialog.setObjectName('settings_dialog') settings_dialog.setWindowIcon(build_icon(u':/icon/openlp-logo.svg')) - settings_dialog.resize(800, 500) + settings_dialog.resize(800, 700) self.dialog_layout = QtGui.QGridLayout(settings_dialog) self.dialog_layout.setObjectName('dialog_layout') self.dialog_layout.setMargin(8) From b7e3a33c4ac33fc815be7839588ac6657184aaa5 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 12 Nov 2014 22:18:57 +0000 Subject: [PATCH 08/17] Mark a song edited from preview as coming from plugin. Fixes bug 1382672 Fixes: https://launchpad.net/bugs/1382672 --- openlp/plugins/songs/lib/mediaitem.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 33c4f2e53..18d964115 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/openlp/plugins/songs/lib/mediaitem.py @@ -339,6 +339,9 @@ class SongMediaItem(MediaManagerItem): self.remote_song = -1 self.remote_triggered = None if item: + if preview: + # A song can only be edited if it comes from plugin, so we set it again for the new item. + item.from_plugin = True return item return None From 28599c8f245a7dcd104bdde3ade95e66452f4e12 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 13 Nov 2014 11:26:44 +0100 Subject: [PATCH 09/17] Make sure that the slidecontroller toolbar layout is correctly adjusted to fit its size. Fixes bug 1387304 Fixes: https://launchpad.net/bugs/1387304 --- openlp/core/ui/slidecontroller.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index d12506452..dfc7db2d8 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -691,7 +691,8 @@ class SlideController(DisplayController, RegistryProperties): self.mediabar.show() self.previous_item.setVisible(not item.is_media()) self.next_item.setVisible(not item.is_media()) - self.set_blank_menu() + # The layout of the toolbar is size dependent, so make sure it fits + self.on_controller_size_changed(self.controller.width()) # Work-around for OS X, hide and then show the toolbar # See bug #791050 self.toolbar.show() From ed13dcbd75b87bbe333873a4ba306e923a4c16b7 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 13 Nov 2014 12:21:54 +0100 Subject: [PATCH 10/17] Only show slide-dropdown on live-slidecontroller when it is populated. Fixes bug 1390238. Fixes: https://launchpad.net/bugs/1390238 --- openlp/core/ui/slidecontroller.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index dfc7db2d8..85fe240d4 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -683,7 +683,8 @@ class SlideController(DisplayController, RegistryProperties): self.play_slides_loop.setChecked(False) self.play_slides_loop.setIcon(build_icon(':/media/media_time.png')) if item.is_text(): - if Settings().value(self.main_window.songs_settings_section + '/display songbar') and self.slide_list: + if (Settings().value(self.main_window.songs_settings_section + '/display songbar') + and not self.song_menu.menu().isEmpty()): self.toolbar.set_widget_visible(['song_menu'], True) if item.is_capable(ItemCapabilities.CanLoop) and len(item.get_frames()) > 1: self.toolbar.set_widget_visible(LOOP_LIST) From 378132d82f299c72a8cc02ae300d727d055d0960 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 13 Nov 2014 20:15:43 +0000 Subject: [PATCH 11/17] 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') From 571b50f016e4bb7df450ec89cd51fc1618461b13 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sat, 15 Nov 2014 22:38:36 +0000 Subject: [PATCH 12/17] Fix for overwriting files on song export. Fixes bug 1216232 Fixes: https://launchpad.net/bugs/1216232 --- openlp/plugins/songs/lib/openlyricsexport.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/lib/openlyricsexport.py b/openlp/plugins/songs/lib/openlyricsexport.py index 0458b893b..84647dbee 100644 --- a/openlp/plugins/songs/lib/openlyricsexport.py +++ b/openlp/plugins/songs/lib/openlyricsexport.py @@ -75,9 +75,14 @@ class OpenLyricsExport(RegistryProperties): filename = '%s (%s)' % (song.title, ', '.join([author.display_name for author in song.authors])) filename = clean_filename(filename) # Ensure the filename isn't too long for some filesystems - filename = '%s.xml' % filename[0:250 - len(self.save_path)] + filename_with_ext = '%s.xml' % filename[0:250 - len(self.save_path)] + # Make sure we're not overwriting an existing file + conflicts = 0 + while os.path.exists(os.path.join(self.save_path, filename_with_ext)): + conflicts += 1 + filename_with_ext = '%s-%d.xml' % (filename[0:247 - len(self.save_path)], conflicts) # Pass a file object, because lxml does not cope with some special # characters in the path (see lp:757673 and lp:744337). - tree.write(open(os.path.join(self.save_path, filename), 'wb'), encoding='utf-8', xml_declaration=True, - pretty_print=True) + tree.write(open(os.path.join(self.save_path, filename_with_ext), 'wb'), encoding='utf-8', + xml_declaration=True, pretty_print=True) return True From c85f49a16c8a1d95b849c1135c3d36f9a6647b54 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sun, 16 Nov 2014 22:44:44 +0000 Subject: [PATCH 13/17] Added test for openlyrics export of songs with same title and author --- .../songs/test_openlyricsexport.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/functional/openlp_plugins/songs/test_openlyricsexport.py diff --git a/tests/functional/openlp_plugins/songs/test_openlyricsexport.py b/tests/functional/openlp_plugins/songs/test_openlyricsexport.py new file mode 100644 index 000000000..df28db035 --- /dev/null +++ b/tests/functional/openlp_plugins/songs/test_openlyricsexport.py @@ -0,0 +1,86 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2014 Raoul Snyman # +# Portions copyright (c) 2008-2014 Tim Bentley, Gerald Britton, Jonathan # +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # +# --------------------------------------------------------------------------- # +# 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 # +############################################################################### +""" +This module contains tests for the OpenLyrics song importer. +""" + +import os +import shutil +from unittest import TestCase +from tempfile import mkdtemp + +from tests.functional import MagicMock, patch +from tests.helpers.testmixin import TestMixin +from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport +from openlp.core.common import Registry + + +class TestOpenLyricsExport(TestCase, TestMixin): + """ + Test the functions in the :mod:`openlyricsexport` module. + """ + def setUp(self): + """ + Create the registry + """ + Registry.create() + self.temp_folder = mkdtemp() + + def tearDown(self): + """ + Cleanup + """ + shutil.rmtree(self.temp_folder) + + def export_same_filename_test(self): + """ + Test that files is not overwritten if songs has same title and author + """ + # GIVEN: A mocked song_to_xml, 2 mocked songs, a mocked application and an OpenLyricsExport instance + with patch('openlp.plugins.songs.lib.openlyricsexport.OpenLyrics.song_to_xml') as mocked_song_to_xml: + mocked_song_to_xml.return_value = '\n' + author = MagicMock() + author.display_name = 'Test Author' + song = MagicMock() + song.authors = [author] + song.title = 'Test Title' + parent = MagicMock() + parent.stop_export_flag = False + mocked_application_object = MagicMock() + Registry().register('application', mocked_application_object) + ol_export = OpenLyricsExport(parent, [song, song], self.temp_folder) + + # WHEN: Doing the export + ol_export.do_export() + + # THEN: The exporter should have created 2 files + self.assertTrue(os.path.exists(os.path.join(self.temp_folder, + '%s (%s).xml' % (song.title, author.display_name)))) + self.assertTrue(os.path.exists(os.path.join(self.temp_folder, + '%s (%s)-1.xml' % (song.title, author.display_name)))) From 4fb0e43c041dcbeb67f6440e66d888a325ba3f00 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Thu, 20 Nov 2014 06:58:33 +0000 Subject: [PATCH 14/17] New start old bugs --- openlp/core/lib/renderer.py | 9 +++- openlp/core/ui/firsttimeform.py | 11 ++++ .../songusage/forms/songusagedetailform.py | 2 +- .../openlp_core_lib/test_renderer.py | 54 ++++++++++++++++++- 4 files changed, 73 insertions(+), 3 deletions(-) diff --git a/openlp/core/lib/renderer.py b/openlp/core/lib/renderer.py index ab4a5a4df..3f7ffc80f 100644 --- a/openlp/core/lib/renderer.py +++ b/openlp/core/lib/renderer.py @@ -250,7 +250,13 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties): # Remove two or more option slide breaks next to each other (causing infinite loop). while '\n[---]\n[---]\n' in text: text = text.replace('\n[---]\n[---]\n', '\n[---]\n') - while True: + while ' [---]' in text: + text = text.replace(' [---]', '[---]') + while '[---] ' in text: + text = text.replace('[---] ', '[---]') + count = 0 + # only loop 5 times as there will never be more than 5 incorrect logical splits on a single slide. + while True and count < 5: slides = text.split('\n[---]\n', 2) # If there are (at least) two occurrences of [---] we use the first two slides (and neglect the last # for now). @@ -296,6 +302,7 @@ class Renderer(OpenLPMixin, RegistryMixin, RegistryProperties): lines = text.strip('\n').split('\n') pages.extend(self._paginate_slide(lines, line_end)) break + count =+ 1 else: # Clean up line endings. pages = self._paginate_slide(text.split('\n'), line_end) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 02d9e65f7..13d7ab0cf 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -95,6 +95,17 @@ class FirstTimeForm(QtGui.QWizard, UiFirstTimeWizard, RegistryProperties): """ self.application.process_events() if self.currentId() == FirstTimePage.Plugins: + if self.has_run_wizard: + self.songs_check_box.setChecked(self.plugin_manager.get_plugin_by_name('songs').is_active()) + self.bible_check_box.setChecked(self.plugin_manager.get_plugin_by_name('bibles').is_active()) + self.presentation_check_box.setChecked(self.plugin_manager.get_plugin_by_name( + 'presentations').is_active()) + self.image_check_box.setChecked(self.plugin_manager.get_plugin_by_name('images').is_active()) + self.media_check_box.setChecked(self.plugin_manager.get_plugin_by_name('media').is_active()) + self.remote_check_box.setChecked(self.plugin_manager.get_plugin_by_name('remotes').is_active()) + self.custom_check_box.setChecked(self.plugin_manager.get_plugin_by_name('custom').is_active()) + self.song_usage_check_box.setChecked(self.plugin_manager.get_plugin_by_name('songusage').is_active()) + self.alert_check_box.setChecked(self.plugin_manager.get_plugin_by_name('alerts').is_active()) if not self.web_access: return FirstTimePage.NoInternet else: diff --git a/openlp/plugins/songusage/forms/songusagedetailform.py b/openlp/plugins/songusage/forms/songusagedetailform.py index e8111c058..5d8470a99 100644 --- a/openlp/plugins/songusage/forms/songusagedetailform.py +++ b/openlp/plugins/songusage/forms/songusagedetailform.py @@ -99,7 +99,7 @@ class SongUsageDetailForm(QtGui.QDialog, Ui_SongUsageDetailDialog, RegistryPrope report_file_name = os.path.join(path, file_name) file_handle = None try: - file_handle = open(report_file_name, 'w') + file_handle = open(report_file_name, 'wb') for instance in usage: record = '\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",\"%s\",' \ '\"%s\",\"%s\"\n' % \ diff --git a/tests/functional/openlp_core_lib/test_renderer.py b/tests/functional/openlp_core_lib/test_renderer.py index 8df4816bb..3a3eaf4cb 100644 --- a/tests/functional/openlp_core_lib/test_renderer.py +++ b/tests/functional/openlp_core_lib/test_renderer.py @@ -34,7 +34,7 @@ from unittest import TestCase from PyQt4 import QtCore from openlp.core.common import Registry -from openlp.core.lib import Renderer, ScreenList +from openlp.core.lib import Renderer, ScreenList, ServiceItem from tests.interfaces import MagicMock @@ -109,3 +109,55 @@ class TestRenderer(TestCase): # THEN: The word lists should be the same. self.assertListEqual(result_words, expected_words) + + def format_slide_logical_split_test(self): + """ + Test that a line with text and a logic break does not break the renderer just returns the input + """ + # GIVEN: A line of with a space text and the logical split + renderer = Renderer() + renderer.empty_height = 25 + given_line = 'a\n[---]\nb' + expected_words = ['a
[---]
b'] + service_item = ServiceItem(None) + + # WHEN: Split the line based on rules + + result_words = renderer.format_slide(given_line, service_item) + + # THEN: The word lists should be the same. + self.assertListEqual(result_words, expected_words) + + def format_slide_blank_before_split_test(self): + """ + Test that a line with blanks before the logical split at handled + """ + # GIVEN: A line of with a space before the logical split + renderer = Renderer() + renderer.empty_height = 25 + given_line = '\n [---]\n' + expected_words = ['
[---]'] + service_item = ServiceItem(None) + + # WHEN: Split the line + result_words = renderer.format_slide(given_line, service_item) + + # THEN: The blanks have been removed. + self.assertListEqual(result_words, expected_words) + + def format_slide_blank_after_split_test(self): + """ + Test that a line with blanks before the logical split at handled + """ + # GIVEN: A line of with a space after the logical split + renderer = Renderer() + renderer.empty_height = 25 + given_line = '\n[---] \n' + expected_words = ['
[---] '] + service_item = ServiceItem(None) + + # WHEN: Split the line + result_words = renderer.format_slide(given_line, service_item) + + # THEN: The blanks have been removed. + self.assertListEqual(result_words, expected_words) \ No newline at end of file From 22a4974d86d353990897ba78f71d320e6413c1ad Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 22 Nov 2014 07:07:08 +0000 Subject: [PATCH 15/17] Comments --- tests/functional/openlp_core_lib/test_renderer.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_renderer.py b/tests/functional/openlp_core_lib/test_renderer.py index 3a3eaf4cb..d1a507373 100644 --- a/tests/functional/openlp_core_lib/test_renderer.py +++ b/tests/functional/openlp_core_lib/test_renderer.py @@ -88,7 +88,7 @@ class TestRenderer(TestCase): expected_tuple = ('{st}{r}Text text text{/r}{/st}', '{st}{r}', '') - # WHEN: + # WHEN: The renderer converts the start tags result = renderer._get_start_tags(given_raw_text) # THEN: Check if the correct tuple is returned. @@ -104,7 +104,7 @@ class TestRenderer(TestCase): given_line = 'beginning asdf \n end asdf' expected_words = ['beginning', 'asdf', 'end', 'asdf'] - # WHEN: Split the line + # WHEN: Split the line based on word split rules result_words = renderer._words_split(given_line) # THEN: The word lists should be the same. @@ -121,7 +121,7 @@ class TestRenderer(TestCase): expected_words = ['a
[---]
b'] service_item = ServiceItem(None) - # WHEN: Split the line based on rules + # WHEN: Split the line based on word split rules result_words = renderer.format_slide(given_line, service_item) @@ -139,7 +139,7 @@ class TestRenderer(TestCase): expected_words = ['
[---]'] service_item = ServiceItem(None) - # WHEN: Split the line + # WHEN: Split the line based on word split rules result_words = renderer.format_slide(given_line, service_item) # THEN: The blanks have been removed. @@ -156,8 +156,8 @@ class TestRenderer(TestCase): expected_words = ['
[---] '] service_item = ServiceItem(None) - # WHEN: Split the line + # WHEN: Split the line based on word split rules result_words = renderer.format_slide(given_line, service_item) # THEN: The blanks have been removed. - self.assertListEqual(result_words, expected_words) \ No newline at end of file + self.assertListEqual(result_words, expected_words) From f4ff654a654d80a12e1ed1460efcafb9a942d409 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 22 Nov 2014 20:26:59 +0000 Subject: [PATCH 16/17] If invlaid bible name stop searching Fixes: https://launchpad.net/bugs/1290246 --- openlp/plugins/bibles/lib/__init__.py | 5 ++++- .../openlp_plugins/bibles/test_lib_parse_reference.py | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index d67319797..fb10e6740 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -330,7 +330,10 @@ def parse_reference(reference, bible, language_selection, book_ref_id=False): if not book_ref_id: book_ref_id = bible.get_book_ref_id_by_localised_name(book, language_selection) elif not bible.get_book_by_book_ref_id(book_ref_id): - book_ref_id = False + return False + # We have not found the book so do not continue + if not book_ref_id: + return False ranges = match.group('ranges') range_list = get_reference_match('range_separator').split(ranges) ref_list = [] diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py index e20105ea1..989d62583 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py @@ -105,3 +105,13 @@ class TestBibleManager(TestCase, TestMixin): # THEN a verse array should be returned self.assertEqual([(54, 1, 1, -1), (54, 2, 1, 1)], results, "The bible verses should match the expected results") + + def parse_reference_four_test(self): + """ + Test the parse_reference method with non existence book + """ + # GIVEN given a bible in the bible manager + # WHEN asking to parse the bible reference + results = parse_reference('Raoul 1', self.manager.db_cache['tests'], MagicMock()) + # THEN a verse array should be returned + self.assertEqual(False, results, "The bible Search should return False") From 21db63b03216b062fe67da6268d9a9e77f965b5e Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Tue, 25 Nov 2014 21:36:52 +0000 Subject: [PATCH 17/17] remove blank line --- tests/functional/openlp_core_lib/test_renderer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_core_lib/test_renderer.py b/tests/functional/openlp_core_lib/test_renderer.py index d1a507373..5e3d087b9 100644 --- a/tests/functional/openlp_core_lib/test_renderer.py +++ b/tests/functional/openlp_core_lib/test_renderer.py @@ -122,7 +122,6 @@ class TestRenderer(TestCase): service_item = ServiceItem(None) # WHEN: Split the line based on word split rules - result_words = renderer.format_slide(given_line, service_item) # THEN: The word lists should be the same.