Various fixes

lp:~phill-ridout/openlp/fixes-mkIII (revision 2794)
https://ci.openlp.io/job/Branch-01-Pull/2301/                          [WAITING]
[RUNNING]
[SUCCESS]
https://ci.openlp.io/job/Branch-02-Functional-Tests/2202/              [WAITING]
[RUNNING]
[SUCCESS]
https://ci.openlp.io/job/Branch-03-Interface-Tests/2080/               [WAITING]
[RUNNING]
[SUCCESS]
https://ci.openlp.io/job/Branch-04a-Code_Analysis/1406/                [WAITING]
[RUNNING]
[SUCCESS]
https://ci.openlp.io/job/B...

bzr-revno: 2788

openlp/core/ui/media/vendor/mediainfoWrapper.py

@@ -25,10 +25,8 @@ information related to the rwquested media.
 """
 import json
 import os
-from subprocess import Popen
+from subprocess import check_output
-from tempfile import mkstemp
 
-import six
 from bs4 import BeautifulSoup, NavigableString
 
 ENV_DICT = os.environ
@@ -80,7 +78,7 @@ class Track(object):
 
     def to_data(self):
         data = {}
-        for k, v in six.iteritems(self.__dict__):
+        for k, v in self.__dict__.items():
             if k != 'xml_dom_fragment':
                 data[k] = v
         return data
@@ -100,20 +98,10 @@ class MediaInfoWrapper(object):
 
     @staticmethod
     def parse(filename, environment=ENV_DICT):
-        command = ["mediainfo", "-f", "--Output=XML", filename]
+        xml = check_output(['mediainfo', '-f', '--Output=XML', '--Inform=OLDXML', filename])
-        fileno_out, fname_out = mkstemp(suffix=".xml", prefix="media-")
+        if not xml.startswith(b'<?xml'):
-        fileno_err, fname_err = mkstemp(suffix=".err", prefix="media-")
+            xml = check_output(['mediainfo', '-f', '--Output=XML', filename])
-        fp_out = os.fdopen(fileno_out, 'r+b')
+        xml_dom = MediaInfoWrapper.parse_xml_data_into_dom(xml)
-        fp_err = os.fdopen(fileno_err, 'r+b')
-        p = Popen(command, stdout=fp_out, stderr=fp_err, env=environment)
-        p.wait()
-        fp_out.seek(0)
-
-        xml_dom = MediaInfoWrapper.parse_xml_data_into_dom(fp_out.read())
-        fp_out.close()
-        fp_err.close()
-        os.unlink(fname_out)
-        os.unlink(fname_err)
         return MediaInfoWrapper(xml_dom)
 
     def _populate_tracks(self):

openlp/core/ui/servicemanager.py

@@ -216,7 +216,7 @@ class Ui_ServiceManager(object):
             text=translate('OpenLP.ServiceManager', 'Go Live'), icon=':/general/general_live.png',
             tooltip=translate('OpenLP.ServiceManager', 'Send the selected item to Live.'),
             category=UiStrings().Service,
-            triggers=self.make_live)
+            triggers=self.on_make_live_action_triggered)
         self.layout.addWidget(self.order_toolbar)
         # Connect up our signals and slots
         self.theme_combo_box.activated.connect(self.on_theme_combo_box_selected)
@@ -704,15 +704,19 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         directory_path = Settings().value(self.main_window.service_manager_settings_section + '/last directory')
         if directory_path:
             default_file_path = directory_path / default_file_path
-        # SaveAs from osz to oszl is not valid as the files will be deleted on exit which is not sensible or usable in
-        # the long term.
         lite_filter = translate('OpenLP.ServiceManager', 'OpenLP Service Files - lite (*.oszl)')
         packaged_filter = translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz)')
-
+        if self._file_name.endswith('oszl'):
+            default_filter = lite_filter
+        else:
+            default_filter = packaged_filter
+        # SaveAs from osz to oszl is not valid as the files will be deleted on exit which is not sensible or usable in
+        # the long term.
         if self._file_name.endswith('oszl') or self.service_has_all_original_files:
             file_path, filter_used = FileDialog.getSaveFileName(
                 self.main_window, UiStrings().SaveService, default_file_path,
-                '{packaged};; {lite}'.format(packaged=packaged_filter, lite=lite_filter))
+                '{packaged};; {lite}'.format(packaged=packaged_filter, lite=lite_filter),
+                default_filter)
         else:
             file_path, filter_used = FileDialog.getSaveFileName(
                 self.main_window, UiStrings().SaveService, default_file_path,
@@ -1730,6 +1734,15 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         self.service_items[item]['service_item'].update_theme(theme)
         self.regenerate_service_items(True)
 
+    def on_make_live_action_triggered(self, checked):
+        """
+        Handle `make_live_action` when the action is triggered.
+
+        :param bool checked: Not Used.
+        :rtype: None
+        """
+        self.make_live()
+
     def get_drop_position(self):
         """
         Getter for drop_position. Used in: MediaManagerItem

