Merge lp:~tomasgroth/openlp/bugfixes19 into lp:openlp

[Tomas Groth]

Delete image thumbnails when the image is removed. Fixes bug 1446491.
Make sure we always add a blank line between verses. Fixes bug 1439311.
Normalize file path to OS standard after drag-and-drop. Fixes bug 1440571.
Make the way we display verse ranges consistent and make use of localized or custom separators. Fixes bug 1081807
Disable button for replacing live background if the webkit player is not available.

[Tomas Groth (tomasgroth)]

lp:~tomasgroth/openlp/bugfixes19 (revision 2535)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/1005/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/928/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/870/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/760/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/358/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/491/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/362/

[Raoul Snyman (raoul-snyman)]

review: Approve

[Tim Bentley (trb143)]

Approved

review: Approve

=== modified file 'openlp/core/common/uistrings.py'
--- openlp/core/common/uistrings.py	2015-02-09 20:49:24 +0000
+++ openlp/core/common/uistrings.py	2015-04-22 21:11:34 +0000
@@ -121,6 +121,8 @@
         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')

=== modified file 'openlp/core/lib/listwidgetwithdnd.py'
--- openlp/core/lib/listwidgetwithdnd.py	2015-01-18 13:39:21 +0000
+++ openlp/core/lib/listwidgetwithdnd.py	2015-04-22 21:11:34 +0000
@@ -95,7 +95,7 @@
             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):

=== modified file 'openlp/core/ui/printserviceform.py'
--- openlp/core/ui/printserviceform.py	2015-01-18 13:39:21 +0000
+++ openlp/core/ui/printserviceform.py	2015-04-22 21:11:34 +0000
@@ -193,13 +193,15 @@
             # 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')

=== modified file 'openlp/plugins/bibles/lib/mediaitem.py'
--- openlp/plugins/bibles/lib/mediaitem.py	2015-01-18 13:39:21 +0000
+++ openlp/plugins/bibles/lib/mediaitem.py	2015-04-22 21:11:34 +0000
@@ -849,7 +849,7 @@
         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

=== modified file 'openlp/plugins/bibles/lib/versereferencelist.py'
--- openlp/plugins/bibles/lib/versereferencelist.py	2015-02-13 20:19:05 +0000
+++ openlp/plugins/bibles/lib/versereferencelist.py	2015-04-22 21:11:34 +0000
@@ -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 @@
         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 @@
                     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(','):

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2015-01-30 21:03:53 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2015-04-22 21:11:34 +0000
@@ -205,6 +205,7 @@
         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 @@
                     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:

=== modified file 'openlp/plugins/media/lib/mediaitem.py'
--- openlp/plugins/media/lib/mediaitem.py	2015-03-10 21:33:35 +0000
+++ openlp/plugins/media/lib/mediaitem.py	2015-04-22 21:11:34 +0000
@@ -91,7 +91,10 @@
         """
         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 @@
         # 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)

=== modified file 'tests/functional/openlp_plugins/images/test_lib.py'
--- tests/functional/openlp_plugins/images/test_lib.py	2015-01-20 07:14:58 +0000
+++ tests/functional/openlp_plugins/images/test_lib.py	2015-04-22 21:11:34 +0000
@@ -97,8 +97,8 @@
             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 @@
             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 @@
             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 @@
             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 @@
             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 @@
         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,29 @@
             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
+            mocked_row_item.text.return_value = ''
+            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')