head

.bzrignore

@@ -7,6 +7,7 @@ cover
 .coverage
 coverage
 .directory
+.vscode
 dist
 *.dll
 documentation/build/doctrees

openlp/core/api/deploy.py

@@ -39,8 +39,8 @@ def deploy_zipfile(app_root_path, zip_name):
     :return: None
     """
     zip_path = app_root_path / zip_name
-    web_zip = ZipFile(str(zip_path))
-    web_zip.extractall(str(app_root_path))
+    web_zip = ZipFile(zip_path)
+    web_zip.extractall(app_root_path)
 
 
 def download_sha256():

openlp/core/app.py

@@ -317,8 +317,7 @@ def set_up_logging(log_path):
     """
     create_paths(log_path, do_not_log=True)
     file_path = log_path / 'openlp.log'
-    # TODO: FileHandler accepts a Path object in Py3.6
-    logfile = logging.FileHandler(str(file_path), 'w', encoding='UTF-8')
+    logfile = logging.FileHandler(file_path, 'w', encoding='UTF-8')
     logfile.setFormatter(logging.Formatter('%(asctime)s %(threadName)s %(name)-55s %(levelname)-8s %(message)s'))
     log.addHandler(logfile)
     if log.isEnabledFor(logging.DEBUG):
@@ -364,7 +363,7 @@ def main(args=None):
         portable_settings_path = data_path / 'OpenLP.ini'
         # Make this our settings file
         log.info('INI file: {name}'.format(name=portable_settings_path))
-        Settings.set_filename(str(portable_settings_path))
+        Settings.set_filename(portable_settings_path)
         portable_settings = Settings()
         # Set our data path
         log.info('Data path: {name}'.format(name=data_path))
@@ -405,7 +404,12 @@ def main(args=None):
                 None, translate('OpenLP', 'Settings Upgrade'),
                 translate('OpenLP', 'Your settings are about to be upgraded. A backup will be created at '
                                     '{back_up_path}').format(back_up_path=back_up_path))
+            try:
                 settings.export(back_up_path)
+            except OSError:
+                QtWidgets.QMessageBox.warning(
+                    None, translate('OpenLP', 'Settings Upgrade'),
+                    translate('OpenLP', 'Settings back up failed.\n\nContinuining to upgrade.'))
         settings.upgrade_settings()
     # First time checks in settings
     if not Settings().value('core/has run wizard'):

openlp/core/common/__init__.py

@@ -474,10 +474,10 @@ def get_file_encoding(file_path):
                 if not chunk:
                     break
                 detector.feed(chunk)
-            detector.close()
-        return detector.result
     except OSError:
         log.exception('Error detecting file encoding')
+    finally:
+        return detector.close()
 
 
 def normalize_str(irregular_string):

openlp/core/common/httputils.py

@@ -27,12 +27,16 @@ import logging
 import sys
 import time
 from random import randint
+from tempfile import gettempdir
 
 import requests
+from PyQt5 import QtCore
 
 from openlp.core.common import trace_error_handler
+from openlp.core.common.path import Path
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import ProxyMode, Settings
+from openlp.core.threading import ThreadWorker
 
 
 log = logging.getLogger(__name__ + '.__init__')
@@ -227,4 +231,46 @@ def download_file(update_object, url, file_path, sha256=None):
     return True
 
 
-__all__ = ['get_web_page']
+class DownloadWorker(ThreadWorker):
+    """
+    This worker allows a file to be downloaded in a thread
+    """
+    download_failed = QtCore.pyqtSignal()
+    download_succeeded = QtCore.pyqtSignal(Path)
+
+    def __init__(self, base_url, file_name):
+        """
+        Set up the worker object
+        """
+        self._base_url = base_url
+        self._file_name = file_name
+        self._download_cancelled = False
+        super().__init__()
+
+    def start(self):
+        """
+        Download the url to the temporary directory
+        """
+        if self._download_cancelled:
+            self.quit.emit()
+            return
+        try:
+            dest_path = Path(gettempdir()) / 'openlp' / self._file_name
+            url = '{url}{name}'.format(url=self._base_url, name=self._file_name)
+            is_success = download_file(self, url, dest_path)
+            if is_success and not self._download_cancelled:
+                self.download_succeeded.emit(dest_path)
+            else:
+                self.download_failed.emit()
+        except Exception:
+            log.exception('Unable to download %s', url)
+            self.download_failed.emit()
+        finally:
+            self.quit.emit()
+
+    @QtCore.pyqtSlot()
+    def cancel_download(self):
+        """
+        A slot to allow the download to be cancelled from outside of the thread
+        """
+        self._download_cancelled = True

openlp/core/common/path.py

@@ -78,7 +78,7 @@ class Path(PathVariant):
         :param onerror: Handler function to handle any errors
         :rtype: None
         """
-        shutil.rmtree(str(self), ignore_errors, onerror)
+        shutil.rmtree(self, ignore_errors, onerror)
 
 
 def replace_params(args, kwargs, params):

openlp/core/common/settings.py

@@ -542,7 +542,7 @@ class Settings(QtCore.QSettings):
                         old_values = [self._convert_value(old_value, default_value)
                                       for old_value, default_value in zip(old_values, default_values)]
                     # Iterate over our rules and check what the old_value should be "converted" to.
-                    new_value = None
+                    new_value = old_values[0]
                     for new_rule, old_rule in rules:
                         # If the value matches with the condition (rule), then use the provided value. This is used to
                         # convert values. E. g. an old value 1 results in True, and 0 in False.

openlp/core/display/html/display.html

@@ -6,7 +6,7 @@
     <style type="text/css">
     body {
       background: transparent !important;
-      color: #fff !important;
+      color: rgb(255, 255, 255) !important;
     }
     sup {
       vertical-align: super !important;

openlp/core/display/html/display.js

@@ -315,7 +315,7 @@ var Display = {
     section.setAttribute("style", "height: 100%; width: 100%; position: relative;");
     var img = document.createElement('img');
     img.src = image;
-    img.setAttribute("style", "position: absolute; top: 0; bottom: 0; left: 0; right: 0; margin: auto;");
+    img.setAttribute("style", "position: absolute; top: 0; bottom: 0; left: 0; right: 0; margin: auto; max-height: 100%; max-width: 100%");
     section.appendChild(img);
     slidesDiv.appendChild(section);
     Display._slides['0'] = 0;

openlp/core/display/render.py

@@ -706,6 +706,7 @@ class Renderer(RegistryBase, LogMixin, RegistryProperties, DisplayWindow):
             else:
                 # The remaining elements do not fit, thus reset the indexes, create a new list and continue.
                 raw_list = raw_list[index + 1:]
+                log.debug(raw_list)
                 raw_list[0] = raw_tags + raw_list[0]
                 html_list = html_list[index + 1:]
                 html_list[0] = html_tags + html_list[0]

openlp/core/display/window.py

@@ -29,16 +29,14 @@ import copy
 
 from PyQt5 import QtCore, QtWebChannel, QtWidgets
 
-from openlp.core.common.path import Path, path_to_str
+from openlp.core.common.path import path_to_str
 from openlp.core.common.settings import Settings
 from openlp.core.common.registry import Registry
+from openlp.core.common.applocation import AppLocation
 from openlp.core.ui import HideMode
 from openlp.core.display.screens import ScreenList
 
 log = logging.getLogger(__name__)
-DISPLAY_PATH = Path(__file__).parent / 'html' / 'display.html'
-CHECKERBOARD_PATH = Path(__file__).parent / 'html' / 'checkerboard.png'
-OPENLP_SPLASH_SCREEN_PATH = Path(__file__).parent / 'html' / 'openlp-splash-screen.png'
 
 
 class MediaWatcher(QtCore.QObject):
@@ -126,7 +124,11 @@ class DisplayWindow(QtWidgets.QWidget):
         self.webview.page().setBackgroundColor(QtCore.Qt.transparent)
         self.layout.addWidget(self.webview)
         self.webview.loadFinished.connect(self.after_loaded)
-        self.set_url(QtCore.QUrl.fromLocalFile(path_to_str(DISPLAY_PATH)))
+        display_base_path = AppLocation.get_directory(AppLocation.AppDir) / 'core' / 'display' / 'html'
+        self.display_path = display_base_path / 'display.html'
+        self.checkerboard_path = display_base_path / 'checkerboard.png'
+        self.openlp_splash_screen_path = display_base_path / 'openlp-splash-screen.png'
+        self.set_url(QtCore.QUrl.fromLocalFile(path_to_str(self.display_path)))
         self.media_watcher = MediaWatcher(self)
         self.channel = QtWebChannel.QWebChannel(self)
         self.channel.registerObject('mediaWatcher', self.media_watcher)
@@ -169,7 +171,7 @@ class DisplayWindow(QtWidgets.QWidget):
         bg_color = Settings().value('core/logo background color')
         image = Settings().value('core/logo file')
         if path_to_str(image).startswith(':'):
-            image = OPENLP_SPLASH_SCREEN_PATH
+            image = self.openlp_splash_screen_path
         image_uri = image.as_uri()
         self.run_javascript('Display.setStartupSplashScreen("{bg_color}", "{image}");'.format(bg_color=bg_color,
                                                                                               image=image_uri))
@@ -329,7 +331,7 @@ class DisplayWindow(QtWidgets.QWidget):
         if theme.background_type == 'transparent' and not self.is_display:
             theme_copy = copy.deepcopy(theme)
             theme_copy.background_type = 'image'
-            theme_copy.background_filename = CHECKERBOARD_PATH
+            theme_copy.background_filename = self.checkerboard_path
             exported_theme = theme_copy.export_theme()
         else:
             exported_theme = theme.export_theme()

openlp/core/lib/__init__.py

@@ -242,16 +242,16 @@ def image_to_byte(image, base_64=True):
     """
     Resize an image to fit on the current screen for the web and returns it as a byte stream.
 
-    :param image: The image to converted.
+    :param image: The image to be converted.
     :param base_64: If True returns the image as Base64 bytes, otherwise the image is returned as a byte array.
         To preserve original intention, this defaults to True
     """
     log.debug('image_to_byte - start')
     byte_array = QtCore.QByteArray()
     # use buffer to store pixmap into byteArray
-    buffie = QtCore.QBuffer(byte_array)
-    buffie.open(QtCore.QIODevice.WriteOnly)
-    image.save(buffie, "PNG")
+    buffer = QtCore.QBuffer(byte_array)
+    buffer.open(QtCore.QIODevice.WriteOnly)
+    image.save(buffer, "PNG")
     log.debug('image_to_byte - end')
     if not base_64:
         return byte_array
@@ -263,15 +263,13 @@ def create_thumb(image_path, thumb_path, return_icon=True, size=None):
     """
     Create a thumbnail from the given image path and depending on ``return_icon`` it returns an icon from this thumb.
 