openlp/plugins/custom/lib/db.py

@@ -26,7 +26,7 @@ the Custom plugin
 from sqlalchemy import Column, Table, types
 from sqlalchemy.orm import mapper
 
-from openlp.core.common.i18n import get_locale_key
+from openlp.core.common.i18n import get_natural_key
 from openlp.core.lib.db import BaseModel, init_db
 
 
@@ -36,10 +36,10 @@ class CustomSlide(BaseModel):
     """
     # By default sort the customs by its title considering language specific characters.
     def __lt__(self, other):
-        return get_locale_key(self.title) < get_locale_key(other.title)
+        return get_natural_key(self.title) < get_natural_key(other.title)
 
     def __eq__(self, other):
-        return get_locale_key(self.title) == get_locale_key(other.title)
+        return get_natural_key(self.title) == get_natural_key(other.title)
 
     def __hash__(self):
         """

openlp/plugins/images/lib/mediaitem.py

@@ -26,7 +26,7 @@ from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import delete_file, get_images_filter
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, create_paths
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -81,8 +81,12 @@ class ImageMediaItem(MediaManagerItem):
         self.add_group_action.setToolTip(UiStrings().AddGroupDot)
         self.replace_action.setText(UiStrings().ReplaceBG)
         self.replace_action.setToolTip(UiStrings().ReplaceLiveBG)
+        self.replace_action_context.setText(UiStrings().ReplaceBG)
+        self.replace_action_context.setToolTip(UiStrings().ReplaceLiveBG)
         self.reset_action.setText(UiStrings().ResetBG)
         self.reset_action.setToolTip(UiStrings().ResetLiveBG)
+        self.reset_action_context.setText(UiStrings().ResetBG)
+        self.reset_action_context.setToolTip(UiStrings().ResetLiveBG)
 
     def required_icons(self):
         """
@@ -184,6 +188,13 @@ class ImageMediaItem(MediaManagerItem):
             self.list_view,
             text=translate('ImagePlugin', 'Add new image(s)'),
             icon=':/general/general_open.png', triggers=self.on_file_click)
+        create_widget_action(self.list_view, separator=True)
+        self.replace_action_context = create_widget_action(
+            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_theme.png',
+            triggers=self.on_replace_click)
+        self.reset_action_context = create_widget_action(
+            self.list_view, text=UiStrings().ReplaceLiveBG, icon=':/system/system_close.png',
+            visible=False, triggers=self.on_reset_click)
 
     def add_start_header_bar(self):
         """
@@ -271,7 +282,7 @@ class ImageMediaItem(MediaManagerItem):
         :param parent_group_id: The ID of the group that will be added recursively.
         """
         image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
-        image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
+        image_groups.sort(key=lambda group_object: get_natural_key(group_object.group_name))
         folder_icon = build_icon(':/images/image_group.png')
         for image_group in image_groups:
             group = QtWidgets.QTreeWidgetItem()
@@ -298,7 +309,7 @@ class ImageMediaItem(MediaManagerItem):
             combobox.clear()
             combobox.top_level_group_added = False
         image_groups = self.manager.get_all_objects(ImageGroups, ImageGroups.parent_id == parent_group_id)
-        image_groups.sort(key=lambda group_object: get_locale_key(group_object.group_name))
+        image_groups.sort(key=lambda group_object: get_natural_key(group_object.group_name))
         for image_group in image_groups:
             combobox.addItem(prefix + image_group.group_name, image_group.id)
             self.fill_groups_combobox(combobox, image_group.id, prefix + '   ')
@@ -355,7 +366,7 @@ class ImageMediaItem(MediaManagerItem):
             self.expand_group(open_group.id)
         # Sort the images by its filename considering language specific.
         # characters.
-        images.sort(key=lambda image_object: get_locale_key(image_object.file_path.name))
+        images.sort(key=lambda image_object: get_natural_key(image_object.file_path.name))
         for image in images:
             log.debug('Loading image: {name}'.format(name=image.file_path))
             file_name = image.file_path.name
@@ -533,9 +544,9 @@ class ImageMediaItem(MediaManagerItem):
                 group_items.append(item)
             if isinstance(item.data(0, QtCore.Qt.UserRole), ImageFilenames):
                 image_items.append(item)
-        group_items.sort(key=lambda item: get_locale_key(item.text(0)))
+        group_items.sort(key=lambda item: get_natural_key(item.text(0)))
         target_group.addChildren(group_items)
-        image_items.sort(key=lambda item: get_locale_key(item.text(0)))
+        image_items.sort(key=lambda item: get_natural_key(item.text(0)))
         target_group.addChildren(image_items)
 
     def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
@@ -659,6 +670,7 @@ class ImageMediaItem(MediaManagerItem):
         Called to reset the Live background with the image selected.
         """
         self.reset_action.setVisible(False)
