From b10aa241199b51386baefac393f262167ca86285 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 14 Feb 2019 22:19:26 +0100 Subject: [PATCH 01/38] Attempt to fix appveyor tests --- .../presentations/lib/impresscontroller.py | 1 + scripts/appveyor.yml | 7 +++++-- tests/functional/openlp_core/api/test_deploy.py | 16 ++++++++-------- tests/functional/openlp_core/common/test_path.py | 2 +- tests/utils/test_bzr_tags.py | 7 +++++-- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/openlp/plugins/presentations/lib/impresscontroller.py b/openlp/plugins/presentations/lib/impresscontroller.py index d231acb05..1db6b9e68 100644 --- a/openlp/plugins/presentations/lib/impresscontroller.py +++ b/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): diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 43f14e69d..3187a7b9f 100644 --- a/scripts/appveyor.yml +++ b/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 @@ -11,7 +14,7 @@ environment: 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 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 @@ -27,7 +30,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.. diff --git a/tests/functional/openlp_core/api/test_deploy.py b/tests/functional/openlp_core/api/test_deploy.py index 784605034..75fc62cfb 100644 --- a/tests/functional/openlp_core/api/test_deploy.py +++ b/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(root_path_str + os.sep + 'site.zip') + mocked_zipfile.extractall.assert_called_once_with(root_path_str) @patch('openlp.core.api.deploy.Registry') @patch('openlp.core.api.deploy.get_web_page') diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py index 7c0a34635..8695b58f1 100644 --- a/tests/functional/openlp_core/common/test_path.py +++ b/tests/functional/openlp_core/common/test_path.py @@ -324,7 +324,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): """ diff --git a/tests/utils/test_bzr_tags.py b/tests/utils/test_bzr_tags.py index e6c00f707..cd057ada4 100644 --- a/tests/utils/test_bzr_tags.py +++ b/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 - bzr = Popen(('bzr', 'tags', '--directory=' + path), stdout=PIPE) + try: + bzr = Popen(('bzr', 'tags', '--directory=' + path), stdout=PIPE) + except: + 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()] From 097225c9d700059a8d1aac031d30c06dc01920fb Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 20:12:28 +0000 Subject: [PATCH 02/38] Code change for json config file. Titulate themes ftw page --- openlp/core/common/httputils.py | 48 ++- openlp/core/ui/firsttimeform.py | 290 +++++++----------- openlp/core/ui/firsttimewizard.py | 66 +++- .../openlp_core/ui/test_firsttimeform.py | 144 +++++---- .../openlp_core/ui/test_firsttimeform.py | 91 ++++++ 5 files changed, 387 insertions(+), 252 deletions(-) create mode 100644 tests/interfaces/openlp_core/ui/test_firsttimeform.py diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index 5eab07047..741dc23ee 100644 --- a/openlp/core/common/httputils.py +++ b/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 = f'{self._base_url}{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: # noqa + 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 diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 99af2decc..abb1efe97 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/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,58 +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 = f'thumbnail_download_{thumbnail}' + run_thread(worker, thread_name) + ftw.thumbnail_download_threads.append(thread_name) # TODO: Already in the application que - 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 +102,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,18 +139,7 @@ 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() + return self.get_next_page_id() def exec(self): """ @@ -172,104 +156,83 @@ 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: + web_config = Path( + 'C:\\Users\\sroom\\Documents\\Phill Ridout\\play_ground\\openlp\\ftw-json\\download_3.0.json' + ).read_text(encoding='utf-8') # TODO: Remove!!!!! + except ConnectionError as e: 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') + '/' - self.web_access = True - except (NoSectionError, NoOptionError, MissingSectionHeaderError): - log.debug('A problem occurred while parsing the downloaded config file') - trace_error_handler(log) + if web_config and self._parse_config(web_config): + self.web_access = True 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://openlp.org/files/frw/' 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 +245,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 +307,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 +353,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,13 +401,9 @@ 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)) - self.max_progress += size + for item in self.themes_list_widget.selectedItems(): + size = get_url_file_size(f'{self.themes_url}{item.file_name}') + self.max_progress += size except urllib.error.URLError: trace_error_handler(log) critical_error_message_box(translate('OpenLP.FirstTimeWizard', 'Download Error'), @@ -579,15 +520,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) - 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)) + 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, f'{self.themes_url}{item.file_name}', themes_destination_path / item.file_name, item.sha256): + missed_files.append(f'Theme: {item.file_name}') if missed_files: file_list = '' for entry in missed_files: diff --git a/openlp/core/ui/firsttimewizard.py b/openlp/core/ui/firsttimewizard.py index 4b4499236..5fe65d8f1 100644 --- a/openlp/core/ui/firsttimewizard.py +++ b/openlp/core/ui/firsttimewizard.py @@ -49,6 +49,40 @@ 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) + self.setWordWrap(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 +209,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_page) + self.themes_select_all_button = QtWidgets.QToolButton(self.themes_page) + self.themes_select_all_button.setIcon(UiIcons().plus) + 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().minus) + 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 +304,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.')) diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py index a6bdd99a1..1a0299f46 100644 --- a/tests/functional/openlp_core/ui/test_firsttimeform.py +++ b/tests/functional/openlp_core/ui/test_firsttimeform.py @@ -25,40 +25,70 @@ 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 = """ - -This is not a config file -Some text - -""" +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 +122,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 +139,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 +153,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://openlp.org/files/frw/' == 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 +165,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 +182,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) as mocked_check_boxes, \ 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 +200,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://openlp.org/files/frw/' == 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 +210,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 +233,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 +242,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: - first_time_form = FirstTimeForm(None) - first_time_form.initialize(MagicMock()) - mocked_get_web_page.return_value = FAKE_BROKEN_CONFIG + # GIVEN: An instance of `FirstTimeForm` + first_time_form = FirstTimeForm(None) - # 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') diff --git a/tests/interfaces/openlp_core/ui/test_firsttimeform.py b/tests/interfaces/openlp_core/ui/test_firsttimeform.py new file mode 100644 index 000000000..62242e8a2 --- /dev/null +++ b/tests/interfaces/openlp_core/ui/test_firsttimeform.py @@ -0,0 +1,91 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 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 + +# TODO: BZR ADD!!!!!!!!!!!!! + + +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()) + 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()) + 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())]) From 6d7c19256b0992203d26d477d06e30e7217e3e0a Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 20:22:02 +0000 Subject: [PATCH 03/38] Fixes --- openlp/core/ui/firsttimeform.py | 7 ++++--- openlp/core/ui/firsttimewizard.py | 2 +- tests/interfaces/openlp_core/ui/test_firsttimeform.py | 4 +--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index da9e38381..536f9e845 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -88,6 +88,7 @@ class ThemeListWidgetItem(QtWidgets.QListWidgetItem): """ self.setIcon(build_icon(thumbnail_path)) + class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): """ This is the Theme Import Wizard, which allows easy creation and editing of OpenLP themes. @@ -172,9 +173,9 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): try: web_config = get_web_page('{host}{name}'.format(host=self.web, name='download_3.0.json'), headers={'User-Agent': user_agent}) - web_config = Path( - 'C:\\Users\\sroom\\Documents\\Phill Ridout\\play_ground\\openlp\\ftw-json\\download_3.0.json' - ).read_text(encoding='utf-8') # TODO: Remove!!!!! + # web_config = Path( + # 'C:\\Users\\sroom\\Documents\\Phill Ridout\\play_ground\\openlp\\ftw-json\\download_3.0.json' + # ).read_text(encoding='utf-8') # TODO: Remove!!!!! except ConnectionError as e: QtWidgets.QMessageBox.critical(self, translate('OpenLP.FirstTimeWizard', 'Network Error'), translate('OpenLP.FirstTimeWizard', 'There was a network error attempting ' diff --git a/openlp/core/ui/firsttimewizard.py b/openlp/core/ui/firsttimewizard.py index 4424dfca4..8388c915d 100644 --- a/openlp/core/ui/firsttimewizard.py +++ b/openlp/core/ui/firsttimewizard.py @@ -220,7 +220,7 @@ class UiFirstTimeWizard(object): self.theme_combo_box.setEditable(False) 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_page) + self.select_buttons_layout = QtWidgets.QHBoxLayout() self.themes_select_all_button = QtWidgets.QToolButton(self.themes_page) self.themes_select_all_button.setIcon(UiIcons().plus) self.select_buttons_layout.addWidget(self.themes_select_all_button, stretch=1, alignment=QtCore.Qt.AlignRight) diff --git a/tests/interfaces/openlp_core/ui/test_firsttimeform.py b/tests/interfaces/openlp_core/ui/test_firsttimeform.py index 62242e8a2..5aa7e1741 100644 --- a/tests/interfaces/openlp_core/ui/test_firsttimeform.py +++ b/tests/interfaces/openlp_core/ui/test_firsttimeform.py @@ -4,7 +4,7 @@ ############################################################################### # OpenLP - Open Source Lyrics Projection # # --------------------------------------------------------------------------- # -# Copyright (c) 2008-2017 OpenLP Developers # +# 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 # @@ -31,8 +31,6 @@ from openlp.core.ui.firsttimeform import ThemeListWidgetItem from openlp.core.ui.icons import UiIcons from tests.helpers.testmixin import TestMixin -# TODO: BZR ADD!!!!!!!!!!!!! - class TestThemeListWidgetItem(TestCase, TestMixin): def setUp(self): From 7ec703a7d782eb6dca4c9df558d80a06930db0cf Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 15 Feb 2019 21:25:37 +0100 Subject: [PATCH 04/38] try out appveyor changes --- scripts/appveyor.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 3187a7b9f..e441af41b 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -18,7 +18,7 @@ install: # 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 @@ -52,7 +52,8 @@ after_test: # - curl -L -O http://downloads.sourceforge.net/project/portableapps/NSIS%20Portable/NSISPortable_3.0_English.paf.exe # - NSISPortable_3.0_English.paf.exe /S # Get the packaging code - - appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz + #- appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz + - appveyor DownloadFile http://bazaar.launchpad.net/~tomasgroth/openlp/packaging-webengine/tarball -FileName packaging.tar.gz - 7z e packaging.tar.gz - 7z x packaging.tar - mv ~openlp-core/openlp/packaging packaging From 85b40c50240dd1e3b019148f3a14b85ec4b0e244 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 15 Feb 2019 21:33:18 +0100 Subject: [PATCH 05/38] another try --- scripts/appveyor.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index e441af41b..90473ebf7 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -56,7 +56,8 @@ after_test: - appveyor DownloadFile http://bazaar.launchpad.net/~tomasgroth/openlp/packaging-webengine/tarball -FileName packaging.tar.gz - 7z e packaging.tar.gz - 7z x packaging.tar - - mv ~openlp-core/openlp/packaging packaging + #- mv ~openlp-core/openlp/packaging packaging + - mv ~tomasgroth/openlp/packaging-webengine packaging # If this is trunk we should also build the manual - ps: >- If (BUILD_DOCS) { From 9b2ef7d8f7eb20fed78fbac0ef64f2226f56cd8d Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 20:47:09 +0000 Subject: [PATCH 06/38] PEP8 --- openlp/core/ui/firsttimeform.py | 2 +- tests/functional/openlp_core/ui/test_firsttimeform.py | 2 +- tests/interfaces/openlp_core/ui/test_firsttimeform.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 536f9e845..b460f61f4 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -176,7 +176,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): # web_config = Path( # 'C:\\Users\\sroom\\Documents\\Phill Ridout\\play_ground\\openlp\\ftw-json\\download_3.0.json' # ).read_text(encoding='utf-8') # TODO: Remove!!!!! - except ConnectionError as e: + 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'), diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py index df49c6da2..0b41ea31f 100644 --- a/tests/functional/openlp_core/ui/test_firsttimeform.py +++ b/tests/functional/openlp_core/ui/test_firsttimeform.py @@ -184,7 +184,7 @@ class TestFirstTimeForm(TestCase, TestMixin): 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) as mocked_check_boxes, \ + 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, \ diff --git a/tests/interfaces/openlp_core/ui/test_firsttimeform.py b/tests/interfaces/openlp_core/ui/test_firsttimeform.py index 5aa7e1741..77d82c7ff 100644 --- a/tests/interfaces/openlp_core/ui/test_firsttimeform.py +++ b/tests/interfaces/openlp_core/ui/test_firsttimeform.py @@ -59,7 +59,7 @@ class TestThemeListWidgetItem(TestCase, TestMixin): 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()) + 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'] @@ -76,7 +76,7 @@ class TestThemeListWidgetItem(TestCase, TestMixin): signal """ # GIVEN: An instance of `DownloadWorker` - instance = ThemeListWidgetItem('url', self.sample_theme_data, MagicMock()) + 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') From de89167a33a498c5523021c0b426adc4cc7dc8dc Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 20:56:15 +0000 Subject: [PATCH 07/38] url change --- openlp/core/ui/firsttimeform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index b460f61f4..c84c21475 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -233,7 +233,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): Set up display at start of theme edit. """ self.restart() - self.web = 'https://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) From 78263294c94c85029c3f178f21934b665700736d Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Fri, 15 Feb 2019 22:50:52 +0100 Subject: [PATCH 08/38] debug print --- scripts/appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 90473ebf7..91b2edc68 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -70,7 +70,7 @@ after_test: &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update -c windows/config-appveyor.ini -b ../openlp-branch -d ../documentation --portable --tag-override TAG } else { cd packaging - &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable --tag-override TAG + &"$env:PYTHON\python.exe" builders/windows-builder.py -v --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable --tag-override TAG } artifacts: From 5fdebcb7a6ed2dda3389639b58b9bfe00ac135b8 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 22:34:53 +0000 Subject: [PATCH 09/38] change select all icons. fix wordwrap --- openlp/core/ui/firsttimeform.py | 3 ++- openlp/core/ui/firsttimewizard.py | 5 ++--- openlp/core/ui/icons.py | 2 ++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index c84c21475..cd4f874b3 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -69,7 +69,8 @@ class ThemeListWidgetItem(QtWidgets.QListWidgetItem): worker.download_succeeded.connect(self._on_thumbnail_downloaded) thread_name = f'thumbnail_download_{thumbnail}' run_thread(worker, thread_name) - ftw.thumbnail_download_threads.append(thread_name) # TODO: Already in the application que + ftw.thumbnail_download_threads.append(thread_name) + def _on_download_failed(self): """ diff --git a/openlp/core/ui/firsttimewizard.py b/openlp/core/ui/firsttimewizard.py index 8388c915d..37b9389a7 100644 --- a/openlp/core/ui/firsttimewizard.py +++ b/openlp/core/ui/firsttimewizard.py @@ -68,7 +68,6 @@ class ThemeListWidget(QtWidgets.QListWidget): self.setResizeMode(QtWidgets.QListView.Adjust) self.setViewMode(QtWidgets.QListView.IconMode) self.setUniformItemSizes(True) - self.setWordWrap(True) def resizeEvent(self, event): """ @@ -222,10 +221,10 @@ class UiFirstTimeWizard(object): 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().plus) + 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().minus) + 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) diff --git a/openlp/core/ui/icons.py b/openlp/core/ui/icons.py index d62e9fce5..5aa157e9f 100644 --- a/openlp/core/ui/icons.py +++ b/openlp/core/ui/icons.py @@ -138,6 +138,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'}, From d15bf8fc4c0727f13c5cc2844239541157d30c78 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 22:44:13 +0000 Subject: [PATCH 10/38] test fixes --- tests/functional/openlp_core/ui/test_firsttimeform.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py index 0b41ea31f..bd4717084 100644 --- a/tests/functional/openlp_core/ui/test_firsttimeform.py +++ b/tests/functional/openlp_core/ui/test_firsttimeform.py @@ -153,7 +153,7 @@ class TestFirstTimeForm(TestCase, TestMixin): # THEN: The default values should have been set mocked_restart.assert_called_once() - assert 'https://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) @@ -200,7 +200,7 @@ class TestFirstTimeForm(TestCase, TestMixin): # THEN: The default values should have been set mocked_restart.assert_called_once() - assert 'https://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) From 5a32b4065393440d5e748dd0758d1b1dd77cc865 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 15 Feb 2019 22:54:45 +0000 Subject: [PATCH 11/38] pep again! --- openlp/core/ui/firsttimeform.py | 1 - 1 file changed, 1 deletion(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index cd4f874b3..499b83675 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -71,7 +71,6 @@ class ThemeListWidgetItem(QtWidgets.QListWidgetItem): run_thread(worker, thread_name) ftw.thumbnail_download_threads.append(thread_name) - def _on_download_failed(self): """ Set an icon to indicate that the thumbnail download has failed. From bd99cee8fd694a8020951a4e6b6cf22fb3df0517 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Feb 2019 08:57:11 +0000 Subject: [PATCH 12/38] minor changes --- openlp/core/ui/firsttimeform.py | 7 ------- tests/functional/openlp_core/ui/test_firsttimeform.py | 1 - tests/interfaces/openlp_core/ui/test_firsttimeform.py | 1 - 3 files changed, 9 deletions(-) diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index 499b83675..ef223e8f1 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -173,9 +173,6 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): try: web_config = get_web_page('{host}{name}'.format(host=self.web, name='download_3.0.json'), headers={'User-Agent': user_agent}) - # web_config = Path( - # 'C:\\Users\\sroom\\Documents\\Phill Ridout\\play_ground\\openlp\\ftw-json\\download_3.0.json' - # ).read_text(encoding='utf-8') # TODO: Remove!!!!! except ConnectionError: QtWidgets.QMessageBox.critical(self, translate('OpenLP.FirstTimeWizard', 'Network Error'), translate('OpenLP.FirstTimeWizard', 'There was a network error attempting ' @@ -192,18 +189,15 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): 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() 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) - for lang in config['bibles'].values(): self.application.process_events() lang_item = QtWidgets.QTreeWidgetItem(self.bibles_tree_widget, [lang['title']]) @@ -215,7 +209,6 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): item.setFlags(item.flags() | QtCore.Qt.ItemIsUserCheckable) self.bibles_tree_widget.expandAll() self.application.process_events() - for theme in config['themes'].values(): ThemeListWidgetItem(self.themes_url, theme, self, self.themes_list_widget) self.application.process_events() diff --git a/tests/functional/openlp_core/ui/test_firsttimeform.py b/tests/functional/openlp_core/ui/test_firsttimeform.py index bd4717084..220e1fff6 100644 --- a/tests/functional/openlp_core/ui/test_firsttimeform.py +++ b/tests/functional/openlp_core/ui/test_firsttimeform.py @@ -48,7 +48,6 @@ class TestThemeListWidgetItem(TestCase): 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() diff --git a/tests/interfaces/openlp_core/ui/test_firsttimeform.py b/tests/interfaces/openlp_core/ui/test_firsttimeform.py index 77d82c7ff..febef440e 100644 --- a/tests/interfaces/openlp_core/ui/test_firsttimeform.py +++ b/tests/interfaces/openlp_core/ui/test_firsttimeform.py @@ -36,7 +36,6 @@ 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() From 2ab8b8ebec0fe82469dbf30f739ca33d797fb8ae Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sat, 16 Feb 2019 22:24:44 +0100 Subject: [PATCH 13/38] remove link to dev branch --- scripts/appveyor.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 91b2edc68..26c9987d8 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -52,12 +52,10 @@ after_test: # - curl -L -O http://downloads.sourceforge.net/project/portableapps/NSIS%20Portable/NSISPortable_3.0_English.paf.exe # - NSISPortable_3.0_English.paf.exe /S # Get the packaging code - #- appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz - - appveyor DownloadFile http://bazaar.launchpad.net/~tomasgroth/openlp/packaging-webengine/tarball -FileName packaging.tar.gz + - appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz - 7z e packaging.tar.gz - 7z x packaging.tar - #- mv ~openlp-core/openlp/packaging packaging - - mv ~tomasgroth/openlp/packaging-webengine packaging + - mv ~openlp-core/openlp/packaging packaging # If this is trunk we should also build the manual - ps: >- If (BUILD_DOCS) { From 8de23cd980f6c1f6867ca542dd056ae9868e29a6 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sun, 17 Feb 2019 21:23:47 +0100 Subject: [PATCH 14/38] Also install webengine. --- scripts/appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 26c9987d8..c1e7a006d 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -14,7 +14,7 @@ environment: install: # Install dependencies from pypi - - "%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 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 @@ -68,7 +68,7 @@ after_test: &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update -c windows/config-appveyor.ini -b ../openlp-branch -d ../documentation --portable --tag-override TAG } else { cd packaging - &"$env:PYTHON\python.exe" builders/windows-builder.py -v --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable --tag-override TAG + &"$env:PYTHON\python.exe" builders/windows-builder.py --skip-update --skip-translations -c windows/config-appveyor.ini -b ../openlp-branch --portable --tag-override TAG } artifacts: From d7b543ef526f49cb74f8275f571899fc67cc33c1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Sun, 17 Feb 2019 21:38:18 +0100 Subject: [PATCH 15/38] retry the test packaging branch --- scripts/appveyor.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index c1e7a006d..5e8cf172d 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -52,10 +52,12 @@ after_test: # - curl -L -O http://downloads.sourceforge.net/project/portableapps/NSIS%20Portable/NSISPortable_3.0_English.paf.exe # - NSISPortable_3.0_English.paf.exe /S # Get the packaging code - - appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz + #- appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz + - appveyor DownloadFile http://bazaar.launchpad.net/~tomasgroth/openlp/packaging-webengine/tarball -FileName packaging.tar.gz - 7z e packaging.tar.gz - 7z x packaging.tar - - mv ~openlp-core/openlp/packaging packaging + #- mv ~openlp-core/openlp/packaging packaging + - mv ~tomasgroth/openlp/packaging-webengine packaging # If this is trunk we should also build the manual - ps: >- If (BUILD_DOCS) { From 0d6607087953f55be3e6fdfe633740423654b696 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 19 Feb 2019 21:46:31 +0100 Subject: [PATCH 16/38] build both 32 and 64 bit, and add debug print --- openlp/core/display/render.py | 1 + scripts/appveyor.yml | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index 35f144ee8..acd2334c4 100644 --- a/openlp/core/display/render.py +++ b/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:] + print(raw_list) raw_list[0] = raw_tags + raw_list[0] html_list = html_list[index + 1:] html_list[0] = html_tags + html_list[0] diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 5e8cf172d..9410159c8 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -10,7 +10,9 @@ clone_script: - mv BRANCHPATH openlp-branch environment: - PYTHON: C:\\Python37-x64 + matrix: + - PYTHON: C:\\Python37-x64 + - PYTHON: C:\\Python37 install: # Install dependencies from pypi From c241b28039cb2c1c985b1f84ed046cd560e12352 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 19 Feb 2019 22:38:44 +0100 Subject: [PATCH 17/38] put debug in the log --- openlp/core/display/render.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/display/render.py b/openlp/core/display/render.py index acd2334c4..06ce89283 100644 --- a/openlp/core/display/render.py +++ b/openlp/core/display/render.py @@ -706,7 +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:] - print(raw_list) + 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] From 67be698ad2a31becf7b73c05d0b9e802333b34c9 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Thu, 21 Feb 2019 21:29:00 +0000 Subject: [PATCH 18/38] Fixes --- openlp/core/common/httputils.py | 4 ++-- openlp/core/ui/firsttimeform.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/openlp/core/common/httputils.py b/openlp/core/common/httputils.py index c6e0f2f58..c9a555a55 100644 --- a/openlp/core/common/httputils.py +++ b/openlp/core/common/httputils.py @@ -256,13 +256,13 @@ class DownloadWorker(ThreadWorker): return try: dest_path = Path(gettempdir()) / 'openlp' / self._file_name - url = f'{self._base_url}{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: # noqa + except Exception: log.exception('Unable to download %s', url) self.download_failed.emit() finally: diff --git a/openlp/core/ui/firsttimeform.py b/openlp/core/ui/firsttimeform.py index ef223e8f1..d5f48e2d7 100644 --- a/openlp/core/ui/firsttimeform.py +++ b/openlp/core/ui/firsttimeform.py @@ -67,7 +67,7 @@ class ThemeListWidgetItem(QtWidgets.QListWidgetItem): worker = DownloadWorker(themes_url, thumbnail) worker.download_failed.connect(self._on_download_failed) worker.download_succeeded.connect(self._on_thumbnail_downloaded) - thread_name = f'thumbnail_download_{thumbnail}' + thread_name = 'thumbnail_download_{thumbnail}'.format(thumbnail=thumbnail) run_thread(worker, thread_name) ftw.thumbnail_download_threads.append(thread_name) @@ -396,7 +396,7 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): iterator += 1 # Loop through the themes list and increase for each selected item for item in self.themes_list_widget.selectedItems(): - size = get_url_file_size(f'{self.themes_url}{item.file_name}') + 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) @@ -517,9 +517,9 @@ class FirstTimeForm(QtWidgets.QWizard, UiFirstTimeWizard, RegistryProperties): 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, f'{self.themes_url}{item.file_name}', themes_destination_path / item.file_name, item.sha256): - missed_files.append(f'Theme: {item.file_name}') + 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: From fb02d06a093230c9b5e30d0bea7abfaaf377abe1 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Wed, 27 Feb 2019 21:17:00 +0100 Subject: [PATCH 19/38] make the path of display web file work on frozen apps --- openlp/core/display/window.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index d811de5b2..c0ef2a1ef 100644 --- a/openlp/core/display/window.py +++ b/openlp/core/display/window.py @@ -32,13 +32,11 @@ from PyQt5 import QtCore, QtWebChannel, QtWidgets from openlp.core.common.path import Path, 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() From c7615920958f688c73ba3b2b668165c99ce694b5 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 3 Mar 2019 09:49:01 +0000 Subject: [PATCH 20/38] Catch some OSErrors to provide user friendly error messages. Few other minor fixes Fixes: https://launchpad.net/bugs/1650910 --- openlp/core/app.py | 7 +++++- openlp/core/common/__init__.py | 6 ++--- openlp/core/lib/__init__.py | 8 +++---- openlp/core/ui/exceptiondialog.py | 6 ++--- openlp/core/ui/exceptionform.py | 21 ++++++++++++------ openlp/core/ui/servicemanager.py | 6 +++-- openlp/core/ui/thememanager.py | 14 ++++++------ openlp/core/version.py | 2 +- openlp/plugins/songs/forms/songimportform.py | 9 ++++++-- .../songusage/forms/songusagedetailform.py | 4 ++-- run_openlp.py | 10 +++++++-- .../openlp_core/common/test_init.py | 22 ++++++++++++------- 12 files changed, 73 insertions(+), 42 deletions(-) diff --git a/openlp/core/app.py b/openlp/core/app.py index 7dafaaf3c..8b274e6b7 100644 --- a/openlp/core/app.py +++ b/openlp/core/app.py @@ -405,7 +405,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)) - settings.export(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'): diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index 6e9274c10..9796a0f7e 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -34,7 +34,7 @@ from ipaddress import IPv4Address, IPv6Address, AddressValueError from shutil import which from subprocess import check_output, CalledProcessError, STDOUT -from PyQt5 import QtGui +from PyQt5 import QtGui, QtWidgets from PyQt5.QtCore import QCryptographicHash as QHash from PyQt5.QtNetwork import QAbstractSocket, QHostAddress, QNetworkInterface from chardet.universaldetector import UniversalDetector @@ -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): diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 0de575d66..9e084fdd3 100644 --- a/openlp/core/lib/__init__.py +++ b/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 diff --git a/openlp/core/ui/exceptiondialog.py b/openlp/core/ui/exceptiondialog.py index 966166a4a..c5f1c19bf 100644 --- a/openlp/core/ui/exceptiondialog.py +++ b/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')) diff --git a/openlp/core/ui/exceptionform.py b/openlp/core/ui/exceptionform.py index 9510b8f07..e6089f7b3 100644 --- a/openlp/core/ui/exceptionform.py +++ b/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. """ - 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: + 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 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): """ diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index cff91d431..ac179663e 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -705,11 +705,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) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 0c8ab094a..188688c3f 100644 --- a/openlp/core/ui/thememanager.py +++ b/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): """ @@ -570,7 +570,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R with zipfile.ZipFile(str(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. diff --git a/openlp/core/version.py b/openlp/core/version.py index e2b77ee07..198b42df2 100644 --- a/openlp/core/version.py +++ b/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 diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 2662529c9..6931c3457 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/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'])) - importer.do_import() - self.progress_label.setText(WizardStrings.FinishedImport) + 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): """ diff --git a/openlp/plugins/songusage/forms/songusagedetailform.py b/openlp/plugins/songusage/forms/songusagedetailform.py index aa8b7c02a..f43ca3b9a 100644 --- a/openlp/plugins/songusage/forms/songusagedetailform.py +++ b/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') diff --git a/run_openlp.py b/run_openlp.py index a754b4c78..c48c19d53 100755 --- a/run_openlp.py +++ b/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 - create_paths(AppLocation.get_directory(AppLocation.CacheDir)) - faulthandler.enable((AppLocation.get_directory(AppLocation.CacheDir) / 'error.log').open('wb')) + 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(): diff --git a/tests/functional/openlp_core/common/test_init.py b/tests/functional/openlp_core/common/test_init.py index 0c8310f4d..99ef55b01 100644 --- a/tests/functional/openlp_core/common/test_init.py +++ b/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 From 07ffb7c0d0ea8604b78aabe6057f0802e11c6aca Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 3 Mar 2019 13:32:31 +0000 Subject: [PATCH 21/38] Fix choruses, bridges & etc. being imported as verses in CCLI txt files --- openlp/core/common/settings.py | 2 +- openlp/plugins/songs/lib/importers/cclifile.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/openlp/core/common/settings.py b/openlp/core/common/settings.py index dc123196b..1539a059c 100644 --- a/openlp/core/common/settings.py +++ b/openlp/core/common/settings.py @@ -540,7 +540,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. diff --git a/openlp/plugins/songs/lib/importers/cclifile.py b/openlp/plugins/songs/lib/importers/cclifile.py index 39cf7abe2..facce65c9 100644 --- a/openlp/plugins/songs/lib/importers/cclifile.py +++ b/openlp/plugins/songs/lib/importers/cclifile.py @@ -254,7 +254,6 @@ class CCLIFileImport(SongImport): 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: @@ -279,7 +278,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'): From 1fe56bc089d5707e633e599b10843271ffb56a65 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 3 Mar 2019 17:44:40 +0000 Subject: [PATCH 22/38] Fix some asserts --- .../songs/forms/test_songmaintenanceform.py | 4 ++-- .../projectors/test_projector_pjlink_commands_01.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py b/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py index b5e6ee49e..e7985fe51 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py +++ b/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') diff --git a/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py b/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py index a03e84db5..eb85ac1f3 100644 --- a/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py +++ b/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py @@ -601,9 +601,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 +623,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): """ From fd502f80dc3e8a2a77694c91e9d00b8d3845f662 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Mon, 4 Mar 2019 21:32:05 +0100 Subject: [PATCH 23/38] Use the normal packaging repo --- scripts/appveyor.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/scripts/appveyor.yml b/scripts/appveyor.yml index 9410159c8..fd9ffac73 100644 --- a/scripts/appveyor.yml +++ b/scripts/appveyor.yml @@ -54,12 +54,10 @@ after_test: # - curl -L -O http://downloads.sourceforge.net/project/portableapps/NSIS%20Portable/NSISPortable_3.0_English.paf.exe # - NSISPortable_3.0_English.paf.exe /S # Get the packaging code - #- appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz - - appveyor DownloadFile http://bazaar.launchpad.net/~tomasgroth/openlp/packaging-webengine/tarball -FileName packaging.tar.gz + - appveyor DownloadFile http://bazaar.launchpad.net/~openlp-core/openlp/packaging/tarball -FileName packaging.tar.gz - 7z e packaging.tar.gz - 7z x packaging.tar - #- mv ~openlp-core/openlp/packaging packaging - - mv ~tomasgroth/openlp/packaging-webengine packaging + - mv ~openlp-core/openlp/packaging packaging # If this is trunk we should also build the manual - ps: >- If (BUILD_DOCS) { From 2a6378f489b03fa58640babfbc9292812b1b226e Mon Sep 17 00:00:00 2001 From: Bob Luursema Date: Mon, 4 Mar 2019 21:37:11 +0100 Subject: [PATCH 24/38] Almost fixed bug 1800761 --- .bzrignore | 1 + openlp/plugins/songs/forms/songselectform.py | 5 ++-- openlp/plugins/songs/lib/songselect.py | 21 ++++++++++---- .../openlp_plugins/songs/test_songselect.py | 29 +++++++++++++------ 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/.bzrignore b/.bzrignore index f60d6cfff..2b43ce13a 100644 --- a/.bzrignore +++ b/.bzrignore @@ -7,6 +7,7 @@ cover .coverage coverage .directory +.vscode dist *.dll documentation/build/doctrees diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py index d8b4db300..423f1f17c 100644 --- a/openlp/plugins/songs/forms/songselectform.py +++ b/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'), diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index bde9b7bf9..01b9a635b 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/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,20 @@ class SongSelectImport(object): return False if callback: callback() - if posted_page.find('input', id='SearchText') is not None: - return True + if posted_page.find('input', id='SearchText') is not None or 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)) + subscription_element = home_page.find('div', {'class': 'subscriptionlevel'}) + if subscription_element is not None: + self.subscription_level = subscription_element.string.strip() + else: + self.subscription_level = 'premium' + return self.subscription_level else: log.debug(posted_page) - return False + return None def logout(self): """ @@ -128,7 +137,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 +154,7 @@ class SongSelectImport(object): 'PrimaryLanguage': '', 'Keys': '', 'Themes': '', - 'List': '', + 'List': '' if self.subscription_level == 'premium' else 'publicdomain', 'Sort': '', 'SearchText': search_text } diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 1351faf51..4ef122c40 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/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): @@ -117,7 +117,11 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_login_page.find.side_effect = [{'value': 'blah'}, None] mocked_posted_page = MagicMock() mocked_posted_page.find.return_value = MagicMock() - MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page] + mocked_home_page = MagicMock() + mocked_return = MagicMock() + mocked_return.string = 'premium' + mocked_home_page.find.return_value = mocked_return + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -128,8 +132,8 @@ class TestSongSelectImport(TestCase, TestMixin): 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 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 3 == mocked_opener.open.call_count, 'opener should have been called 3 times' + assert result is 'premium', '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 +150,11 @@ 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() + mocked_return = MagicMock() + mocked_return.string = 'premium' + mocked_home_page.find.return_value = mocked_return + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -158,7 +166,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 'premium', '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 +199,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 +240,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 +292,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) From c68d6dc3de3c7165e43b8b666d179f64e18a0b30 Mon Sep 17 00:00:00 2001 From: Bob Luursema Date: Tue, 5 Mar 2019 21:55:37 +0100 Subject: [PATCH 25/38] Fixed bug 1800761 --- openlp/plugins/songs/forms/songselectform.py | 7 ++++++ openlp/plugins/songs/lib/songselect.py | 23 ++++++++++++++----- .../openlp_plugins/songs/test_songselect.py | 15 ++++-------- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py index 423f1f17c..46d27e2af 100644 --- a/openlp/plugins/songs/forms/songselectform.py +++ b/openlp/plugins/songs/forms/songselectform.py @@ -273,6 +273,13 @@ 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()) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 01b9a635b..5d5307cd9 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -115,20 +115,31 @@ class SongSelectImport(object): return False if callback: callback() - if posted_page.find('input', id='SearchText') is not None or posted_page.find('div', id="select-organization") is not None: + # Page if user is in an organization + if posted_page.find('input', id='SearchText') is not None: + 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)) - subscription_element = home_page.find('div', {'class': 'subscriptionlevel'}) - if subscription_element is not None: - self.subscription_level = subscription_element.string.strip() - else: - self.subscription_level = 'premium' + self.subscription_level = self.find_subscription_level(home_page) return self.subscription_level else: log.debug(posted_page) 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[^']+)", tag.string) + if match: + return match.group('subscription_level') + log.error('Could not determine SongSelect subscription level') + return 'unkown' def logout(self): """ diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 4ef122c40..d12d84e46 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -117,11 +117,7 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_login_page.find.side_effect = [{'value': 'blah'}, None] mocked_posted_page = MagicMock() mocked_posted_page.find.return_value = MagicMock() - mocked_home_page = MagicMock() - mocked_return = MagicMock() - mocked_return.string = 'premium' - mocked_home_page.find.return_value = mocked_return - MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -132,8 +128,8 @@ class TestSongSelectImport(TestCase, TestMixin): 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 on the login page' assert 1 == mocked_posted_page.find.call_count, 'find should have been called once on the posted page' - assert 3 == mocked_opener.open.call_count, 'opener should have been called 3 times' - assert result is 'premium', 'The login method should have returned the subscription level' + assert 2 == mocked_opener.open.call_count, 'opener should have been called twice' + assert result is 'unkown', 'The login method should have returned the subscription level' @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') @@ -151,9 +147,6 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_posted_page = MagicMock() mocked_posted_page.find.return_value = MagicMock() mocked_home_page = MagicMock() - mocked_return = MagicMock() - mocked_return.string = 'premium' - mocked_home_page.find.return_value = mocked_return MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -166,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 'premium', 'The login method should have returned the subscription level' + assert result is 'unkown', 'The login method should have returned the subscription level' @patch('openlp.plugins.songs.lib.songselect.build_opener') def test_logout(self, mocked_build_opener): From 40ceb07f18aa56a5d37e8495031aad2bcc98de01 Mon Sep 17 00:00:00 2001 From: Bob Luursema Date: Tue, 5 Mar 2019 22:02:49 +0100 Subject: [PATCH 26/38] Hotfix --- openlp/plugins/songs/lib/songselect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 5d5307cd9..7aa1519a2 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -165,7 +165,7 @@ class SongSelectImport(object): 'PrimaryLanguage': '', 'Keys': '', 'Themes': '', - 'List': '' if self.subscription_level == 'premium' else 'publicdomain', + 'List': 'publicdomain' if self.subscription_level == 'Free' else '', 'Sort': '', 'SearchText': search_text } From 490f9bbe15b4f6d8c291449b5bfc1ff38d04418f Mon Sep 17 00:00:00 2001 From: Bob Luursema Date: Tue, 5 Mar 2019 22:21:12 +0100 Subject: [PATCH 27/38] Fix linting --- openlp/plugins/songs/forms/songselectform.py | 11 ++++++----- openlp/plugins/songs/lib/songselect.py | 4 ++-- .../openlp_plugins/songs/test_songselect.py | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py index 46d27e2af..68e393a82 100644 --- a/openlp/plugins/songs/forms/songselectform.py +++ b/openlp/plugins/songs/forms/songselectform.py @@ -264,7 +264,7 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties) self.application.process_events() # Log the user in subscription_level = self.song_select_importer.login( - self.username_edit.text(), self.password_edit.text(), self._update_login_progress) + self.username_edit.text(), self.password_edit.text(), self._update_login_progress) if not subscription_level: QtWidgets.QMessageBox.critical( self, @@ -275,10 +275,11 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties) 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.') + 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()) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 7aa1519a2..71e093cb1 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -130,7 +130,7 @@ class SongSelectImport(object): else: log.debug(posted_page) return None - + def find_subscription_level(self, page): scripts = page.find_all('script') for tag in scripts: @@ -139,7 +139,7 @@ class SongSelectImport(object): if match: return match.group('subscription_level') log.error('Could not determine SongSelect subscription level') - return 'unkown' + return None def logout(self): """ diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index d12d84e46..1affd66c2 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -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 'unkown', 'The login method should have returned the subscription level' + 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') @@ -159,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 'unkown', 'The login method should have returned the subscription level' + 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): From b6a36f2324db7ae07a95ff52d6e5b60a50e94840 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Thu, 7 Mar 2019 20:23:04 +0100 Subject: [PATCH 28/38] pep8 fixes --- openlp/core/display/window.py | 2 +- tests/utils/test_bzr_tags.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/core/display/window.py b/openlp/core/display/window.py index c0ef2a1ef..35442b5ed 100644 --- a/openlp/core/display/window.py +++ b/openlp/core/display/window.py @@ -29,7 +29,7 @@ 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 diff --git a/tests/utils/test_bzr_tags.py b/tests/utils/test_bzr_tags.py index cd057ada4..66b159c96 100644 --- a/tests/utils/test_bzr_tags.py +++ b/tests/utils/test_bzr_tags.py @@ -44,7 +44,7 @@ class TestBzrTags(TestCase): # WHEN getting the branches tags try: bzr = Popen(('bzr', 'tags', '--directory=' + path), stdout=PIPE) - except: + except Exception: raise SkipTest('bzr is not installed') std_out = bzr.communicate()[0] count = len(TAGS1) From cc4b8e42421d3569b123d317d4ee4899ffda0d77 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 8 Mar 2019 07:19:57 -0800 Subject: [PATCH 29/38] PJLink2 Update U --- openlp/core/projectors/db.py | 8 +++- openlp/core/projectors/manager.py | 41 ++++++------------- openlp/core/projectors/pjlink.py | 19 +++++++-- openlp/core/ui/icons.py | 12 ++++-- setup.cfg | 1 + .../openlp_core/lib/test_image_manager.py | 4 +- .../openlp_plugins/media/test_mediaplugin.py | 2 +- .../openlp_plugins/songs/test_lib.py | 2 +- .../songusage/test_songusage.py | 4 +- .../projectors/test_projector_db.py | 7 ++-- .../test_projector_pjlink_commands_01.py | 6 ++- 11 files changed, 59 insertions(+), 47 deletions(-) diff --git a/openlp/core/projectors/db.py b/openlp/core/projectors/db.py index ef233999b..9249f0b60 100644 --- a/openlp/core/projectors/db.py +++ b/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: diff --git a/openlp/core/projectors/manager.py b/openlp/core/projectors/manager.py index 966da08f8..9445c3f70 100644 --- a/openlp/core/projectors/manager.py +++ b/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, @@ -505,7 +484,8 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM projector = list_item.data(QtCore.Qt.UserRole) try: projector.link.connect_to_host() - except Exception: + except Exception as e: + print(e) continue def on_delete_projector(self, opt=None): @@ -880,6 +860,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 +881,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 +892,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 +915,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) diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index 02d6c444b..524bc8c3c 100644 --- a/openlp/core/projectors/pjlink.py +++ b/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, - data=self.source_available)) + 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', diff --git a/openlp/core/ui/icons.py b/openlp/core/ui/icons.py index 5aa157e9f..0e83a01d8 100644 --- a/openlp/core/ui/icons.py +++ b/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'}, diff --git a/setup.cfg b/setup.cfg index 71cd36460..5b443dcf7 100644 --- a/setup.cfg +++ b/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] diff --git a/tests/functional/openlp_core/lib/test_image_manager.py b/tests/functional/openlp_core/lib/test_image_manager.py index 33207f69b..9cb77ece9 100644 --- a/tests/functional/openlp_core/lib/test_image_manager.py +++ b/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') diff --git a/tests/functional/openlp_plugins/media/test_mediaplugin.py b/tests/functional/openlp_plugins/media/test_mediaplugin.py index eabf35aa0..d5bfcac0b 100644 --- a/tests/functional/openlp_plugins/media/test_mediaplugin.py +++ b/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 diff --git a/tests/functional/openlp_plugins/songs/test_lib.py b/tests/functional/openlp_plugins/songs/test_lib.py index eedbe85c7..e9c657df3 100644 --- a/tests/functional/openlp_plugins/songs/test_lib.py +++ b/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.' diff --git a/tests/functional/openlp_plugins/songusage/test_songusage.py b/tests/functional/openlp_plugins/songusage/test_songusage.py index 03847a601..e80c31c03 100644 --- a/tests/functional/openlp_plugins/songusage/test_songusage.py +++ b/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): diff --git a/tests/openlp_core/projectors/test_projector_db.py b/tests/openlp_core/projectors/test_projector_db.py index ae25e011c..bc5899aeb 100644 --- a/tests/openlp_core/projectors/test_projector_db.py +++ b/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() diff --git a/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py b/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py index a03e84db5..361b5c4bb 100644 --- a/tests/openlp_core/projectors/test_projector_pjlink_commands_01.py +++ b/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'] From eb8864f5696fb5d3e24940bce71f5fda92a42ef2 Mon Sep 17 00:00:00 2001 From: Ken Roberts Date: Fri, 8 Mar 2019 19:53:20 -0800 Subject: [PATCH 30/38] Minor cleanups --- openlp/core/projectors/manager.py | 3 +-- openlp/core/projectors/pjlink.py | 4 ++-- openlp/core/ui/advancedtab.py | 4 ++-- openlp/core/ui/themestab.py | 2 +- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/openlp/core/projectors/manager.py b/openlp/core/projectors/manager.py index 9445c3f70..c8c01401e 100644 --- a/openlp/core/projectors/manager.py +++ b/openlp/core/projectors/manager.py @@ -484,8 +484,7 @@ class ProjectorManager(QtWidgets.QWidget, RegistryBase, UiProjectorManager, LogM projector = list_item.data(QtCore.Qt.UserRole) try: projector.link.connect_to_host() - except Exception as e: - print(e) + except Exception: continue def on_delete_projector(self, opt=None): diff --git a/openlp/core/projectors/pjlink.py b/openlp/core/projectors/pjlink.py index 524bc8c3c..24f1b04ed 100644 --- a/openlp/core/projectors/pjlink.py +++ b/openlp/core/projectors/pjlink.py @@ -912,11 +912,11 @@ class PJLink(QtNetwork.QTcpSocket, PJLinkCommands): # 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) + 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) + code, message = self._get_status(status) if msg is not None: message = msg elif message is None: diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index 3627e8f45..33909bdc2 100644 --- a/openlp/core/ui/advancedtab.py +++ b/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) @@ -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): diff --git a/openlp/core/ui/themestab.py b/openlp/core/ui/themestab.py index d1998ccca..ee87b79d0 100644 --- a/openlp/core/ui/themestab.py +++ b/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): From 50598e9058ccb5a411519b2b7b9fe41dbf769874 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 10 Mar 2019 21:01:39 +0000 Subject: [PATCH 31/38] pathlib cleanups --- openlp/core/api/deploy.py | 4 ++-- openlp/core/app.py | 5 ++--- openlp/core/common/path.py | 2 +- openlp/core/lib/db.py | 4 ++-- openlp/core/lib/serviceitem.py | 6 +++--- openlp/core/ui/advancedtab.py | 2 +- openlp/core/ui/mainwindow.py | 2 +- openlp/core/ui/media/__init__.py | 2 +- openlp/core/ui/media/vlcplayer.py | 2 +- openlp/core/ui/servicemanager.py | 18 +++++++++--------- openlp/core/ui/thememanager.py | 6 +++--- openlp/core/widgets/edits.py | 2 +- openlp/core/widgets/wizard.py | 4 ++-- openlp/plugins/bibles/lib/bibleimport.py | 4 ++-- .../bibles/lib/importers/wordproject.py | 2 +- openlp/plugins/media/lib/mediaitem.py | 6 +++--- openlp/plugins/songs/lib/importers/cclifile.py | 2 +- .../functional/openlp_core/api/test_deploy.py | 4 ++-- .../functional/openlp_core/common/test_path.py | 8 +++----- .../openlp_core/lib/test_serviceitem.py | 4 ++-- .../openlp_core/ui/test_thememanager.py | 6 +++--- .../common/test_network_interfaces.py | 2 +- .../projectors/test_projector_sourceform.py | 4 ++-- 23 files changed, 49 insertions(+), 52 deletions(-) diff --git a/openlp/core/api/deploy.py b/openlp/core/api/deploy.py index a9889df9c..fe77cc61d 100644 --- a/openlp/core/api/deploy.py +++ b/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(): diff --git a/openlp/core/app.py b/openlp/core/app.py index 8b274e6b7..e0438b8f3 100644 --- a/openlp/core/app.py +++ b/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)) diff --git a/openlp/core/common/path.py b/openlp/core/common/path.py index 7f3c156fc..79adbf5b6 100644 --- a/openlp/core/common/path.py +++ b/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): diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index 2e587b8e9..bbdfb5907 100644 --- a/openlp/core/lib/db.py +++ b/openlp/core/lib/db.py @@ -144,7 +144,7 @@ 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_name): # TODO: To pathlib """ Log and report to the user that a database cannot be loaded @@ -159,7 +159,7 @@ def handle_db_error(plugin_name, db_file_name): 'OpenLP cannot load your database.\n\nDatabase: {db}').format(db=db_path)) -def init_url(plugin_name, db_file_name=None): +def init_url(plugin_name, db_file_name=None): # TODO: Pathlib """ Return the database URL. diff --git a/openlp/core/lib/serviceitem.py b/openlp/core/lib/serviceitem.py index 6b8bc05e6..2d43697dc 100644 --- a/openlp/core/lib/serviceitem.py +++ b/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']: diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index 3627e8f45..4957b86d0 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -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.') diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index dabfca79e..461ba82f9 100644 --- a/openlp/core/ui/mainwindow.py +++ b/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() diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 4e38cc122..5f3e4a072 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -54,7 +54,7 @@ class ItemMediaInfo(object): """ This class hold the media related info """ - file_info = None + file_info = None # TODO: Ptahlib? volume = 100 is_background = False can_loop_playback = False diff --git a/openlp/core/ui/media/vlcplayer.py b/openlp/core/ui/media/vlcplayer.py index 0cadc8837..19ce7863c 100644 --- a/openlp/core/ui/media/vlcplayer.py +++ b/openlp/core/ui/media/vlcplayer.py @@ -197,7 +197,7 @@ class VlcPlayer(MediaPlayer): """ return get_vlc() is not None - def load(self, display, file): + def load(self, display, file): # TODO: pathlib """ Load a video into VLC diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index ac179663e..2ce4b5d19 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -234,7 +234,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 +590,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 +679,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 +692,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) @@ -1239,11 +1239,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): """ diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 188688c3f..4ae79ee6b 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -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,7 +567,7 @@ 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 once the upgrade path from 2.4 is no longer required diff --git a/openlp/core/widgets/edits.py b/openlp/core/widgets/edits.py index a90aa2cd0..b80c61ee6 100644 --- a/openlp/core/widgets/edits.py +++ b/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) diff --git a/openlp/core/widgets/wizard.py b/openlp/core/widgets/wizard.py index cf37e5bbf..284caba96 100644 --- a/openlp/core/widgets/wizard.py +++ b/openlp/core/widgets/wizard.py @@ -300,7 +300,7 @@ class OpenLPWizard(QtWidgets.QWizard, RegistryProperties): 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)) + editbox.setText(str(file_path)) # TODO: to pathdedit Settings().setValue(self.plugin.settings_section + '/' + setting_name, file_path.parent) def get_folder(self, title, editbox, setting_name): @@ -316,5 +316,5 @@ class OpenLPWizard(QtWidgets.QWizard, RegistryProperties): self, title, Settings().value(self.plugin.settings_section + '/' + setting_name), FileDialog.ShowDirsOnly) if folder_path: - editbox.setText(str(folder_path)) + editbox.setText(str(folder_path)) # TODO: to pathedit Settings().setValue(self.plugin.settings_section + '/' + setting_name, folder_path) diff --git a/openlp/plugins/bibles/lib/bibleimport.py b/openlp/plugins/bibles/lib/bibleimport.py index 66f06c4ab..7c62236c6 100644 --- a/openlp/plugins/bibles/lib/bibleimport.py +++ b/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.' diff --git a/openlp/plugins/bibles/lib/importers/wordproject.py b/openlp/plugins/bibles/lib/importers/wordproject.py index 248080f98..22ab5e165 100644 --- a/openlp/plugins/bibles/lib/importers/wordproject.py +++ b/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) diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index 72a0e5c50..dad964dd5 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -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 diff --git a/openlp/plugins/songs/lib/importers/cclifile.py b/openlp/plugins/songs/lib/importers/cclifile.py index facce65c9..d307c6ca7 100644 --- a/openlp/plugins/songs/lib/importers/cclifile.py +++ b/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) diff --git a/tests/functional/openlp_core/api/test_deploy.py b/tests/functional/openlp_core/api/test_deploy.py index 784605034..439dc2b8c 100644 --- a/tests/functional/openlp_core/api/test_deploy.py +++ b/tests/functional/openlp_core/api/test_deploy.py @@ -63,8 +63,8 @@ class TestRemoteDeploy(TestCase): 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') diff --git a/tests/functional/openlp_core/common/test_path.py b/tests/functional/openlp_core/common/test_path.py index 7c0a34635..d6668ea67 100644 --- a/tests/functional/openlp_core/common/test_path.py +++ b/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): """ diff --git a/tests/functional/openlp_core/lib/test_serviceitem.py b/tests/functional/openlp_core/lib/test_serviceitem.py index 98436de7e..d2152bdec 100644 --- a/tests/functional/openlp_core/lib/test_serviceitem.py +++ b/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,7 +154,7 @@ 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_PATHb) # THEN: We should get back a valid service item assert service_item.is_valid is True, 'The new service item should be valid' diff --git a/tests/functional/openlp_core/ui/test_thememanager.py b/tests/functional/openlp_core/ui/test_thememanager.py index 3318653a0..1c0d5aa25 100644 --- a/tests/functional/openlp_core/ui/test_thememanager.py +++ b/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): """ diff --git a/tests/openlp_core/common/test_network_interfaces.py b/tests/openlp_core/common/test_network_interfaces.py index 26fe15af0..dc1482c60 100644 --- a/tests/openlp_core/common/test_network_interfaces.py +++ b/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 diff --git a/tests/openlp_core/projectors/test_projector_sourceform.py b/tests/openlp_core/projectors/test_projector_sourceform.py index 08748f106..a738f701f 100644 --- a/tests/openlp_core/projectors/test_projector_sourceform.py +++ b/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: From 296adb59a08a5064e9ba51c64ea8a9264bc25811 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Mar 2019 10:20:46 +0000 Subject: [PATCH 32/38] Bible import issues --- openlp/core/ui/generaltab.py | 1 - openlp/plugins/bibles/forms/bibleimportform.py | 4 ++-- openlp/plugins/bibles/lib/db.py | 5 +++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openlp/core/ui/generaltab.py b/openlp/core/ui/generaltab.py index 0095f64d5..2a81820af 100644 --- a/openlp/core/ui/generaltab.py +++ b/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' diff --git a/openlp/plugins/bibles/forms/bibleimportform.py b/openlp/plugins/bibles/forms/bibleimportform.py index a53e73558..4804863d6 100644 --- a/openlp/plugins/bibles/forms/bibleimportform.py +++ b/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'): diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index 545bf27a8..6df2efab8 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -750,7 +750,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 +765,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, ' From d8644648a496b9b305d94f00f64c027f94c8b389 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Mar 2019 10:26:05 +0000 Subject: [PATCH 33/38] Fit the logo to the Main Display Fixes: https://launchpad.net/bugs/1819763 --- openlp/core/display/html/display.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/display/html/display.js b/openlp/core/display/html/display.js index 9c6eda634..02980a393 100644 --- a/openlp/core/display/html/display.js +++ b/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; From cae0c2eb099b7b10df63dcee31b6cd6667809d8d Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Mar 2019 10:58:59 +0000 Subject: [PATCH 34/38] Revert commented code --- openlp/plugins/songs/songsplugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/songsplugin.py b/openlp/plugins/songs/songsplugin.py index 963da89c7..d5c52edf0 100644 --- a/openlp/plugins/songs/songsplugin.py +++ b/openlp/plugins/songs/songsplugin.py @@ -121,7 +121,7 @@ class SongsPlugin(Plugin): self.song_export_item.setVisible(True) self.song_tools_menu.menuAction().setVisible(True) action_list = ActionList.get_instance() - #action_list.add_action(self.song_import_item, UiStrings().Import) + action_list.add_action(self.song_import_item, UiStrings().Import) action_list.add_action(self.song_export_item, UiStrings().Export) action_list.add_action(self.tools_reindex_item, UiStrings().Tools) action_list.add_action(self.tools_find_duplicates, UiStrings().Tools) From 7c284ad520afc83b244e81dc8236f29bb5c85aa0 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 16 Mar 2019 21:07:44 +0000 Subject: [PATCH 35/38] Fix deleting bibles Fixes: https://launchpad.net/bugs/1748719 --- openlp/plugins/bibles/lib/db.py | 6 +++--- openlp/plugins/bibles/lib/manager.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index 6df2efab8..bccff3559 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -159,10 +159,10 @@ class BibleDB(Manager): 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 diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index e3423ccf1..d638512b2 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/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): """ From eb115a0ad44bbd7929322c3972caab550aec39d0 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 17 Mar 2019 10:01:52 +0000 Subject: [PATCH 36/38] More pathlib clean ups --- openlp/core/lib/__init__.py | 6 +-- openlp/core/lib/db.py | 28 ++++++++------ openlp/core/lib/mediamanageritem.py | 8 ++-- openlp/core/ui/media/__init__.py | 2 +- openlp/core/widgets/wizard.py | 38 ------------------- openlp/plugins/bibles/lib/db.py | 1 - openlp/plugins/bibles/lib/mediaitem.py | 6 +-- openlp/plugins/custom/lib/mediaitem.py | 7 +--- openlp/plugins/images/lib/mediaitem.py | 6 +-- openlp/plugins/media/lib/mediaitem.py | 6 +-- openlp/plugins/presentations/lib/mediaitem.py | 6 +-- .../presentations/lib/messagelistener.py | 10 +---- openlp/plugins/songs/lib/mediaitem.py | 6 +-- .../openlp_plugins/bibles/test_manager.py | 5 ++- 14 files changed, 45 insertions(+), 90 deletions(-) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 9e084fdd3..990b45adb 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -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 diff --git a/openlp/core/lib/db.py b/openlp/core/lib/db.py index bbdfb5907..fd459f4ef 100644 --- a/openlp/core/lib/db.py +++ b/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,27 +145,30 @@ 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): # TODO: To pathlib +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', 'OpenLP cannot load your database.\n\nDatabase: {db}').format(db=db_path)) -def init_url(plugin_name, db_file_name=None): # TODO: Pathlib +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) @@ -345,7 +349,7 @@ class Manager(object): Runs the initialisation process that includes creating the connection to the database and the tables if they do not exist. - :param plugin_name: The name to setup paths and settings section names + :param plugin_name: The name to setup paths and settings section names :param init_schema: The init_schema function for this database :param openlp.core.common.path.Path db_file_path: The file name to use for this database. Defaults to None resulting in the plugin_name being used. @@ -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 diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index b84b7a24f..35adc0bfc 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -454,15 +454,17 @@ 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, xml_version=False, 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') @@ -634,7 +636,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ 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, xml_version=xml_version, remote=remote, context=context): return service_item else: return None diff --git a/openlp/core/ui/media/__init__.py b/openlp/core/ui/media/__init__.py index 5f3e4a072..4e38cc122 100644 --- a/openlp/core/ui/media/__init__.py +++ b/openlp/core/ui/media/__init__.py @@ -54,7 +54,7 @@ class ItemMediaInfo(object): """ This class hold the media related info """ - file_info = None # TODO: Ptahlib? + file_info = None volume = 100 is_background = False can_loop_playback = False diff --git a/openlp/core/widgets/wizard.py b/openlp/core/widgets/wizard.py index 284caba96..5619d5a3e 100644 --- a/openlp/core/widgets/wizard.py +++ b/openlp/core/widgets/wizard.py @@ -280,41 +280,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)) # TODO: to pathdedit - 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)) # TODO: to pathedit - Settings().setValue(self.plugin.settings_section + '/' + setting_name, folder_path) diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index bccff3559..5f127bf05 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -158,7 +158,6 @@ class BibleDB(Manager): self.name = kwargs['name'] if not isinstance(self.name, str): self.name = str(self.name, 'utf-8') - # TODO: To path object self.file_path = Path(clean_filename(self.name) + '.sqlite') if 'file' in kwargs: self.file_path = kwargs['file'] diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index f35e9dcfa..925744142 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/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: diff --git a/openlp/plugins/custom/lib/mediaitem.py b/openlp/plugins/custom/lib/mediaitem.py index 0225f0cc4..fcf42f490 100644 --- a/openlp/plugins/custom/lib/mediaitem.py +++ b/openlp/plugins/custom/lib/mediaitem.py @@ -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) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 69bd46b27..b2eb25721 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/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: diff --git a/openlp/plugins/media/lib/mediaitem.py b/openlp/plugins/media/lib/mediaitem.py index dad964dd5..292312626 100644 --- a/openlp/plugins/media/lib/mediaitem.py +++ b/openlp/plugins/media/lib/mediaitem.py @@ -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() diff --git a/openlp/plugins/presentations/lib/mediaitem.py b/openlp/plugins/presentations/lib/mediaitem.py index 106720c9d..ddec6d143 100644 --- a/openlp/plugins/presentations/lib/mediaitem.py +++ b/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] diff --git a/openlp/plugins/presentations/lib/messagelistener.py b/openlp/plugins/presentations/lib/messagelistener.py index ba50e621c..ef9dd1544 100644 --- a/openlp/plugins/presentations/lib/messagelistener.py +++ b/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 diff --git a/openlp/plugins/songs/lib/mediaitem.py b/openlp/plugins/songs/lib/mediaitem.py index 96b94c9f7..4af0f0096 100644 --- a/openlp/plugins/songs/lib/mediaitem.py +++ b/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)) diff --git a/tests/functional/openlp_plugins/bibles/test_manager.py b/tests/functional/openlp_plugins/bibles/test_manager.py index 8e295a52b..39a693446 100644 --- a/tests/functional/openlp_plugins/bibles/test_manager.py +++ b/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') From 573f31fd452e814b7228c37c4cc6353438522409 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 17 Mar 2019 10:36:12 +0000 Subject: [PATCH 37/38] Minor tidy ups --- openlp/core/display/html/display.html | 2 +- openlp/core/lib/db.py | 2 +- openlp/core/lib/mediamanageritem.py | 10 ++++------ openlp/core/lib/serviceitem.py | 2 +- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/openlp/core/display/html/display.html b/openlp/core/display/html/display.html index 83e0bac90..154d3855d 100644 --- a/openlp/core/display/html/display.html +++ b/openlp/core/display/html/display.html @@ -6,7 +6,7 @@