-    :param image_path: The image file to create the icon from.
-    :param thumb_path: The filename to save the thumbnail to.
+    :param openlp.core.common.path.Path image_path: The image file to create the icon from.
+    :param openlp.core.common.path.Path thumb_path: The filename to save the thumbnail to.
     :param return_icon: States if an icon should be build and returned from the thumb. Defaults to ``True``.
     :param size: Allows to state a own size (QtCore.QSize) to use. Defaults to ``None``, which means that a default
      height of 88 is used.
     :return: The final icon.
     """
-    # TODO: To path object
-    thumb_path = Path(thumb_path)
     reader = QtGui.QImageReader(str(image_path))
     if size is None:
         # No size given; use default height of 88

openlp/core/lib/db.py

@@ -132,8 +132,9 @@ def get_db_path(plugin_name, db_file_name=None):
     Create a path to a database from the plugin name and database name
 
     :param plugin_name: Name of plugin
-    :param db_file_name: File name of database
-    :return: The path to the database as type str
+    :param openlp.core.common.path.Path | str | None db_file_name: File name of database
+    :return: The path to the database
+    :rtype: str
     """
     if db_file_name is None:
         return 'sqlite:///{path}/{plugin}.sqlite'.format(path=AppLocation.get_section_data_path(plugin_name),
@@ -144,15 +145,15 @@ def get_db_path(plugin_name, db_file_name=None):
         return 'sqlite:///{path}/{name}'.format(path=AppLocation.get_section_data_path(plugin_name), name=db_file_name)
 
 
-def handle_db_error(plugin_name, db_file_name):
+def handle_db_error(plugin_name, db_file_path):
     """
     Log and report to the user that a database cannot be loaded
 
     :param plugin_name: Name of plugin
-    :param db_file_name: File name of database
+    :param openlp.core.common.path.Path db_file_path: File name of database
     :return: None
     """
-    db_path = get_db_path(plugin_name, db_file_name)
+    db_path = get_db_path(plugin_name, db_file_path)
     log.exception('Error loading database: {db}'.format(db=db_path))
     critical_error_message_box(translate('OpenLP.Manager', 'Database Error'),
                                translate('OpenLP.Manager',
@@ -161,10 +162,13 @@ def handle_db_error(plugin_name, db_file_name):
 
 def init_url(plugin_name, db_file_name=None):
     """
-    Return the database URL.
+    Construct the connection string for a database.
 
     :param plugin_name: The name of the plugin for the database creation.
-    :param db_file_name: The database file name. Defaults to None resulting in the plugin_name being used.
+    :param openlp.core.common.path.Path | str | None db_file_name: The database file name. Defaults to None resulting
+                                                                   in the plugin_name being used.
+    :return: The database URL
+    :rtype: str
     """
     settings = Settings()
     settings.beginGroup(plugin_name)
@@ -357,14 +361,14 @@ class Manager(object):
         self.db_url = None
         if db_file_path:
             log.debug('Manager: Creating new DB url')
-            self.db_url = init_url(plugin_name, str(db_file_path))
+            self.db_url = init_url(plugin_name, str(db_file_path))  # TOdO :PATHLIB
         else:
             self.db_url = init_url(plugin_name)
         if upgrade_mod:
             try:
                 db_ver, up_ver = upgrade_db(self.db_url, upgrade_mod)
             except (SQLAlchemyError, DBAPIError):
-                handle_db_error(plugin_name, str(db_file_path))
+                handle_db_error(plugin_name, db_file_path)
                 return
             if db_ver > up_ver:
                 critical_error_message_box(
@@ -379,7 +383,7 @@ class Manager(object):
             try:
                 self.session = init_schema(self.db_url)
             except (SQLAlchemyError, DBAPIError):
-                handle_db_error(plugin_name, str(db_file_path))
+                handle_db_error(plugin_name, db_file_path)
         else:
             self.session = session
 

openlp/core/lib/mediamanageritem.py

@@ -454,15 +454,16 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
         """
         pass
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Live):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Live,
+                            file_path=None):
         """
         Generate the slide data. Needs to be implemented by the plugin.
+
         :param service_item: The service Item to be processed
         :param item: The database item to be used to build the service item
-        :param xml_version:
         :param remote: Was this remote triggered (False)
         :param context: The service context
+        :param openlp.core.common.path.Path file_path:
         """
         raise NotImplementedError('MediaManagerItem.generate_slide_data needs to be defined by the plugin')
 
@@ -624,17 +625,16 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties):
                                                             'You must select a {title} '
                                                             'service item.').format(title=self.title))
 
-    def build_service_item(self, item=None, xml_version=False, remote=False, context=ServiceItemContext.Live):
+    def build_service_item(self, item=None, remote=False, context=ServiceItemContext.Live):
         """
         Common method for generating a service item
         :param item: Service Item to be built.
-        :param xml_version: version of XML (False)
         :param remote: Remote triggered (False)
         :param context: The context on which this is called
         """
         service_item = ServiceItem(self.plugin)
         service_item.add_icon()
-        if self.generate_slide_data(service_item, item, xml_version, remote, context):
+        if self.generate_slide_data(service_item, item=item, remote=remote, context=context):
             return service_item
         else:
             return None

openlp/core/lib/serviceitem.py

@@ -263,7 +263,7 @@ class ServiceItem(RegistryProperties):
             file_location = os.path.join(path, file_name)
             file_location_hash = md5_hash(file_location.encode('utf-8'))
             image = os.path.join(str(AppLocation.get_section_data_path(self.name)), 'thumbnails',
-                                 file_location_hash, ntpath.basename(image))
+                                 file_location_hash, ntpath.basename(image))  # TODO: Pathlib
         self.slides.append({'title': file_name, 'image': image, 'path': path, 'display_title': display_title,
                             'notes': notes, 'thumbnail': image})
         # if self.is_capable(ItemCapabilities.HasThumbnails):
@@ -361,7 +361,7 @@ class ServiceItem(RegistryProperties):
                 if isinstance(file_path, str):
                     # Handle service files prior to OpenLP 3.0
                     # Windows can handle both forward and backward slashes, so we use ntpath to get the basename
-                    file_path = Path(path, ntpath.basename(file_path))
+                    file_path = path / ntpath.basename(file_path)
                 self.background_audio.append(file_path)
         self.theme_overwritten = header.get('theme_overwritten', False)
         if self.service_item_type == ServiceItemType.Text:
@@ -374,7 +374,7 @@ class ServiceItem(RegistryProperties):
             if path:
                 self.has_original_files = False
                 for text_image in service_item['serviceitem']['data']:
-                    file_path = os.path.join(path, text_image)
+                    file_path = path / text_image
                     self.add_from_image(file_path, text_image, background)
             else:
                 for text_image in service_item['serviceitem']['data']:

openlp/core/projectors/db.py

@@ -417,11 +417,17 @@ class ProjectorDB(Manager):
                   value: (str) From ProjectorSource, Sources tables or PJLink default code list
         """
         source_dict = {}
+        # Apparently, there was a change to the projector object. Test for which object has db id
+        if hasattr(projector, "id"):
+            chk = projector.id
+        elif hasattr(projector.entry, "id"):
+            chk = projector.entry.id
+
         # Get default list first
         for key in projector.source_available:
             item = self.get_object_filtered(ProjectorSource,
                                             and_(ProjectorSource.code == key,
-                                                 ProjectorSource.projector_id == projector.id))
+                                                 ProjectorSource.projector_id == chk))
             if item is None:
                 source_dict[key] = PJLINK_DEFAULT_CODES[key]
             else:

openlp/core/projectors/manager.py

@@ -46,31 +46,10 @@ from openlp.core.projectors.sourceselectform import SourceSelectSingle, SourceSe
 from openlp.core.ui.icons import UiIcons
 from openlp.core.widgets.toolbar import OpenLPToolbar
 
-
 log = logging.getLogger(__name__)
 log.debug('projectormanager loaded')
 
 
-# Dict for matching projector status to display icon
-STATUS_ICONS = {
-    S_NOT_CONNECTED: ':/projector/projector_item_disconnect.png',
-    S_CONNECTING: ':/projector/projector_item_connect.png',
-    S_CONNECTED: ':/projector/projector_off.png',
-    S_OFF: ':/projector/projector_off.png',
-    S_INITIALIZE: ':/projector/projector_off.png',
-    S_STANDBY: ':/projector/projector_off.png',
-    S_WARMUP: ':/projector/projector_warmup.png',
-    S_ON: ':/projector/projector_on.png',
-    S_COOLDOWN: ':/projector/projector_cooldown.png',
-    E_ERROR: ':/projector/projector_error.png',
-    E_NETWORK: ':/projector/projector_not_connected_error.png',
-    E_SOCKET_TIMEOUT: ':/projector/projector_not_connected_error.png',
-    E_AUTHENTICATION: ':/projector/projector_not_connected_error.png',
-    E_UNKNOWN_SOCKET_ERROR: ':/projector/projector_not_connected_error.png',
-    E_NOT_CONNECTED: ':/projector/projector_not_connected_error.png'
-}
-
-
 class UiProjectorManager(object):
     """
     UI part of the Projector Manager
@@ -122,7 +101,7 @@ class UiProjectorManager(object):
         self.one_toolbar.add_toolbar_action('connect_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Connect to selected projector.'),
-                                            icon=UiIcons().projector_connect,
+                                            icon=UiIcons().projector_select_connect,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Connect to selected projector.'),
                                             triggers=self.on_connect_projector)
@@ -136,7 +115,7 @@ class UiProjectorManager(object):
         self.one_toolbar.add_toolbar_action('disconnect_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Disconnect from selected projectors'),
-                                            icon=UiIcons().projector_disconnect,
+                                            icon=UiIcons().projector_select_disconnect,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Disconnect from selected projector.'),
                                             triggers=self.on_disconnect_projector)
@@ -151,7 +130,7 @@ class UiProjectorManager(object):
         self.one_toolbar.add_toolbar_action('poweron_projector',
                                             text=translate('OpenLP.ProjectorManager',
                                                            'Power on selected projector'),
-                                            icon=UiIcons().projector_on,
+                                            icon=UiIcons().projector_power_on,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Power on selected projector.'),
                                             triggers=self.on_poweron_projector)
@@ -164,7 +143,7 @@ class UiProjectorManager(object):
                                             triggers=self.on_poweron_projector)
         self.one_toolbar.add_toolbar_action('poweroff_projector',
                                             text=translate('OpenLP.ProjectorManager', 'Standby selected projector'),
-                                            icon=UiIcons().projector_off,
+                                            icon=UiIcons().projector_power_off,
                                             tooltip=translate('OpenLP.ProjectorManager',
                                                               'Put selected projector in standby.'),
                                             triggers=self.on_poweroff_projector)
@@ -309,7 +288,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
             S_INITIALIZE: UiIcons().projector_on,
             S_STANDBY: UiIcons().projector_off,
             S_WARMUP: UiIcons().projector_warmup,
-            S_ON: UiIcons().projector_off,
+            S_ON: UiIcons().projector_on,
             S_COOLDOWN: UiIcons().projector_cooldown,
             E_ERROR: UiIcons().projector_error,
             E_NETWORK: UiIcons().error,
@@ -880,6 +859,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
         """
         Update the icons when the selected projectors change
         """
+        log.debug('update_icons(): Checking for selected projector items in list')
         count = len(self.projector_list_widget.selectedItems())
         projector = None
         if count == 0:
@@ -900,6 +880,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
             self.get_toolbar_item('blank_projector_multiple', hidden=True)
             self.get_toolbar_item('show_projector_multiple', hidden=True)
         elif count == 1:
+            log.debug('update_icons(): Found one item selected')
             projector = self.projector_list_widget.selectedItems()[0].data(QtCore.Qt.UserRole)
             connected = QSOCKET_STATE[projector.link.state()] == S_CONNECTED
             power = projector.link.power == S_ON
@@ -910,12 +891,14 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
             self.get_toolbar_item('blank_projector_multiple', hidden=True)
             self.get_toolbar_item('show_projector_multiple', hidden=True)
             if connected:
+                log.debug('update_icons(): Updating icons for connected state')
                 self.get_toolbar_item('view_projector', enabled=True)
                 self.get_toolbar_item('source_view_projector',
-                                      enabled=connected and power and projector.link.source_available is not None)
+                                      enabled=projector.link.source_available is not None and connected and power)
                 self.get_toolbar_item('edit_projector', hidden=True)
                 self.get_toolbar_item('delete_projector', hidden=True)
             else:
+                log.debug('update_icons(): Updating for not connected state')
                 self.get_toolbar_item('view_projector', hidden=True)
                 self.get_toolbar_item('source_view_projector', hidden=True)
                 self.get_toolbar_item('edit_projector', enabled=True)
@@ -931,6 +914,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM
                 self.get_toolbar_item('blank_projector', enabled=False)
                 self.get_toolbar_item('show_projector', enabled=False)
         else:
+            log.debug('update_icons(): Updating for multiple items selected')
             self.get_toolbar_item('edit_projector', enabled=False)
             self.get_toolbar_item('delete_projector', enabled=False)
             self.get_toolbar_item('view_projector', hidden=True)

openlp/core/projectors/pjlink.py

@@ -528,8 +528,9 @@ class PJLinkCommands(object):
             sources.append(source)
         sources.sort()
         self.source_available = sources
-        log.debug('({ip}) Setting projector sources_available to "{data}"'.format(ip=self.entry.name,
+        log.debug('({ip}) Setting projector source_available to "{data}"'.format(ip=self.entry.name,
                                                                                  data=self.source_available))
+        self.projectorUpdateIcons.emit()
         return
 
     def process_lamp(self, data):
@@ -886,6 +887,9 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
         elif status >= S_NOT_CONNECTED and status in QSOCKET_STATE:
             # Socket connection status update
             self.status_connect = status
+            # Check if we need to update error state as well
+            if self.error_status != S_OK and status != S_NOT_CONNECTED:
+                self.error_status = S_OK
         elif status >= S_NOT_CONNECTED and status in PROJECTOR_STATE:
             # Only affects the projector status
             self.projector_status = status
@@ -905,7 +909,14 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
                                                                     message=status_message if msg is None else msg))
 
         # Now that we logged extra information for debugging, broadcast the original change/message
-        (code, message) = self._get_status(status)
+        # Check for connection errors first
+        if self.error_status != S_OK:
+            log.debug('({ip}) Signalling error code'.format(ip=self.entry.name))
+            code, message = self._get_status(self.error_status)
+            status = self.error_status
+        else:
+            log.debug('({ip}) Signalling status code'.format(ip=self.entry.name))
+            code, message = self._get_status(status)
         if msg is not None:
             message = msg
         elif message is None:
@@ -953,7 +964,7 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands):
             log.error('({ip}) Invalid initial packet received - closing socket'.format(ip=self.entry.name))
             return self.disconnect_from_host()
         # Convert the initial login prompt with the expected PJLink normal command format for processing
-        log.debug('({ip}) check_login(): Formatting initial connection prompt'
+        log.debug('({ip}) check_login(): Formatting initial connection prompt '
                   'to PJLink packet'.format(ip=self.entry.name))
         return self.get_data('{start}{clss}{data}'.format(start=PJLINK_PREFIX,
                                                           clss='1',

openlp/core/ui/advancedtab.py

@@ -455,7 +455,7 @@ class AdvancedTab(SettingsTab):
         Service Name options changed
         """
         self.service_name_day.setEnabled(default_service_enabled)
-        time_enabled = default_service_enabled and self.service_name_day.currentIndex() is not 7
+        time_enabled = default_service_enabled and self.service_name_day.currentIndex() != 7
         self.service_name_time.setEnabled(time_enabled)
         self.service_name_edit.setEnabled(default_service_enabled)
         self.service_name_revert_button.setEnabled(default_service_enabled)
@@ -478,7 +478,7 @@ class AdvancedTab(SettingsTab):
                 minute=self.service_name_time.time().minute()
             )
         try:
-            service_name_example = format_time(str(self.service_name_edit.text()), local_time)
+            service_name_example = format_time(self.service_name_edit.text(), local_time)
         except ValueError:
             preset_is_valid = False
             service_name_example = translate('OpenLP.AdvancedTab', 'Syntax error.')
@@ -497,7 +497,7 @@ class AdvancedTab(SettingsTab):
         """
         React to the day of the service name changing.
         """
-        self.service_name_time.setEnabled(service_day is not 7)
+        self.service_name_time.setEnabled(service_day != 7)
         self.update_service_name_example(None)
 
     def on_service_name_revert_button_clicked(self):

openlp/core/ui/exceptiondialog.py

@@ -77,11 +77,11 @@ class Ui_ExceptionDialog(object):
         self.save_report_button = create_button(exception_dialog, 'save_report_button',
                                                 icon=UiIcons().save,
                                                 click=self.on_save_report_button_clicked)
-        self.attach_tile_button = create_button(exception_dialog, 'attach_tile_button',
+        self.attach_file_button = create_button(exception_dialog, 'attach_file_button',
                                                 icon=UiIcons().open,
                                                 click=self.on_attach_file_button_clicked)
         self.button_box = create_button_box(exception_dialog, 'button_box', ['close'],
-                                            [self.send_report_button, self.save_report_button, self.attach_tile_button])
+                                            [self.send_report_button, self.save_report_button, self.attach_file_button])
         self.exception_layout.addWidget(self.button_box)
 
         self.retranslate_ui(exception_dialog)
@@ -112,4 +112,4 @@ class Ui_ExceptionDialog(object):
                       ).format(first_part=exception_part1))
         self.send_report_button.setText(translate('OpenLP.ExceptionDialog', 'Send E-Mail'))
         self.save_report_button.setText(translate('OpenLP.ExceptionDialog', 'Save to File'))
-        self.attach_tile_button.setText(translate('OpenLP.ExceptionDialog', 'Attach File'))
+        self.attach_file_button.setText(translate('OpenLP.ExceptionDialog', 'Attach File'))

openlp/core/ui/exceptionform.py

@@ -95,12 +95,14 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties):
         """
         Saving exception log and system information to a file.
         """
+        while True:
             file_path, filter_used = FileDialog.getSaveFileName(
                 self,
                 translate('OpenLP.ExceptionForm', 'Save Crash Report'),
                 Settings().value(self.settings_section + '/last directory'),
                 translate('OpenLP.ExceptionForm', 'Text files (*.txt *.log *.text)'))
