From a019b426363719e0add869a681662655848aa8fe Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 2 Nov 2019 07:05:08 +0000 Subject: [PATCH 1/2] Force the display checkbox to be selected for at least 1 screen --- openlp/core/widgets/widgets.py | 17 +++++++++ tests/openlp_core/widgets/test_widgets.py | 45 +++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/openlp/core/widgets/widgets.py b/openlp/core/widgets/widgets.py index 7e3811eec..acb280aae 100644 --- a/openlp/core/widgets/widgets.py +++ b/openlp/core/widgets/widgets.py @@ -25,6 +25,7 @@ from PyQt5 import QtCore, QtWidgets from openlp.core.common.i18n import translate from openlp.core.common.settings import ProxyMode, Settings +from openlp.core.lib.ui import critical_error_message_box SCREENS_LAYOUT_STYLE = """ @@ -281,6 +282,8 @@ class ScreenSelectionWidget(QtWidgets.QWidget): self.layout.addStretch() # Signals and slots + self.display_group_box.clicked.connect(self.on_display_clicked) + self.use_screen_check_box.clicked.connect(self.on_display_clicked) self.use_screen_check_box.toggled.connect(self.display_group_box.setChecked) self.custom_geometry_button.toggled.connect(self.height_spin_box.setEnabled) self.custom_geometry_button.toggled.connect(self.left_spin_box.setEnabled) @@ -306,6 +309,20 @@ class ScreenSelectionWidget(QtWidgets.QWidget): self.height_label.setText(translate('OpenLP.ScreensTab', 'Height:')) self.identify_button.setText(translate('OpenLP.ScreensTab', 'Identify Screens')) + def on_display_clicked(self, is_checked): + if not is_checked: + critical_error_message_box(translate('OpenLP.ScreensTab', 'Select a Display'), + translate('OpenLP.ScreensTab', 'You need to select at least one screen to be ' + 'used as a display. Select the screen you wish to use as a display, ' + 'and check the checkbox for that screen.'), + parent=self, question=False) + self.use_screen_check_box.setChecked(True) + self.display_group_box.setChecked(True) + else: + for screen in self.screens: + screen.is_display = False + self.current_screen.is_display = True + def _save_screen(self, screen): """ Save the details in the UI to the screen diff --git a/tests/openlp_core/widgets/test_widgets.py b/tests/openlp_core/widgets/test_widgets.py index bbe52a2ac..5754e4346 100644 --- a/tests/openlp_core/widgets/test_widgets.py +++ b/tests/openlp_core/widgets/test_widgets.py @@ -480,3 +480,48 @@ class TestScreenSelectionWidget(TestCase, TestMixin): mocked_label.show.assert_called_once() assert instance.identify_labels == [mocked_label] instance.timer.start.assert_called_once() + + def test_on_display_clicked_with_checked(self): + """ + Test that the on_display_clicked() sets the first screen as display when the checkbx is checked + """ + # GIVEN: A ScreenSelectionWidget and a bunch o' mocks + instance = ScreenSelectionWidget() + mocked_screen_1 = MagicMock() + mocked_screen_2 = MagicMock() + mocked_screen_2.is_display = True + instance.screens = [mocked_screen_1, mocked_screen_2] + instance.current_screen = mocked_screen_1 + + # WHEN: on_display_clicked() is called when the checkbox is checked + instance.on_display_clicked(True) + + # THEN: The first screen should be marked as a display + assert mocked_screen_1.is_display is True + assert mocked_screen_2.is_display is False + + def test_on_display_clicked_with_unchecked(self): + """ + Test that the on_display_clicked() disallows the checkbox to be unchecked + """ + # GIVEN: A ScreenSelectionWidget and a bunch o' mocks + instance = ScreenSelectionWidget() + mocked_screen_1 = MagicMock() + mocked_screen_2 = MagicMock() + mocked_screen_2.is_display = True + instance.screens = [mocked_screen_1, mocked_screen_2] + instance.current_screen = mocked_screen_2 + + # WHEN: on_display_clicked() is called when the checkbox is checked + with patch('openlp.core.widgets.widgets.translate') as mocked_translate, \ + patch('openlp.core.widgets.widgets.critical_error_message_box') as mocked_error: + mocked_translate.side_effect = lambda c, s: s + instance.on_display_clicked(False) + + # THEN: The first screen should be marked as a display + mocked_error.assert_called_once_with('Select a Display', + 'You need to select at least one screen to be used as a display. ' + 'Select the screen you wish to use as a display, and check the ' + 'checkbox for that screen.', parent=instance, question=False) + assert instance.use_screen_check_box.isChecked() is True + assert instance.display_group_box.isChecked() is True From 1ac6c052c74aee91ce6f070bfbb7eecde31c2ea6 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 2 Nov 2019 07:14:10 +0000 Subject: [PATCH 2/2] Fix clone theme with missing background source --- openlp/core/ui/thememanager.py | 17 +++++++---- .../openlp_core/ui/test_thememanager.py | 29 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 261bdbbbe..e2d3e2c68 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -324,11 +324,13 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R :param str new_theme_name: The new theme name of the theme :rtype: None """ + old_background = None if theme_data.background_type == 'image' or theme_data.background_type == 'video': + old_background = theme_data.background_filename theme_data.background_filename = self.theme_path / new_theme_name / theme_data.background_filename.name theme_data.theme_name = new_theme_name theme_data.extend_image_filename(self.theme_path) - self.save_theme(theme_data) + self.save_theme(theme_data, background_override=old_background) self.load_themes() def on_edit_theme(self, field=None): @@ -644,12 +646,13 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R return False return True - def save_theme(self, theme, image=None): + def save_theme(self, theme, image=None, background_override=None): """ Writes the theme to the disk and including the background image and thumbnail if necessary :param Theme theme: The theme data object. :param image: The theme thumbnail. Optionally. + :param background_override: Background to use rather than background_source. Optionally. :rtype: None """ name = theme.theme_name @@ -662,13 +665,17 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R except OSError: self.log_exception('Saving theme to file failed') if theme.background_source and theme.background_filename: + background_file = background_override + # Use theme source image if override doesn't exist + if not background_file or not background_file.exists(): + background_file = theme.background_source if self.old_background_image_path and theme.background_filename != self.old_background_image_path: delete_file(self.old_background_image_path) - if not theme.background_source.exists(): + if not background_file.exists(): self.log_warning('Background does not exist, retaining cached background') - elif theme.background_source != theme.background_filename: + elif background_file != theme.background_filename: try: - shutil.copyfile(theme.background_source, theme.background_filename) + shutil.copyfile(background_file, theme.background_filename) except OSError: self.log_exception('Failed to save theme image') if image: diff --git a/tests/functional/openlp_core/ui/test_thememanager.py b/tests/functional/openlp_core/ui/test_thememanager.py index 230c3014e..b7aae7c2c 100644 --- a/tests/functional/openlp_core/ui/test_thememanager.py +++ b/tests/functional/openlp_core/ui/test_thememanager.py @@ -219,6 +219,35 @@ class TestThemeManager(TestCase): # THEN: A warning should have happened due to attempting to copy a missing file mocked_log_warning.assert_called_once_with('Background does not exist, retaining cached background') + @patch('openlp.core.ui.thememanager.shutil') + @patch('openlp.core.ui.thememanager.delete_file') + @patch('openlp.core.ui.thememanager.create_paths') + def test_save_theme_background_override(self, mocked_paths, mocked_delete, mocked_shutil): + """ + Test that we log a warning if the new background is missing + """ + # GIVEN: A new theme manager instance, with invalid files. Setup as if the user + # has changed the background to a invalid path. + # Not using resource dir because I could potentially copy a file + folder_path = Path(mkdtemp()) + theme_manager = ThemeManager(None) + theme_manager.old_background_image_path = folder_path / 'old.png' + theme_manager.update_preview_images = MagicMock() + theme_manager.theme_path = MagicMock() + mocked_theme = MagicMock() + mocked_theme.theme_name = 'themename' + mocked_theme.background_filename = folder_path / 'new_cached.png' + # mocked_theme.background_source.exists() will return True + mocked_theme.background_source = MagicMock() + # override_background.exists() will return True + override_background = MagicMock() + + # WHEN: Calling save_theme with a background override + theme_manager.save_theme(mocked_theme, background_override=override_background) + + # THEN: The override_background should have been copied rather than the background_source + mocked_shutil.copyfile.assert_called_once_with(override_background, mocked_theme.background_filename) + def test_save_theme_special_char_name(self): """ Test that we can save themes with special characters in the name