From c294ea5752ccdb55b3c3f81b478959474de6a081 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 7 Apr 2015 23:01:52 +0100 Subject: [PATCH 1/7] Make the way we display verse ranges consistent and make use of localized or custom separators. Fixes bug 1081807 Fixes: https://launchpad.net/bugs/1081807 --- openlp/plugins/bibles/lib/mediaitem.py | 2 +- .../plugins/bibles/lib/versereferencelist.py | 30 +++++++++++++------ 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index d2b877b81..0d3a40008 100644 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -849,7 +849,7 @@ class BibleMediaItem(MediaManagerItem): service_item.add_capability(ItemCapabilities.CanWordSplit) service_item.add_capability(ItemCapabilities.CanEditTitle) # Service Item: Title - service_item.title = create_separated_list(raw_title) + service_item.title = '%s %s' % (verses.format_verses(), verses.format_versions()) # Service Item: Theme if not self.settings.bible_theme: service_item.theme = None diff --git a/openlp/plugins/bibles/lib/versereferencelist.py b/openlp/plugins/bibles/lib/versereferencelist.py index a04617877..38496d956 100644 --- a/openlp/plugins/bibles/lib/versereferencelist.py +++ b/openlp/plugins/bibles/lib/versereferencelist.py @@ -20,6 +20,8 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### +from openlp.plugins.bibles.lib import get_reference_separator + class VerseReferenceList(object): """ @@ -53,29 +55,39 @@ class VerseReferenceList(object): self.version_list.append({'version': version, 'copyright': copyright, 'permission': permission}) def format_verses(self): + verse_sep = get_reference_separator('sep_v_display') + range_sep = get_reference_separator('sep_r_display') + list_sep = get_reference_separator('sep_l_display') result = '' for index, verse in enumerate(self.verse_list): if index == 0: - result = '%s %s:%s' % (verse['book'], verse['chapter'], verse['start']) + result = '%s %s%s%s' % (verse['book'], verse['chapter'], verse_sep, verse['start']) if verse['start'] != verse['end']: - result = '%s-%s' % (result, verse['end']) + result = '%s%s%s' % (result, range_sep, verse['end']) continue prev = index - 1 if self.verse_list[prev]['version'] != verse['version']: result = '%s (%s)' % (result, self.verse_list[prev]['version']) - result += ', ' + result += '%s ' % list_sep if self.verse_list[prev]['book'] != verse['book']: - result = '%s%s %s:' % (result, verse['book'], verse['chapter']) + result = '%s%s %s%s' % (result, verse['book'], verse['chapter'], verse_sep) elif self.verse_list[prev]['chapter'] != verse['chapter']: - result = '%s%s:' % (result, verse['chapter']) + result = '%s%s%s' % (result, verse['chapter'], verse_sep) result += str(verse['start']) if verse['start'] != verse['end']: - result = '%s-%s' % (result, verse['end']) + result = '%s%s%s' % (result, range_sep, verse['end']) if len(self.version_list) > 1: result = '%s (%s)' % (result, verse['version']) return result - def format_versions(self): + def format_versions(self, copyright=True, permission=True): + """ + Format a string with the bible versions used + + :param copyright: If the copyright info should be included, default is true. + :param permission: If the permission info should be included, default is true. + :return: A formatted string with the bible versions used + """ result = '' for index, version in enumerate(self.version_list): if index > 0: @@ -83,9 +95,9 @@ class VerseReferenceList(object): result += ';' result += ' ' result += version['version'] - if version['copyright'].strip(): + if copyright and version['copyright'].strip(): result += ', ' + version['copyright'] - if version['permission'].strip(): + if permission and version['permission'].strip(): result += ', ' + version['permission'] result = result.rstrip() if result.endswith(','): From 6f5a13932928c3200ea97d103d42d99f54cc1ebf Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 7 Apr 2015 23:20:30 +0100 Subject: [PATCH 2/7] Normalize file path to OS standard after drag-and-drop. Fixes bug 1440571. Fixes: https://launchpad.net/bugs/1440571 --- openlp/core/lib/listwidgetwithdnd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/lib/listwidgetwithdnd.py b/openlp/core/lib/listwidgetwithdnd.py index 2d5bcf6f3..4da1cf81e 100644 --- a/openlp/core/lib/listwidgetwithdnd.py +++ b/openlp/core/lib/listwidgetwithdnd.py @@ -95,7 +95,7 @@ class ListWidgetWithDnD(QtGui.QListWidget): event.accept() files = [] for url in event.mimeData().urls(): - local_file = url.toLocalFile() + local_file = os.path.normpath(url.toLocalFile()) if os.path.isfile(local_file): files.append(local_file) elif os.path.isdir(local_file): From cd7c4f6197e09f541c4f4046b81bf6018b34a1aa Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sat, 11 Apr 2015 23:13:30 +0100 Subject: [PATCH 3/7] Make sure we always add a blank line between verses. Fixes bug 1439311. Fixes: https://launchpad.net/bugs/1439311 --- openlp/core/ui/printserviceform.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openlp/core/ui/printserviceform.py b/openlp/core/ui/printserviceform.py index f74738947..05e60b911 100644 --- a/openlp/core/ui/printserviceform.py +++ b/openlp/core/ui/printserviceform.py @@ -193,13 +193,15 @@ class PrintServiceForm(QtGui.QDialog, Ui_PrintServiceDialog, RegistryProperties) # Add the text of the service item. if item.is_text(): verse_def = None + verse_html = None for slide in item.get_frames(): - if not verse_def or verse_def != slide['verseTag']: + if not verse_def or verse_def != slide['verseTag'] or verse_html == slide['html']: text_div = self._add_element('div', parent=div, classId='itemText') else: self._add_element('br', parent=text_div) self._add_element('span', slide['html'], text_div) verse_def = slide['verseTag'] + verse_html = slide['html'] # Break the page before the div element. if index != 0 and self.page_break_after_text.isChecked(): div.set('class', 'item newPage') From f8114d0b97edd650748b326843c625fcee7768a9 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 21 Apr 2015 22:49:22 +0100 Subject: [PATCH 4/7] Disable button for replacing live background if the webkit player is not available. --- openlp/core/common/uistrings.py | 2 ++ openlp/plugins/media/lib/mediaitem.py | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/openlp/core/common/uistrings.py b/openlp/core/common/uistrings.py index bebf4add0..8add1a35c 100644 --- a/openlp/core/common/uistrings.py +++ b/openlp/core/common/uistrings.py @@ -121,6 +121,8 @@ class UiStrings(object): self.Projectors = translate('OpenLP.Ui', 'Projectors', 'Plural') self.ReplaceBG = translate('OpenLP.Ui', 'Replace Background') self.ReplaceLiveBG = translate('OpenLP.Ui', 'Replace live background.') + self.ReplaceLiveBGDisabled = translate('OpenLP.Ui', 'Replace live background is not available on this ' + 'platform in this version of OpenLP.') self.ResetBG = translate('OpenLP.Ui', 'Reset Background') self.ResetLiveBG = translate('OpenLP.Ui', 'Reset live background.') self.Seconds = translate('OpenLP.Ui', 's', 'The abbreviated unit for seconds') diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index e3b8137eb..4fd1620f3 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -91,7 +91,10 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): """ self.on_new_prompt = translate('MediaPlugin.MediaItem', 'Select Media') self.replace_action.setText(UiStrings().ReplaceBG) - self.replace_action.setToolTip(UiStrings().ReplaceLiveBG) + if 'webkit' in get_media_players()[0]: + self.replace_action.setToolTip(UiStrings().ReplaceLiveBG) + else: + self.replace_action.setToolTip(UiStrings().ReplaceLiveBGDisabled) self.reset_action.setText(UiStrings().ResetBG) self.reset_action.setToolTip(UiStrings().ResetLiveBG) self.automatic = UiStrings().Automatic @@ -139,6 +142,8 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties): # Replace backgrounds do not work at present so remove functionality. self.replace_action = self.toolbar.add_toolbar_action('replace_action', icon=':/slides/slide_blank.png', triggers=self.on_replace_click) + if 'webkit' not in get_media_players()[0]: + self.replace_action.setDisabled(True) self.reset_action = self.toolbar.add_toolbar_action('reset_action', icon=':/system/system_close.png', visible=False, triggers=self.on_reset_click) self.media_widget = QtGui.QWidget(self) From 29f5efeb91fa1f1a8f33947587d930746e04baa0 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 22 Apr 2015 20:37:32 +0100 Subject: [PATCH 5/7] Delete image thumbnails when the image is removed. Fixes bug 1446491. Fixes: https://launchpad.net/bugs/1446491 --- openlp/plugins/images/lib/mediaitem.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index a52b25d8a..8c8bfe811 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -205,6 +205,7 @@ class ImageMediaItem(MediaManagerItem): images = self.manager.get_all_objects(ImageFilenames, ImageFilenames.group_id == image_group.id) for image in images: delete_file(os.path.join(self.service_path, os.path.split(image.filename)[1])) + delete_file(self.generate_thumbnail_path(image)) self.manager.delete_object(ImageFilenames, image.id) image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == image_group.id) for group in image_groups: @@ -227,6 +228,7 @@ class ImageMediaItem(MediaManagerItem): item_data = row_item.data(0, QtCore.Qt.UserRole) if isinstance(item_data, ImageFilenames): delete_file(os.path.join(self.service_path, row_item.text(0))) + delete_file(self.generate_thumbnail_path(item_data)) if item_data.group_id == 0: self.list_view.takeTopLevelItem(self.list_view.indexOfTopLevelItem(row_item)) else: From cbeece73750a01c4632eb822ad6275bce995738a Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 22 Apr 2015 21:57:33 +0100 Subject: [PATCH 6/7] Added test for on_delete_click in the image plugin. --- .../openlp_plugins/images/test_lib.py | 53 ++++++++++++++----- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/tests/functional/openlp_plugins/images/test_lib.py b/tests/functional/openlp_plugins/images/test_lib.py index 9dfb324cc..8327ef78d 100644 --- a/tests/functional/openlp_plugins/images/test_lib.py +++ b/tests/functional/openlp_plugins/images/test_lib.py @@ -97,8 +97,8 @@ class TestImageMediaItem(TestCase): self.media_item.save_new_images_list(image_list) # THEN: The save_object() method should not have been called - assert self.media_item.manager.save_object.call_count == 0, \ - 'The save_object() method should not have been called' + self.assertEquals(self.media_item.manager.save_object.call_count, 0, + 'The save_object() method should not have been called') def save_new_images_list_single_image_with_reload_test(self): """ @@ -114,7 +114,7 @@ class TestImageMediaItem(TestCase): self.media_item.save_new_images_list(image_list, reload_list=True) # THEN: load_full_list() should have been called - assert mocked_load_full_list.call_count == 1, 'load_full_list() should have been called' + self.assertEquals(mocked_load_full_list.call_count, 1, 'load_full_list() should have been called') # CLEANUP: Remove added attribute from ImageFilenames delattr(ImageFilenames, 'filename') @@ -132,7 +132,7 @@ class TestImageMediaItem(TestCase): self.media_item.save_new_images_list(image_list, reload_list=False) # THEN: load_full_list() should not have been called - assert mocked_load_full_list.call_count == 0, 'load_full_list() should not have been called' + self.assertEquals(mocked_load_full_list.call_count, 0, 'load_full_list() should not have been called') def save_new_images_list_multiple_images_test(self): """ @@ -147,8 +147,8 @@ class TestImageMediaItem(TestCase): self.media_item.save_new_images_list(image_list, reload_list=False) # THEN: load_full_list() should not have been called - assert self.media_item.manager.save_object.call_count == 3, \ - 'load_full_list() should have been called three times' + self.assertEquals(self.media_item.manager.save_object.call_count, 3, + 'load_full_list() should have been called three times') def save_new_images_list_other_objects_in_list_test(self): """ @@ -163,8 +163,8 @@ class TestImageMediaItem(TestCase): self.media_item.save_new_images_list(image_list, reload_list=False) # THEN: load_full_list() should not have been called - assert self.media_item.manager.save_object.call_count == 2, \ - 'load_full_list() should have been called only once' + self.assertEquals(self.media_item.manager.save_object.call_count, 2, + 'load_full_list() should have been called only once') def on_reset_click_test(self): """ @@ -185,22 +185,22 @@ class TestImageMediaItem(TestCase): Test that recursively_delete_group() works """ # GIVEN: An ImageGroups object and mocked functions - with patch('openlp.core.utils.delete_file') as mocked_delete_file: + with patch('openlp.plugins.images.lib.mediaitem.delete_file') as mocked_delete_file: ImageFilenames.group_id = 1 ImageGroups.parent_id = 1 self.media_item.manager = MagicMock() self.media_item.manager.get_all_objects.side_effect = self._recursively_delete_group_side_effect - self.media_item.service_path = "" + self.media_item.service_path = '' test_group = ImageGroups() test_group.id = 1 # WHEN: recursively_delete_group() is called self.media_item.recursively_delete_group(test_group) - # THEN: - assert mocked_delete_file.call_count == 0, 'delete_file() should not be called' - assert self.media_item.manager.delete_object.call_count == 7, \ - 'manager.delete_object() should be called exactly 7 times' + # THEN: delete_file() should have been called 12 times and manager.delete_object() 7 times. + self.assertEquals(mocked_delete_file.call_count, 12, 'delete_file() should have been called 12 times') + self.assertEquals(self.media_item.manager.delete_object.call_count, 7, + 'manager.delete_object() should be called exactly 7 times') # CLEANUP: Remove added attribute from Image Filenames and ImageGroups delattr(ImageFilenames, 'group_id') @@ -230,3 +230,28 @@ class TestImageMediaItem(TestCase): returned_object1.id = 1 return [returned_object1] return [] + + def on_delete_click_test(self): + """ + Test that on_delete_click() works + """ + # GIVEN: An ImageGroups object and mocked functions + with patch('openlp.plugins.images.lib.mediaitem.delete_file') as mocked_delete_file, \ + patch('openlp.plugins.images.lib.mediaitem.check_item_selected') as mocked_check_item_selected: + mocked_check_item_selected.return_value = True + test_image = ImageFilenames() + test_image.id = 1 + test_image.group_id = 1 + test_image.filename = 'imagefile.png' + self.media_item.manager = MagicMock() + self.media_item.service_path = '' + self.media_item.list_view = MagicMock() + mocked_row_item = MagicMock() + mocked_row_item.data.return_value = test_image + self.media_item.list_view.selectedItems.return_value = [mocked_row_item] + + # WHEN: Calling on_delete_click + self.media_item.on_delete_click() + + # THEN: delete_file should have been called twice + self.assertEquals(mocked_delete_file.call_count, 2, 'delete_file() should have been called twice') From ae785815c8a9df764bde191a2f61e68775cab480 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 22 Apr 2015 22:05:46 +0100 Subject: [PATCH 7/7] Fix test on windows. --- tests/functional/openlp_plugins/images/test_lib.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/openlp_plugins/images/test_lib.py b/tests/functional/openlp_plugins/images/test_lib.py index 8327ef78d..070b4b40d 100644 --- a/tests/functional/openlp_plugins/images/test_lib.py +++ b/tests/functional/openlp_plugins/images/test_lib.py @@ -248,6 +248,7 @@ class TestImageMediaItem(TestCase): self.media_item.list_view = MagicMock() mocked_row_item = MagicMock() mocked_row_item.data.return_value = test_image + mocked_row_item.text.return_value = '' self.media_item.list_view.selectedItems.return_value = [mocked_row_item] # WHEN: Calling on_delete_click