-        if file_path:
+            if file_path is None:
+                break
             Settings().setValue(self.settings_section + '/last directory', file_path.parent)
             opts = self._create_report()
             report_text = self.report_text.format(version=opts['version'], description=opts['description'],
@@ -108,8 +110,13 @@ class ExceptionForm(QtWidgets.QDialog, Ui_ExceptionDialog, RegistryProperties):
             try:
                 with file_path.open('w') as report_file:
                     report_file.write(report_text)
-            except OSError:
+                    break
+            except OSError as e:
                 log.exception('Failed to write crash report')
+                QtWidgets.QMessageBox.warning(
+                    self, translate('OpenLP.ExceptionDialog', 'Failed to Save Report'),
+                    translate('OpenLP.ExceptionDialog', 'The following error occured when saving the report.\n\n'
+                                                        '{exception}').format(file_name=file_path, exception=e))
 
     def on_send_report_button_clicked(self):
         """

openlp/core/ui/firsttimeform.py

@@ -22,19 +22,19 @@
 """
 This module contains the first time wizard.
 """
+import json
 import logging
 import time
 import urllib.error
 import urllib.parse
 import urllib.request
-from configparser import ConfigParser, MissingSectionHeaderError, NoOptionError, NoSectionError
 from tempfile import gettempdir
 
 from PyQt5 import QtCore, QtWidgets
 
 from openlp.core.common import clean_button_text, trace_error_handler
 from openlp.core.common.applocation import AppLocation
-from openlp.core.common.httputils import download_file, get_url_file_size, get_web_page
+from openlp.core.common.httputils import DownloadWorker, download_file, get_url_file_size, get_web_page
 from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.path import Path, create_paths
@@ -43,57 +43,50 @@ from openlp.core.common.settings import Settings
 from openlp.core.lib import build_icon
 from openlp.core.lib.plugin import PluginStatus
 from openlp.core.lib.ui import critical_error_message_box
-from openlp.core.threading import ThreadWorker, get_thread_worker, is_thread_finished, run_thread
+from openlp.core.threading import get_thread_worker, is_thread_finished, run_thread
 from openlp.core.ui.firsttimewizard import FirstTimePage, UiFirstTimeWizard
+from openlp.core.ui.icons import UiIcons
 
 
 log = logging.getLogger(__name__)
 
 
-class ThemeScreenshotWorker(ThreadWorker):
+class ThemeListWidgetItem(QtWidgets.QListWidgetItem):
     """
-    This thread downloads a theme's screenshot
+    Subclass a QListWidgetItem to allow dynamic loading of thumbnails from an online resource
     """
-    screenshot_downloaded = QtCore.pyqtSignal(str, str, str)
+    def __init__(self, themes_url, sample_theme_data, ftw, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        title = sample_theme_data['title']
+        thumbnail = sample_theme_data['thumbnail']
+        self.file_name = sample_theme_data['file_name']
+        self.sha256 = sample_theme_data['sha256']
+        self.setIcon(UiIcons().picture)  # Set a place holder icon whilst the thumbnails download
+        self.setText(title)
+        self.setToolTip(title)
+        worker = DownloadWorker(themes_url, thumbnail)
+        worker.download_failed.connect(self._on_download_failed)
+        worker.download_succeeded.connect(self._on_thumbnail_downloaded)
+        thread_name = 'thumbnail_download_{thumbnail}'.format(thumbnail=thumbnail)
+        run_thread(worker, thread_name)
+        ftw.thumbnail_download_threads.append(thread_name)
 
-    def __init__(self, themes_url, title, filename, sha256, screenshot):
+    def _on_download_failed(self):
         """
-        Set up the worker object
-        """
-        self.was_cancelled = False
-        self.themes_url = themes_url
-        self.title = title
-        self.filename = filename
-        self.sha256 = sha256
-        self.screenshot = screenshot
-        super().__init__()
+        Set an icon to indicate that the thumbnail download has failed.
 
-    def start(self):
+        :rtype: None
         """
-        Run the worker
-        """
-        if self.was_cancelled:
-            return
-        try:
-            download_path = Path(gettempdir()) / 'openlp' / self.screenshot
-            is_success = download_file(self, '{host}{name}'.format(host=self.themes_url, name=self.screenshot),
-                                       download_path)
-            if is_success and not self.was_cancelled:
-                # Signal that the screenshot has been downloaded
-                self.screenshot_downloaded.emit(self.title, self.filename, self.sha256)
-        except:                                                                 # noqa
-            log.exception('Unable to download screenshot')
-        finally:
-            self.quit.emit()
+        self.setIcon(UiIcons().exception)
 
-    @QtCore.pyqtSlot(bool)
-    def set_download_canceled(self, toggle):
+    def _on_thumbnail_downloaded(self, thumbnail_path):
         """
-        Externally set if the download was canceled
+        Load the thumbnail as the icon when it has downloaded.
 
-        :param toggle: Set if the download was canceled or not
+        :param Path thumbnail_path: Path to the file to use as a thumbnail
+        :rtype: None
         """
-        self.was_download_cancelled = toggle
+        self.setIcon(build_icon(thumbnail_path))
 
 
 class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
@@ -110,6 +103,9 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         self.web_access = True
         self.web = ''
         self.setup_ui(self)
+        self.themes_list_widget.itemSelectionChanged.connect(self.on_themes_list_widget_selection_changed)
+        self.themes_deselect_all_button.clicked.connect(self.themes_list_widget.clearSelection)
+        self.themes_select_all_button.clicked.connect(self.themes_list_widget.selectAll)
 
     def get_next_page_id(self):
         """
@@ -144,17 +140,6 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
             return -1
         elif self.currentId() == FirstTimePage.NoInternet:
             return FirstTimePage.Progress
-        elif self.currentId() == FirstTimePage.Themes:
-            self.application.set_busy_cursor()
-            while not all([is_thread_finished(thread_name) for thread_name in self.theme_screenshot_threads]):
-                time.sleep(0.1)
-                self.application.process_events()
-            # Build the screenshot icons, as this can not be done in the thread.
-            self._build_theme_screenshots()
-            self.application.set_normal_cursor()
-            self.theme_screenshot_threads = []
-            return self.get_next_page_id()
-        else:
         return self.get_next_page_id()
 
     def exec(self):
@@ -172,104 +157,76 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         """
         self.screens = screens
         self.was_cancelled = False
-        self.theme_screenshot_threads = []
+        self.thumbnail_download_threads = []
         self.has_run_wizard = False
 
-        self.themes_list_widget.itemChanged.connect(self.on_theme_selected)
-
     def _download_index(self):
         """
         Download the configuration file and kick off the theme screenshot download threads
         """
         # check to see if we have web access
         self.web_access = False
-        self.config = ConfigParser()
+        self.config = ''
+        web_config = None
         user_agent = 'OpenLP/' + Registry().get('application').applicationVersion()
         self.application.process_events()
         try:
-            web_config = get_web_page('{host}{name}'.format(host=self.web, name='download.cfg'),
+            web_config = get_web_page('{host}{name}'.format(host=self.web, name='download_3.0.json'),
                                       headers={'User-Agent': user_agent})
         except ConnectionError:
             QtWidgets.QMessageBox.critical(self, translate('OpenLP.FirstTimeWizard', 'Network Error'),
                                            translate('OpenLP.FirstTimeWizard', 'There was a network error attempting '
                                                      'to connect to retrieve initial configuration information'),
                                            QtWidgets.QMessageBox.Ok)
-            web_config = False
-        if web_config:
-            try:
-                self.config.read_string(web_config)
-                self.web = self.config.get('general', 'base url')
-                self.songs_url = self.web + self.config.get('songs', 'directory') + '/'
-                self.bibles_url = self.web + self.config.get('bibles', 'directory') + '/'
-                self.themes_url = self.web + self.config.get('themes', 'directory') + '/'
+        if web_config and self._parse_config(web_config):
             self.web_access = True
-            except (NoSectionError, NoOptionError, MissingSectionHeaderError):
-                log.debug('A problem occurred while parsing the downloaded config file')
-                trace_error_handler(log)
         self.application.process_events()
         self.downloading = translate('OpenLP.FirstTimeWizard', 'Downloading {name}...')
-        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.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())
         self.application.set_normal_cursor()
-        # Sort out internet access for downloads
-        if self.web_access:
-            songs = self.config.get('songs', 'languages')
-            songs = songs.split(',')
-            for song in songs:
+
+    def _parse_config(self, web_config):
+        try:
+            config = json.loads(web_config)
+            meta = config['_meta']
+            self.web = meta['base_url']
+            self.songs_url = self.web + meta['songs_dir'] + '/'
+            self.bibles_url = self.web + meta['bibles_dir'] + '/'
+            self.themes_url = self.web + meta['themes_dir'] + '/'
+            for song in config['songs'].values():
                 self.application.process_events()
-                title = self.config.get('songs_{song}'.format(song=song), 'title')
-                filename = self.config.get('songs_{song}'.format(song=song), 'filename')
-                sha256 = self.config.get('songs_{song}'.format(song=song), 'sha256', fallback='')
-                item = QtWidgets.QListWidgetItem(title, self.songs_list_widget)
-                item.setData(QtCore.Qt.UserRole, (filename, sha256))
+                item = QtWidgets.QListWidgetItem(song['title'], self.songs_list_widget)
+                item.setData(QtCore.Qt.UserRole, (song['file_name'], song['sha256']))
                 item.setCheckState(QtCore.Qt.Unchecked)
                 item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
-            bible_languages = self.config.get('bibles', 'languages')
-            bible_languages = bible_languages.split(',')
-            for lang in bible_languages:
+            for lang in config['bibles'].values():
                 self.application.process_events()
-                language = self.config.get('bibles_{lang}'.format(lang=lang), 'title')
-                lang_item = QtWidgets.QTreeWidgetItem(self.bibles_tree_widget, [language])
-                bibles = self.config.get('bibles_{lang}'.format(lang=lang), 'translations')
-                bibles = bibles.split(',')
-                for bible in bibles:
+                lang_item = QtWidgets.QTreeWidgetItem(self.bibles_tree_widget, [lang['title']])
+                for translation in lang['translations'].values():
                     self.application.process_events()
-                    title = self.config.get('bible_{bible}'.format(bible=bible), 'title')
-                    filename = self.config.get('bible_{bible}'.format(bible=bible), 'filename')
-                    sha256 = self.config.get('bible_{bible}'.format(bible=bible), 'sha256', fallback='')
-                    item = QtWidgets.QTreeWidgetItem(lang_item, [title])
-                    item.setData(0, QtCore.Qt.UserRole, (filename, sha256))
+                    item = QtWidgets.QTreeWidgetItem(lang_item, [translation['title']])
+                    item.setData(0, QtCore.Qt.UserRole, (translation['file_name'], translation['sha256']))
                     item.setCheckState(0, QtCore.Qt.Unchecked)
                     item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
             self.bibles_tree_widget.expandAll()
             self.application.process_events()
-            # Download the theme screenshots
-            themes = self.config.get('themes', 'files').split(',')
-            for theme in themes:
-                title = self.config.get('theme_{theme}'.format(theme=theme), 'title')
-                filename = self.config.get('theme_{theme}'.format(theme=theme), 'filename')
-                sha256 = self.config.get('theme_{theme}'.format(theme=theme), 'sha256', fallback='')
-                screenshot = self.config.get('theme_{theme}'.format(theme=theme), 'screenshot')
-                worker = ThemeScreenshotWorker(self.themes_url, title, filename, sha256, screenshot)
-                worker.screenshot_downloaded.connect(self.on_screenshot_downloaded)
-                thread_name = 'theme_screenshot_{title}'.format(title=title)
-                run_thread(worker, thread_name)
-                self.theme_screenshot_threads.append(thread_name)
+            for theme in config['themes'].values():
+                ThemeListWidgetItem(self.themes_url, theme, self, self.themes_list_widget)
             self.application.process_events()
+        except Exception:
+            log.exception('Unable to parse sample config file %s', web_config)
+            critical_error_message_box(
+                translate('OpenLP.FirstTimeWizard', 'Invalid index file'),
+                translate('OpenLP.FirstTimeWizard', 'OpenLP was unable to read the resource index file. '
+                                                    'Please try again later.'))
+            return False
+        return True
 
     def set_defaults(self):
         """
         Set up display at start of theme edit.
         """
         self.restart()
-        self.web = 'http://openlp.org/files/frw/'
+        self.web = 'https://get.openlp.org/ftw/'
         self.cancel_button.clicked.connect(self.on_cancel_button_clicked)
         self.no_internet_finish_button.clicked.connect(self.on_no_internet_finish_button_clicked)
         self.no_internet_cancel_button.clicked.connect(self.on_no_internet_cancel_button_clicked)
@@ -282,9 +239,18 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         create_paths(Path(gettempdir(), 'openlp'))
         self.theme_combo_box.clear()
         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.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())
             # Add any existing themes to list.
-            for theme in self.theme_manager.get_themes():
-                self.theme_combo_box.addItem(theme)
+            self.theme_combo_box.insertSeparator(0)
+            self.theme_combo_box.addItems(sorted(self.theme_manager.get_themes()))
             default_theme = Settings().value('themes/global theme')
             # Pre-select the current default theme.
             index = self.theme_combo_box.findText(default_theme)
@@ -335,49 +301,34 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         Process the triggering of the cancel button.
         """
         self.was_cancelled = True
-        if self.theme_screenshot_threads:
-            for thread_name in self.theme_screenshot_threads:
+        if self.thumbnail_download_threads:  # TODO: Use main thread list
+            for thread_name in self.thumbnail_download_threads:
                 worker = get_thread_worker(thread_name)
                 if worker:
-                    worker.set_download_canceled(True)
+                    worker.cancel_download()
         # Was the thread created.
-        if self.theme_screenshot_threads:
-            while any([not is_thread_finished(thread_name) for thread_name in self.theme_screenshot_threads]):
+        if self.thumbnail_download_threads:
+            while any([not is_thread_finished(thread_name) for thread_name in self.thumbnail_download_threads]):
                 time.sleep(0.1)
         self.application.set_normal_cursor()
 
-    def on_screenshot_downloaded(self, title, filename, sha256):
+    def on_themes_list_widget_selection_changed(self):
         """
-        Add an item to the list when a theme has been downloaded
+        Update the `theme_combo_box` with the selected items
 
-        :param title: The title of the theme
-        :param filename: The filename of the theme
-        """
-        self.themes_list_widget.blockSignals(True)
-        item = QtWidgets.QListWidgetItem(title, self.themes_list_widget)
-        item.setData(QtCore.Qt.UserRole, (filename, sha256))
-        item.setCheckState(QtCore.Qt.Unchecked)
-        item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable)
-        self.themes_list_widget.blockSignals(False)
-
-    def on_theme_selected(self, item):
-        """
-        Add or remove a de/selected sample theme from the theme_combo_box
-
-        :param QtWidgets.QListWidgetItem item: The item that has been de/selected
         :rtype: None
         """
-        theme_name = item.text()
-        if self.theme_manager and theme_name in self.theme_manager.get_themes():
-            return True
-        if item.checkState() == QtCore.Qt.Checked:
-            self.theme_combo_box.addItem(theme_name)
-            return True
-        else:
-            index = self.theme_combo_box.findText(theme_name)
-            if index != -1:
-                self.theme_combo_box.removeItem(index)
-            return True
+        existing_themes = []
+        if self.theme_manager:
+            existing_themes = self.theme_manager.get_themes()
+        for list_index in range(self.themes_list_widget.count()):
+            item = self.themes_list_widget.item(list_index)
+            if item.text() not in existing_themes:
+                cbox_index = self.theme_combo_box.findText(item.text())
+                if item.isSelected() and cbox_index == -1:
+                    self.theme_combo_box.insertItem(0, item.text())
+                elif not item.isSelected() and cbox_index != -1:
+                    self.theme_combo_box.removeItem(cbox_index)
 
     def on_no_internet_finish_button_clicked(self):
         """
@@ -396,18 +347,6 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
         self.was_cancelled = True
         self.close()
 
-    def _build_theme_screenshots(self):
-        """
-        This method builds the theme screenshots' icons for all items in the ``self.themes_list_widget``.
-        """
-        themes = self.config.get('themes', 'files')
-        themes = themes.split(',')
-        for index, theme in enumerate(themes):
-            screenshot = self.config.get('theme_{theme}'.format(theme=theme), 'screenshot')
-            item = self.themes_list_widget.item(index)
-            if item:
-                item.setIcon(build_icon(Path(gettempdir(), 'openlp', screenshot)))
-
     def update_progress(self, count, block_size):
         """
         Calculate and display the download progress. This method is called by download_file().
@@ -456,12 +395,8 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                     self.max_progress += size
                 iterator += 1
             # Loop through the themes list and increase for each selected item
-            for i in range(self.themes_list_widget.count()):
-                self.application.process_events()
-                item = self.themes_list_widget.item(i)
-                if item.checkState() == QtCore.Qt.Checked:
-                    filename, sha256 = item.data(QtCore.Qt.UserRole)
-                    size = get_url_file_size('{path}{name}'.format(path=self.themes_url, name=filename))
+            for item in self.themes_list_widget.selectedItems():
+                size = get_url_file_size('{url}{file}'.format(url=self.themes_url, file=item.file_name))
                 self.max_progress += size
         except urllib.error.URLError:
             trace_error_handler(log)
@@ -579,15 +514,12 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties):
                     missed_files.append('Bible: {name}'.format(name=bible))
             bibles_iterator += 1
         # Download themes
-        for i in range(self.themes_list_widget.count()):
-            item = self.themes_list_widget.item(i)
-            if item.checkState() == QtCore.Qt.Checked:
-                theme, sha256 = item.data(QtCore.Qt.UserRole)
-                self._increment_progress_bar(self.downloading.format(name=theme), 0)
+        for item in self.themes_list_widget.selectedItems():
+            self._increment_progress_bar(self.downloading.format(name=item.file_name), 0)
             self.previous_size = 0
