From 03bcc194ea2ee8de89c0be2371175be08d68a502 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 4 Aug 2017 18:40:57 +0100 Subject: [PATCH 1/9] Update the path edit component to use the pathlib module. Add a few utility methods --- openlp/core/common/path.py | 61 ++++++ openlp/core/lib/__init__.py | 35 ++++ openlp/core/ui/advancedtab.py | 13 +- openlp/core/ui/generaltab.py | 8 +- openlp/core/ui/lib/filedialogpatches.py | 118 +++++++++++ openlp/core/ui/lib/pathedit.py | 57 ++++-- openlp/core/ui/themeform.py | 15 +- openlp/core/ui/themewizard.py | 2 + openlp/plugins/bibles/lib/importers/http.py | 4 +- .../presentations/lib/presentationtab.py | 15 +- .../songusage/forms/songusagedetailform.py | 8 +- .../openlp_core_common/test_path.py | 88 ++++++++ tests/functional/openlp_core_lib/test_lib.py | 39 +++- .../openlp_core_ui/test_themeform.py | 3 +- ...st_color_button.py => test_colorbutton.py} | 0 .../test_filedialogpatches.py | 191 ++++++++++++++++++ .../{test_path_edit.py => test_pathedit.py} | 103 +++++----- 17 files changed, 650 insertions(+), 110 deletions(-) create mode 100644 openlp/core/common/path.py create mode 100755 openlp/core/ui/lib/filedialogpatches.py create mode 100644 tests/functional/openlp_core_common/test_path.py rename tests/functional/openlp_core_ui_lib/{test_color_button.py => test_colorbutton.py} (100%) create mode 100755 tests/functional/openlp_core_ui_lib/test_filedialogpatches.py rename tests/functional/openlp_core_ui_lib/{test_path_edit.py => test_pathedit.py} (77%) diff --git a/openlp/core/common/path.py b/openlp/core/common/path.py new file mode 100644 index 000000000..12bef802d --- /dev/null +++ b/openlp/core/common/path.py @@ -0,0 +1,61 @@ +# -*- 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 # +############################################################################### + +from pathlib import Path + + +def path_to_str(path): + """ + A utility function to convert a Path object or NoneType to a string equivalent. + + :param path: The value to convert to a string + :type: pathlib.Path or None + + :return: An empty string if :param:`path` is None, else a string representation of the :param:`path` + :rtype: str + """ + if not isinstance(path, Path) and path is not None: + raise TypeError('parameter \'path\' must be of type Path or NoneType') + if path is None: + return '' + else: + return str(path) + + +def str_to_path(string): + """ + A utility function to convert a str object to a Path or NoneType. + + This function is of particular use because initating a Path object with an empty string causes the Path object to + point to the current working directory. + + :param string: The string to convert + :type string: str + + :return: None if :param:`string` is empty, or a Path object representation of :param:`string` + :rtype: pathlib.Path or None + """ + if not isinstance(string, str): + raise TypeError('parameter \'string\' must be of type str') + if string == '': + return None + return Path(string) diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index a8b5771b6..86eac88ee 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -608,6 +608,41 @@ def create_separated_list(string_list): return list_to_string +def replace_params(args, kwargs, params): + """ + Apply a transformation function to the specified args or kwargs + + :param args: Positional arguments + :type args: (,) + + :param kwargs: Key Word arguments + :type kwargs: dict + + :param params: A tuple of tuples with the position and the key word to replace. + :type params: ((int, str, path_to_str),) + + :return: The modified positional and keyword arguments + :rtype: (tuple, dict) + + + Usage: + Take a method with the following signature, and assume we which to apply the str function to arg2: + def method(arg1=None, arg2=None, arg3=None) + + As arg2 can be specified postitionally as the second argument (1 with a zero index) or as a keyword, the we + would call this function as follows: + + replace_params(args, kwargs, ((1, 'arg2', str),)) + """ + args = list(args) + for position, key_word, transform in params: + if len(args) > position: + args[position] = transform(args[position]) + elif key_word in kwargs: + kwargs[key_word] = transform(kwargs[key_word]) + return tuple(args), kwargs + + from .exceptions import ValidationError from .filedialog import FileDialog from .screen import ScreenList diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index bf7294ca8..58989ad8a 100644 --- a/openlp/core/ui/advancedtab.py +++ b/openlp/core/ui/advancedtab.py @@ -30,6 +30,7 @@ from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import AppLocation, Settings, SlideLimits, UiStrings, translate from openlp.core.common.languagemanager import format_time +from openlp.core.common.path import path_to_str from openlp.core.lib import SettingsTab, build_icon from openlp.core.ui.lib import PathEdit, PathType @@ -156,7 +157,7 @@ class AdvancedTab(SettingsTab): self.data_directory_new_label = QtWidgets.QLabel(self.data_directory_group_box) self.data_directory_new_label.setObjectName('data_directory_current_label') self.data_directory_path_edit = PathEdit(self.data_directory_group_box, path_type=PathType.Directories, - default_path=str(AppLocation.get_directory(AppLocation.DataDir))) + default_path=AppLocation.get_directory(AppLocation.DataDir)) self.data_directory_layout.addRow(self.data_directory_new_label, self.data_directory_path_edit) self.new_data_directory_has_files_label = QtWidgets.QLabel(self.data_directory_group_box) self.new_data_directory_has_files_label.setObjectName('new_data_directory_has_files_label') @@ -373,7 +374,7 @@ class AdvancedTab(SettingsTab): self.new_data_directory_has_files_label.hide() self.data_directory_cancel_button.hide() # Since data location can be changed, make sure the path is present. - self.data_directory_path_edit.path = str(AppLocation.get_data_path()) + self.data_directory_path_edit.path = AppLocation.get_data_path() # Don't allow data directory move if running portable. if settings.value('advanced/is portable'): self.data_directory_group_box.hide() @@ -497,12 +498,12 @@ class AdvancedTab(SettingsTab): 'closed.').format(path=new_data_path), defaultButton=QtWidgets.QMessageBox.No) if answer != QtWidgets.QMessageBox.Yes: - self.data_directory_path_edit.path = str(AppLocation.get_data_path()) + self.data_directory_path_edit.path = AppLocation.get_data_path() return # Check if data already exists here. - self.check_data_overwrite(new_data_path) + self.check_data_overwrite(path_to_str(new_data_path)) # Save the new location. - self.main_window.set_new_data_path(new_data_path) + self.main_window.set_new_data_path(path_to_str(new_data_path)) self.data_directory_cancel_button.show() def on_data_directory_copy_check_box_toggled(self): @@ -550,7 +551,7 @@ class AdvancedTab(SettingsTab): """ Cancel the data directory location change """ - self.data_directory_path_edit.path = str(AppLocation.get_data_path()) + self.data_directory_path_edit.path = AppLocation.get_data_path() self.data_directory_copy_check_box.setChecked(False) self.main_window.set_new_data_path(None) self.main_window.set_copy_data(False) diff --git a/openlp/core/ui/generaltab.py b/openlp/core/ui/generaltab.py index 0ed208ec5..a6760d56e 100644 --- a/openlp/core/ui/generaltab.py +++ b/openlp/core/ui/generaltab.py @@ -23,6 +23,7 @@ The general tab of the configuration dialog. """ import logging +from pathlib import Path from PyQt5 import QtCore, QtGui, QtWidgets @@ -172,7 +173,8 @@ class GeneralTab(SettingsTab): self.logo_layout.setObjectName('logo_layout') self.logo_file_label = QtWidgets.QLabel(self.logo_group_box) self.logo_file_label.setObjectName('logo_file_label') - self.logo_file_path_edit = PathEdit(self.logo_group_box, default_path=':/graphics/openlp-splash-screen.png') + self.logo_file_path_edit = PathEdit(self.logo_group_box, + default_path=Path(':/graphics/openlp-splash-screen.png')) self.logo_layout.addRow(self.logo_file_label, self.logo_file_path_edit) self.logo_color_label = QtWidgets.QLabel(self.logo_group_box) self.logo_color_label.setObjectName('logo_color_label') @@ -266,7 +268,7 @@ class GeneralTab(SettingsTab): self.audio_group_box.setTitle(translate('OpenLP.GeneralTab', 'Background Audio')) self.start_paused_check_box.setText(translate('OpenLP.GeneralTab', 'Start background audio paused')) self.repeat_list_check_box.setText(translate('OpenLP.GeneralTab', 'Repeat track list')) - self.logo_file_path_edit.dialog_caption = dialog_caption = translate('OpenLP.AdvancedTab', 'Select Logo File') + self.logo_file_path_edit.dialog_caption = translate('OpenLP.AdvancedTab', 'Select Logo File') self.logo_file_path_edit.filters = '{text};;{names} (*)'.format( text=get_images_filter(), names=UiStrings().AllFiles) @@ -325,7 +327,7 @@ class GeneralTab(SettingsTab): settings.setValue('auto open', self.auto_open_check_box.isChecked()) settings.setValue('show splash', self.show_splash_check_box.isChecked()) settings.setValue('logo background color', self.logo_background_color) - settings.setValue('logo file', self.logo_file_path_edit.path) + settings.setValue('logo file', str(self.logo_file_path_edit.path)) settings.setValue('logo hide on startup', self.logo_hide_on_startup_check_box.isChecked()) settings.setValue('update check', self.check_for_updates_check_box.isChecked()) settings.setValue('save prompt', self.save_check_service_check_box.isChecked()) diff --git a/openlp/core/ui/lib/filedialogpatches.py b/openlp/core/ui/lib/filedialogpatches.py new file mode 100755 index 000000000..f4b8d60ce --- /dev/null +++ b/openlp/core/ui/lib/filedialogpatches.py @@ -0,0 +1,118 @@ +# -*- 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 # +############################################################################### +""" Patch the QFileDialog so it accepts and returns Path objects""" +from functools import wraps +from pathlib import Path + +from PyQt5 import QtWidgets + +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import replace_params + + +class PQFileDialog(QtWidgets.QFileDialog): + @classmethod + @wraps(QtWidgets.QFileDialog.getExistingDirectory) + def getExistingDirectory(cls, *args, **kwargs): + """ + Reimplement `getExistingDirectory` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path, str] + """ + + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + return_value = super().getExistingDirectory(*args, **kwargs) + + # getExistingDirectory returns a str that represents the path. The string is empty if the user cancels the + # dialog. + return str_to_path(return_value) + + @classmethod + @wraps(QtWidgets.QFileDialog.getOpenFileName) + def getOpenFileName(cls, *args, **kwargs): + """ + Reimplement `getOpenFileName` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path, str] + """ + + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + return_value = super().getOpenFileName(*args, **kwargs) + + # getOpenFileName returns a tuple. The first item is a of str's that represents the path. The string is empty if + # the user cancels the dialog. + return str_to_path(return_value[0]), return_value[1] + + @classmethod + def getOpenFileNames(cls, *args, **kwargs): + """ + Reimplement `getOpenFileNames` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[list[Path], str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + return_value = super().getOpenFileNames(*args, **kwargs) + + # getSaveFileName returns a tuple. The first item is a list of str's that represents the path. The list is + # empty if the user cancels the dialog. + paths = [str_to_path(path) for path in return_value[0]] + return paths, return_value[1] + + @classmethod + def getSaveFileName(cls, *args, **kwargs): + """ + Reimplement `getSaveFileName` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path or None, str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + return_value = super().getSaveFileName(*args, **kwargs) + + # getSaveFileName returns a tuple. The first item represents the path as a str. The string is empty if the user + # cancels the dialog. + return str_to_path(return_value[0]), return_value[1] diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 9c66478b2..90cb6ef1e 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -20,12 +20,14 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### from enum import Enum -import os.path +from pathlib import Path from PyQt5 import QtCore, QtWidgets from openlp.core.common import UiStrings, translate +from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib import build_icon +from openlp.core.ui.lib.filedialogpatches import PQFileDialog class PathType(Enum): @@ -38,11 +40,12 @@ class PathEdit(QtWidgets.QWidget): The :class:`~openlp.core.ui.lib.pathedit.PathEdit` class subclasses QWidget to create a custom widget for use when a file or directory needs to be selected. """ - pathChanged = QtCore.pyqtSignal(str) + + pathChanged = QtCore.pyqtSignal(Path) def __init__(self, parent=None, path_type=PathType.Files, default_path=None, dialog_caption=None, show_revert=True): """ - Initalise the PathEdit widget + Initialise the PathEdit widget :param parent: The parent of the widget. This is just passed to the super method. :type parent: QWidget or None @@ -51,9 +54,9 @@ class PathEdit(QtWidgets.QWidget): :type dialog_caption: str :param default_path: The default path. This is set as the path when the revert button is clicked - :type default_path: str + :type default_path: pathlib.Path - :param show_revert: Used to determin if the 'revert button' should be visible. + :param show_revert: Used to determine if the 'revert button' should be visible. :type show_revert: bool :return: None @@ -79,7 +82,6 @@ class PathEdit(QtWidgets.QWidget): widget_layout = QtWidgets.QHBoxLayout() widget_layout.setContentsMargins(0, 0, 0, 0) self.line_edit = QtWidgets.QLineEdit(self) - self.line_edit.setText(self._path) widget_layout.addWidget(self.line_edit) self.browse_button = QtWidgets.QToolButton(self) self.browse_button.setIcon(build_icon(':/general/general_open.png')) @@ -101,7 +103,7 @@ class PathEdit(QtWidgets.QWidget): A property getter method to return the selected path. :return: The selected path - :rtype: str + :rtype: pathlib.Path """ return self._path @@ -111,11 +113,15 @@ class PathEdit(QtWidgets.QWidget): A Property setter method to set the selected path :param path: The path to set the widget to - :type path: str + :type path: pathlib.Path + + :return: None + :rtype: None """ self._path = path - self.line_edit.setText(path) - self.line_edit.setToolTip(path) + text = path_to_str(path) + self.line_edit.setText(text) + self.line_edit.setToolTip(text) @property def path_type(self): @@ -124,7 +130,7 @@ class PathEdit(QtWidgets.QWidget): selecting a file or directory. :return: The type selected - :rtype: Enum of PathEdit + :rtype: PathType """ return self._path_type @@ -133,8 +139,11 @@ class PathEdit(QtWidgets.QWidget): """ A Property setter method to set the path type - :param path: The type of path to select - :type path: Enum of PathEdit + :param path_type: The type of path to select + :type path_type: PathType + + :return: None + :rtype: None """ self._path_type = path_type self.update_button_tool_tips() @@ -142,7 +151,9 @@ class PathEdit(QtWidgets.QWidget): def update_button_tool_tips(self): """ Called to update the tooltips on the buttons. This is changing path types, and when the widget is initalised + :return: None + :rtype: None """ if self._path_type == PathType.Directories: self.browse_button.setToolTip(translate('OpenLP.PathEdit', 'Browse for directory.')) @@ -156,21 +167,21 @@ class PathEdit(QtWidgets.QWidget): A handler to handle a click on the browse button. Show the QFileDialog and process the input from the user + :return: None + :rtype: None """ caption = self.dialog_caption - path = '' + path = None if self._path_type == PathType.Directories: if not caption: caption = translate('OpenLP.PathEdit', 'Select Directory') - path = QtWidgets.QFileDialog.getExistingDirectory(self, caption, - self._path, QtWidgets.QFileDialog.ShowDirsOnly) + path = PQFileDialog.getExistingDirectory(self, caption, self._path, PQFileDialog.ShowDirsOnly) elif self._path_type == PathType.Files: if not caption: caption = self.dialog_caption = translate('OpenLP.PathEdit', 'Select File') - path, filter_used = QtWidgets.QFileDialog.getOpenFileName(self, caption, self._path, self.filters) + path, filter_used = PQFileDialog.getOpenFileName(self, caption, self._path, self.filters) if path: - path = os.path.normpath(path) self.on_new_path(path) def on_revert_button_clicked(self): @@ -178,16 +189,21 @@ class PathEdit(QtWidgets.QWidget): A handler to handle a click on the revert button. Set the new path to the value of the default_path instance variable. + :return: None + :rtype: None """ self.on_new_path(self.default_path) def on_line_edit_editing_finished(self): """ A handler to handle when the line edit has finished being edited. + :return: None + :rtype: None """ - self.on_new_path(self.line_edit.text()) + path = str_to_path(self.line_edit.text()) + self.on_new_path(path) def on_new_path(self, path): """ @@ -196,9 +212,10 @@ class PathEdit(QtWidgets.QWidget): Emits the pathChanged Signal :param path: The new path - :type path: str + :type path: pathlib.Path :return: None + :rtype: None """ if self._path != path: self.path = path diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index c92d5c663..0c36a9131 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -28,12 +28,15 @@ import os from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, RegistryProperties, UiStrings, translate, get_images_filter, is_not_image_file +from openlp.core.common.path import path_to_str from openlp.core.lib.theme import BackgroundType, BackgroundGradientType from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui import ThemeLayoutForm from openlp.core.ui.media.webkitplayer import VIDEO_EXT from .themewizard import Ui_ThemeWizard +from pathlib import Path + log = logging.getLogger(__name__) @@ -316,11 +319,11 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties): self.setField('background_type', 1) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Image): self.image_color_button.color = self.theme.background_border_color - self.image_path_edit.path = self.theme.background_filename + self.image_path_edit.path = path_to_str(self.theme.background_filename) self.setField('background_type', 2) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Video): self.video_color_button.color = self.theme.background_border_color - self.video_path_edit.path = self.theme.background_filename + self.video_path_edit.path = path_to_str(self.theme.background_filename) self.setField('background_type', 4) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Transparent): self.setField('background_type', 3) @@ -448,18 +451,18 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties): """ self.theme.background_end_color = color - def on_image_path_edit_path_changed(self, filename): + def on_image_path_edit_path_changed(self, file_path): """ Background Image button pushed. """ - self.theme.background_filename = filename + self.theme.background_filename = path_to_str(file_path) self.set_background_page_values() - def on_video_path_edit_path_changed(self, filename): + def on_video_path_edit_path_changed(self, file_path): """ Background video button pushed. """ - self.theme.background_filename = filename + self.theme.background_filename = path_to_str(file_path) self.set_background_page_values() def on_main_color_changed(self, color): diff --git a/openlp/core/ui/themewizard.py b/openlp/core/ui/themewizard.py index 65464e063..3a1eba9f0 100644 --- a/openlp/core/ui/themewizard.py +++ b/openlp/core/ui/themewizard.py @@ -30,6 +30,8 @@ from openlp.core.lib.theme import HorizontalType, BackgroundType, BackgroundGrad from openlp.core.lib.ui import add_welcome_page, create_valign_selection_widgets from openlp.core.ui.lib import ColorButton, PathEdit +from pathlib import Path + class Ui_ThemeWizard(object): """ diff --git a/openlp/plugins/bibles/lib/importers/http.py b/openlp/plugins/bibles/lib/importers/http.py index 4ed5fa844..1616ebcf7 100644 --- a/openlp/plugins/bibles/lib/importers/http.py +++ b/openlp/plugins/bibles/lib/importers/http.py @@ -255,7 +255,7 @@ class BGExtract(RegistryProperties): chapter=chapter, version=version) soup = get_soup_for_bible_ref( - 'http://biblegateway.com/passage/?{url}'.format(url=url_params), + 'http://www.biblegateway.com/passage/?{url}'.format(url=url_params), pre_parse_regex=r'', pre_parse_substitute='') if not soup: return None @@ -284,7 +284,7 @@ class BGExtract(RegistryProperties): """ log.debug('BGExtract.get_books_from_http("{version}")'.format(version=version)) url_params = urllib.parse.urlencode({'action': 'getVersionInfo', 'vid': '{version}'.format(version=version)}) - reference_url = 'http://biblegateway.com/versions/?{url}#books'.format(url=url_params) + reference_url = 'http://www.biblegateway.com/versions/?{url}#books'.format(url=url_params) page = get_web_page(reference_url) if not page: send_error_message('download') diff --git a/openlp/plugins/presentations/lib/presentationtab.py b/openlp/plugins/presentations/lib/presentationtab.py index 2af73d369..f801d54d1 100644 --- a/openlp/plugins/presentations/lib/presentationtab.py +++ b/openlp/plugins/presentations/lib/presentationtab.py @@ -20,10 +20,11 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### -from PyQt5 import QtGui, QtWidgets +from PyQt5 import QtWidgets from openlp.core.common import Settings, UiStrings, translate -from openlp.core.lib import SettingsTab, build_icon +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import SettingsTab from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui.lib import PathEdit from openlp.plugins.presentations.lib.pdfcontroller import PdfController @@ -156,7 +157,7 @@ class PresentationTab(SettingsTab): self.program_path_edit.setEnabled(enable_pdf_program) pdf_program = Settings().value(self.settings_section + '/pdf_program') if pdf_program: - self.program_path_edit.path = pdf_program + self.program_path_edit.path = str_to_path(pdf_program) def save(self): """ @@ -192,7 +193,7 @@ class PresentationTab(SettingsTab): Settings().setValue(setting_key, self.ppt_window_check_box.checkState()) changed = True # Save pdf-settings - pdf_program = self.program_path_edit.path + pdf_program = path_to_str(self.program_path_edit.path) enable_pdf_program = self.pdf_program_check_box.checkState() # If the given program is blank disable using the program if pdf_program == '': @@ -219,12 +220,12 @@ class PresentationTab(SettingsTab): checkbox.setEnabled(controller.is_available()) self.set_controller_text(checkbox, controller) - def on_program_path_edit_path_changed(self, filename): + def on_program_path_edit_path_changed(self, new_path): """ Select the mudraw or ghostscript binary that should be used. """ - if filename: - if not PdfController.process_check_binary(filename): + if new_path: + if not PdfController.process_check_binary(new_path): critical_error_message_box(UiStrings().Error, translate('PresentationPlugin.PresentationTab', 'The program is not ghostscript or mudraw which is required.')) diff --git a/openlp/plugins/songusage/forms/songusagedetailform.py b/openlp/plugins/songusage/forms/songusagedetailform.py index 172cca6b1..595d56fcd 100644 --- a/openlp/plugins/songusage/forms/songusagedetailform.py +++ b/openlp/plugins/songusage/forms/songusagedetailform.py @@ -27,6 +27,7 @@ from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_ from openlp.core.common import RegistryProperties, Settings, check_directory_exists, translate +from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib.ui import critical_error_message_box from openlp.plugins.songusage.lib.db import SongUsageItem from .songusagedetaildialog import Ui_SongUsageDetailDialog @@ -55,20 +56,21 @@ class SongUsageDetailForm(QtWidgets.QDialog, Ui_SongUsageDetailDialog, RegistryP """ self.from_date_calendar.setSelectedDate(Settings().value(self.plugin.settings_section + '/from date')) self.to_date_calendar.setSelectedDate(Settings().value(self.plugin.settings_section + '/to date')) - self.report_path_edit.path = Settings().value(self.plugin.settings_section + '/last directory export') + self.report_path_edit.path = str_to_path( + Settings().value(self.plugin.settings_section + '/last directory export')) def on_report_path_edit_path_changed(self, file_path): """ Triggered when the Directory selection button is clicked """ - Settings().setValue(self.plugin.settings_section + '/last directory export', file_path) + Settings().setValue(self.plugin.settings_section + '/last directory export', path_to_str(file_path)) def accept(self): """ Ok was triggered so lets save the data and run the report """ log.debug('accept') - path = self.report_path_edit.path + path = path_to_str(self.report_path_edit.path) if not path: self.main_window.error_message( translate('SongUsagePlugin.SongUsageDetailForm', 'Output Path Not Selected'), diff --git a/tests/functional/openlp_core_common/test_path.py b/tests/functional/openlp_core_common/test_path.py new file mode 100644 index 000000000..e1d015e6a --- /dev/null +++ b/tests/functional/openlp_core_common/test_path.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-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.common.path package. +""" +import os +from pathlib import Path +from unittest import TestCase + +from openlp.core.common.path import path_to_str, str_to_path + + +class TestPath(TestCase): + """ + Tests for the :mod:`openlp.core.common.path` module + """ + + def test_path_to_str_type_error(self): + """ + Test that `path_to_str` raises a type error when called with an invalid type + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling `path_to_str` with an invalid Type + # THEN: A TypeError should have been raised + with self.assertRaises(TypeError): + path_to_str(str()) + + def test_path_to_str_none(self): + """ + Test that `path_to_str` correctly converts the path parameter when passed with None + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling the `path_to_str` function with None + result = path_to_str(None) + + # THEN: `path_to_str` should return an empty string + self.assertEqual(result, '') + + def test_path_to_str_path_object(self): + """ + Test that `path_to_str` correctly converts the path parameter when passed a Path object + """ + # GIVEN: The `path_to_str` function + # WHEN: Calling the `path_to_str` function with a Path object + result = path_to_str(Path('test/path')) + + # THEN: `path_to_str` should return a string representation of the Path object + self.assertEqual(result, os.path.join('test', 'path')) + + def test_str_to_path_type_error(self): + """ + Test that `str_to_path` raises a type error when called with an invalid type + """ + # GIVEN: The `str_to_path` function + # WHEN: Calling `str_to_path` with an invalid Type + # THEN: A TypeError should have been raised + with self.assertRaises(TypeError): + str_to_path(Path()) + + def test_str_to_path_empty_str(self): + """ + Test that `str_to_path` correctly converts the string parameter when passed with and empty string + """ + # GIVEN: The `str_to_path` function + # WHEN: Calling the `str_to_path` function with None + result = str_to_path('') + + # THEN: `path_to_str` should return None + self.assertEqual(result, None) \ No newline at end of file diff --git a/tests/functional/openlp_core_lib/test_lib.py b/tests/functional/openlp_core_lib/test_lib.py index 77f093625..5e9b52dfd 100644 --- a/tests/functional/openlp_core_lib/test_lib.py +++ b/tests/functional/openlp_core_lib/test_lib.py @@ -29,10 +29,9 @@ from unittest.mock import MagicMock, patch from PyQt5 import QtCore, QtGui -from openlp.core.lib import FormattingTags, expand_chords_for_printing -from openlp.core.lib import build_icon, check_item_selected, clean_tags, create_thumb, create_separated_list, \ - expand_tags, get_text_file_string, image_to_byte, resize_image, str_to_bool, validate_thumb, expand_chords, \ - compare_chord_lyric, find_formatting_tags +from openlp.core.lib import FormattingTags, build_icon, check_item_selected, clean_tags, compare_chord_lyric, \ + create_separated_list, create_thumb, expand_chords, expand_chords_for_printing, expand_tags, find_formatting_tags, \ + get_text_file_string, image_to_byte, replace_params, resize_image, str_to_bool, validate_thumb TEST_PATH = os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..', 'resources')) @@ -652,6 +651,38 @@ class TestLib(TestCase): mocked_os.stat.assert_any_call(thumb_path) assert result is False, 'The result should be False' + def test_replace_params_no_params(self): + """ + Test replace_params when called with and empty tuple instead of parameters to replace + """ + # GIVEN: Some test data + test_args = (1, 2) + test_kwargs = {'arg3': 3, 'arg4': 4} + test_params = tuple() + + # WHEN: Calling replace_params + result_args, result_kwargs = replace_params(test_args, test_kwargs, test_params) + + # THEN: The positional and keyword args should not have changed + self.assertEqual(test_args, result_args) + self.assertEqual(test_kwargs, result_kwargs) + + def test_replace_params_params(self): + """ + Test replace_params when given a positional and a keyword argument to change + """ + # GIVEN: Some test data + test_args = (1, 2) + test_kwargs = {'arg3': 3, 'arg4': 4} + test_params = ((1, 'arg2', str), (2, 'arg3', str)) + + # WHEN: Calling replace_params + result_args, result_kwargs = replace_params(test_args, test_kwargs, test_params) + + # THEN: The positional and keyword args should have have changed + self.assertEqual(result_args, (1, '2')) + self.assertEqual(result_kwargs, {'arg3': '3', 'arg4': 4}) + def test_resize_thumb(self): """ Test the resize_thumb() function diff --git a/tests/functional/openlp_core_ui/test_themeform.py b/tests/functional/openlp_core_ui/test_themeform.py index f19255fd4..97152feed 100644 --- a/tests/functional/openlp_core_ui/test_themeform.py +++ b/tests/functional/openlp_core_ui/test_themeform.py @@ -22,6 +22,7 @@ """ Package to test the openlp.core.ui.themeform package. """ +from pathlib import Path from unittest import TestCase from unittest.mock import MagicMock, patch @@ -45,7 +46,7 @@ class TestThemeManager(TestCase): self.instance.theme = MagicMock() # WHEN: `on_image_path_edit_path_changed` is clicked - self.instance.on_image_path_edit_path_changed('/new/pat.h') + self.instance.on_image_path_edit_path_changed(Path('/', 'new', 'pat.h')) # THEN: The theme background file should be set and `set_background_page_values` should have been called self.assertEqual(self.instance.theme.background_filename, '/new/pat.h') diff --git a/tests/functional/openlp_core_ui_lib/test_color_button.py b/tests/functional/openlp_core_ui_lib/test_colorbutton.py similarity index 100% rename from tests/functional/openlp_core_ui_lib/test_color_button.py rename to tests/functional/openlp_core_ui_lib/test_colorbutton.py diff --git a/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py b/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py new file mode 100755 index 000000000..2e6fe318c --- /dev/null +++ b/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py @@ -0,0 +1,191 @@ +import os +import sys +from unittest import TestCase +from unittest.mock import patch +from pathlib import Path + +from PyQt5 import QtWidgets + +from openlp.core.ui.lib.filedialogpatches import PQFileDialog + + +class TestFileDialogPatches(TestCase): + """ + Tests for the :mod:`openlp.core.ui.lib.filedialogpatches` module + """ + + def test_pq_file_dialog(self): + """ + Test that the :class:`PQFileDialog` instantiates correctly + """ + #app = QtWidgets.QApplication(sys.argv) + + # GIVEN: The PQFileDialog class + # WHEN: Creating an instance + instance = PQFileDialog() + + # THEN: The instance should be an instance of QFileDialog + self.assertIsInstance(instance, QtWidgets.QFileDialog) + + def test_get_existing_directory_user_abort(self): + """ + Test that `getExistingDirectory` handles the case when the user cancels the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getExistingDirectory method + # WHEN: Calling PQFileDialog.getExistingDirectory and the user cancels the dialog returns a empty string + with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=''): + result = PQFileDialog.getExistingDirectory() + + # THEN: The result should be None + self.assertEqual(result, None) + + def test_get_existing_directory_user_accepts(self): + """ + Test that `getExistingDirectory` handles the case when the user accepts the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getExistingDirectory method + # WHEN: Calling PQFileDialog.getExistingDirectory, the user chooses a file and accepts the dialog (it returns a + # string pointing to the directory) + with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=os.path.join('test', 'dir')): + result = PQFileDialog.getExistingDirectory() + + # THEN: getExistingDirectory() should return a Path object pointing to the chosen file + self.assertEqual(result, Path('test', 'dir')) + + def test_get_existing_directory_param_order(self): + """ + Test that `getExistingDirectory` passes the parameters to `QFileDialog.getExistingDirectory` in the correct + order + """ + # GIVEN: PQFileDialog + with patch('openlp.core.ui.lib.filedialogpatches.QtWidgets.QFileDialog.getExistingDirectory', return_value='') \ + as mocked_get_existing_directory: + + # WHEN: Calling the getExistingDirectory method with all parameters set + PQFileDialog.getExistingDirectory('Parent', 'Caption', Path('test', 'dir'), 'Options') + + # THEN: The `QFileDialog.getExistingDirectory` should have been called with the parameters in the correct + # order + mocked_get_existing_directory.assert_called_once_with('Parent', 'Caption', os.path.join('test', 'dir'), + 'Options') + + def test_get_open_file_name_user_abort(self): + """ + Test that `getOpenFileName` handles the case when the user cancels the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileName method + # WHEN: Calling PQFileDialog.getOpenFileName and the user cancels the dialog (it returns a tuple with the first + # value set as an empty string) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')): + result = PQFileDialog.getOpenFileName() + + # THEN: First value should be None + self.assertEqual(result[0], None) + + def test_get_open_file_name_user_accepts(self): + """ + Test that `getOpenFileName` handles the case when the user accepts the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileName method + # WHEN: Calling PQFileDialog.getOpenFileName, the user chooses a file and accepts the dialog (it returns a + # tuple with the first value set as an string pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', + return_value=(os.path.join('test', 'chosen.file'), '')): + result = PQFileDialog.getOpenFileName() + + # THEN: getOpenFileName() should return a tuple with the first value set to a Path object pointing to the + # chosen file + self.assertEqual(result[0], Path('test', 'chosen.file')) + + def test_get_open_file_name_selected_filter(self): + """ + Test that `getOpenFileName` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileName` + """ + # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling PQFileDialog.getOpenFileName, and `QFileDialog.getOpenFileName` returns a known `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', 'selected filter')): + result = PQFileDialog.getOpenFileName() + + # THEN: getOpenFileName() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') + + def test_get_open_file_names_user_abort(self): + """ + Test that `getOpenFileNames` handles the case when the user cancels the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling PQFileDialog.getOpenFileNames and the user cancels the dialog (it returns a tuple with the first + # value set as an empty list) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], '')): + result = PQFileDialog.getOpenFileNames() + + # THEN: First value should be an empty list + self.assertEqual(result[0], []) + + def test_get_open_file_names_user_accepts(self): + """ + Test that `getOpenFileNames` handles the case when the user accepts the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling PQFileDialog.getOpenFileNames, the user chooses some files and accepts the dialog (it returns a + # tuple with the first value set as a list of strings pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', + return_value=([os.path.join('test', 'chosen.file1'), os.path.join('test', 'chosen.file2')], '')): + result = PQFileDialog.getOpenFileNames() + + # THEN: getOpenFileNames() should return a tuple with the first value set to a list of Path objects pointing + # to the chosen file + self.assertEqual(result[0], [Path('test', 'chosen.file1'), Path('test', 'chosen.file2')]) + + def test_get_open_file_names_selected_filter(self): + """ + Test that `getOpenFileNames` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileNames` + """ + # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling PQFileDialog.getOpenFileNames, and `QFileDialog.getOpenFileNames` returns a known + # `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], 'selected filter')): + result = PQFileDialog.getOpenFileNames() + + # THEN: getOpenFileNames() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') + + def test_get_save_file_name_user_abort(self): + """ + Test that `getSaveFileName` handles the case when the user cancels the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling PQFileDialog.getSaveFileName and the user cancels the dialog (it returns a tuple with the first + # value set as an empty string) + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', '')): + result = PQFileDialog.getSaveFileName() + + # THEN: First value should be None + self.assertEqual(result[0], None) + + def test_get_save_file_name_user_accepts(self): + """ + Test that `getSaveFileName` handles the case when the user accepts the dialog + """ + # GIVEN: PQFileDialog with a mocked QDialog.getSaveFileName method + # WHEN: Calling PQFileDialog.getSaveFileName, the user chooses a file and accepts the dialog (it returns a + # tuple with the first value set as an string pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', + return_value=(os.path.join('test', 'chosen.file'), '')): + result = PQFileDialog.getSaveFileName() + + # THEN: getSaveFileName() should return a tuple with the first value set to a Path object pointing to the + # chosen file + self.assertEqual(result[0], Path('test', 'chosen.file')) + + def test_get_save_file_name_selected_filter(self): + """ + Test that `getSaveFileName` does not modify the selectedFilter as returned by `QFileDialog.getSaveFileName` + """ + # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling PQFileDialog.getSaveFileName, and `QFileDialog.getSaveFileName` returns a known `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', 'selected filter')): + result = PQFileDialog.getSaveFileName() + + # THEN: getSaveFileName() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') diff --git a/tests/functional/openlp_core_ui_lib/test_path_edit.py b/tests/functional/openlp_core_ui_lib/test_pathedit.py similarity index 77% rename from tests/functional/openlp_core_ui_lib/test_path_edit.py rename to tests/functional/openlp_core_ui_lib/test_pathedit.py index 111951622..e4ce99c41 100755 --- a/tests/functional/openlp_core_ui_lib/test_path_edit.py +++ b/tests/functional/openlp_core_ui_lib/test_pathedit.py @@ -22,12 +22,13 @@ """ This module contains tests for the openlp.core.ui.lib.pathedit module """ +import os +from pathlib import Path from unittest import TestCase - -from PyQt5 import QtWidgets +from unittest.mock import MagicMock, PropertyMock, patch from openlp.core.ui.lib import PathEdit, PathType -from unittest.mock import MagicMock, PropertyMock, patch +from openlp.core.ui.lib.filedialogpatches import PQFileDialog class TestPathEdit(TestCase): @@ -43,11 +44,11 @@ class TestPathEdit(TestCase): Test the `path` property getter. """ # GIVEN: An instance of PathEdit with the `_path` instance variable set - self.widget._path = 'getter/test/pat.h' + self.widget._path = Path('getter', 'test', 'pat.h') # WHEN: Reading the `path` property # THEN: The value that we set should be returned - self.assertEqual(self.widget.path, 'getter/test/pat.h') + self.assertEqual(self.widget.path, Path('getter', 'test', 'pat.h')) def test_path_setter(self): """ @@ -57,13 +58,13 @@ class TestPathEdit(TestCase): self.widget.line_edit = MagicMock() # WHEN: Writing to the `path` property - self.widget.path = 'setter/test/pat.h' + self.widget.path = Path('setter', 'test', 'pat.h') # THEN: The `_path` instance variable should be set with the test data. The `line_edit` text and tooltip # should have also been set. - self.assertEqual(self.widget._path, 'setter/test/pat.h') - self.widget.line_edit.setToolTip.assert_called_once_with('setter/test/pat.h') - self.widget.line_edit.setText.assert_called_once_with('setter/test/pat.h') + self.assertEqual(self.widget._path, Path('setter', 'test', 'pat.h')) + self.widget.line_edit.setToolTip.assert_called_once_with(os.path.join('setter', 'test', 'pat.h')) + self.widget.line_edit.setText.assert_called_once_with(os.path.join('setter', 'test', 'pat.h')) def test_path_type_getter(self): """ @@ -125,22 +126,20 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Directories` and a mocked # QFileDialog.getExistingDirectory - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getExistingDirectory', return_value='') as \ + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory', return_value=None) as \ mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName') as \ - mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath: + patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName') as mocked_get_open_file_name: self.widget._path_type = PathType.Directories - self.widget._path = 'test/path/' + self.widget._path = Path('test', 'path') # WHEN: Calling on_browse_button_clicked self.widget.on_browse_button_clicked() # THEN: The FileDialog.getExistingDirectory should have been called with the default caption - mocked_get_existing_directory.assert_called_once_with(self.widget, 'Select Directory', 'test/path/', - QtWidgets.QFileDialog.ShowDirsOnly) + mocked_get_existing_directory.assert_called_once_with(self.widget, 'Select Directory', + Path('test', 'path'), + PQFileDialog.ShowDirsOnly) self.assertFalse(mocked_get_open_file_name.called) - self.assertFalse(mocked_normpath.called) def test_on_browse_button_clicked_directory_custom_caption(self): """ @@ -149,45 +148,40 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Directories` and a mocked # QFileDialog.getExistingDirectory with `default_caption` set. - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getExistingDirectory', return_value='') as \ + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory', return_value=None) as \ mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName') as \ - mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath: + patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName') as mocked_get_open_file_name: self.widget._path_type = PathType.Directories - self.widget._path = 'test/path/' + self.widget._path = Path('test', 'path') self.widget.dialog_caption = 'Directory Caption' # WHEN: Calling on_browse_button_clicked self.widget.on_browse_button_clicked() # THEN: The FileDialog.getExistingDirectory should have been called with the custom caption - mocked_get_existing_directory.assert_called_once_with(self.widget, 'Directory Caption', 'test/path/', - QtWidgets.QFileDialog.ShowDirsOnly) + mocked_get_existing_directory.assert_called_once_with(self.widget, 'Directory Caption', + Path('test', 'path'), + PQFileDialog.ShowDirsOnly) self.assertFalse(mocked_get_open_file_name.called) - self.assertFalse(mocked_normpath.called) def test_on_browse_button_clicked_file(self): """ Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Files. """ # GIVEN: An instance of PathEdit with the `path_type` set to `Files` and a mocked QFileDialog.getOpenFileName - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getExistingDirectory') as \ - mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')) as \ - mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath: + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory') as mocked_get_existing_directory, \ + patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + mocked_get_open_file_name: self.widget._path_type = PathType.Files - self.widget._path = 'test/pat.h' + self.widget._path = Path('test', 'pat.h') # WHEN: Calling on_browse_button_clicked self.widget.on_browse_button_clicked() # THEN: The FileDialog.getOpenFileName should have been called with the default caption - mocked_get_open_file_name.assert_called_once_with(self.widget, 'Select File', 'test/pat.h', + mocked_get_open_file_name.assert_called_once_with(self.widget, 'Select File', Path('test', 'pat.h'), self.widget.filters) self.assertFalse(mocked_get_existing_directory.called) - self.assertFalse(mocked_normpath.called) def test_on_browse_button_clicked_file_custom_caption(self): """ @@ -196,23 +190,20 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Files` and a mocked QFileDialog.getOpenFileName # with `default_caption` set. - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getExistingDirectory') as \ - mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')) as \ - mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath: + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory') as mocked_get_existing_directory, \ + patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + mocked_get_open_file_name: self.widget._path_type = PathType.Files - self.widget._path = 'test/pat.h' + self.widget._path = Path('test', 'pat.h') self.widget.dialog_caption = 'File Caption' # WHEN: Calling on_browse_button_clicked self.widget.on_browse_button_clicked() # THEN: The FileDialog.getOpenFileName should have been called with the custom caption - mocked_get_open_file_name.assert_called_once_with(self.widget, 'File Caption', 'test/pat.h', + mocked_get_open_file_name.assert_called_once_with(self.widget, 'File Caption', Path('test', 'pat.h'), self.widget.filters) self.assertFalse(mocked_get_existing_directory.called) - self.assertFalse(mocked_normpath.called) def test_on_browse_button_clicked_user_cancels(self): """ @@ -221,16 +212,14 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked QFileDialog.getOpenFileName which returns an empty str for the # file path. - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')) as \ - mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath: + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + mocked_get_open_file_name: # WHEN: Calling on_browse_button_clicked self.widget.on_browse_button_clicked() # THEN: normpath should not have been called self.assertTrue(mocked_get_open_file_name.called) - self.assertFalse(mocked_normpath.called) def test_on_browse_button_clicked_user_accepts(self): """ @@ -239,9 +228,8 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked QFileDialog.getOpenFileName which returns a str for the file # path. - with patch('openlp.core.ui.lib.pathedit.QtWidgets.QFileDialog.getOpenFileName', - return_value=('/test/pat.h', '')) as mocked_get_open_file_name, \ - patch('openlp.core.ui.lib.pathedit.os.path.normpath') as mocked_normpath, \ + with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', + return_value=(Path('test', 'pat.h'), '')) as mocked_get_open_file_name, \ patch.object(self.widget, 'on_new_path'): # WHEN: Calling on_browse_button_clicked @@ -249,7 +237,6 @@ class TestPathEdit(TestCase): # THEN: normpath and `on_new_path` should have been called self.assertTrue(mocked_get_open_file_name.called) - mocked_normpath.assert_called_once_with('/test/pat.h') self.assertTrue(self.widget.on_new_path.called) def test_on_revert_button_clicked(self): @@ -258,13 +245,13 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked `on_new_path`, and the `default_path` set. with patch.object(self.widget, 'on_new_path') as mocked_on_new_path: - self.widget.default_path = '/default/pat.h' + self.widget.default_path = Path('default', 'pat.h') # WHEN: Calling `on_revert_button_clicked` self.widget.on_revert_button_clicked() # THEN: on_new_path should have been called with the default path - mocked_on_new_path.assert_called_once_with('/default/pat.h') + mocked_on_new_path.assert_called_once_with(Path('default', 'pat.h')) def test_on_line_edit_editing_finished(self): """ @@ -272,13 +259,13 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked `line_edit` and `on_new_path`. with patch.object(self.widget, 'on_new_path') as mocked_on_new_path: - self.widget.line_edit = MagicMock(**{'text.return_value': '/test/pat.h'}) + self.widget.line_edit = MagicMock(**{'text.return_value': 'test/pat.h'}) # WHEN: Calling `on_line_edit_editing_finished` self.widget.on_line_edit_editing_finished() # THEN: on_new_path should have been called with the path enetered in `line_edit` - mocked_on_new_path.assert_called_once_with('/test/pat.h') + mocked_on_new_path.assert_called_once_with(Path('test', 'pat.h')) def test_on_new_path_no_change(self): """ @@ -286,11 +273,11 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a test path and mocked `pathChanged` signal with patch('openlp.core.ui.lib.pathedit.PathEdit.path', new_callable=PropertyMock): - self.widget._path = '/old/test/pat.h' + self.widget._path = Path('/old', 'test', 'pat.h') self.widget.pathChanged = MagicMock() # WHEN: Calling `on_new_path` with the same path as the existing path - self.widget.on_new_path('/old/test/pat.h') + self.widget.on_new_path(Path('/old', 'test', 'pat.h')) # THEN: The `pathChanged` signal should not be emitted self.assertFalse(self.widget.pathChanged.emit.called) @@ -301,11 +288,11 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a test path and mocked `pathChanged` signal with patch('openlp.core.ui.lib.pathedit.PathEdit.path', new_callable=PropertyMock): - self.widget._path = '/old/test/pat.h' + self.widget._path = Path('/old', 'test', 'pat.h') self.widget.pathChanged = MagicMock() # WHEN: Calling `on_new_path` with the a new path - self.widget.on_new_path('/new/test/pat.h') + self.widget.on_new_path(Path('/new', 'test', 'pat.h')) # THEN: The `pathChanged` signal should be emitted - self.widget.pathChanged.emit.assert_called_once_with('/new/test/pat.h') + self.widget.pathChanged.emit.assert_called_once_with(Path('/new', 'test', 'pat.h')) From a7750e680bdf5cf2b9917231cbaa1e7cf28c22d4 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 4 Aug 2017 18:52:55 +0100 Subject: [PATCH 2/9] Pathedit line endings --- openlp/core/ui/lib/pathedit.py | 444 ++++++++++++++++----------------- 1 file changed, 222 insertions(+), 222 deletions(-) diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 90cb6ef1e..abcd56fef 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -1,222 +1,222 @@ -# -*- 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 # -############################################################################### -from enum import Enum -from pathlib import Path - -from PyQt5 import QtCore, QtWidgets - -from openlp.core.common import UiStrings, translate -from openlp.core.common.path import path_to_str, str_to_path -from openlp.core.lib import build_icon -from openlp.core.ui.lib.filedialogpatches import PQFileDialog - - -class PathType(Enum): - Files = 1 - Directories = 2 - - -class PathEdit(QtWidgets.QWidget): - """ - The :class:`~openlp.core.ui.lib.pathedit.PathEdit` class subclasses QWidget to create a custom widget for use when - a file or directory needs to be selected. - """ - - pathChanged = QtCore.pyqtSignal(Path) - - def __init__(self, parent=None, path_type=PathType.Files, default_path=None, dialog_caption=None, show_revert=True): - """ - Initialise the PathEdit widget - - :param parent: The parent of the widget. This is just passed to the super method. - :type parent: QWidget or None - - :param dialog_caption: Used to customise the caption in the QFileDialog. - :type dialog_caption: str - - :param default_path: The default path. This is set as the path when the revert button is clicked - :type default_path: pathlib.Path - - :param show_revert: Used to determine if the 'revert button' should be visible. - :type show_revert: bool - - :return: None - :rtype: None - """ - super().__init__(parent) - self.default_path = default_path - self.dialog_caption = dialog_caption - self._path_type = path_type - self._path = None - self.filters = '{all_files} (*)'.format(all_files=UiStrings().AllFiles) - self._setup(show_revert) - - def _setup(self, show_revert): - """ - Set up the widget - :param show_revert: Show or hide the revert button - :type show_revert: bool - - :return: None - :rtype: None - """ - widget_layout = QtWidgets.QHBoxLayout() - widget_layout.setContentsMargins(0, 0, 0, 0) - self.line_edit = QtWidgets.QLineEdit(self) - widget_layout.addWidget(self.line_edit) - self.browse_button = QtWidgets.QToolButton(self) - self.browse_button.setIcon(build_icon(':/general/general_open.png')) - widget_layout.addWidget(self.browse_button) - self.revert_button = QtWidgets.QToolButton(self) - self.revert_button.setIcon(build_icon(':/general/general_revert.png')) - self.revert_button.setVisible(show_revert) - widget_layout.addWidget(self.revert_button) - self.setLayout(widget_layout) - # Signals and Slots - self.browse_button.clicked.connect(self.on_browse_button_clicked) - self.revert_button.clicked.connect(self.on_revert_button_clicked) - self.line_edit.editingFinished.connect(self.on_line_edit_editing_finished) - self.update_button_tool_tips() - - @property - def path(self): - """ - A property getter method to return the selected path. - - :return: The selected path - :rtype: pathlib.Path - """ - return self._path - - @path.setter - def path(self, path): - """ - A Property setter method to set the selected path - - :param path: The path to set the widget to - :type path: pathlib.Path - - :return: None - :rtype: None - """ - self._path = path - text = path_to_str(path) - self.line_edit.setText(text) - self.line_edit.setToolTip(text) - - @property - def path_type(self): - """ - A property getter method to return the path_type. Path type allows you to sepecify if the user is restricted to - selecting a file or directory. - - :return: The type selected - :rtype: PathType - """ - return self._path_type - - @path_type.setter - def path_type(self, path_type): - """ - A Property setter method to set the path type - - :param path_type: The type of path to select - :type path_type: PathType - - :return: None - :rtype: None - """ - self._path_type = path_type - self.update_button_tool_tips() - - def update_button_tool_tips(self): - """ - Called to update the tooltips on the buttons. This is changing path types, and when the widget is initalised - - :return: None - :rtype: None - """ - if self._path_type == PathType.Directories: - self.browse_button.setToolTip(translate('OpenLP.PathEdit', 'Browse for directory.')) - self.revert_button.setToolTip(translate('OpenLP.PathEdit', 'Revert to default directory.')) - else: - self.browse_button.setToolTip(translate('OpenLP.PathEdit', 'Browse for file.')) - self.revert_button.setToolTip(translate('OpenLP.PathEdit', 'Revert to default file.')) - - def on_browse_button_clicked(self): - """ - A handler to handle a click on the browse button. - - Show the QFileDialog and process the input from the user - - :return: None - :rtype: None - """ - caption = self.dialog_caption - path = None - if self._path_type == PathType.Directories: - if not caption: - caption = translate('OpenLP.PathEdit', 'Select Directory') - path = PQFileDialog.getExistingDirectory(self, caption, self._path, PQFileDialog.ShowDirsOnly) - elif self._path_type == PathType.Files: - if not caption: - caption = self.dialog_caption = translate('OpenLP.PathEdit', 'Select File') - path, filter_used = PQFileDialog.getOpenFileName(self, caption, self._path, self.filters) - if path: - self.on_new_path(path) - - def on_revert_button_clicked(self): - """ - A handler to handle a click on the revert button. - - Set the new path to the value of the default_path instance variable. - - :return: None - :rtype: None - """ - self.on_new_path(self.default_path) - - def on_line_edit_editing_finished(self): - """ - A handler to handle when the line edit has finished being edited. - - :return: None - :rtype: None - """ - path = str_to_path(self.line_edit.text()) - self.on_new_path(path) - - def on_new_path(self, path): - """ - A method called to validate and set a new path. - - Emits the pathChanged Signal - - :param path: The new path - :type path: pathlib.Path - - :return: None - :rtype: None - """ - if self._path != path: - self.path = path - self.pathChanged.emit(path) +# -*- 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 # +############################################################################### +from enum import Enum +from pathlib import Path + +from PyQt5 import QtCore, QtWidgets + +from openlp.core.common import UiStrings, translate +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import build_icon +from openlp.core.ui.lib.filedialogpatches import PQFileDialog + + +class PathType(Enum): + Files = 1 + Directories = 2 + + +class PathEdit(QtWidgets.QWidget): + """ + The :class:`~openlp.core.ui.lib.pathedit.PathEdit` class subclasses QWidget to create a custom widget for use when + a file or directory needs to be selected. + """ + + pathChanged = QtCore.pyqtSignal(Path) + + def __init__(self, parent=None, path_type=PathType.Files, default_path=None, dialog_caption=None, show_revert=True): + """ + Initialise the PathEdit widget + + :param parent: The parent of the widget. This is just passed to the super method. + :type parent: QWidget or None + + :param dialog_caption: Used to customise the caption in the QFileDialog. + :type dialog_caption: str + + :param default_path: The default path. This is set as the path when the revert button is clicked + :type default_path: pathlib.Path + + :param show_revert: Used to determine if the 'revert button' should be visible. + :type show_revert: bool + + :return: None + :rtype: None + """ + super().__init__(parent) + self.default_path = default_path + self.dialog_caption = dialog_caption + self._path_type = path_type + self._path = None + self.filters = '{all_files} (*)'.format(all_files=UiStrings().AllFiles) + self._setup(show_revert) + + def _setup(self, show_revert): + """ + Set up the widget + :param show_revert: Show or hide the revert button + :type show_revert: bool + + :return: None + :rtype: None + """ + widget_layout = QtWidgets.QHBoxLayout() + widget_layout.setContentsMargins(0, 0, 0, 0) + self.line_edit = QtWidgets.QLineEdit(self) + widget_layout.addWidget(self.line_edit) + self.browse_button = QtWidgets.QToolButton(self) + self.browse_button.setIcon(build_icon(':/general/general_open.png')) + widget_layout.addWidget(self.browse_button) + self.revert_button = QtWidgets.QToolButton(self) + self.revert_button.setIcon(build_icon(':/general/general_revert.png')) + self.revert_button.setVisible(show_revert) + widget_layout.addWidget(self.revert_button) + self.setLayout(widget_layout) + # Signals and Slots + self.browse_button.clicked.connect(self.on_browse_button_clicked) + self.revert_button.clicked.connect(self.on_revert_button_clicked) + self.line_edit.editingFinished.connect(self.on_line_edit_editing_finished) + self.update_button_tool_tips() + + @property + def path(self): + """ + A property getter method to return the selected path. + + :return: The selected path + :rtype: pathlib.Path + """ + return self._path + + @path.setter + def path(self, path): + """ + A Property setter method to set the selected path + + :param path: The path to set the widget to + :type path: pathlib.Path + + :return: None + :rtype: None + """ + self._path = path + text = path_to_str(path) + self.line_edit.setText(text) + self.line_edit.setToolTip(text) + + @property + def path_type(self): + """ + A property getter method to return the path_type. Path type allows you to sepecify if the user is restricted to + selecting a file or directory. + + :return: The type selected + :rtype: PathType + """ + return self._path_type + + @path_type.setter + def path_type(self, path_type): + """ + A Property setter method to set the path type + + :param path_type: The type of path to select + :type path_type: PathType + + :return: None + :rtype: None + """ + self._path_type = path_type + self.update_button_tool_tips() + + def update_button_tool_tips(self): + """ + Called to update the tooltips on the buttons. This is changing path types, and when the widget is initalised + + :return: None + :rtype: None + """ + if self._path_type == PathType.Directories: + self.browse_button.setToolTip(translate('OpenLP.PathEdit', 'Browse for directory.')) + self.revert_button.setToolTip(translate('OpenLP.PathEdit', 'Revert to default directory.')) + else: + self.browse_button.setToolTip(translate('OpenLP.PathEdit', 'Browse for file.')) + self.revert_button.setToolTip(translate('OpenLP.PathEdit', 'Revert to default file.')) + + def on_browse_button_clicked(self): + """ + A handler to handle a click on the browse button. + + Show the QFileDialog and process the input from the user + + :return: None + :rtype: None + """ + caption = self.dialog_caption + path = None + if self._path_type == PathType.Directories: + if not caption: + caption = translate('OpenLP.PathEdit', 'Select Directory') + path = PQFileDialog.getExistingDirectory(self, caption, self._path, PQFileDialog.ShowDirsOnly) + elif self._path_type == PathType.Files: + if not caption: + caption = self.dialog_caption = translate('OpenLP.PathEdit', 'Select File') + path, filter_used = PQFileDialog.getOpenFileName(self, caption, self._path, self.filters) + if path: + self.on_new_path(path) + + def on_revert_button_clicked(self): + """ + A handler to handle a click on the revert button. + + Set the new path to the value of the default_path instance variable. + + :return: None + :rtype: None + """ + self.on_new_path(self.default_path) + + def on_line_edit_editing_finished(self): + """ + A handler to handle when the line edit has finished being edited. + + :return: None + :rtype: None + """ + path = str_to_path(self.line_edit.text()) + self.on_new_path(path) + + def on_new_path(self, path): + """ + A method called to validate and set a new path. + + Emits the pathChanged Signal + + :param path: The new path + :type path: pathlib.Path + + :return: None + :rtype: None + """ + if self._path != path: + self.path = path + self.pathChanged.emit(path) From 1c8474b8e2acf65fd22a1efdd3192601f2f5e21c Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 4 Aug 2017 19:06:43 +0100 Subject: [PATCH 3/9] Pep fixes --- openlp/core/ui/lib/filedialogpatches.py | 16 ++++++++-------- tests/functional/openlp_core_common/test_path.py | 2 +- .../openlp_core_ui_lib/test_filedialogpatches.py | 2 -- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/openlp/core/ui/lib/filedialogpatches.py b/openlp/core/ui/lib/filedialogpatches.py index f4b8d60ce..8b8b9a105 100755 --- a/openlp/core/ui/lib/filedialogpatches.py +++ b/openlp/core/ui/lib/filedialogpatches.py @@ -36,10 +36,10 @@ class PQFileDialog(QtWidgets.QFileDialog): """ Reimplement `getExistingDirectory` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget or None :type caption: str :type directory: pathlib.Path - :type options: QtWidgets.QFileDialog.Options + :type options: QtWidgets.QFileDialog.Options :rtype: tuple[Path, str] """ @@ -57,12 +57,12 @@ class PQFileDialog(QtWidgets.QFileDialog): """ Reimplement `getOpenFileName` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget or None :type caption: str :type directory: pathlib.Path :type filter: str :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options + :type options: QtWidgets.QFileDialog.Options :rtype: tuple[Path, str] """ @@ -79,12 +79,12 @@ class PQFileDialog(QtWidgets.QFileDialog): """ Reimplement `getOpenFileNames` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget or None :type caption: str :type directory: pathlib.Path :type filter: str :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options + :type options: QtWidgets.QFileDialog.Options :rtype: tuple[list[Path], str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) @@ -101,12 +101,12 @@ class PQFileDialog(QtWidgets.QFileDialog): """ Reimplement `getSaveFileName` so that it can be called with, and return Path objects - :type parent: QtWidgets.QWidget or None + :type parent: QtWidgets.QWidget or None :type caption: str :type directory: pathlib.Path :type filter: str :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options + :type options: QtWidgets.QFileDialog.Options :rtype: tuple[Path or None, str] """ args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) diff --git a/tests/functional/openlp_core_common/test_path.py b/tests/functional/openlp_core_common/test_path.py index e1d015e6a..2d9490012 100644 --- a/tests/functional/openlp_core_common/test_path.py +++ b/tests/functional/openlp_core_common/test_path.py @@ -85,4 +85,4 @@ class TestPath(TestCase): result = str_to_path('') # THEN: `path_to_str` should return None - self.assertEqual(result, None) \ No newline at end of file + self.assertEqual(result, None) diff --git a/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py b/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py index 2e6fe318c..af5c466d2 100755 --- a/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py +++ b/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py @@ -18,8 +18,6 @@ class TestFileDialogPatches(TestCase): """ Test that the :class:`PQFileDialog` instantiates correctly """ - #app = QtWidgets.QApplication(sys.argv) - # GIVEN: The PQFileDialog class # WHEN: Creating an instance instance = PQFileDialog() From 11a9f58cb6584ba641575bc091e4cdf2b5c6feb2 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 4 Aug 2017 22:53:02 +0100 Subject: [PATCH 4/9] path conversion fixes --- openlp/core/ui/generaltab.py | 5 +++-- openlp/core/ui/themeform.py | 7 +++---- openlp/plugins/presentations/lib/presentationtab.py | 1 + 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/openlp/core/ui/generaltab.py b/openlp/core/ui/generaltab.py index a6760d56e..dc084eb2b 100644 --- a/openlp/core/ui/generaltab.py +++ b/openlp/core/ui/generaltab.py @@ -28,6 +28,7 @@ from pathlib import Path from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, Settings, UiStrings, translate, get_images_filter +from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib import SettingsTab, ScreenList from openlp.core.ui.lib import ColorButton, PathEdit @@ -293,7 +294,7 @@ class GeneralTab(SettingsTab): self.auto_open_check_box.setChecked(settings.value('auto open')) self.show_splash_check_box.setChecked(settings.value('show splash')) self.logo_background_color = settings.value('logo background color') - self.logo_file_path_edit.path = settings.value('logo file') + self.logo_file_path_edit.path = str_to_path(settings.value('logo file')) self.logo_hide_on_startup_check_box.setChecked(settings.value('logo hide on startup')) self.logo_color_button.color = self.logo_background_color self.check_for_updates_check_box.setChecked(settings.value('update check')) @@ -327,7 +328,7 @@ class GeneralTab(SettingsTab): settings.setValue('auto open', self.auto_open_check_box.isChecked()) settings.setValue('show splash', self.show_splash_check_box.isChecked()) settings.setValue('logo background color', self.logo_background_color) - settings.setValue('logo file', str(self.logo_file_path_edit.path)) + settings.setValue('logo file', path_to_str(self.logo_file_path_edit.path)) settings.setValue('logo hide on startup', self.logo_hide_on_startup_check_box.isChecked()) settings.setValue('update check', self.check_for_updates_check_box.isChecked()) settings.setValue('save prompt', self.save_check_service_check_box.isChecked()) diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index 0c36a9131..ed490ab62 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -28,14 +28,13 @@ import os from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, RegistryProperties, UiStrings, translate, get_images_filter, is_not_image_file -from openlp.core.common.path import path_to_str +from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib.theme import BackgroundType, BackgroundGradientType from openlp.core.lib.ui import critical_error_message_box from openlp.core.ui import ThemeLayoutForm from openlp.core.ui.media.webkitplayer import VIDEO_EXT from .themewizard import Ui_ThemeWizard -from pathlib import Path log = logging.getLogger(__name__) @@ -319,11 +318,11 @@ class ThemeForm(QtWidgets.QWizard, Ui_ThemeWizard, RegistryProperties): self.setField('background_type', 1) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Image): self.image_color_button.color = self.theme.background_border_color - self.image_path_edit.path = path_to_str(self.theme.background_filename) + self.image_path_edit.path = str_to_path(self.theme.background_filename) self.setField('background_type', 2) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Video): self.video_color_button.color = self.theme.background_border_color - self.video_path_edit.path = path_to_str(self.theme.background_filename) + self.video_path_edit.path = str_to_path(self.theme.background_filename) self.setField('background_type', 4) elif self.theme.background_type == BackgroundType.to_string(BackgroundType.Transparent): self.setField('background_type', 3) diff --git a/openlp/plugins/presentations/lib/presentationtab.py b/openlp/plugins/presentations/lib/presentationtab.py index f801d54d1..fa804f2d3 100644 --- a/openlp/plugins/presentations/lib/presentationtab.py +++ b/openlp/plugins/presentations/lib/presentationtab.py @@ -224,6 +224,7 @@ class PresentationTab(SettingsTab): """ Select the mudraw or ghostscript binary that should be used. """ + new_path = path_to_str(new_path) if new_path: if not PdfController.process_check_binary(new_path): critical_error_message_box(UiStrings().Error, From e6faf233ed1cb8cd11ee98a490f003f0054f4fe4 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 7 Aug 2017 21:50:01 +0100 Subject: [PATCH 5/9] Merge fixes --- openlp/core/lib/__init__.py | 1 - openlp/core/lib/filedialog.py | 58 ------ openlp/core/lib/mediamanageritem.py | 19 +- openlp/core/ui/lib/filedialogpatches.py | 118 ----------- openlp/core/ui/lib/pathedit.py | 7 +- openlp/core/ui/themeform.py | 1 - openlp/core/ui/thememanager.py | 21 +- openlp/core/ui/themewizard.py | 4 +- openlp/plugins/songs/forms/editsongform.py | 12 +- openlp/plugins/songs/forms/songimportform.py | 10 +- .../openlp_core_lib/test_file_dialog.py | 50 ----- .../test_filedialogpatches.py | 189 ------------------ .../openlp_core_ui_lib/test_pathedit.py | 26 +-- 13 files changed, 55 insertions(+), 461 deletions(-) delete mode 100644 openlp/core/lib/filedialog.py delete mode 100755 openlp/core/ui/lib/filedialogpatches.py delete mode 100755 tests/functional/openlp_core_ui_lib/test_filedialogpatches.py diff --git a/openlp/core/lib/__init__.py b/openlp/core/lib/__init__.py index 86eac88ee..329eb0149 100644 --- a/openlp/core/lib/__init__.py +++ b/openlp/core/lib/__init__.py @@ -644,7 +644,6 @@ def replace_params(args, kwargs, params): from .exceptions import ValidationError -from .filedialog import FileDialog from .screen import ScreenList from .formattingtags import FormattingTags from .plugin import PluginStatus, StringContent, Plugin diff --git a/openlp/core/lib/filedialog.py b/openlp/core/lib/filedialog.py deleted file mode 100644 index 5fd8015e3..000000000 --- a/openlp/core/lib/filedialog.py +++ /dev/null @@ -1,58 +0,0 @@ -# -*- 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 # -############################################################################### -""" -Provide a work around for a bug in QFileDialog -""" -import logging -import os -from urllib import parse - -from PyQt5 import QtWidgets - -from openlp.core.common import UiStrings - -log = logging.getLogger(__name__) - - -class FileDialog(QtWidgets.QFileDialog): - """ - Subclass QFileDialog to work round a bug - """ - @staticmethod - def getOpenFileNames(parent, *args, **kwargs): - """ - Reimplement getOpenFileNames to fix the way it returns some file names that url encoded when selecting multiple - files - """ - files, filter_used = QtWidgets.QFileDialog.getOpenFileNames(parent, *args, **kwargs) - file_list = [] - for file in files: - if not os.path.exists(file): - log.info('File not found. Attempting to unquote.') - file = parse.unquote(file) - if not os.path.exists(file): - log.error('File {text} not found.'.format(text=file)) - QtWidgets.QMessageBox.information(parent, UiStrings().FileNotFound, - UiStrings().FileNotFoundMessage.format(name=file)) - continue - file_list.append(file) - return file_list diff --git a/openlp/core/lib/mediamanageritem.py b/openlp/core/lib/mediamanageritem.py index 0adb471f4..6b04d076b 100644 --- a/openlp/core/lib/mediamanageritem.py +++ b/openlp/core/lib/mediamanageritem.py @@ -26,12 +26,14 @@ import logging import os import re -from PyQt5 import QtCore, QtGui, QtWidgets +from PyQt5 import QtCore, QtWidgets from openlp.core.common import Registry, RegistryProperties, Settings, UiStrings, translate -from openlp.core.lib import FileDialog, ServiceItem, StringContent, ServiceItemContext +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import ServiceItem, StringContent, ServiceItemContext from openlp.core.lib.searchedit import SearchEdit from openlp.core.lib.ui import create_widget_action, critical_error_message_box +from openlp.core.ui.lib.filedialog import FileDialog from openlp.core.ui.lib.listwidgetwithdnd import ListWidgetWithDnD from openlp.core.ui.lib.toolbar import OpenLPToolbar @@ -309,13 +311,14 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties): """ Add a file to the list widget to make it available for showing """ - files = FileDialog.getOpenFileNames(self, self.on_new_prompt, - Settings().value(self.settings_section + '/last directory'), - self.on_new_file_masks) - log.info('New files(s) {files}'.format(files=files)) - if files: + file_paths, selected_filter = FileDialog.getOpenFileNames( + self, self.on_new_prompt, + str_to_path(Settings().value(self.settings_section + '/last directory')), + self.on_new_file_masks) + log.info('New files(s) {file_paths}'.format(file_paths=file_paths)) + if file_paths: self.application.set_busy_cursor() - self.validate_and_load(files) + self.validate_and_load([path_to_str(path) for path in file_paths]) self.application.set_normal_cursor() def load_file(self, data): diff --git a/openlp/core/ui/lib/filedialogpatches.py b/openlp/core/ui/lib/filedialogpatches.py deleted file mode 100755 index 8b8b9a105..000000000 --- a/openlp/core/ui/lib/filedialogpatches.py +++ /dev/null @@ -1,118 +0,0 @@ -# -*- 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 # -############################################################################### -""" Patch the QFileDialog so it accepts and returns Path objects""" -from functools import wraps -from pathlib import Path - -from PyQt5 import QtWidgets - -from openlp.core.common.path import path_to_str, str_to_path -from openlp.core.lib import replace_params - - -class PQFileDialog(QtWidgets.QFileDialog): - @classmethod - @wraps(QtWidgets.QFileDialog.getExistingDirectory) - def getExistingDirectory(cls, *args, **kwargs): - """ - Reimplement `getExistingDirectory` so that it can be called with, and return Path objects - - :type parent: QtWidgets.QWidget or None - :type caption: str - :type directory: pathlib.Path - :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path, str] - """ - - args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) - - return_value = super().getExistingDirectory(*args, **kwargs) - - # getExistingDirectory returns a str that represents the path. The string is empty if the user cancels the - # dialog. - return str_to_path(return_value) - - @classmethod - @wraps(QtWidgets.QFileDialog.getOpenFileName) - def getOpenFileName(cls, *args, **kwargs): - """ - Reimplement `getOpenFileName` so that it can be called with, and return Path objects - - :type parent: QtWidgets.QWidget or None - :type caption: str - :type directory: pathlib.Path - :type filter: str - :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path, str] - """ - - args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) - - return_value = super().getOpenFileName(*args, **kwargs) - - # getOpenFileName returns a tuple. The first item is a of str's that represents the path. The string is empty if - # the user cancels the dialog. - return str_to_path(return_value[0]), return_value[1] - - @classmethod - def getOpenFileNames(cls, *args, **kwargs): - """ - Reimplement `getOpenFileNames` so that it can be called with, and return Path objects - - :type parent: QtWidgets.QWidget or None - :type caption: str - :type directory: pathlib.Path - :type filter: str - :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[list[Path], str] - """ - args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) - - return_value = super().getOpenFileNames(*args, **kwargs) - - # getSaveFileName returns a tuple. The first item is a list of str's that represents the path. The list is - # empty if the user cancels the dialog. - paths = [str_to_path(path) for path in return_value[0]] - return paths, return_value[1] - - @classmethod - def getSaveFileName(cls, *args, **kwargs): - """ - Reimplement `getSaveFileName` so that it can be called with, and return Path objects - - :type parent: QtWidgets.QWidget or None - :type caption: str - :type directory: pathlib.Path - :type filter: str - :type initialFilter: str - :type options: QtWidgets.QFileDialog.Options - :rtype: tuple[Path or None, str] - """ - args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) - - return_value = super().getSaveFileName(*args, **kwargs) - - # getSaveFileName returns a tuple. The first item represents the path as a str. The string is empty if the user - # cancels the dialog. - return str_to_path(return_value[0]), return_value[1] diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index abcd56fef..dd6066931 100644 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -27,7 +27,7 @@ from PyQt5 import QtCore, QtWidgets from openlp.core.common import UiStrings, translate from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib import build_icon -from openlp.core.ui.lib.filedialogpatches import PQFileDialog +from openlp.core.ui.lib.filedialog import FileDialog class PathType(Enum): @@ -40,7 +40,6 @@ class PathEdit(QtWidgets.QWidget): The :class:`~openlp.core.ui.lib.pathedit.PathEdit` class subclasses QWidget to create a custom widget for use when a file or directory needs to be selected. """ - pathChanged = QtCore.pyqtSignal(Path) def __init__(self, parent=None, path_type=PathType.Files, default_path=None, dialog_caption=None, show_revert=True): @@ -176,11 +175,11 @@ class PathEdit(QtWidgets.QWidget): if self._path_type == PathType.Directories: if not caption: caption = translate('OpenLP.PathEdit', 'Select Directory') - path = PQFileDialog.getExistingDirectory(self, caption, self._path, PQFileDialog.ShowDirsOnly) + path = FileDialog.getExistingDirectory(self, caption, self._path, FileDialog.ShowDirsOnly) elif self._path_type == PathType.Files: if not caption: caption = self.dialog_caption = translate('OpenLP.PathEdit', 'Select File') - path, filter_used = PQFileDialog.getOpenFileName(self, caption, self._path, self.filters) + path, filter_used = FileDialog.getOpenFileName(self, caption, self._path, self.filters) if path: self.on_new_path(path) diff --git a/openlp/core/ui/themeform.py b/openlp/core/ui/themeform.py index ed490ab62..bb471d0f1 100644 --- a/openlp/core/ui/themeform.py +++ b/openlp/core/ui/themeform.py @@ -35,7 +35,6 @@ from openlp.core.ui import ThemeLayoutForm from openlp.core.ui.media.webkitplayer import VIDEO_EXT from .themewizard import Ui_ThemeWizard - log = logging.getLogger(__name__) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index b1033646f..d7338db91 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -22,7 +22,6 @@ """ The Theme Manager manages adding, deleteing and modifying of themes. """ -import json import os import zipfile import shutil @@ -32,12 +31,14 @@ from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ UiStrings, check_directory_exists, translate, is_win, get_filesystem_encoding, delete_file -from openlp.core.lib import FileDialog, ImageSource, ValidationError, get_text_file_string, build_icon, \ +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import ImageSource, ValidationError, get_text_file_string, build_icon, \ check_item_selected, create_thumb, validate_thumb from openlp.core.lib.theme import Theme, BackgroundType from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.ui import FileRenameForm, ThemeForm from openlp.core.ui.lib import OpenLPToolbar +from openlp.core.ui.lib.filedialog import FileDialog from openlp.core.common.languagemanager import get_locale_key @@ -424,15 +425,17 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage those files. This process will only load version 2 themes. :param field: """ - files = FileDialog.getOpenFileNames(self, - translate('OpenLP.ThemeManager', 'Select Theme Import File'), - Settings().value(self.settings_section + '/last directory import'), - translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) - self.log_info('New Themes {name}'.format(name=str(files))) - if not files: + file_paths, selected_filter = FileDialog.getOpenFileNames( + self, + translate('OpenLP.ThemeManager', 'Select Theme Import File'), + str_to_path(Settings().value(self.settings_section + '/last directory import')), + translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) + self.log_info('New Themes {file_paths}'.format(file_paths=file_paths)) + if not file_paths: return self.application.set_busy_cursor() - for file_name in files: + for file_path in file_paths: + file_name = path_to_str(file_path) Settings().setValue(self.settings_section + '/last directory import', str(file_name)) self.unzip_theme(file_name, self.path) self.load_themes() diff --git a/openlp/core/ui/themewizard.py b/openlp/core/ui/themewizard.py index 3a1eba9f0..cd1748aae 100644 --- a/openlp/core/ui/themewizard.py +++ b/openlp/core/ui/themewizard.py @@ -22,6 +22,8 @@ """ The Create/Edit theme wizard """ +from pathlib import Path + from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import UiStrings, translate, is_macosx @@ -30,8 +32,6 @@ from openlp.core.lib.theme import HorizontalType, BackgroundType, BackgroundGrad from openlp.core.lib.ui import add_welcome_page, create_valign_selection_widgets from openlp.core.ui.lib import ColorButton, PathEdit -from pathlib import Path - class Ui_ThemeWizard(object): """ diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 61d5b1ddd..77573c5ac 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -28,12 +28,15 @@ import logging import re import os import shutil +from pathlib import Path from PyQt5 import QtCore, QtWidgets from openlp.core.common import Registry, RegistryProperties, AppLocation, UiStrings, check_directory_exists, translate -from openlp.core.lib import FileDialog, PluginStatus, MediaType, create_separated_list +from openlp.core.common.path import path_to_str +from openlp.core.lib import PluginStatus, MediaType, create_separated_list from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box +from openlp.core.ui.lib.filedialog import FileDialog from openlp.core.common.languagemanager import get_natural_key from openlp.plugins.songs.lib import VerseType, clean_song from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile, SongBookEntry @@ -925,9 +928,10 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): Loads file(s) from the filesystem. """ filters = '{text} (*)'.format(text=UiStrings().AllFiles) - file_names = FileDialog.getOpenFileNames(self, translate('SongsPlugin.EditSongForm', 'Open File(s)'), '', - filters) - for filename in file_names: + file_paths, selected_filter = FileDialog.getOpenFileNames( + self, translate('SongsPlugin.EditSongForm', 'Open File(s)'), Path(), filters) + for file_path in file_paths: + filename = path_to_str(file_path) item = QtWidgets.QListWidgetItem(os.path.split(str(filename))[1]) item.setData(QtCore.Qt.UserRole, filename) self.audio_list_widget.addItem(item) diff --git a/openlp/plugins/songs/forms/songimportform.py b/openlp/plugins/songs/forms/songimportform.py index 3547521c9..e3c0eb620 100644 --- a/openlp/plugins/songs/forms/songimportform.py +++ b/openlp/plugins/songs/forms/songimportform.py @@ -29,8 +29,9 @@ import os from PyQt5 import QtCore, QtWidgets from openlp.core.common import RegistryProperties, Settings, UiStrings, translate -from openlp.core.lib import FileDialog +from openlp.core.common.path import path_to_str, str_to_path from openlp.core.lib.ui import critical_error_message_box +from openlp.core.ui.lib.filedialog import FileDialog from openlp.core.ui.lib.wizard import OpenLPWizard, WizardStrings from openlp.plugins.songs.lib.importer import SongFormat, SongFormatSelect @@ -237,10 +238,11 @@ class SongImportForm(OpenLPWizard, RegistryProperties): if filters: filters += ';;' filters += '{text} (*)'.format(text=UiStrings().AllFiles) - file_names = FileDialog.getOpenFileNames( + file_paths, selected_filter = FileDialog.getOpenFileNames( self, title, - Settings().value(self.plugin.settings_section + '/last directory import'), filters) - if file_names: + str_to_path(Settings().value(self.plugin.settings_section + '/last directory import')), filters) + if file_paths: + file_names = [path_to_str(file_path) for file_path in file_paths] listbox.addItems(file_names) Settings().setValue(self.plugin.settings_section + '/last directory import', os.path.split(str(file_names[0]))[0]) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py index 488900fc7..13d833fcc 100644 --- a/tests/functional/openlp_core_lib/test_file_dialog.py +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -25,7 +25,6 @@ Package to test the openlp.core.lib.filedialog package. from unittest import TestCase from unittest.mock import MagicMock, call, patch -from openlp.core.lib.filedialog import FileDialog class TestFileDialog(TestCase): @@ -45,52 +44,3 @@ class TestFileDialog(TestCase): self.os_patcher.stop() self.qt_gui_patcher.stop() self.ui_strings_patcher.stop() - - def test_get_open_file_names_canceled(self): - """ - Test that FileDialog.getOpenFileNames() returns and empty QStringList when QFileDialog is canceled - (returns an empty QStringList) - """ - self.mocked_os.reset_mock() - - # GIVEN: An empty QStringList as a return value from QFileDialog.getOpenFileNames - self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = ([], []) - - # WHEN: FileDialog.getOpenFileNames is called - result = FileDialog.getOpenFileNames(self.mocked_parent) - - # THEN: The returned value should be an empty QStringList and os.path.exists should not have been called - assert not self.mocked_os.path.exists.called - self.assertEqual(result, [], - 'FileDialog.getOpenFileNames should return and empty list when QFileDialog.getOpenFileNames ' - 'is canceled') - - def test_returned_file_list(self): - """ - Test that FileDialog.getOpenFileNames handles a list of files properly when QFileList.getOpenFileNames - returns a good file name, a url encoded file name and a non-existing file - """ - self.mocked_os.rest_mock() - self.mocked_qt_gui.reset_mock() - - # GIVEN: A List of known values as a return value from QFileDialog.getOpenFileNames and a list of valid file - # names. - self.mocked_qt_gui.QFileDialog.getOpenFileNames.return_value = ([ - '/Valid File', '/url%20encoded%20file%20%231', '/non-existing'], []) - self.mocked_os.path.exists.side_effect = lambda file_name: file_name in [ - '/Valid File', '/url encoded file #1'] - self.mocked_ui_strings().FileNotFound = 'File Not Found' - self.mocked_ui_strings().FileNotFoundMessage = 'File {name} not found.\nPlease try selecting it individually.' - - # WHEN: FileDialog.getOpenFileNames is called - result = FileDialog.getOpenFileNames(self.mocked_parent) - - # THEN: os.path.exists should have been called with known args. QmessageBox.information should have been - # called. The returned result should correlate with the input. - call_list = [call('/Valid File'), call('/url%20encoded%20file%20%231'), call('/url encoded file #1'), - call('/non-existing'), call('/non-existing')] - self.mocked_os.path.exists.assert_has_calls(call_list) - self.mocked_qt_gui.QMessageBox.information.assert_called_with( - self.mocked_parent, 'File Not Found', - 'File /non-existing not found.\nPlease try selecting it individually.') - self.assertEqual(result, ['/Valid File', '/url encoded file #1'], 'The returned file list is incorrect') diff --git a/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py b/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py deleted file mode 100755 index af5c466d2..000000000 --- a/tests/functional/openlp_core_ui_lib/test_filedialogpatches.py +++ /dev/null @@ -1,189 +0,0 @@ -import os -import sys -from unittest import TestCase -from unittest.mock import patch -from pathlib import Path - -from PyQt5 import QtWidgets - -from openlp.core.ui.lib.filedialogpatches import PQFileDialog - - -class TestFileDialogPatches(TestCase): - """ - Tests for the :mod:`openlp.core.ui.lib.filedialogpatches` module - """ - - def test_pq_file_dialog(self): - """ - Test that the :class:`PQFileDialog` instantiates correctly - """ - # GIVEN: The PQFileDialog class - # WHEN: Creating an instance - instance = PQFileDialog() - - # THEN: The instance should be an instance of QFileDialog - self.assertIsInstance(instance, QtWidgets.QFileDialog) - - def test_get_existing_directory_user_abort(self): - """ - Test that `getExistingDirectory` handles the case when the user cancels the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getExistingDirectory method - # WHEN: Calling PQFileDialog.getExistingDirectory and the user cancels the dialog returns a empty string - with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=''): - result = PQFileDialog.getExistingDirectory() - - # THEN: The result should be None - self.assertEqual(result, None) - - def test_get_existing_directory_user_accepts(self): - """ - Test that `getExistingDirectory` handles the case when the user accepts the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getExistingDirectory method - # WHEN: Calling PQFileDialog.getExistingDirectory, the user chooses a file and accepts the dialog (it returns a - # string pointing to the directory) - with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=os.path.join('test', 'dir')): - result = PQFileDialog.getExistingDirectory() - - # THEN: getExistingDirectory() should return a Path object pointing to the chosen file - self.assertEqual(result, Path('test', 'dir')) - - def test_get_existing_directory_param_order(self): - """ - Test that `getExistingDirectory` passes the parameters to `QFileDialog.getExistingDirectory` in the correct - order - """ - # GIVEN: PQFileDialog - with patch('openlp.core.ui.lib.filedialogpatches.QtWidgets.QFileDialog.getExistingDirectory', return_value='') \ - as mocked_get_existing_directory: - - # WHEN: Calling the getExistingDirectory method with all parameters set - PQFileDialog.getExistingDirectory('Parent', 'Caption', Path('test', 'dir'), 'Options') - - # THEN: The `QFileDialog.getExistingDirectory` should have been called with the parameters in the correct - # order - mocked_get_existing_directory.assert_called_once_with('Parent', 'Caption', os.path.join('test', 'dir'), - 'Options') - - def test_get_open_file_name_user_abort(self): - """ - Test that `getOpenFileName` handles the case when the user cancels the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileName method - # WHEN: Calling PQFileDialog.getOpenFileName and the user cancels the dialog (it returns a tuple with the first - # value set as an empty string) - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')): - result = PQFileDialog.getOpenFileName() - - # THEN: First value should be None - self.assertEqual(result[0], None) - - def test_get_open_file_name_user_accepts(self): - """ - Test that `getOpenFileName` handles the case when the user accepts the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileName method - # WHEN: Calling PQFileDialog.getOpenFileName, the user chooses a file and accepts the dialog (it returns a - # tuple with the first value set as an string pointing to the file) - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', - return_value=(os.path.join('test', 'chosen.file'), '')): - result = PQFileDialog.getOpenFileName() - - # THEN: getOpenFileName() should return a tuple with the first value set to a Path object pointing to the - # chosen file - self.assertEqual(result[0], Path('test', 'chosen.file')) - - def test_get_open_file_name_selected_filter(self): - """ - Test that `getOpenFileName` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileName` - """ - # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method - # WHEN: Calling PQFileDialog.getOpenFileName, and `QFileDialog.getOpenFileName` returns a known `selectedFilter` - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', 'selected filter')): - result = PQFileDialog.getOpenFileName() - - # THEN: getOpenFileName() should return a tuple with the second value set to a the selected filter - self.assertEqual(result[1], 'selected filter') - - def test_get_open_file_names_user_abort(self): - """ - Test that `getOpenFileNames` handles the case when the user cancels the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method - # WHEN: Calling PQFileDialog.getOpenFileNames and the user cancels the dialog (it returns a tuple with the first - # value set as an empty list) - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], '')): - result = PQFileDialog.getOpenFileNames() - - # THEN: First value should be an empty list - self.assertEqual(result[0], []) - - def test_get_open_file_names_user_accepts(self): - """ - Test that `getOpenFileNames` handles the case when the user accepts the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method - # WHEN: Calling PQFileDialog.getOpenFileNames, the user chooses some files and accepts the dialog (it returns a - # tuple with the first value set as a list of strings pointing to the file) - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', - return_value=([os.path.join('test', 'chosen.file1'), os.path.join('test', 'chosen.file2')], '')): - result = PQFileDialog.getOpenFileNames() - - # THEN: getOpenFileNames() should return a tuple with the first value set to a list of Path objects pointing - # to the chosen file - self.assertEqual(result[0], [Path('test', 'chosen.file1'), Path('test', 'chosen.file2')]) - - def test_get_open_file_names_selected_filter(self): - """ - Test that `getOpenFileNames` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileNames` - """ - # GIVEN: PQFileDialog with a mocked QDialog.getOpenFileNames method - # WHEN: Calling PQFileDialog.getOpenFileNames, and `QFileDialog.getOpenFileNames` returns a known - # `selectedFilter` - with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], 'selected filter')): - result = PQFileDialog.getOpenFileNames() - - # THEN: getOpenFileNames() should return a tuple with the second value set to a the selected filter - self.assertEqual(result[1], 'selected filter') - - def test_get_save_file_name_user_abort(self): - """ - Test that `getSaveFileName` handles the case when the user cancels the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method - # WHEN: Calling PQFileDialog.getSaveFileName and the user cancels the dialog (it returns a tuple with the first - # value set as an empty string) - with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', '')): - result = PQFileDialog.getSaveFileName() - - # THEN: First value should be None - self.assertEqual(result[0], None) - - def test_get_save_file_name_user_accepts(self): - """ - Test that `getSaveFileName` handles the case when the user accepts the dialog - """ - # GIVEN: PQFileDialog with a mocked QDialog.getSaveFileName method - # WHEN: Calling PQFileDialog.getSaveFileName, the user chooses a file and accepts the dialog (it returns a - # tuple with the first value set as an string pointing to the file) - with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', - return_value=(os.path.join('test', 'chosen.file'), '')): - result = PQFileDialog.getSaveFileName() - - # THEN: getSaveFileName() should return a tuple with the first value set to a Path object pointing to the - # chosen file - self.assertEqual(result[0], Path('test', 'chosen.file')) - - def test_get_save_file_name_selected_filter(self): - """ - Test that `getSaveFileName` does not modify the selectedFilter as returned by `QFileDialog.getSaveFileName` - """ - # GIVEN: PQFileDialog with a mocked QDialog.get_save_file_name method - # WHEN: Calling PQFileDialog.getSaveFileName, and `QFileDialog.getSaveFileName` returns a known `selectedFilter` - with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', 'selected filter')): - result = PQFileDialog.getSaveFileName() - - # THEN: getSaveFileName() should return a tuple with the second value set to a the selected filter - self.assertEqual(result[1], 'selected filter') diff --git a/tests/functional/openlp_core_ui_lib/test_pathedit.py b/tests/functional/openlp_core_ui_lib/test_pathedit.py index e4ce99c41..9ef4dff4b 100755 --- a/tests/functional/openlp_core_ui_lib/test_pathedit.py +++ b/tests/functional/openlp_core_ui_lib/test_pathedit.py @@ -28,7 +28,7 @@ from unittest import TestCase from unittest.mock import MagicMock, PropertyMock, patch from openlp.core.ui.lib import PathEdit, PathType -from openlp.core.ui.lib.filedialogpatches import PQFileDialog +from openlp.core.ui.lib.filedialog import FileDialog class TestPathEdit(TestCase): @@ -126,9 +126,9 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Directories` and a mocked # QFileDialog.getExistingDirectory - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory', return_value=None) as \ + with patch('openlp.core.ui.lib.pathedit.FileDialog.getExistingDirectory', return_value=None) as \ mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName') as mocked_get_open_file_name: + patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName') as mocked_get_open_file_name: self.widget._path_type = PathType.Directories self.widget._path = Path('test', 'path') @@ -138,7 +138,7 @@ class TestPathEdit(TestCase): # THEN: The FileDialog.getExistingDirectory should have been called with the default caption mocked_get_existing_directory.assert_called_once_with(self.widget, 'Select Directory', Path('test', 'path'), - PQFileDialog.ShowDirsOnly) + FileDialog.ShowDirsOnly) self.assertFalse(mocked_get_open_file_name.called) def test_on_browse_button_clicked_directory_custom_caption(self): @@ -148,9 +148,9 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Directories` and a mocked # QFileDialog.getExistingDirectory with `default_caption` set. - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory', return_value=None) as \ + with patch('openlp.core.ui.lib.pathedit.FileDialog.getExistingDirectory', return_value=None) as \ mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName') as mocked_get_open_file_name: + patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName') as mocked_get_open_file_name: self.widget._path_type = PathType.Directories self.widget._path = Path('test', 'path') self.widget.dialog_caption = 'Directory Caption' @@ -161,7 +161,7 @@ class TestPathEdit(TestCase): # THEN: The FileDialog.getExistingDirectory should have been called with the custom caption mocked_get_existing_directory.assert_called_once_with(self.widget, 'Directory Caption', Path('test', 'path'), - PQFileDialog.ShowDirsOnly) + FileDialog.ShowDirsOnly) self.assertFalse(mocked_get_open_file_name.called) def test_on_browse_button_clicked_file(self): @@ -169,8 +169,8 @@ class TestPathEdit(TestCase): Test the `browse_button` `clicked` handler on_browse_button_clicked when the `path_type` is set to Files. """ # GIVEN: An instance of PathEdit with the `path_type` set to `Files` and a mocked QFileDialog.getOpenFileName - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory') as mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + with patch('openlp.core.ui.lib.pathedit.FileDialog.getExistingDirectory') as mocked_get_existing_directory, \ + patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName', return_value=(None, '')) as \ mocked_get_open_file_name: self.widget._path_type = PathType.Files self.widget._path = Path('test', 'pat.h') @@ -190,8 +190,8 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with the `path_type` set to `Files` and a mocked QFileDialog.getOpenFileName # with `default_caption` set. - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getExistingDirectory') as mocked_get_existing_directory, \ - patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + with patch('openlp.core.ui.lib.pathedit.FileDialog.getExistingDirectory') as mocked_get_existing_directory, \ + patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName', return_value=(None, '')) as \ mocked_get_open_file_name: self.widget._path_type = PathType.Files self.widget._path = Path('test', 'pat.h') @@ -212,7 +212,7 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked QFileDialog.getOpenFileName which returns an empty str for the # file path. - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', return_value=(None, '')) as \ + with patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName', return_value=(None, '')) as \ mocked_get_open_file_name: # WHEN: Calling on_browse_button_clicked @@ -228,7 +228,7 @@ class TestPathEdit(TestCase): """ # GIVEN: An instance of PathEdit with a mocked QFileDialog.getOpenFileName which returns a str for the file # path. - with patch('openlp.core.ui.lib.pathedit.PQFileDialog.getOpenFileName', + with patch('openlp.core.ui.lib.pathedit.FileDialog.getOpenFileName', return_value=(Path('test', 'pat.h'), '')) as mocked_get_open_file_name, \ patch.object(self.widget, 'on_new_path'): From c4eedc6dcadc39133eb5e33fbf88d957796bf341 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 7 Aug 2017 22:01:16 +0100 Subject: [PATCH 6/9] add forgotten files --- openlp/core/ui/lib/filedialog.py | 116 +++++++++++ .../openlp_core_ui_lib/test_filedialog.py | 188 ++++++++++++++++++ 2 files changed, 304 insertions(+) create mode 100755 openlp/core/ui/lib/filedialog.py create mode 100755 tests/functional/openlp_core_ui_lib/test_filedialog.py diff --git a/openlp/core/ui/lib/filedialog.py b/openlp/core/ui/lib/filedialog.py new file mode 100755 index 000000000..8159078e0 --- /dev/null +++ b/openlp/core/ui/lib/filedialog.py @@ -0,0 +1,116 @@ +# -*- 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 # +############################################################################### +""" Patch the QFileDialog so it accepts and returns Path objects""" +from functools import wraps +from pathlib import Path + +from PyQt5 import QtWidgets + +from openlp.core.common.path import path_to_str, str_to_path +from openlp.core.lib import replace_params + + +class FileDialog(QtWidgets.QFileDialog): + @classmethod + @wraps(QtWidgets.QFileDialog.getExistingDirectory) + def getExistingDirectory(cls, *args, **kwargs): + """ + Reimplement `getExistingDirectory` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path, str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + return_value = super().getExistingDirectory(*args, **kwargs) + + # getExistingDirectory returns a str that represents the path. The string is empty if the user cancels the + # dialog. + return str_to_path(return_value) + + @classmethod + @wraps(QtWidgets.QFileDialog.getOpenFileName) + def getOpenFileName(cls, *args, **kwargs): + """ + Reimplement `getOpenFileName` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path, str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + file_name, selected_filter = super().getOpenFileName(*args, **kwargs) + + # getOpenFileName returns a tuple. The first item is a str that represents the path. The string is empty if + # the user cancels the dialog. + return str_to_path(file_name), selected_filter + + @classmethod + def getOpenFileNames(cls, *args, **kwargs): + """ + Reimplement `getOpenFileNames` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[list[Path], str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + file_names, selected_filter = super().getOpenFileNames(*args, **kwargs) + + # getSaveFileName returns a tuple. The first item is a list of str's that represents the path. The list is + # empty if the user cancels the dialog. + paths = [str_to_path(path) for path in file_names] + return paths, selected_filter + + @classmethod + def getSaveFileName(cls, *args, **kwargs): + """ + Reimplement `getSaveFileName` so that it can be called with, and return Path objects + + :type parent: QtWidgets.QWidget or None + :type caption: str + :type directory: pathlib.Path + :type filter: str + :type initialFilter: str + :type options: QtWidgets.QFileDialog.Options + :rtype: tuple[Path or None, str] + """ + args, kwargs = replace_params(args, kwargs, ((2, 'directory', path_to_str),)) + + file_name, selected_filter = super().getSaveFileName(*args, **kwargs) + + # getSaveFileName returns a tuple. The first item represents the path as a str. The string is empty if the user + # cancels the dialog. + return str_to_path(file_name), selected_filter diff --git a/tests/functional/openlp_core_ui_lib/test_filedialog.py b/tests/functional/openlp_core_ui_lib/test_filedialog.py new file mode 100755 index 000000000..6ec045d47 --- /dev/null +++ b/tests/functional/openlp_core_ui_lib/test_filedialog.py @@ -0,0 +1,188 @@ +import os +from unittest import TestCase +from unittest.mock import patch +from pathlib import Path + +from PyQt5 import QtWidgets + +from openlp.core.ui.lib.filedialog import FileDialog + + +class TestFileDialogPatches(TestCase): + """ + Tests for the :mod:`openlp.core.ui.lib.filedialogpatches` module + """ + + def test_file_dialog(self): + """ + Test that the :class:`FileDialog` instantiates correctly + """ + # GIVEN: The FileDialog class + # WHEN: Creating an instance + instance = FileDialog() + + # THEN: The instance should be an instance of QFileDialog + self.assertIsInstance(instance, QtWidgets.QFileDialog) + + def test_get_existing_directory_user_abort(self): + """ + Test that `getExistingDirectory` handles the case when the user cancels the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getExistingDirectory method + # WHEN: Calling FileDialog.getExistingDirectory and the user cancels the dialog returns a empty string + with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=''): + result = FileDialog.getExistingDirectory() + + # THEN: The result should be None + self.assertEqual(result, None) + + def test_get_existing_directory_user_accepts(self): + """ + Test that `getExistingDirectory` handles the case when the user accepts the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getExistingDirectory method + # WHEN: Calling FileDialog.getExistingDirectory, the user chooses a file and accepts the dialog (it returns a + # string pointing to the directory) + with patch('PyQt5.QtWidgets.QFileDialog.getExistingDirectory', return_value=os.path.join('test', 'dir')): + result = FileDialog.getExistingDirectory() + + # THEN: getExistingDirectory() should return a Path object pointing to the chosen file + self.assertEqual(result, Path('test', 'dir')) + + def test_get_existing_directory_param_order(self): + """ + Test that `getExistingDirectory` passes the parameters to `QFileDialog.getExistingDirectory` in the correct + order + """ + # GIVEN: FileDialog + with patch('openlp.core.ui.lib.filedialog.QtWidgets.QFileDialog.getExistingDirectory', return_value='') \ + as mocked_get_existing_directory: + + # WHEN: Calling the getExistingDirectory method with all parameters set + FileDialog.getExistingDirectory('Parent', 'Caption', Path('test', 'dir'), 'Options') + + # THEN: The `QFileDialog.getExistingDirectory` should have been called with the parameters in the correct + # order + mocked_get_existing_directory.assert_called_once_with('Parent', 'Caption', os.path.join('test', 'dir'), + 'Options') + + def test_get_open_file_name_user_abort(self): + """ + Test that `getOpenFileName` handles the case when the user cancels the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getOpenFileName method + # WHEN: Calling FileDialog.getOpenFileName and the user cancels the dialog (it returns a tuple with the first + # value set as an empty string) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', '')): + result = FileDialog.getOpenFileName() + + # THEN: First value should be None + self.assertEqual(result[0], None) + + def test_get_open_file_name_user_accepts(self): + """ + Test that `getOpenFileName` handles the case when the user accepts the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getOpenFileName method + # WHEN: Calling FileDialog.getOpenFileName, the user chooses a file and accepts the dialog (it returns a + # tuple with the first value set as an string pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', + return_value=(os.path.join('test', 'chosen.file'), '')): + result = FileDialog.getOpenFileName() + + # THEN: getOpenFileName() should return a tuple with the first value set to a Path object pointing to the + # chosen file + self.assertEqual(result[0], Path('test', 'chosen.file')) + + def test_get_open_file_name_selected_filter(self): + """ + Test that `getOpenFileName` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileName` + """ + # GIVEN: FileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling FileDialog.getOpenFileName, and `QFileDialog.getOpenFileName` returns a known `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileName', return_value=('', 'selected filter')): + result = FileDialog.getOpenFileName() + + # THEN: getOpenFileName() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') + + def test_get_open_file_names_user_abort(self): + """ + Test that `getOpenFileNames` handles the case when the user cancels the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling FileDialog.getOpenFileNames and the user cancels the dialog (it returns a tuple with the first + # value set as an empty list) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], '')): + result = FileDialog.getOpenFileNames() + + # THEN: First value should be an empty list + self.assertEqual(result[0], []) + + def test_get_open_file_names_user_accepts(self): + """ + Test that `getOpenFileNames` handles the case when the user accepts the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling FileDialog.getOpenFileNames, the user chooses some files and accepts the dialog (it returns a + # tuple with the first value set as a list of strings pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', + return_value=([os.path.join('test', 'chosen.file1'), os.path.join('test', 'chosen.file2')], '')): + result = FileDialog.getOpenFileNames() + + # THEN: getOpenFileNames() should return a tuple with the first value set to a list of Path objects pointing + # to the chosen file + self.assertEqual(result[0], [Path('test', 'chosen.file1'), Path('test', 'chosen.file2')]) + + def test_get_open_file_names_selected_filter(self): + """ + Test that `getOpenFileNames` does not modify the selectedFilter as returned by `QFileDialog.getOpenFileNames` + """ + # GIVEN: FileDialog with a mocked QDialog.getOpenFileNames method + # WHEN: Calling FileDialog.getOpenFileNames, and `QFileDialog.getOpenFileNames` returns a known + # `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getOpenFileNames', return_value=([], 'selected filter')): + result = FileDialog.getOpenFileNames() + + # THEN: getOpenFileNames() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') + + def test_get_save_file_name_user_abort(self): + """ + Test that `getSaveFileName` handles the case when the user cancels the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling FileDialog.getSaveFileName and the user cancels the dialog (it returns a tuple with the first + # value set as an empty string) + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', '')): + result = FileDialog.getSaveFileName() + + # THEN: First value should be None + self.assertEqual(result[0], None) + + def test_get_save_file_name_user_accepts(self): + """ + Test that `getSaveFileName` handles the case when the user accepts the dialog + """ + # GIVEN: FileDialog with a mocked QDialog.getSaveFileName method + # WHEN: Calling FileDialog.getSaveFileName, the user chooses a file and accepts the dialog (it returns a + # tuple with the first value set as an string pointing to the file) + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', + return_value=(os.path.join('test', 'chosen.file'), '')): + result = FileDialog.getSaveFileName() + + # THEN: getSaveFileName() should return a tuple with the first value set to a Path object pointing to the + # chosen file + self.assertEqual(result[0], Path('test', 'chosen.file')) + + def test_get_save_file_name_selected_filter(self): + """ + Test that `getSaveFileName` does not modify the selectedFilter as returned by `QFileDialog.getSaveFileName` + """ + # GIVEN: FileDialog with a mocked QDialog.get_save_file_name method + # WHEN: Calling FileDialog.getSaveFileName, and `QFileDialog.getSaveFileName` returns a known `selectedFilter` + with patch('PyQt5.QtWidgets.QFileDialog.getSaveFileName', return_value=('', 'selected filter')): + result = FileDialog.getSaveFileName() + + # THEN: getSaveFileName() should return a tuple with the second value set to a the selected filter + self.assertEqual(result[1], 'selected filter') From 6340b7e89ffa57ce2ba6e77515fb3cc6e4f9d6d3 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 7 Aug 2017 22:12:42 +0100 Subject: [PATCH 7/9] pep --- tests/functional/openlp_core_lib/test_file_dialog.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_file_dialog.py b/tests/functional/openlp_core_lib/test_file_dialog.py index 13d833fcc..6336ce0a0 100644 --- a/tests/functional/openlp_core_lib/test_file_dialog.py +++ b/tests/functional/openlp_core_lib/test_file_dialog.py @@ -20,11 +20,10 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### """ -Package to test the openlp.core.lib.filedialog package. +Package to test the openlp.core.ui.lib.filedialog package. """ from unittest import TestCase -from unittest.mock import MagicMock, call, patch - +from unittest.mock import MagicMock, patch class TestFileDialog(TestCase): @@ -32,9 +31,9 @@ class TestFileDialog(TestCase): Test the functions in the :mod:`filedialog` module. """ def setUp(self): - self.os_patcher = patch('openlp.core.lib.filedialog.os') - self.qt_gui_patcher = patch('openlp.core.lib.filedialog.QtWidgets') - self.ui_strings_patcher = patch('openlp.core.lib.filedialog.UiStrings') + self.os_patcher = patch('openlp.core.ui.lib.filedialog.os') + self.qt_gui_patcher = patch('openlp.core.ui.lib.filedialog.QtWidgets') + self.ui_strings_patcher = patch('openlp.core.ui.lib.filedialog.UiStrings') self.mocked_os = self.os_patcher.start() self.mocked_qt_gui = self.qt_gui_patcher.start() self.mocked_ui_strings = self.ui_strings_patcher.start() From 469e7f3bb65056c7a95dd93c458aeaeb9b9cf394 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Wed, 9 Aug 2017 21:15:02 +0100 Subject: [PATCH 8/9] remove the wraps decoartor --- openlp/core/ui/lib/filedialog.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/openlp/core/ui/lib/filedialog.py b/openlp/core/ui/lib/filedialog.py index 8159078e0..379b34177 100755 --- a/openlp/core/ui/lib/filedialog.py +++ b/openlp/core/ui/lib/filedialog.py @@ -31,10 +31,9 @@ from openlp.core.lib import replace_params class FileDialog(QtWidgets.QFileDialog): @classmethod - @wraps(QtWidgets.QFileDialog.getExistingDirectory) def getExistingDirectory(cls, *args, **kwargs): """ - Reimplement `getExistingDirectory` so that it can be called with, and return Path objects + Wraps `getExistingDirectory` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str @@ -51,10 +50,9 @@ class FileDialog(QtWidgets.QFileDialog): return str_to_path(return_value) @classmethod - @wraps(QtWidgets.QFileDialog.getOpenFileName) def getOpenFileName(cls, *args, **kwargs): """ - Reimplement `getOpenFileName` so that it can be called with, and return Path objects + Wraps `getOpenFileName` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str @@ -75,7 +73,7 @@ class FileDialog(QtWidgets.QFileDialog): @classmethod def getOpenFileNames(cls, *args, **kwargs): """ - Reimplement `getOpenFileNames` so that it can be called with, and return Path objects + Wrap `getOpenFileNames` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str @@ -97,7 +95,7 @@ class FileDialog(QtWidgets.QFileDialog): @classmethod def getSaveFileName(cls, *args, **kwargs): """ - Reimplement `getSaveFileName` so that it can be called with, and return Path objects + Wrap `getSaveFileName` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str From 5d99c48fe0608dd2216fe5f465dc323aa11e9c04 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Thu, 10 Aug 2017 07:28:30 +0100 Subject: [PATCH 9/9] Merge fix --- openlp/core/ui/lib/filedialog.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openlp/core/ui/lib/filedialog.py b/openlp/core/ui/lib/filedialog.py index 379b34177..bd2bb5109 100755 --- a/openlp/core/ui/lib/filedialog.py +++ b/openlp/core/ui/lib/filedialog.py @@ -20,7 +20,6 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### """ Patch the QFileDialog so it accepts and returns Path objects""" -from functools import wraps from pathlib import Path from PyQt5 import QtWidgets @@ -73,7 +72,7 @@ class FileDialog(QtWidgets.QFileDialog): @classmethod def getOpenFileNames(cls, *args, **kwargs): """ - Wrap `getOpenFileNames` so that it can be called with, and return Path objects + Wraps `getOpenFileNames` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str @@ -95,7 +94,7 @@ class FileDialog(QtWidgets.QFileDialog): @classmethod def getSaveFileName(cls, *args, **kwargs): """ - Wrap `getSaveFileName` so that it can be called with, and return Path objects + Wraps `getSaveFileName` so that it can be called with, and return Path objects :type parent: QtWidgets.QWidget or None :type caption: str