+        self.reset_action_context.setVisible(False)
         self.live_controller.display.reset_image()
 
     def live_theme_changed(self):
@@ -666,6 +678,7 @@ class ImageMediaItem(MediaManagerItem):
         Triggered by the change of theme in the slide controller.
         """
         self.reset_action.setVisible(False)
+        self.reset_action_context.setVisible(False)
 
     def on_replace_click(self):
         """
@@ -683,6 +696,7 @@ class ImageMediaItem(MediaManagerItem):
             if file_path.exists():
                 if self.live_controller.display.direct_image(str(file_path), background):
                     self.reset_action.setVisible(True)
+                    self.reset_action_context.setVisible(True)
                 else:
                     critical_error_message_box(
                         UiStrings().LiveBGError,

openlp/plugins/media/lib/mediaitem.py

@@ -26,7 +26,7 @@ import os
 from PyQt5 import QtCore, QtWidgets
 
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, path_to_str, create_paths
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
@@ -176,7 +176,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
     def add_custom_context_actions(self):
         create_widget_action(self.list_view, separator=True)
         self.replace_action_context = create_widget_action(
-            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_blank.png',
+            self.list_view, text=UiStrings().ReplaceBG, icon=':/slides/slide_theme.png',
             triggers=self.on_replace_click)
         self.reset_action_context = create_widget_action(
             self.list_view, text=UiStrings().ReplaceLiveBG, icon=':/system/system_close.png',
@@ -362,7 +362,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         :param media: The media
         :param target_group:
         """
-        media.sort(key=lambda file_name: get_locale_key(os.path.split(str(file_name))[1]))
+        media.sort(key=lambda file_name: get_natural_key(os.path.split(str(file_name))[1]))
         for track in media:
             track_info = QtCore.QFileInfo(track)
             item_name = None
@@ -404,7 +404,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         :return: The media list
         """
         media_file_paths = Settings().value(self.settings_section + '/media files')
-        media_file_paths.sort(key=lambda file_path: get_locale_key(file_path.name))
+        media_file_paths.sort(key=lambda file_path: get_natural_key(file_path.name))
         if media_type == MediaType.Audio:
             extension = self.media_controller.audio_extensions_list
         else:

openlp/plugins/presentations/lib/mediaitem.py

@@ -23,7 +23,7 @@ import logging
 
 from PyQt5 import QtCore, QtGui, QtWidgets
 
-from openlp.core.common.i18n import UiStrings, translate, get_locale_key
+from openlp.core.common.i18n import UiStrings, translate, get_natural_key
 from openlp.core.common.path import Path, path_to_str, str_to_path
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
@@ -165,7 +165,7 @@ class PresentationMediaItem(MediaManagerItem):
         if not initial_load:
             self.main_window.display_progress_bar(len(file_paths))
         # Sort the presentations by its filename considering language specific characters.
-        file_paths.sort(key=lambda file_path: get_locale_key(file_path.name))
+        file_paths.sort(key=lambda file_path: get_natural_key(file_path.name))
         for file_path in file_paths:
             if not initial_load:
                 self.main_window.increment_progress_bar()
@@ -243,7 +243,7 @@ class PresentationMediaItem(MediaManagerItem):
         """
         Clean up the files created such as thumbnails
 
-        :param openlp.core.common.path.Path file_path: File path of the presention to clean up after
+        :param openlp.core.common.path.Path file_path: File path of the presentation to clean up after
         :param bool clean_for_update: Only clean thumbnails if update is needed
         :rtype: None
         """

openlp/plugins/presentations/lib/messagelistener.py

@@ -191,7 +191,7 @@ class Controller(object):
         """
         Based on the handler passed at startup triggers the previous slide event.
         """
-        log.debug('Live = {live}, previous'.formta(live=self.is_live))
+        log.debug('Live = {live}, previous'.format(live=self.is_live))
         if not self.doc:
             return
         if not self.is_live:

openlp/plugins/presentations/lib/pdfcontroller.py

@@ -19,8 +19,6 @@
 # with this program; if not, write to the Free Software Foundation, Inc., 59  #
 # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
 ###############################################################################
-
-import os
 import logging
 import re
 from subprocess import check_output, CalledProcessError

openlp/plugins/presentations/lib/presentationcontroller.py

@@ -139,7 +139,8 @@ class PresentationDocument(object):
         :return: The path to the thumbnail
         :rtype: openlp.core.common.path.Path
         """
-        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in
+        #       get_temp_folder and PresentationPluginapp_startup is removed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(bytes(self.file_path))
         else:
@@ -153,7 +154,8 @@ class PresentationDocument(object):
         :return: The path to the temporary file folder
         :rtype: openlp.core.common.path.Path
         """
-        # TODO: If statement can be removed when the upgrade path from 2.0.x to 2.2.x is no longer needed
+        # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in
+        #       get_thumbnail_folder and PresentationPluginapp_startup is removed
         if Settings().value('presentations/thumbnail_scheme') == 'md5':
             folder = md5_hash(bytes(self.file_path))
         else:

openlp/plugins/presentations/presentationplugin.py

@@ -31,6 +31,7 @@ from PyQt5 import QtCore
 from openlp.core.api.http import register_endpoint
 from openlp.core.common import extension_loader
 from openlp.core.common.i18n import translate
+from openlp.core.common.settings import Settings
 from openlp.core.lib import Plugin, StringContent, build_icon
 from openlp.plugins.presentations.endpoint import api_presentations_endpoint, presentations_endpoint
 from openlp.plugins.presentations.lib import PresentationController, PresentationMediaItem, PresentationTab
@@ -136,6 +137,20 @@ class PresentationPlugin(Plugin):
             self.register_controllers(controller)
         return bool(self.controllers)
 
+    def app_startup(self):
+        """
+        Perform tasks on application startup.
+        """
+        # TODO: Can be removed when the upgrade path to OpenLP 3.0 is no longer needed, also ensure code in
+        #       PresentationDocument.get_thumbnail_folder and PresentationDocument.get_temp_folder is removed
+        super().app_startup()
+        files_from_config = Settings().value('presentations/presentations files')
+        for file in files_from_config:
+            self.media_item.clean_up_thumbnails(file, clean_for_update=True)
+        self.media_item.list_view.clear()
+        Settings().setValue('presentations/thumbnail_scheme', 'md5')
+        self.media_item.validate_and_load(files_from_config)
+
     @staticmethod
     def about():
         """

tests/functional/openlp_plugins/images/test_lib.py

@@ -173,12 +173,14 @@ class TestImageMediaItem(TestCase):
         """
         # GIVEN: A mocked version of reset_action
         self.media_item.reset_action = MagicMock()
+        self.media_item.reset_action_context = MagicMock()
 
         # WHEN: on_reset_click is called
         self.media_item.on_reset_click()
 
         # THEN: the reset_action should be set visible, and the image should be reset
         self.media_item.reset_action.setVisible.assert_called_with(False)
+        self.media_item.reset_action_context.setVisible.assert_called_with(False)
         self.media_item.live_controller.display.reset_image.assert_called_with()
 
     @patch('openlp.plugins.images.lib.mediaitem.delete_file')

tests/functional/openlp_plugins/presentations/test_mediaitem.py

@@ -133,3 +133,27 @@ class TestMediaItem(TestCase, TestMixin):
 
         # THEN: doc.presentation_deleted should have been called since the presentation file did not exists.
         mocked_doc.assert_has_calls([call.get_thumbnail_path(1, True), call.presentation_deleted()], True)
+
+    @patch('openlp.plugins.presentations.lib.mediaitem.MediaManagerItem._setup')
+    @patch('openlp.plugins.presentations.lib.mediaitem.PresentationMediaItem.setup_item')
+    @patch('openlp.plugins.presentations.lib.mediaitem.Settings')
+    def test_search(self, mocked_settings, *unreferenced_mocks):
+        """
+        Test that the search method finds the correct results
+        """
+        # GIVEN: A mocked Settings class which returns a list of Path objects,
+        #        and an instance of the PresentationMediaItem
+        path_1 = Path('some_dir', 'Impress_file_1')
+        path_2 = Path('some_other_dir', 'impress_file_2')
+        path_3 = Path('another_dir', 'ppt_file')
+        mocked_returned_settings = MagicMock()
+        mocked_returned_settings.value.return_value = [path_1, path_2, path_3]
+        mocked_settings.return_value = mocked_returned_settings
+        media_item = PresentationMediaItem(None, MagicMock(), None)
+        media_item.settings_section = ''
+
+        # WHEN: Calling search
+        results = media_item.search('IMPRE', False)
+
+        # THEN: The first two results should have been returned
+        assert results == [[str(path_1), 'Impress_file_1'], [str(path_2), 'impress_file_2']]