-                if not download_file(self, '{path}{name}'.format(path=self.themes_url, name=theme),
-                                     themes_destination_path / theme, sha256):
-                    missed_files.append('Theme: {name}'.format(name=theme))
+            if not download_file(self, '{url}{file}'.format(url=self.themes_url, file=item.file_name),
+                                 themes_destination_path / item.file_name, item.sha256):
+                missed_files.append('Theme: name'.format(name=item.file_name))
         if missed_files:
             file_list = ''
             for entry in missed_files:

openlp/core/ui/firsttimewizard.py

@@ -49,6 +49,39 @@ class FirstTimePage(object):
     Progress = 8
 
 
+class ThemeListWidget(QtWidgets.QListWidget):
+    """
+    Subclass a QListWidget so we can make it look better when it resizes.
+    """
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.setEditTriggers(QtWidgets.QAbstractItemView.NoEditTriggers)
+        self.setHorizontalScrollBarPolicy(QtCore.Qt.ScrollBarAlwaysOff)
+        self.setVerticalScrollBarPolicy(QtCore.Qt.ScrollBarAlwaysOn)
+        self.setVerticalScrollMode(QtWidgets.QAbstractItemView.ScrollPerPixel)
+        self.setSelectionMode(QtWidgets.QAbstractItemView.MultiSelection)
+        self.setSelectionBehavior(QtWidgets.QAbstractItemView.SelectRows)
+        self.setIconSize(QtCore.QSize(133, 100))
+        self.setMovement(QtWidgets.QListView.Static)
+        self.setFlow(QtWidgets.QListView.LeftToRight)
+        self.setProperty("isWrapping", True)
+        self.setResizeMode(QtWidgets.QListView.Adjust)
+        self.setViewMode(QtWidgets.QListView.IconMode)
+        self.setUniformItemSizes(True)
+
+    def resizeEvent(self, event):
+        """
+        Resize the grid so the list looks better when its resized/
+
+        :param QtGui.QResizeEvent event: Not used
+        :return: None
+        """
+        nominal_width = 141  # Icon width of 133 + 4 each side
+        max_items_per_row = self.viewport().width() // nominal_width or 1  # or 1 to avoid divide by 0 errors
+        col_size = (self.viewport().width() - 1) / max_items_per_row
+        self.setGridSize(QtCore.QSize(col_size, 140))
+
+
 class UiFirstTimeWizard(object):
     """
     The UI widgets for the first time wizard.
@@ -175,27 +208,26 @@ class UiFirstTimeWizard(object):
         self.themes_page = QtWidgets.QWizardPage()
         self.themes_page.setObjectName('themes_page')
         self.themes_layout = QtWidgets.QVBoxLayout(self.themes_page)
-        self.themes_layout.setContentsMargins(20, 50, 20, 60)
         self.themes_layout.setObjectName('themes_layout')
-        self.themes_list_widget = QtWidgets.QListWidget(self.themes_page)
-        self.themes_list_widget.setViewMode(QtWidgets.QListView.IconMode)
-        self.themes_list_widget.setMovement(QtWidgets.QListView.Static)
-        self.themes_list_widget.setFlow(QtWidgets.QListView.LeftToRight)
-        self.themes_list_widget.setSpacing(4)
-        self.themes_list_widget.setUniformItemSizes(True)
-        self.themes_list_widget.setIconSize(QtCore.QSize(133, 100))
-        self.themes_list_widget.setWrapping(False)
-        self.themes_list_widget.setObjectName('themes_list_widget')
+        self.themes_list_widget = ThemeListWidget(self.themes_page)
         self.themes_layout.addWidget(self.themes_list_widget)
+        self.theme_options_layout = QtWidgets.QHBoxLayout()
         self.default_theme_layout = QtWidgets.QHBoxLayout()
         self.theme_label = QtWidgets.QLabel(self.themes_page)
         self.default_theme_layout.addWidget(self.theme_label)
         self.theme_combo_box = QtWidgets.QComboBox(self.themes_page)
         self.theme_combo_box.setEditable(False)
-        self.theme_combo_box.setInsertPolicy(QtWidgets.QComboBox.NoInsert)
-        self.theme_combo_box.setSizeAdjustPolicy(QtWidgets.QComboBox.AdjustToContents)
-        self.default_theme_layout.addWidget(self.theme_combo_box)
-        self.themes_layout.addLayout(self.default_theme_layout)
+        self.default_theme_layout.addWidget(self.theme_combo_box, stretch=1)
+        self.theme_options_layout.addLayout(self.default_theme_layout, stretch=1)
+        self.select_buttons_layout = QtWidgets.QHBoxLayout()
+        self.themes_select_all_button = QtWidgets.QToolButton(self.themes_page)
+        self.themes_select_all_button.setIcon(UiIcons().select_all)
+        self.select_buttons_layout.addWidget(self.themes_select_all_button, stretch=1, alignment=QtCore.Qt.AlignRight)
+        self.themes_deselect_all_button = QtWidgets.QToolButton(self.themes_page)
+        self.themes_deselect_all_button.setIcon(UiIcons().select_none)
+        self.select_buttons_layout.addWidget(self.themes_deselect_all_button)
+        self.theme_options_layout.addLayout(self.select_buttons_layout, stretch=1)
+        self.themes_layout.addLayout(self.theme_options_layout)
         first_time_wizard.setPage(FirstTimePage.Themes, self.themes_page)
         # Progress page
         self.progress_page = QtWidgets.QWizardPage()
@@ -271,9 +303,12 @@ class UiFirstTimeWizard(object):
         self.songs_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Select and download public domain songs.'))
         self.bibles_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Sample Bibles'))
         self.bibles_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Select and download free Bibles.'))
+        # Themes Page
         self.themes_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Sample Themes'))
         self.themes_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Select and download sample themes.'))
-        self.theme_label.setText(translate('OpenLP.FirstTimeWizard', 'Select default theme:'))
+        self.theme_label.setText(translate('OpenLP.FirstTimeWizard', 'Default theme:'))
+        self.themes_select_all_button.setToolTip(translate('OpenLP.FirstTimeWizard', 'Select all'))
+        self.themes_deselect_all_button.setToolTip(translate('OpenLP.FirstTimeWizard', 'Deselect all'))
         self.progress_page.setTitle(translate('OpenLP.FirstTimeWizard', 'Downloading and Configuring'))
         self.progress_page.setSubTitle(translate('OpenLP.FirstTimeWizard', 'Please wait while resources are downloaded '
                                                                            'and OpenLP is configured.'))

openlp/core/ui/generaltab.py

@@ -47,7 +47,6 @@ class GeneralTab(SettingsTab):
         """
         Initialise the general settings tab
         """
-        self.logo_file = ':/graphics/openlp-splash-screen.png'
         self.logo_background_color = '#ffffff'
         self.screens = ScreenList()
         self.icon_path = ':/icon/openlp-logo.svg'

openlp/core/ui/icons.py

@@ -117,13 +117,17 @@ class UiIcons(object):
             'presentation': {'icon': 'fa.bar-chart'},
             'preview': {'icon': 'fa.laptop'},
             'projector': {'icon': 'op.video'},
-            'projector_connect': {'icon': 'fa.plug'},
+            'projector_connect': {'icon': 'fa.plug'},  # Projector connect
             'projector_cooldown': {'icon': 'fa.video-camera', 'attr': 'blue'},
-            'projector_disconnect': {'icon': 'fa.plug', 'attr': 'lightGray'},
+            'projector_disconnect': {'icon': 'fa.plug', 'attr': 'lightGray'},  # Projector disconnect
             'projector_error': {'icon': 'fa.video-camera', 'attr': 'red'},
             'projector_hdmi': {'icon': 'op.hdmi'},
-            'projector_off': {'icon': 'fa.video-camera', 'attr': 'black'},
-            'projector_on': {'icon': 'fa.video-camera', 'attr': 'green'},
+            'projector_power_off': {'icon': 'fa.video-camera', 'attr': 'red'},  # Toolbar power off
+            'projector_power_on': {'icon': 'fa.video-camera', 'attr': 'green'},  # Toolbar power on
+            'projector_off': {'icon': 'fa.video-camera', 'attr': 'black'},  # Projector off
+            'projector_on': {'icon': 'fa.video-camera', 'attr': 'green'},  # Projector on
+            'projector_select_connect': {'icon': 'fa.plug', 'attr': 'green'},  # Toolbar connect
+            'projector_select_disconnect': {'icon': 'fa.plug', 'attr': 'red'},  # Toolbar disconnect
             'projector_warmup': {'icon': 'fa.video-camera', 'attr': 'yellow'},
             'picture': {'icon': 'fa.picture-o'},
             'print': {'icon': 'fa.print'},
@@ -138,6 +142,8 @@ class UiIcons(object):
             'search_plus': {'icon': 'fa.search-plus'},
             'search_ref': {'icon': 'fa.institution'},
             'search_text': {'icon': 'op.search-text'},
+            'select_all': {'icon': 'fa.check-square-o'},
+            'select_none': {'icon': 'fa.square-o'},
             'settings': {'icon': 'fa.cogs'},
             'shortcuts': {'icon': 'fa.wrench'},
             'song_usage': {'icon': 'fa.line-chart'},

openlp/core/ui/mainwindow.py

@@ -1334,7 +1334,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
                 self.show_status_message(
                     translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} '
                               '- Please wait for copy to finish').format(path=self.new_data_path))
-                dir_util.copy_tree(str(old_data_path), str(self.new_data_path))
+                dir_util.copy_tree(old_data_path, self.new_data_path)
                 self.log_info('Copy successful')
             except (OSError, DistutilsFileError) as why:
                 self.application.set_normal_cursor()

openlp/core/ui/servicemanager.py

@@ -25,7 +25,6 @@ The service manager sets up, loads, saves and manages services.
 import html
 import json
 import os
-import shutil
 import zipfile
 from contextlib import suppress
 from datetime import datetime, timedelta
@@ -234,7 +233,7 @@ class Ui_ServiceManager(object):
         self.service_manager_list.itemExpanded.connect(self.expanded)
         # Last little bits of setting up
         self.service_theme = Settings().value(self.main_window.service_manager_settings_section + '/service theme')
-        self.service_path = str(AppLocation.get_section_data_path('servicemanager'))
+        self.service_path = AppLocation.get_section_data_path('servicemanager')
         # build the drag and drop context menu
         self.dnd_menu = QtWidgets.QMenu()
         self.new_action = self.dnd_menu.addAction(translate('OpenLP.ServiceManager', '&Add New Item'))
@@ -590,11 +589,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                 self.main_window.increment_progress_bar(service_content_size)
                 # Finally add all the listed media files.
                 for write_path in write_list:
-                    zip_file.write(str(write_path), str(write_path))
+                    zip_file.write(write_path, write_path)
                     self.main_window.increment_progress_bar(write_path.stat().st_size)
                 with suppress(FileNotFoundError):
                     file_path.unlink()
-                os.link(temp_file.name, str(file_path))
+                os.link(temp_file.name, file_path)
             Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent)
         except (PermissionError, OSError) as error:
             self.log_exception('Failed to save service to disk: {name}'.format(name=file_path))
@@ -679,7 +678,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         service_data = None
         self.application.set_busy_cursor()
         try:
-            with zipfile.ZipFile(str(file_path)) as zip_file:
+            with zipfile.ZipFile(file_path) as zip_file:
                 compressed_size = 0
                 for zip_info in zip_file.infolist():
                     compressed_size += zip_info.compress_size
@@ -692,7 +691,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                             service_data = json_file.read()
                     else:
                         zip_info.filename = os.path.basename(zip_info.filename)
-                        zip_file.extract(zip_info, str(self.service_path))
+                        zip_file.extract(zip_info, self.service_path)
                     self.main_window.increment_progress_bar(zip_info.compress_size)
             if service_data:
                 items = json.loads(service_data, cls=OpenLPJsonDecoder)
@@ -705,11 +704,13 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             else:
                 raise ValidationError(msg='No service data found')
         except (NameError, OSError, ValidationError, zipfile.BadZipFile):
+            self.application.set_normal_cursor()
             self.log_exception('Problem loading service file {name}'.format(name=file_path))
             critical_error_message_box(
                 message=translate('OpenLP.ServiceManager',
-                                  'The service file {file_path} could not be loaded because it is either corrupt, or '
-                                  'not a valid OpenLP 2 or OpenLP 3 service file.'.format(file_path=file_path)))
+                                  'The service file {file_path} could not be loaded because it is either corrupt, '
+                                  'inaccessible, or not a valid OpenLP 2 or OpenLP 3 service file.'
+                                  ).format(file_path=file_path))
         self.main_window.finished_progress_bar()
         self.application.set_normal_cursor()
         self.repaint_service_list(-1, -1)
@@ -1237,11 +1238,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         """
         Empties the service_path of temporary files on system exit.
         """
-        for file_name in os.listdir(self.service_path):
-            file_path = Path(self.service_path, file_name)
+        for file_path in self.service_path.iterdir():
             delete_file(file_path)
-        if os.path.exists(os.path.join(self.service_path, 'audio')):
-            shutil.rmtree(os.path.join(self.service_path, 'audio'), True)
+        audio_path = self.service_path / 'audio'
+        if audio_path.exists():
+            audio_path.rmtree(True)
 
     def on_theme_combo_box_selected(self, current_index):
         """

openlp/core/ui/thememanager.py

@@ -150,7 +150,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.global_theme = Settings().value(self.settings_section + '/global theme')
         self.build_theme_path()
         self.load_first_time_themes()
-        self.upgrade_themes()
+        self.upgrade_themes()  # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed
 
     def bootstrap_post_set_up(self):
         """
@@ -422,10 +422,10 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         :rtype: bool
         """
         try:
-            with zipfile.ZipFile(str(theme_path), 'w') as theme_zip:
+            with zipfile.ZipFile(theme_path, 'w') as theme_zip:
                 source_path = self.theme_path / theme_name
                 for file_path in source_path.iterdir():
-                    theme_zip.write(str(file_path), os.path.join(theme_name, file_path.name))
+                    theme_zip.write(file_path, Path(theme_name, file_path.name))
             return True
         except OSError as ose:
             self.log_exception('Export Theme Failed')
@@ -567,10 +567,10 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         json_theme = False
         theme_name = ""
         try:
-            with zipfile.ZipFile(str(file_path)) as theme_zip:
+            with zipfile.ZipFile(file_path) as theme_zip:
                 json_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.json']
                 if len(json_file) != 1:
-                    # TODO: remove XML handling after the 2.6 release.
+                    # TODO: remove XML handling after once the upgrade path from 2.4 is no longer required
                     xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml']
                     if len(xml_file) != 1:
                         self.log_error('Theme contains "{val:d}" theme files'.format(val=len(xml_file)))
@@ -607,12 +607,12 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
                     else:
                         with full_name.open('wb') as out_file:
                             out_file.write(theme_zip.read(zipped_file))
-        except (OSError, zipfile.BadZipFile):
+        except (OSError, ValidationError, zipfile.BadZipFile):
             self.log_exception('Importing theme from zip failed {name}'.format(name=file_path))
-            raise ValidationError
-        except ValidationError:
-            critical_error_message_box(translate('OpenLP.ThemeManager', 'Validation Error'),
-                                       translate('OpenLP.ThemeManager', 'File is not a valid theme.'))
+            critical_error_message_box(
+                translate('OpenLP.ThemeManager', 'Import Error'),
+                translate('OpenLP.ThemeManager', 'There was a problem imoorting {file_name}.\n\nIt is corrupt,'
+                                                 'inaccessible or not a valid theme.').format(file_name=file_path))
         finally:
             if not abort_import:
                 # As all files are closed, we can create the Theme.

