Fix issue with theme backgrounds disappearing

openlp/core/ui/thememanager.py

@@ -371,7 +371,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
             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, background_override=old_background)
+        self.save_theme(theme_data, background_file=old_background)
         self.update_preview_images([new_theme_name])
         self.load_themes()
 
@@ -691,12 +691,12 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
             return False
         return True
 
-    def save_theme(self, theme, background_override=None):
+    def save_theme(self, theme, background_file=None):
         """
         Writes the theme to the disk and including the background image and thumbnail if necessary
 
         :param Theme theme: The theme data object.
-        :param background_override: Background to use rather than background_source. Optionally.
+        :param background_file: Background to use rather than background_source. Optional.
         :rtype: None
         """
         name = theme.theme_name
@@ -708,15 +708,14 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
             theme_path.write_text(theme_pretty)
         except OSError:
             self.log_exception('Saving theme to file failed')
-        if theme.background_source and theme.background_filename and theme.background_type != 'stream':
+        if theme.background_filename and theme.background_type != 'stream':
-            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 background_file.exists():
+            if not background_file or not background_file.exists():
-                self.log_warning('Background does not exist, retaining cached background')
+                self.log_warning('Background source does not exist, retaining cached background')
             elif background_file != theme.background_filename:
                 try:
                     shutil.copyfile(background_file, theme.background_filename)

tests/openlp_core/ui/test_thememanager.py

@@ -32,6 +32,7 @@ from PyQt5 import QtWidgets
 
 from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
+from openlp.core.lib.theme import Theme
 from openlp.core.ui.thememanager import ThemeManager
 from tests.utils.constants import RESOURCE_PATH
 
@@ -265,7 +266,7 @@ def test_save_theme_missing_new(mocked_paths, mocked_delete, mocked_log_warning,
     theme_manager.save_theme(mocked_theme)
 
     # 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')
+    mocked_log_warning.assert_called_once_with('Background source does not exist, retaining cached background')
 
 
 @patch('openlp.core.ui.thememanager.shutil')
@@ -292,7 +293,7 @@ def test_save_theme_background_override(mocked_paths, mocked_delete, mocked_shut
     override_background = MagicMock()
 
     # WHEN: Calling save_theme with a background override
-    theme_manager.save_theme(mocked_theme, background_override=override_background)
+    theme_manager.save_theme(mocked_theme, background_file=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)
@@ -505,3 +506,27 @@ def test_bootstrap_post(mocked_rename_form, mocked_theme_form, theme_manager):
     assert theme_manager.file_rename_form is not None
     theme_manager.upgrade_themes.assert_called_once()
     theme_manager.load_themes.assert_called_once()
+
+
+def test_clone_theme_data(theme_manager):
+    """Test that cloning the theme data works correctly"""
+    # GIVEN: A theme manager, a theme (without a background source) and a new theme name
+    existing_theme = Theme()
+    existing_theme.theme_name = 'Existing Theme'
+    existing_theme.background_type = 'image'
+    existing_theme.background_filename = Path('Existing Theme', 'background.jpg')
+
+    theme_manager.theme_path = Path('openlp', 'themes')
+    theme_manager.save_theme = MagicMock()
+    theme_manager.update_preview_images = MagicMock()
+    theme_manager.load_themes = MagicMock()
+
+    # WHEN: The theme is cloned
+    theme_manager.clone_theme_data(existing_theme, 'New Theme')
+
+    # THEN: The theme data should have been updated
+    assert existing_theme.theme_name == 'New Theme'
+    theme_manager.save_theme.assert_called_once_with(existing_theme, background_file=Path('Existing Theme',
+                                                                                          'background.jpg'))
+    theme_manager.update_preview_images.assert_called_once_with(['New Theme'])
+    theme_manager.load_themes.assert_called_once_with()