openlp/core/ui/themestab.py

@@ -206,7 +206,7 @@ class ThemesTab(SettingsTab):
         find_and_set_in_combo_box(self.default_combo_box, self.global_theme)
         # self.renderer.set_global_theme()
         self.renderer.set_theme_level(self.theme_level)
-        if self.global_theme is not '':
+        if self.global_theme != '':
             self._preview_global_theme()
 
     def _preview_global_theme(self):

openlp/core/version.py

@@ -200,7 +200,7 @@ def get_library_versions():
     """
     library_versions = OrderedDict([(library, _get_lib_version(*args)) for library, args in LIBRARIES.items()])
     # Just update the dict for display purposes, changing the None to a '-'
-    for library, version in library_versions:
+    for library, version in library_versions.items():
         if version is None:
             library_versions[library] = '-'
     return library_versions

openlp/core/widgets/edits.py

@@ -352,7 +352,7 @@ class PathEdit(QtWidgets.QWidget):
         :rtype: None
         """
         if self._path != path:
-            self._path = path
+            self.path = path
             self.pathChanged.emit(path)
 
 

openlp/core/widgets/wizard.py

@@ -27,13 +27,11 @@ import logging
 from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import is_macosx
-from openlp.core.common.i18n import UiStrings, translate
+from openlp.core.common.i18n import translate
 from openlp.core.common.mixins import RegistryProperties
 from openlp.core.common.registry import Registry
-from openlp.core.common.settings import Settings
 from openlp.core.lib.ui import add_welcome_page
 from openlp.core.ui.icons import UiIcons
-from openlp.core.widgets.dialogs import FileDialog
 
 
 log = logging.getLogger(__name__)
@@ -280,41 +278,3 @@ class OpenLPWizard(QtWidgets.QWizard, RegistryProperties):
         self.finish_button.setVisible(True)
         self.cancel_button.setVisible(False)
         self.application.process_events()
-
-    def get_file_name(self, title, editbox, setting_name, filters=''):
-        """
-        Opens a FileDialog and saves the filename to the given editbox.
-
-        :param str title: The title of the dialog.
-        :param QtWidgets.QLineEdit editbox:  An QLineEdit.
-        :param str setting_name: The place where to save the last opened directory.
-        :param str filters: The file extension filters. It should contain the file description
-            as well as the file extension. For example::
-
-                'OpenLP 2 Databases (*.sqlite)'
-        :rtype: None
-        """
-        if filters:
-            filters += ';;'
-        filters += '%s (*)' % UiStrings().AllFiles
-        file_path, filter_used = FileDialog.getOpenFileName(
-            self, title, Settings().value(self.plugin.settings_section + '/' + setting_name), filters)
-        if file_path:
-            editbox.setText(str(file_path))
-            Settings().setValue(self.plugin.settings_section + '/' + setting_name, file_path.parent)
-
-    def get_folder(self, title, editbox, setting_name):
-        """
-        Opens a FileDialog and saves the selected folder to the given editbox.
-
-        :param str title: The title of the dialog.
-        :param QtWidgets.QLineEdit editbox: An QLineEditbox.
-        :param str setting_name: The place where to save the last opened directory.
-        :rtype: None
-        """
-        folder_path = FileDialog.getExistingDirectory(
-            self, title, Settings().value(self.plugin.settings_section + '/' + setting_name),
-            FileDialog.ShowDirsOnly)
-        if folder_path:
-            editbox.setText(str(folder_path))
-            Settings().setValue(self.plugin.settings_section + '/' + setting_name, folder_path)

openlp/plugins/bibles/forms/bibleimportform.py

@@ -463,14 +463,14 @@ class BibleImportForm(OpenLPWizard):
                         UiStrings().NFSs, translate('BiblesPlugin.ImportWizardForm',
                                                     'You need to specify a file with books of the Bible to use in the '
                                                     'import.'))
-                    self.csv_books_edit.setFocus()
+                    self.csv_books_path_edit.setFocus()
                     return False
                 elif not self.field('csv_versefile'):
                     critical_error_message_box(
                         UiStrings().NFSs,
                         translate('BiblesPlugin.ImportWizardForm', 'You need to specify a file of Bible verses to '
                                                                    'import.'))
-                    self.csv_verses_edit.setFocus()
+                    self.csv_verses_pathedit.setFocus()
                     return False
             elif self.field('source_format') == BibleFormat.OpenSong:
                 if not self.field('opensong_file'):

openlp/plugins/bibles/lib/bibleimport.py

@@ -48,9 +48,9 @@ class BibleImport(BibleDB, LogMixin, RegistryProperties):
         """
         Check if the supplied file is compressed
 
-        :param file_path: A path to the file to check
+        :param openlp.core.common.path.Path file_path: A path to the file to check
         """
-        if is_zipfile(str(file_path)):
+        if is_zipfile(file_path):
             critical_error_message_box(
                 message=translate('BiblesPlugin.BibleImport',
                                   'The file "{file}" you supplied is compressed. You must decompress it before import.'

openlp/plugins/bibles/lib/db.py

@@ -158,11 +158,10 @@ class BibleDB(Manager):
             self.name = kwargs['name']
             if not isinstance(self.name, str):
                 self.name = str(self.name, 'utf-8')
-            # TODO: To path object
-            file_path = Path(clean_filename(self.name) + '.sqlite')
+            self.file_path = Path(clean_filename(self.name) + '.sqlite')
         if 'file' in kwargs:
-            file_path = kwargs['file']
-        Manager.__init__(self, 'bibles', init_schema, file_path, upgrade)
+            self.file_path = kwargs['file']
+        Manager.__init__(self, 'bibles', init_schema, self.file_path, upgrade)
         if self.session and 'file' in kwargs:
             self.get_name()
         self._is_web_bible = None
@@ -750,7 +749,7 @@ class BiblesResourcesDB(QtCore.QObject, Manager):
         ]
 
 
-class AlternativeBookNamesDB(QtCore.QObject, Manager):
+class AlternativeBookNamesDB(Manager):
     """
     This class represents a database-bound alternative book names system.
     """
@@ -765,8 +764,9 @@ class AlternativeBookNamesDB(QtCore.QObject, Manager):
         """
         if AlternativeBookNamesDB.cursor is None:
             file_path = AppLocation.get_directory(AppLocation.DataDir) / 'bibles' / 'alternative_book_names.sqlite'
+            exists = file_path.exists()
             AlternativeBookNamesDB.conn = sqlite3.connect(str(file_path))
-            if not file_path.exists():
+            if not exists:
                 # create new DB, create table alternative_book_names
                 AlternativeBookNamesDB.conn.execute(
                     'CREATE TABLE alternative_book_names(id INTEGER NOT NULL, '

openlp/plugins/bibles/lib/importers/wordproject.py

@@ -51,7 +51,7 @@ class WordProjectBible(BibleImport):
         Unzip the file to a temporary directory
         """
         self.tmp = TemporaryDirectory()
-        with ZipFile(str(self.file_path)) as zip_file:
+        with ZipFile(self.file_path) as zip_file:
             zip_file.extractall(self.tmp.name)
         self.base_path = Path(self.tmp.name, self.file_path.stem)
 

openlp/plugins/bibles/lib/manager.py

@@ -187,7 +187,7 @@ class BibleManager(LogMixin, RegistryProperties):
         bible = self.db_cache[name]
         bible.session.close_all()
         bible.session = None
-        return delete_file(Path(bible.path, bible.file))
+        return delete_file(bible.path, bible.file_path)
 
     def get_bibles(self):
         """

openlp/plugins/bibles/lib/mediaitem.py

@@ -911,16 +911,16 @@ class BibleMediaItem(MediaManagerItem):
             list_widget_items.append(bible_verse)
         return list_widget_items
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         log.debug('generating slide data')
         if item:

openlp/plugins/custom/lib/mediaitem.py

@@ -28,7 +28,7 @@ from sqlalchemy.sql import and_, func, or_
 from openlp.core.common.i18n import UiStrings, translate
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
-from openlp.core.lib import ServiceItemContext, check_item_selected
+from openlp.core.lib import check_item_selected
 from openlp.core.lib.mediamanageritem import MediaManagerItem
 from openlp.core.lib.plugin import PluginStatus
 from openlp.core.lib.serviceitem import ItemCapabilities
@@ -219,15 +219,12 @@ class CustomMediaItem(MediaManagerItem):
         self.search_text_edit.setFocus()
         self.search_text_edit.selectAll()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False,
-                            remote=False, context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
         :param service_item: To be updated
         :param item: The custom database item to be used
-        :param xml_version: No used
-        :param remote: Is this triggered by the Preview Controller or Service Manager.
-        :param context: Why is this item required to be build (Default Service).
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         item_id = self._get_id_of_item_to_generate(item, self.remote_custom)
         service_item.add_capability(ItemCapabilities.CanEdit)

openlp/plugins/images/lib/mediaitem.py

@@ -542,16 +542,16 @@ class ImageMediaItem(MediaManagerItem):
         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,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         background = QtGui.QColor(Settings().value(self.settings_section + '/background color'))
         if item:

openlp/plugins/media/lib/mediaitem.py

@@ -29,7 +29,7 @@ from openlp.core.state import State
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.i18n import UiStrings, get_natural_key, translate
 from openlp.core.common.mixins import RegistryProperties
-from openlp.core.common.path import Path, create_paths, path_to_str
+from openlp.core.common.path import create_paths, path_to_str
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.lib import MediaType, ServiceItemContext, check_item_selected
@@ -166,16 +166,16 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         # self.display_type_combo_box.currentIndexChanged.connect(self.override_player_changed)
         pass
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         if item is None:
             item = self.list_view.currentItem()
@@ -229,8 +229,8 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         Initialize media item.
         """
         self.list_view.clear()
-        self.service_path = str(AppLocation.get_section_data_path(self.settings_section) / 'thumbnails')
-        create_paths(Path(self.service_path))
+        self.service_path = AppLocation.get_section_data_path(self.settings_section) / 'thumbnails'
+        create_paths(self.service_path)
         self.load_list([path_to_str(file) for file in Settings().value(self.settings_section + '/media files')])
         self.rebuild_players()
 
@@ -264,7 +264,7 @@ class MediaMediaItem(MediaManagerItem, RegistryProperties):
         :param media: The media
         :param target_group:
         """
-        media.sort(key=lambda file_name: get_natural_key(os.path.split(str(file_name))[1]))
+        media.sort(key=lambda file_path: get_natural_key(file_path.name))
         for track in media:
             track_info = QtCore.QFileInfo(track)
             item_name = None

openlp/plugins/presentations/lib/impresscontroller.py

@@ -46,6 +46,7 @@ from openlp.plugins.presentations.lib.presentationcontroller import Presentation
 if is_win():
     from win32com.client import Dispatch
     import pywintypes
+    uno_available = False
     # Declare an empty exception to match the exception imported from UNO
 
     class ErrorCodeIOException(Exception):

openlp/plugins/presentations/lib/mediaitem.py

@@ -260,16 +260,16 @@ class PresentationMediaItem(MediaManagerItem):
                     doc.presentation_deleted()
                 doc.close_presentation()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service, file_path=None):
+    def generate_slide_data(self, service_item, *, item=None, remote=False, context=ServiceItemContext.Service,
+                            file_path=None, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
         :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         if item:
             items = [item]

openlp/plugins/presentations/lib/messagelistener.py

@@ -337,14 +337,8 @@ class MessageListener(object):
             # Create a copy of the original item, and then clear the original item so it can be filled with images
             item_cpy = copy.copy(item)
             item.__init__(None)
-            if is_live:
-                # TODO: To Path object
-                self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Live,
-                                                    str(file_path))
-            else:
-                # TODO: To Path object
-                self.media_item.generate_slide_data(item, item_cpy, False, False, ServiceItemContext.Preview,
-                                                    str(file_path))
+            context = ServiceItemContext.Live if is_live else ServiceItemContext.Preview
+            self.media_item.generate_slide_data(item, item=item_cpy, context=context, file_path=file_path)
             # Some of the original serviceitem attributes is needed in the new serviceitem
             item.footer = item_cpy.footer
             item.from_service = item_cpy.from_service

openlp/plugins/songs/forms/songimportform.py

@@ -329,8 +329,13 @@ class SongImportForm(OpenLPWizard, RegistryProperties):
             importer = self.plugin.import_songs(
                 source_format,
                 file_paths=self.get_list_of_paths(self.format_widgets[source_format]['file_list_widget']))
+        try:
             importer.do_import()
             self.progress_label.setText(WizardStrings.FinishedImport)
+        except OSError as e:
+            log.exception('Importing songs failed')
+            self.progress_label.setText(translate('SongsPlugin.ImportWizardForm',
+                                                  'Your Song import failed. {error}').format(error=e))
 
     def on_error_copy_to_button_clicked(self):
         """

openlp/plugins/songs/forms/songselectform.py

@@ -263,8 +263,9 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties)
         self.login_progress_bar.setVisible(True)
         self.application.process_events()
         # Log the user in
-        if not self.song_select_importer.login(
-                self.username_edit.text(), self.password_edit.text(), self._update_login_progress):
+        subscription_level = self.song_select_importer.login(
+            self.username_edit.text(), self.password_edit.text(), self._update_login_progress)
+        if not subscription_level:
             QtWidgets.QMessageBox.critical(
                 self,
                 translate('SongsPlugin.SongSelectForm', 'Error Logging In'),
@@ -272,6 +273,14 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties)
                           'There was a problem logging in, perhaps your username or password is incorrect?')
             )
         else:
+            if subscription_level == 'Free':
+                QtWidgets.QMessageBox.information(
+                    self,
+                    translate('SongsPlugin.SongSelectForm', 'Free user'),
+                    translate('SongsPlugin.SongSelectForm', 'You logged in with a free account, '
+                                                            'the search will be limited to songs '
+                                                            'in the public domain.')
+                )
             if self.save_password_checkbox.isChecked():
                 Settings().setValue(self.plugin.settings_section + '/songselect username', self.username_edit.text())
                 Settings().setValue(self.plugin.settings_section + '/songselect password', self.password_edit.text())

openlp/plugins/songs/lib/importers/cclifile.py

@@ -67,7 +67,7 @@ class CCLIFileImport(SongImport):
                         details = {'confidence': 1, 'encoding': 'utf-8'}
                     except UnicodeDecodeError:
                         details = chardet.detect(detect_content)
-                in_file = codecs.open(str(file_path), 'r', details['encoding'])
+                in_file = codecs.open(file_path, 'r', details['encoding'])
                 if not in_file.read(1) == '\ufeff':
                     # not UTF or no BOM was found
                     in_file.seek(0)
@@ -251,10 +251,10 @@ class CCLIFileImport(SongImport):
         line_number = 0
         check_first_verse_line = False
         verse_text = ''
+        verse_type = VerseType.tags[VerseType.Verse]
         song_author = ''
         verse_start = False
         for line in text_list:
-            verse_type = 'v'
             clean_line = line.strip()
             if not clean_line:
                 if line_number == 0:
@@ -263,6 +263,7 @@ class CCLIFileImport(SongImport):
                     if verse_text:
                         self.add_verse(verse_text, verse_type)
                         verse_text = ''
+                        verse_type = VerseType.tags[VerseType.Verse]
                         verse_start = False
             else:
                 # line_number=0, song title
@@ -279,7 +280,7 @@ class CCLIFileImport(SongImport):
                     elif not verse_start:
                         # We have the verse descriptor
                         verse_desc_parts = clean_line.split(' ')
-                        if len(verse_desc_parts) == 2:
+                        if len(verse_desc_parts):
                             if verse_desc_parts[0].startswith('Ver'):
                                 verse_type = VerseType.tags[VerseType.Verse]
                             elif verse_desc_parts[0].startswith('Ch'):

openlp/plugins/songs/lib/mediaitem.py

@@ -557,16 +557,14 @@ class SongMediaItem(MediaManagerItem):
             self.plugin.manager.save_object(new_song)
         self.on_song_list_load()
 
-    def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False,
-                            context=ServiceItemContext.Service):
+    def generate_slide_data(self, service_item, *, item=None, context=ServiceItemContext.Service, **kwargs):
         """
         Generate the slide data. Needs to be implemented by the plugin.
 
         :param service_item: The service item to be built on
         :param item: The Song item to be used
-        :param xml_version: The xml version (not used)
-        :param remote: Triggered from remote
         :param context: Why is it being generated
+        :param kwargs: Consume other unused args specified by the base implementation, but not use by this one.
         """
         log.debug('generate_slide_data: {service}, {item}, {remote}'.format(service=service_item, item=item,
                                                                             remote=self.remote_song))

openlp/plugins/songs/lib/songselect.py

@@ -81,7 +81,7 @@ class SongSelectImport(object):
         :param username: SongSelect username
         :param password: SongSelect password
         :param callback: Method to notify of progress.
-        :return: True on success, False on failure.
+        :return: subscription level on success, None on failure.
         """
         if callback:
             callback()
@@ -115,11 +115,31 @@ class SongSelectImport(object):
             return False
         if callback:
             callback()
+        # Page if user is in an organization
         if posted_page.find('input', id='SearchText') is not None:
-            return True
+            self.subscription_level = self.find_subscription_level(posted_page)
+            return self.subscription_level
+        # Page if user is not in an organization
+        elif posted_page.find('div', id="select-organization") is not None:
+            try:
+                home_page = BeautifulSoup(self.opener.open(BASE_URL).read(), 'lxml')
+            except (TypeError, URLError) as error:
+                log.exception('Could not reach SongSelect, {error}'.format(error=error))
+            self.subscription_level = self.find_subscription_level(home_page)
+            return self.subscription_level
         else:
             log.debug(posted_page)
-            return False
+            return None
+
+    def find_subscription_level(self, page):
+        scripts = page.find_all('script')
+        for tag in scripts:
+            if tag.string:
+                match = re.search("'Subscription': '(?P<subscription_level>[^']+)", tag.string)
+                if match:
+                    return match.group('subscription_level')
+        log.error('Could not determine SongSelect subscription level')
+        return None
 
     def logout(self):
         """
@@ -128,7 +148,7 @@ class SongSelectImport(object):
         try:
             self.opener.open(LOGOUT_URL)
         except (TypeError, URLError) as error:
-            log.exception('Could not log of SongSelect, {error}'.format(error=error))
+            log.exception('Could not log out of SongSelect, {error}'.format(error=error))
 
     def search(self, search_text, max_results, callback=None):
         """
@@ -145,7 +165,7 @@ class SongSelectImport(object):
             'PrimaryLanguage': '',
             'Keys': '',
             'Themes': '',
-            'List': '',
+            'List': 'publicdomain' if self.subscription_level == 'Free' else '',
             'Sort': '',
             'SearchText': search_text
         }

openlp/plugins/songusage/forms/songusagedetailform.py

@@ -85,7 +85,7 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP
             self.main_window.error_message(
                 translate('SongUsagePlugin.SongUsageDetailForm', 'Output Path Not Selected'),
                 translate('SongUsagePlugin.SongUsageDetailForm', 'You have not set a valid output location for your'
-                          ' song usage report. \nPlease select an existing path on your computer.')
+                          ' song usage report.\nPlease select an existing path on your computer.')
             )
             return
         create_paths(path)
@@ -112,7 +112,7 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP
                 self.main_window.information_message(
                     translate('SongUsagePlugin.SongUsageDetailForm', 'Report Creation'),
                     translate('SongUsagePlugin.SongUsageDetailForm',
-                              'Report \n{name} \nhas been successfully created. ').format(name=report_file_name)
+                              'Report\n{name}\nhas been successfully created.').format(name=report_file_name)
                 )
         except OSError as ose:
             log.exception('Failed to write out song usage records')

run_openlp.py

@@ -24,6 +24,7 @@
 The entrypoint for OpenLP
 """
 import faulthandler
+import logging
 import multiprocessing
 import sys
 
@@ -34,14 +35,19 @@ from openlp.core.common import is_macosx, is_win
 from openlp.core.common.applocation import AppLocation
 from openlp.core.common.path import create_paths
 
+log = logging.getLogger(__name__)
+
 
 def set_up_fault_handling():
     """
     Set up the Python fault handler
     """
     # Create the cache directory if it doesn't exist, and enable the fault handler to log to an error log file
+    try:
         create_paths(AppLocation.get_directory(AppLocation.CacheDir))
         faulthandler.enable((AppLocation.get_directory(AppLocation.CacheDir) / 'error.log').open('wb'))
+    except OSError:
+        log.exception('An exception occurred when enabling the fault handler')
 
 
 def start():

scripts/appveyor.yml

@@ -1,5 +1,8 @@
 version: OpenLP-win-ci-b{build}
 
+image:
+  - Visual Studio 2017
+
 clone_script:
   - curl -L https://bazaar.launchpad.net/BRANCHPATH/tarball -o sourcecode.tar.gz
   - 7z e sourcecode.tar.gz
@@ -7,15 +10,17 @@ clone_script:
   - mv BRANCHPATH openlp-branch
 
 environment:
-  PYTHON: C:\\Python37-x64
+  matrix:
+    - PYTHON: C:\\Python37-x64
+    - PYTHON: C:\\Python37
 
 install:
   # Install dependencies from pypi
-  - "%PYTHON%\\python.exe -m pip install sqlalchemy alembic appdirs chardet beautifulsoup4 lxml Mako mysql-connector-python nose mock pyodbc psycopg2 pypiwin32 websockets asyncio waitress six webob requests QtAwesome PyQt5 pymediainfo"
+  - "%PYTHON%\\python.exe -m pip install sqlalchemy alembic appdirs chardet beautifulsoup4 lxml Mako mysql-connector-python pytest mock pyodbc psycopg2 pypiwin32 websockets asyncio waitress six webob requests QtAwesome PyQt5 PyQtWebEngine pymediainfo"
   # Download and unpack mupdf
   - appveyor DownloadFile https://mupdf.com/downloads/archive/mupdf-1.14.0-windows.zip
   - 7z x mupdf-1.14.0-windows.zip
-  - cp mupdf-1.14.0-windows/mupdf.exe openlp-branch/mupdf.exe
+  - cp mupdf-1.14.0-windows/mutool.exe openlp-branch/mutool.exe
   # Download and unpack mediainfo
   - appveyor DownloadFile https://mediaarea.net/download/binary/mediainfo/18.08.1/MediaInfo_CLI_18.08.1_Windows_i386.zip
   - mkdir MediaInfo
@@ -27,7 +32,7 @@ build: off
 test_script:
   - cd openlp-branch
   # Run the tests
-  - "%PYTHON%\\python.exe -m nose -v tests"
+  - "%PYTHON%\\python.exe -m pytest -v tests"
   # Go back to the user root folder
   - cd..
 

setup.cfg

@@ -9,6 +9,7 @@ ignore = E402,E722,W503,W504
 
 [flake8]
 exclude=resources.py,vlc.py
+max-line-length = 120
 ignore = E402,W503,W504,D
 
 [pycodestyle]

tests/functional/openlp_core/api/test_deploy.py

@@ -15,13 +15,12 @@
 # FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
 # more details.                                                               #
 #                                                                             #
-
-
-# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
-###############################################################################
-from tempfile import mkdtemp
 # 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                          #
+###############################################################################
+import os
+from tempfile import mkdtemp
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
@@ -57,14 +56,15 @@ class TestRemoteDeploy(TestCase):
         # GIVEN: A new downloaded zip file
         mocked_zipfile = MagicMock()
         MockZipFile.return_value = mocked_zipfile
-        root_path = Path('/tmp/remotes')
+        root_path_str = '{sep}tmp{sep}remotes'.format(sep=os.sep)
+        root_path = Path(root_path_str)
 
         # WHEN: deploy_zipfile() is called
         deploy_zipfile(root_path, 'site.zip')
 
         # THEN: the zip file should have been extracted to the right location
-        MockZipFile.assert_called_once_with('/tmp/remotes/site.zip')
-        mocked_zipfile.extractall.assert_called_once_with('/tmp/remotes')
+        MockZipFile.assert_called_once_with(Path('/tmp/remotes/site.zip'))
+        mocked_zipfile.extractall.assert_called_once_with(Path('/tmp/remotes'))
 
     @patch('openlp.core.api.deploy.Registry')
     @patch('openlp.core.api.deploy.get_web_page')

tests/functional/openlp_core/common/test_init.py

@@ -309,9 +309,9 @@ class TestInit(TestCase, TestMixin):
         """
         # GIVEN: A mocked UniversalDetector instance with done attribute set to True after first iteration
         with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
-                patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open:
+                patch.object(Path, 'open', return_value=BytesIO(b'data' * 260)) as mocked_open:
             encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
-            mocked_universal_detector_inst = MagicMock(result=encoding_result)
+            mocked_universal_detector_inst = MagicMock(**{'close.return_value': encoding_result})
             type(mocked_universal_detector_inst).done = PropertyMock(side_effect=[False, True])
             mocked_universal_detector.return_value = mocked_universal_detector_inst
 
@@ -320,7 +320,7 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: The feed method of UniversalDetector should only br called once before returning a result
             mocked_open.assert_called_once_with('rb')
-            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256)]
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b'data' * 256)]
             mocked_universal_detector_inst.close.assert_called_once_with()
             assert result == encoding_result
 
@@ -331,10 +331,10 @@ class TestInit(TestCase, TestMixin):
         # GIVEN: A mocked UniversalDetector instance which isn't set to done and a mocked open, with 1040 bytes of test
         #       data (enough to run the iterator twice)
         with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
-                patch.object(Path, 'open', return_value=BytesIO(b"data" * 260)) as mocked_open:
+                patch.object(Path, 'open', return_value=BytesIO(b'data' * 260)) as mocked_open:
             encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
             mocked_universal_detector_inst = MagicMock(mock=mocked_universal_detector,
-                                                       **{'done': False, 'result': encoding_result})
+                                                       **{'done': False, 'close.return_value': encoding_result})
             mocked_universal_detector.return_value = mocked_universal_detector_inst
 
             # WHEN: Calling get_file_encoding
@@ -342,7 +342,7 @@ class TestInit(TestCase, TestMixin):
 
             # THEN: The feed method of UniversalDetector should have been called twice before returning a result
             mocked_open.assert_called_once_with('rb')
-            assert mocked_universal_detector_inst.feed.mock_calls == [call(b"data" * 256), call(b"data" * 4)]
+            assert mocked_universal_detector_inst.feed.mock_calls == [call(b'data' * 256), call(b'data' * 4)]
             mocked_universal_detector_inst.close.assert_called_once_with()
             assert result == encoding_result
 
@@ -352,13 +352,19 @@ class TestInit(TestCase, TestMixin):
         """
         # GIVEN: A mocked UniversalDetector instance which isn't set to done and a mocked open, with 1040 bytes of test
         #       data (enough to run the iterator twice)
-        with patch('openlp.core.common.UniversalDetector'), \
+        with patch('openlp.core.common.UniversalDetector') as mocked_universal_detector, \
                 patch('builtins.open', side_effect=OSError), \
                 patch('openlp.core.common.log') as mocked_log:
+            encoding_result = {'encoding': 'UTF-8', 'confidence': 0.99}
+            mocked_universal_detector_inst = MagicMock(mock=mocked_universal_detector,
+                                                       **{'done': False, 'close.return_value': encoding_result})
+            mocked_universal_detector.return_value = mocked_universal_detector_inst
 
             # WHEN: Calling get_file_encoding
             result = get_file_encoding(Path('file name'))
 
             # THEN: log.exception should be called and get_file_encoding should return None
             mocked_log.exception.assert_called_once_with('Error detecting file encoding')
-            assert result is None
+            mocked_universal_detector_inst.feed.assert_not_called()
+            mocked_universal_detector_inst.close.assert_called_once_with()
+            assert result == encoding_result

tests/functional/openlp_core/common/test_path.py

@@ -179,9 +179,8 @@ class TestShutil(TestCase):
             # WHEN: Calling :func:`openlp.core.common.path.rmtree` with the path parameter as Path object type
             path.rmtree()
 
-            # THEN: :func:`shutil.rmtree` should have been called with the str equivalents of the Path object.
-            mocked_shutil_rmtree.assert_called_once_with(
-                os.path.join('test', 'path'), False, None)
+            # THEN: :func:`shutil.rmtree` should have been called with the the Path object.
+            mocked_shutil_rmtree.assert_called_once_with(Path('test', 'path'), False, None)
 
     def test_rmtree_optional_params(self):
         """
@@ -198,8 +197,7 @@ class TestShutil(TestCase):
 
             # THEN: :func:`shutil.rmtree` should have been called with the optional parameters, with out any of the
             #       values being modified
-            mocked_shutil_rmtree.assert_called_once_with(
-                os.path.join('test', 'path'), True, mocked_on_error)
+            mocked_shutil_rmtree.assert_called_once_with(path, True, mocked_on_error)
 
     def test_which_no_command(self):
         """
@@ -324,7 +322,7 @@ class TestPath(TestCase):
         obj = path.json_object(extra=1, args=2)
 
         # THEN: A JSON decodable object should have been returned.
-        assert obj == {'__Path__': ('/', 'base', 'path', 'to', 'fi.le')}
+        assert obj == {'__Path__': (os.sep, 'base', 'path', 'to', 'fi.le')}
 
     def test_path_json_object_base_path(self):
         """

tests/functional/openlp_core/lib/test_image_manager.py

@@ -178,7 +178,7 @@ class TestImageManager(TestCase, TestMixin):
         # THEN a KeyError is thrown
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(TEST_PATH, 'church1.jpg')
-        assert context.exception is not '', 'KeyError exception should have been thrown for missing image'
+        assert context.exception != '', 'KeyError exception should have been thrown for missing image'
 
     @patch('openlp.core.lib.imagemanager.run_thread')
     def test_different_dimension_image(self, mocked_run_thread):
@@ -211,7 +211,7 @@ class TestImageManager(TestCase, TestMixin):
         # WHEN: calling with correct image, but wrong dimensions
         with self.assertRaises(KeyError) as context:
             self.image_manager.get_image(full_path, 'church.jpg', 120, 120)
-        assert context.exception is not '', 'KeyError exception should have been thrown for missing dimension'
+        assert context.exception != '', 'KeyError exception should have been thrown for missing dimension'
 
     @patch('openlp.core.lib.imagemanager.resize_image')
     @patch('openlp.core.lib.imagemanager.image_to_byte')

tests/functional/openlp_core/lib/test_serviceitem.py

@@ -141,7 +141,7 @@ class TestServiceItem(TestCase, TestMixin):
         """
         # GIVEN: A new service item and a mocked add icon function
         image_name = 'image_1.jpg'
-        test_file = os.path.join(str(TEST_PATH), image_name)
+        test_file = TEST_PATH / image_name
         frame_array = {'path': test_file, 'title': image_name}
 
         service_item = ServiceItem(None)
@@ -154,13 +154,13 @@ class TestServiceItem(TestCase, TestMixin):
                 mocked_get_section_data_path:
             mocked_exists.return_value = True
             mocked_get_section_data_path.return_value = Path('/path/')
-            service_item.set_from_service(line, str(TEST_PATH))
+            service_item.set_from_service(line, TEST_PATH)
 
         # THEN: We should get back a valid service item
         assert service_item.is_valid is True, 'The new service item should be valid'
         assert test_file == service_item.get_rendered_frame(0), 'The first frame should match the path to the image'
         assert frame_array == service_item.get_frames()[0], 'The return should match frame array1'
-        assert test_file == str(service_item.get_frame_path(0)), \
+        assert test_file == service_item.get_frame_path(0), \
             'The frame path should match the full path to the image'
         assert image_name == service_item.get_frame_title(0), 'The frame title should match the image name'
         assert image_name == service_item.get_display_title(), 'The display title should match the first image name'
@@ -328,7 +328,7 @@ class TestServiceItem(TestCase, TestMixin):
 
         # WHEN: We add a custom from a saved service
         line = convert_file_service_item(TEST_PATH, 'serviceitem-song-linked-audio.osj')
-        service_item.set_from_service(line, '/test/')
+        service_item.set_from_service(line, Path('/test/'))
 
         # THEN: We should get back a valid service item
         assert service_item.is_valid is True, 'The new service item should be valid'

tests/functional/openlp_core/ui/test_firsttimeform.py

@@ -25,40 +25,69 @@ Package to test the openlp.core.ui.firsttimeform package.
 import os
 import tempfile
 from unittest import TestCase
-from unittest.mock import MagicMock, call, patch
+from unittest.mock import MagicMock, call, patch, DEFAULT
 
 from openlp.core.common.path import Path
 from openlp.core.common.registry import Registry
-from openlp.core.ui.firsttimeform import FirstTimeForm
+from openlp.core.ui.firsttimeform import FirstTimeForm, ThemeListWidgetItem
 from tests.helpers.testmixin import TestMixin
 
 
-FAKE_CONFIG = """
-[general]
-base url = http://example.com/frw/
-[songs]
-directory = songs
-[bibles]
-directory = bibles
-[themes]
-directory = themes
+INVALID_CONFIG = """
+{
+  "_comments": "The most recent version should be added to https://openlp.org/files/frw/download_3.0.json",
+  "_meta": {
+}
 """
 
-FAKE_BROKEN_CONFIG = """
-[general]
-base url = http://example.com/frw/
-[songs]
-directory = songs
-[bibles]
-directory = bibles
-"""
 
-FAKE_INVALID_CONFIG = """
-<html>
-<head><title>This is not a config file</title></head>
-<body>Some text</body>
-</html>
-"""
+class TestThemeListWidgetItem(TestCase):
+    """
+    Test the :class:`ThemeListWidgetItem` class
+    """
+    def setUp(self):
+        self.sample_theme_data = {'file_name': 'BlueBurst.otz', 'sha256': 'sha_256_hash',
+                                  'thumbnail': 'BlueBurst.png', 'title': 'Blue Burst'}
+        download_worker_patcher = patch('openlp.core.ui.firsttimeform.DownloadWorker')
+        self.addCleanup(download_worker_patcher.stop)
+        self.mocked_download_worker = download_worker_patcher.start()
+        run_thread_patcher = patch('openlp.core.ui.firsttimeform.run_thread')
+        self.addCleanup(run_thread_patcher.stop)
+        self.mocked_run_thread = run_thread_patcher.start()
+
+    def test_init_sample_data(self):
+        """
+        Test that the theme data is loaded correctly in to a ThemeListWidgetItem object when instantiated
+        """
+        # GIVEN: A sample theme dictanary object
+        # WHEN: Creating an instance of `ThemeListWidgetItem`
+        instance = ThemeListWidgetItem('url', self.sample_theme_data, MagicMock())
+
+        # THEN: The data should have been set correctly
+        assert instance.file_name == 'BlueBurst.otz'
+        assert instance.sha256 == 'sha_256_hash'
+        assert instance.text() == 'Blue Burst'
+        assert instance.toolTip() == 'Blue Burst'
+        self.mocked_download_worker.assert_called_once_with('url', 'BlueBurst.png')
+
+    def test_init_download_worker(self):
+        """
+        Test that the `DownloadWorker` worker is set up correctly and that the thread is started.
+        """
+        # GIVEN: A sample theme dictanary object
+        mocked_ftw = MagicMock(spec=FirstTimeForm)
+        mocked_ftw.thumbnail_download_threads = []
+
+        # WHEN: Creating an instance of `ThemeListWidgetItem`
+        instance = ThemeListWidgetItem('url', self.sample_theme_data, mocked_ftw)
+
+        # THEN: The `DownloadWorker` should have been set up with the appropriate data
+        self.mocked_download_worker.assert_called_once_with('url', 'BlueBurst.png')
+        self.mocked_download_worker.download_failed.connect.called_once_with(instance._on_download_failed())
+        self.mocked_download_worker.download_succeeded.connect.called_once_with(instance._on_thumbnail_downloaded)
+        self.mocked_run_thread.assert_called_once_with(
+            self.mocked_download_worker(), 'thumbnail_download_BlueBurst.png')
+        assert mocked_ftw.thumbnail_download_threads == ['thumbnail_download_BlueBurst.png']
 
 
 class TestFirstTimeForm(TestCase, TestMixin):
@@ -92,7 +121,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
         assert expected_screens == frw.screens, 'The screens should be correct'
         assert frw.web_access is True, 'The default value of self.web_access should be True'
         assert frw.was_cancelled is False, 'The default value of self.was_cancelled should be False'
-        assert [] == frw.theme_screenshot_threads, 'The list of threads should be empty'
+        assert [] == frw.thumbnail_download_threads, 'The list of threads should be empty'
         assert frw.has_run_wizard is False, 'has_run_wizard should be False'
 
     def test_set_defaults(self):
@@ -109,6 +138,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
                 patch.object(frw, 'no_internet_finish_button') as mocked_no_internet_finish_btn, \
                 patch.object(frw, 'currentIdChanged') as mocked_currentIdChanged, \
                 patch.object(frw, 'theme_combo_box') as mocked_theme_combo_box, \
+                patch.object(frw, 'songs_check_box') as mocked_songs_check_box, \
                 patch.object(Registry, 'register_function') as mocked_register_function, \
                 patch('openlp.core.ui.firsttimeform.Settings', return_value=mocked_settings), \
                 patch('openlp.core.ui.firsttimeform.gettempdir', return_value='temp') as mocked_gettempdir, \
@@ -122,7 +152,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
 
             # THEN: The default values should have been set
             mocked_restart.assert_called_once()
-            assert 'http://openlp.org/files/frw/' == frw.web, 'The default URL should be set'
+            assert 'https://get.openlp.org/ftw/' == frw.web, 'The default URL should be set'
             mocked_cancel_button.clicked.connect.assert_called_once_with(frw.on_cancel_button_clicked)
             mocked_no_internet_finish_btn.clicked.connect.assert_called_once_with(
                 frw.on_no_internet_finish_button_clicked)
@@ -134,6 +164,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
             mocked_create_paths.assert_called_once_with(Path('temp', 'openlp'))
             mocked_theme_combo_box.clear.assert_called_once()
             mocked_theme_manager.assert_not_called()
+            mocked_songs_check_box.assert_not_called()
 
     def test_set_defaults_rerun(self):
         """
@@ -150,12 +181,17 @@ class TestFirstTimeForm(TestCase, TestMixin):
                 patch.object(frw, 'no_internet_finish_button') as mocked_no_internet_finish_btn, \
                 patch.object(frw, 'currentIdChanged') as mocked_currentIdChanged, \
                 patch.object(frw, 'theme_combo_box', **{'findText.return_value': 3}) as mocked_theme_combo_box, \
+                patch.multiple(frw, songs_check_box=DEFAULT, bible_check_box=DEFAULT, presentation_check_box=DEFAULT,
+                               image_check_box=DEFAULT, media_check_box=DEFAULT, custom_check_box=DEFAULT,
+                               song_usage_check_box=DEFAULT, alert_check_box=DEFAULT), \
                 patch.object(Registry, 'register_function') as mocked_register_function, \
                 patch('openlp.core.ui.firsttimeform.Settings', return_value=mocked_settings), \
                 patch('openlp.core.ui.firsttimeform.gettempdir', return_value='temp') as mocked_gettempdir, \
                 patch('openlp.core.ui.firsttimeform.create_paths') as mocked_create_paths, \
                 patch.object(frw.application, 'set_normal_cursor'):
-            mocked_theme_manager = MagicMock(**{'get_themes.return_value': ['a', 'b', 'c']})
+            mocked_plugin_manager = MagicMock()
+            mocked_theme_manager = MagicMock(**{'get_themes.return_value': ['b', 'a', 'c']})
+            Registry().register('plugin_manager', mocked_plugin_manager)
             Registry().register('theme_manager', mocked_theme_manager)
 
             # WHEN: The set_defaults() method is run
@@ -163,7 +199,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
 
             # THEN: The default values should have been set
             mocked_restart.assert_called_once()
-            assert 'http://openlp.org/files/frw/' == frw.web, 'The default URL should be set'
+            assert 'https://get.openlp.org/ftw/' == frw.web, 'The default URL should be set'
             mocked_cancel_button.clicked.connect.assert_called_once_with(frw.on_cancel_button_clicked)
             mocked_no_internet_finish_btn.clicked.connect.assert_called_once_with(
                 frw.on_no_internet_finish_button_clicked)
@@ -173,9 +209,13 @@ class TestFirstTimeForm(TestCase, TestMixin):
             mocked_settings.value.assert_has_calls([call('core/has run wizard'), call('themes/global theme')])
             mocked_gettempdir.assert_called_once()
             mocked_create_paths.assert_called_once_with(Path('temp', 'openlp'))
-            mocked_theme_manager.assert_not_called()
+            mocked_theme_manager.get_themes.assert_called_once()
             mocked_theme_combo_box.clear.assert_called_once()
-            mocked_theme_combo_box.addItem.assert_has_calls([call('a'), call('b'), call('c')])
+            mocked_plugin_manager.get_plugin_by_name.assert_has_calls(
+                [call('songs'), call('bibles'), call('presentations'), call('images'), call('media'), call('custom'),
+                 call('songusage'), call('alerts')], any_order=True)
+            mocked_plugin_manager.get_plugin_by_name.assert_has_calls([call().is_active()] * 8, any_order=True)
+            mocked_theme_combo_box.addItems.assert_called_once_with(['a', 'b', 'c'])
             mocked_theme_combo_box.findText.assert_called_once_with('Default Theme')
             mocked_theme_combo_box.setCurrentIndex(3)
 
@@ -192,7 +232,7 @@ class TestFirstTimeForm(TestCase, TestMixin):
         mocked_is_thread_finished.side_effect = [False, True]
         frw = FirstTimeForm(None)
         frw.initialize(MagicMock())
-        frw.theme_screenshot_threads = ['test_thread']
+        frw.thumbnail_download_threads = ['test_thread']
         with patch.object(frw.application, 'set_normal_cursor') as mocked_set_normal_cursor:
 
             # WHEN: on_cancel_button_clicked() is called
@@ -201,43 +241,26 @@ class TestFirstTimeForm(TestCase, TestMixin):
             # THEN: The right things should be called in the right order
             assert frw.was_cancelled is True, 'The was_cancelled property should have been set to True'
             mocked_get_thread_worker.assert_called_once_with('test_thread')
-            mocked_worker.set_download_canceled.assert_called_with(True)
+            mocked_worker.cancel_download.assert_called_once()
             mocked_is_thread_finished.assert_called_with('test_thread')
             assert mocked_is_thread_finished.call_count == 2, 'isRunning() should have been called twice'
             mocked_time.sleep.assert_called_once_with(0.1)
             mocked_set_normal_cursor.assert_called_once_with()
 
-    def test_broken_config(self):
+    @patch('openlp.core.ui.firsttimeform.critical_error_message_box')
+    def test__parse_config_invalid_config(self, mocked_critical_error_message_box):
         """
-        Test if we can handle an config file with missing data
+        Test `FirstTimeForm._parse_config` when called with invalid data
         """
-        # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked broken config file
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
+        # GIVEN: An instance of `FirstTimeForm`
         first_time_form = FirstTimeForm(None)
-            first_time_form.initialize(MagicMock())
-            mocked_get_web_page.return_value = FAKE_BROKEN_CONFIG
 
-            # WHEN: The First Time Wizard is downloads the config file
-            first_time_form._download_index()
+        # WHEN: Calling _parse_config with a string containing invalid data
+        result = first_time_form._parse_config(INVALID_CONFIG)
 
-            # THEN: The First Time Form should not have web access
-            assert first_time_form.web_access is False, 'There should not be web access with a broken config file'
-
-    def test_invalid_config(self):
-        """
-        Test if we can handle an config file in invalid format
-        """
-        # GIVEN: A mocked get_web_page, a First Time Wizard, an expected screen object, and a mocked invalid config file
-        with patch('openlp.core.ui.firsttimeform.get_web_page') as mocked_get_web_page:
-            first_time_form = FirstTimeForm(None)
-            first_time_form.initialize(MagicMock())
-            mocked_get_web_page.return_value = FAKE_INVALID_CONFIG
-
-            # WHEN: The First Time Wizard is downloads the config file
-            first_time_form._download_index()
-
-            # THEN: The First Time Form should not have web access
-            assert first_time_form.web_access is False, 'There should not be web access with an invalid config file'
+        # THEN: _parse_data should return False and the user should have should have been informed.
+        assert result is False
+        mocked_critical_error_message_box.assert_called_once()
 
     @patch('openlp.core.ui.firsttimeform.get_web_page')
     @patch('openlp.core.ui.firsttimeform.QtWidgets.QMessageBox')

tests/functional/openlp_core/ui/test_thememanager.py

@@ -66,9 +66,9 @@ class TestThemeManager(TestCase):
         theme_manager._export_theme(Path('some', 'path', 'Default.otz'), 'Default')
 
         # THEN: The zipfile should be created at the given path
-        mocked_zipfile_init.assert_called_with(os.path.join('some', 'path', 'Default.otz'), 'w')
-        mocked_zipfile_write.assert_called_with(str(RESOURCE_PATH / 'themes' / 'Default' / 'Default.xml'),
-                                                os.path.join('Default', 'Default.xml'))
+        mocked_zipfile_init.assert_called_with(Path('some', 'path', 'Default.otz'), 'w')
+        mocked_zipfile_write.assert_called_with(RESOURCE_PATH / 'themes' / 'Default' / 'Default.xml',
+                                                Path('Default', 'Default.xml'))
 
     def test_initial_theme_manager(self):
         """

tests/functional/openlp_plugins/bibles/test_manager.py

@@ -55,7 +55,8 @@ class TestManager(TestCase):
             instance = BibleManager(MagicMock())
             # We need to keep a reference to the mock for close_all as it gets set to None later on!
             mocked_close_all = MagicMock()
-            mocked_bible = MagicMock(file='KJV.sqlite', path='bibles', **{'session.close_all': mocked_close_all})
+            mocked_bible = MagicMock(file_path='KJV.sqlite', path=Path('bibles'),
+                                     **{'session.close_all': mocked_close_all})
             instance.db_cache = {'KJV': mocked_bible}
 
             # WHEN: Calling delete_bible with 'KJV'
@@ -66,4 +67,4 @@ class TestManager(TestCase):
             assert result is True
             mocked_close_all.assert_called_once_with()
             assert mocked_bible.session is None
-            mocked_delete_file.assert_called_once_with(Path('bibles', 'KJV.sqlite'))
+            mocked_delete_file.assert_called_once_with(Path('bibles'), 'KJV.sqlite')

tests/functional/openlp_plugins/media/test_mediaplugin.py

@@ -57,4 +57,4 @@ class MediaPluginTest(TestCase, TestMixin):
         # THEN: about() should return a string object
         assert isinstance(MediaPlugin.about(), str)
         # THEN: about() should return a non-empty string
-        assert len(MediaPlugin.about()) is not 0
+        assert len(MediaPlugin.about()) != 0

tests/functional/openlp_plugins/songs/test_lib.py

@@ -199,7 +199,7 @@ class TestLib(TestCase):
         result = _remove_typos(diff)
 
         # THEN: There should be no typos in the middle anymore. The remaining equals should have been merged.
-        assert len(result) is 1, 'The result should contain only one element.'
+        assert len(result) == 1, 'The result should contain only one element.'
         assert result[0][0] == 'equal', 'The result should contain an equal element.'
         assert result[0][1] == 0, 'The start indices should be kept.'
         assert result[0][2] == 22, 'The stop indices should be kept.'

tests/functional/openlp_plugins/songs/test_songselect.py

@@ -64,7 +64,7 @@ class TestSongSelectImport(TestCase, TestMixin):
     @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup')
     def test_login_fails(self, MockedBeautifulSoup, mocked_build_opener):
         """
-        Test that when logging in to SongSelect fails, the login method returns False
+        Test that when logging in to SongSelect fails, the login method returns None
         """
         # GIVEN: A bunch of mocked out stuff and an importer object
         mocked_opener = MagicMock()
@@ -80,12 +80,12 @@ class TestSongSelectImport(TestCase, TestMixin):
         # WHEN: The login method is called after being rigged to fail
         result = importer.login('username', 'password', mock_callback)
 
-        # THEN: callback was called 3 times, open was called twice, find was called twice, and False was returned
+        # THEN: callback was called 3 times, open was called twice, find was called twice, and None was returned
         assert 3 == mock_callback.call_count, 'callback should have been called 3 times'
         assert 2 == mocked_login_page.find.call_count, 'find should have been called twice'
-        assert 1 == mocked_posted_page.find.call_count, 'find should have been called once'
+        assert 2 == mocked_posted_page.find.call_count, 'find should have been called twice'
         assert 2 == mocked_opener.open.call_count, 'opener should have been called twice'
-        assert result is False, 'The login method should have returned False'
+        assert result is None, 'The login method should have returned None'
 
     @patch('openlp.plugins.songs.lib.songselect.build_opener')
     def test_login_except(self, mocked_build_opener):
@@ -129,7 +129,7 @@ class TestSongSelectImport(TestCase, TestMixin):
         assert 2 == mocked_login_page.find.call_count, 'find should have been called twice on the login page'
         assert 1 == mocked_posted_page.find.call_count, 'find should have been called once on the posted page'
         assert 2 == mocked_opener.open.call_count, 'opener should have been called twice'
-        assert result is True, 'The login method should have returned True'
+        assert result is None, 'The login method should have returned the subscription level'
 
     @patch('openlp.plugins.songs.lib.songselect.build_opener')
     @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup')
@@ -146,7 +146,8 @@ class TestSongSelectImport(TestCase, TestMixin):
         mocked_login_page.find.side_effect = [{'value': 'blah'}, mocked_form]
         mocked_posted_page = MagicMock()
         mocked_posted_page.find.return_value = MagicMock()
-        MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page]
+        mocked_home_page = MagicMock()
+        MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page]
         mock_callback = MagicMock()
         importer = SongSelectImport(None)
 
@@ -158,7 +159,7 @@ class TestSongSelectImport(TestCase, TestMixin):
         assert 2 == mocked_login_page.find.call_count, 'find should have been called twice on the login page'
         assert 1 == mocked_posted_page.find.call_count, 'find should have been called once on the posted page'
         assert 'https://profile.ccli.com/do/login', mocked_opener.open.call_args_list[1][0][0]
-        assert result is True, 'The login method should have returned True'
+        assert result is None, 'The login method should have returned the subscription level'
 
     @patch('openlp.plugins.songs.lib.songselect.build_opener')
     def test_logout(self, mocked_build_opener):
@@ -191,6 +192,7 @@ class TestSongSelectImport(TestCase, TestMixin):
         MockedBeautifulSoup.return_value = mocked_results_page
         mock_callback = MagicMock()
         importer = SongSelectImport(None)
+        importer.subscription_level = 'premium'
 
         # WHEN: The login method is called after being rigged to fail
         results = importer.search('text', 1000, mock_callback)
@@ -231,6 +233,7 @@ class TestSongSelectImport(TestCase, TestMixin):
         MockedBeautifulSoup.return_value = mocked_results_page
         mock_callback = MagicMock()
         importer = SongSelectImport(None)
+        importer.subscription_level = 'premium'
 
         # WHEN: The search method is called
         results = importer.search('text', 1000, mock_callback)
@@ -282,6 +285,7 @@ class TestSongSelectImport(TestCase, TestMixin):
         MockedBeautifulSoup.return_value = mocked_results_page
         mock_callback = MagicMock()
         importer = SongSelectImport(None)
+        importer.subscription_level = 'premium'
 
         # WHEN: The search method is called
         results = importer.search('text', 2, mock_callback)

tests/functional/openlp_plugins/songusage/test_songusage.py

@@ -45,8 +45,8 @@ class TestSongUsage(TestCase):
         # THEN: about() should return a string object
         assert isinstance(SongUsagePlugin.about(), str)
         # THEN: about() should return a non-empty string
-        assert len(SongUsagePlugin.about()) is not 0
-        assert len(SongUsagePlugin.about()) is not 0
+        assert len(SongUsagePlugin.about()) != 0
+        assert len(SongUsagePlugin.about()) != 0
 
     @patch('openlp.plugins.songusage.songusageplugin.Manager')
     def test_song_usage_init(self, MockedManager):

tests/interfaces/openlp_core/ui/test_firsttimeform.py

@@ -0,0 +1,88 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2019 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# 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                          #
+###############################################################################
+"""
+Package to test the openlp.core.ui.firsttimeform package.
+"""
+from unittest import TestCase
+from unittest.mock import MagicMock, call, patch
+
+from openlp.core.common.path import Path
+from openlp.core.common.registry import Registry
+from openlp.core.ui.firsttimeform import ThemeListWidgetItem
+from openlp.core.ui.icons import UiIcons
+from tests.helpers.testmixin import TestMixin
+
+
+class TestThemeListWidgetItem(TestCase, TestMixin):
+    def setUp(self):
+        self.sample_theme_data = {'file_name': 'BlueBurst.otz', 'sha256': 'sha_256_hash',
+                                  'thumbnail': 'BlueBurst.png', 'title': 'Blue Burst'}
+        Registry.create()
+        self.registry = Registry()
+        mocked_app = MagicMock()
+        mocked_app.worker_threads = {}
+        Registry().register('application', mocked_app)
+        self.setup_application()
+
+        move_to_thread_patcher = patch('openlp.core.ui.firsttimeform.DownloadWorker.moveToThread')
+        self.addCleanup(move_to_thread_patcher.stop)
+        move_to_thread_patcher.start()
+        set_icon_patcher = patch('openlp.core.ui.firsttimeform.ThemeListWidgetItem.setIcon')
+        self.addCleanup(set_icon_patcher.stop)
+        self.mocked_set_icon = set_icon_patcher.start()
+        q_thread_patcher = patch('openlp.core.ui.firsttimeform.QtCore.QThread')
+        self.addCleanup(q_thread_patcher.stop)
+        q_thread_patcher.start()
+
+    def test_failed_download(self):
+        """
+        Test that icon get set to indicate a failure when `DownloadWorker` emits the download_failed signal
+        """
+        # GIVEN: An instance of `DownloadWorker`
+        instance = ThemeListWidgetItem('url', self.sample_theme_data, MagicMock())  # noqa Overcome GC issue
+        worker_threads = Registry().get('application').worker_threads
+        worker = worker_threads['thumbnail_download_BlueBurst.png']['worker']
+
+        # WHEN: `DownloadWorker` emits the `download_failed` signal
+        worker.download_failed.emit()
+
+        # THEN: Then the initial loading icon should have been replaced by the exception icon
+        self.mocked_set_icon.assert_has_calls([call(UiIcons().picture), call(UiIcons().exception)])
+
+    @patch('openlp.core.ui.firsttimeform.build_icon')
+    def test_successful_download(self, mocked_build_icon):
+        """
+        Test that the downloaded thumbnail is set as the icon when `DownloadWorker` emits the `download_succeeded`
+        signal
+        """
+        # GIVEN: An instance of `DownloadWorker`
+        instance = ThemeListWidgetItem('url', self.sample_theme_data, MagicMock())  # noqa Overcome GC issue
+        worker_threads = Registry().get('application').worker_threads
+        worker = worker_threads['thumbnail_download_BlueBurst.png']['worker']
+        test_path = Path('downlaoded', 'file')
+
+        # WHEN: `DownloadWorker` emits the `download_succeeded` signal
+        worker.download_succeeded.emit(test_path)
+
+        # THEN: An icon should have been built from the downloaded file and used to replace the loading icon
+        mocked_build_icon.assert_called_once_with(test_path)
+        self.mocked_set_icon.assert_has_calls([call(UiIcons().picture), call(mocked_build_icon())])

tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py

@@ -236,8 +236,8 @@ class TestSongMaintenanceForm(TestCase, TestMixin):
         assert MockedQListWidgetItem.call_args_list == expected_widget_item_calls, MockedQListWidgetItem.call_args_list
         mocked_author_item1.setData.assert_called_once_with(QtCore.Qt.UserRole, 2)
         mocked_author_item2.setData.assert_called_once_with(QtCore.Qt.UserRole, 1)
-        mocked_authors_list_widget.addItem.call_args_list == [
-            call(mocked_author_item1), call(mocked_author_item2)]
+        mocked_authors_list_widget.addItem.assert_has_calls([
+            call(mocked_author_item1), call(mocked_author_item2)])
 
     @patch('openlp.plugins.songs.forms.songmaintenanceform.QtWidgets.QListWidgetItem')
     @patch('openlp.plugins.songs.forms.songmaintenanceform.Topic')

tests/openlp_core/common/test_network_interfaces.py

@@ -70,7 +70,7 @@ class FakeIP4InterfaceEntry(QObject):
         """
         Return a QFlags enum with IsUp and IsRunning
         """
-        return (QNetworkInterface.IsUp | QNetworkInterface.IsRunning)
+        return QNetworkInterface.IsUp | QNetworkInterface.IsRunning
 
     def name(self):
         return self.my_name

tests/openlp_core/projectors/test_projector_db.py

@@ -134,6 +134,7 @@ class TestProjectorDB(TestCase, TestMixin):
         """
         Set up anything necessary for all tests
         """
+        self.tmp_folder = mkdtemp(prefix='openlp_')
         # Create a test app to keep from segfaulting
         Registry.create()
         self.registry = Registry()
@@ -153,11 +154,11 @@ class TestProjectorDB(TestCase, TestMixin):
                 patch('openlp.core.ui.mainwindow.ThemeManager'), \
                 patch('openlp.core.ui.mainwindow.ProjectorManager'), \
                 patch('openlp.core.ui.mainwindow.websockets.WebSocketServer'), \
-                patch('openlp.core.ui.mainwindow.server.HttpServer'):
+                patch('openlp.core.ui.mainwindow.server.HttpServer'), \
+                patch('openlp.core.state.State.list_plugins') as mock_plugins:
+            mock_plugins.return_value = []
             self.main_window = MainWindow()
 
-        # Create a temporary database directory and database
-        self.tmp_folder = mkdtemp(prefix='openlp_')
         tmpdb_url = 'sqlite:///{db}'.format(db=os.path.join(self.tmp_folder, TEST_DB))
         mocked_init_url.return_value = tmpdb_url
         self.projector = ProjectorDB()

tests/openlp_core/projectors/test_projector_pjlink_commands_01.py

@@ -452,12 +452,16 @@ class TestPJLinkCommands(TestCase):
         """
         Test saving video source available information
         """
+
         # GIVEN: Test object
         with patch.object(openlp.core.projectors.pjlink, 'log') as mock_log:
             pjlink = PJLink(Projector(**TEST1_DATA), no_poll=True)
             pjlink.source_available = []
-            log_debug_calls = [call('({ip}) Setting projector sources_available to '
+            log_debug_calls = [call('({ip}) reset_information() connect status is '
+                                    'S_NOT_CONNECTED'.format(ip=pjlink.name)),
+                               call('({ip}) Setting projector source_available to '
                                     '"[\'11\', \'12\', \'21\', \'22\', \'31\', \'32\']"'.format(ip=pjlink.name))]
+
             chk_data = '21 12 11 22 32 31'  # Although they should already be sorted, use unsorted to verify method
             chk_test = ['11', '12', '21', '22', '31', '32']
 
@@ -601,9 +605,9 @@ class TestPJLinkCommands(TestCase):
 
             # THEN: Power should be set to ON
             assert pjlink.power == S_STANDBY, 'Power should not have changed'
-            assert mock_UpdateIcons.emit.called is False, 'projectorUpdateIcons() should not have been called'
-            mock_change_status.called is False, 'change_status() should not have been called'
-            mock_send_command.called is False, 'send_command() should not have been called'
+            mock_UpdateIcons.emit.assert_not_called()
+            mock_change_status.assert_not_called()
+            mock_send_command.assert_not_called()
             mock_log.warning.assert_has_calls(log_warn_calls)
 
     def test_projector_process_powr_off(self):
@@ -623,9 +627,9 @@ class TestPJLinkCommands(TestCase):
 
             # THEN: Power should be set to ON
             assert pjlink.power == S_STANDBY, 'Power should have changed to S_STANDBY'
-            assert mock_UpdateIcons.emit.called is True, 'projectorUpdateIcons should have been called'
-            mock_change_status.called is True, 'change_status should have been called'
-            mock_send_command.called is False, 'send_command should not have been called'
+            mock_UpdateIcons.emit.assert_called_with()
+            mock_change_status.assert_called_with(313)
+            mock_send_command.assert_not_called()
 
     def test_projector_process_rfil_save(self):
         """

tests/openlp_core/projectors/test_projector_sourceform.py

@@ -83,8 +83,8 @@ class ProjectorSourceFormTest(TestCase, TestMixin):
         Delete all C++ objects at end so we don't segfault.
         """
         self.projectordb.session.close()
-        del(self.projectordb)
-        del(self.projector)
+        del self.projectordb
+        del self.projector
         retries = 0
         while retries < 5:
             try:

tests/utils/test_bzr_tags.py

@@ -24,7 +24,7 @@ Package to test for proper bzr tags.
 """
 import os
 from subprocess import PIPE, Popen
-from unittest import TestCase
+from unittest import TestCase, SkipTest
 
 
 TAGS1 = {'1.9.0', '1.9.1', '1.9.2', '1.9.3', '1.9.4', '1.9.5', '1.9.6', '1.9.7', '1.9.8', '1.9.9', '1.9.10',
@@ -42,7 +42,10 @@ class TestBzrTags(TestCase):
         path = os.path.dirname(__file__)
 
         # WHEN getting the branches tags
+        try:
             bzr = Popen(('bzr', 'tags', '--directory=' + path), stdout=PIPE)
+        except Exception:
+            raise SkipTest('bzr is not installed')
         std_out = bzr.communicate()[0]
         count = len(TAGS1)
         tags = [line.decode('utf-8').split()[0] for line in std_out.splitlines()]