Fix a few service bugs

openlp/core/api/versions/v2/controller.py

@@ -115,16 +115,14 @@ def set_theme_level():
         abort(400)
     if theme_level == 'global':
         Registry().get('settings').setValue('themes/theme level', 1)
-        Registry().execute('theme_update_global')
     elif theme_level == 'service':
         Registry().get('settings').setValue('themes/theme level', 2)
-        Registry().execute('theme_update_service')
     elif theme_level == 'song':
         Registry().get('settings').setValue('themes/theme level', 3)
-        Registry().execute('theme_update_global')
     else:
         log.error('Unsupported data passed ' + theme_level)
         abort(400)
+    Registry().get('theme_manager').theme_level_updated.emit()
     return '', 204
 
 

openlp/core/ui/mainwindow.py

@@ -522,7 +522,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
         # Timestamp for latest screen-change-popup. Used to prevent spamming the user with popups
         self.screen_change_timestamp = None
         # Simple message boxes
-        Registry().register_function('theme_update_global', self.default_theme_changed)
+        Registry().register_function('theme_change_global', self.default_theme_changed)
         Registry().register_function('config_screen_changed', self.screen_changed)
         Registry().register_function('bootstrap_post_set_up', self.bootstrap_post_set_up)
         # Reset the cursor
@@ -576,7 +576,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
         """
         self.load_settings()
         self.restore_current_media_manager_item()
-        Registry().execute('theme_update_global')
+        Registry().execute('theme_change_global')
 
     def restore_current_media_manager_item(self):
         """
@@ -716,7 +716,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
                 else:
                     self.active_plugin.toggle_status(PluginStatus.Inactive)
         # Set global theme and
-        Registry().execute('theme_update_global')
+        Registry().execute('theme_change_global')
         # Check if any Bibles downloaded.  If there are, they will be processed.
         Registry().execute('bibles_load_list')
         self.application.set_normal_cursor()

openlp/core/ui/screenstab.py

@@ -88,4 +88,6 @@ class ScreensTab(SettingsTab):
         self.screen_selection_widget.save()
         self.settings.setValue('core/display on monitor', self.display_on_monitor_check.isChecked())
         # On save update the screens as well
+        if self.tab_visited:
             self.settings_form.register_post_process('config_screen_changed')
+        self.tab_visited = False

openlp/core/ui/servicemanager.py

@@ -299,10 +299,6 @@ class Ui_ServiceManager(object):
         self.service_manager_list.addActions([self.move_down_action, self.move_up_action, self.make_live_action,
                                               self.move_top_action, self.move_bottom_action, self.expand_action,
                                               self.collapse_action])
-        Registry().register_function('theme_update_list', self.update_theme_list)
-        Registry().register_function('config_screen_changed', self.regenerate_service_items)
-        Registry().register_function('theme_update_global', self.theme_change)
-        Registry().register_function('mediaitem_suffix_reset', self.reset_supported_suffixes)
 
 
 class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixin, RegistryProperties):
@@ -346,7 +342,14 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         self.servicemanager_next_item.connect(self.next_item)
         self.servicemanager_previous_item.connect(self.previous_item)
         self.servicemanager_new_file.connect(self.new_file)
-        self.theme_update_service.connect(self.service_theme_change)
+        # This signal is used to update the theme on the ui thread from the web api thread
+        self.theme_update_service.connect(self.on_service_theme_change)
+        Registry().register_function('theme_update_list', self.update_theme_list)
+        Registry().register_function('theme_level_changed', self.on_theme_level_changed)
+        Registry().register_function('config_screen_changed', self.regenerate_service_items)
+        Registry().register_function('theme_change_global', self.regenerate_service_items)
+        Registry().register_function('theme_change_service', self.regenerate_changed_service_items)
+        Registry().register_function('mediaitem_suffix_reset', self.reset_supported_suffixes)
 
     def bootstrap_post_set_up(self):
         """
@@ -1370,11 +1373,11 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
 
         :param current_index: The combo box index for the selected item
         """
-        self.service_theme = self.theme_combo_box.currentText()
+        new_service_theme = self.theme_combo_box.currentText()
-        self.settings.setValue('servicemanager/service theme', self.service_theme)
+        self.settings.setValue('servicemanager/service theme', new_service_theme)
-        self.service_theme_change()
+        Registry().execute('theme_change_service')
 
-    def theme_change(self):
+    def on_theme_level_changed(self):
         """
         The theme may have changed in the settings dialog so make sure the theme combo box is in the correct state.
         """
@@ -1383,13 +1386,19 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         self.toolbar.actions['theme_label'].setVisible(visible)
         self.regenerate_service_items()
 
-    def service_theme_change(self):
+    def on_service_theme_change(self):
         """
-        Set the theme for the current service remotely
+        Set the theme for the current service from the settings
         """
         self.service_theme = self.settings.value('servicemanager/service theme')
         find_and_set_in_combo_box(self.theme_combo_box, self.service_theme)
-        self.regenerate_service_items(True)
+        Registry().execute('theme_change_service')
+
+    def regenerate_changed_service_items(self):
+        """
+        Regenerate the changed service items, marking the service as unsaved.
+        """
+        self.regenerate_service_items(changed=True)
 
     def regenerate_service_items(self, changed=False):
         """
@@ -1398,6 +1407,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         :param changed: True if the list has changed for new / removed items. False for a theme change.
         """
         self.application.set_busy_cursor()
+        was_modified = self.is_modified()
         # force reset of renderer as theme data has changed
         self.service_has_all_original_files = True
         if self.service_items:
@@ -1423,8 +1433,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                 self.add_service_item(item['service_item'], False, expand=item['expanded'], repaint=False,
                                       selected=item['selected'])
             # Set to False as items may have changed rendering does not impact the saved song so True may also be valid
-            if changed:
+            self.set_modified(changed or was_modified)
-                self.set_modified()
             # Repaint it once only at the end
             self.repaint_service_list(-1, -1)
         self.application.set_normal_cursor()
@@ -1719,7 +1728,6 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             theme_group.addAction(create_widget_action(self.theme_menu, theme, text=theme, checked=False,
                                   triggers=self.on_theme_change_action))
         find_and_set_in_combo_box(self.theme_combo_box, self.service_theme)
-        self.regenerate_service_items()
 
     def on_theme_change_action(self, field=None):
         """

openlp/core/ui/slidecontroller.py

@@ -153,14 +153,21 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         Registry().set_flag('has doubleclick added item to service', True)
         Registry().set_flag('replace service manager item', False)
 
-    def post_set_up(self):
+    def initialise(self):
         """
         Call by bootstrap functions
         """
-        self.initialise()
+        self.setup_ui()
         self.setup_displays()
         self.screen_size_changed()
 
+    def post_set_up(self):
+        # Update the theme whenever the theme is changed (hot reload)
+        Registry().register_function('theme_update_list', self.on_theme_changed)
+        Registry().register_function('theme_level_changed', self.on_theme_changed)
+        Registry().register_function('theme_change_global', self.on_theme_changed)
+        Registry().register_function('theme_change_service', self.on_theme_changed)
+
     def setup_displays(self):
         """
         Set up the display
@@ -186,7 +193,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
     def display(self):
         return self.displays[0] if self.displays else None
 
-    def initialise(self):
+    def setup_ui(self):
         """
         Initialise the UI elements of the controller
         """
@@ -510,10 +517,6 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
             self.mediacontroller_live_stop.connect(self.media_controller.on_media_stop)
         else:
             getattr(self, 'slidecontroller_preview_clear').connect(self.on_clear)
-        # Update the theme whenever global or service theme updated
-        # theme_update_list catches changes to themes AND if the global theme changes
-        Registry().register_function('theme_update_list', self.theme_updated)
-        Registry().register_function('theme_update_service', self.theme_updated)
 
     def new_song_menu(self):
         """
@@ -883,7 +886,7 @@ class SlideController(QtWidgets.QWidget, LogMixin, RegistryProperties):
         for display in self.displays:
             display.set_background_image(image_path)
 
-    def theme_updated(self, var=None):
+    def on_theme_changed(self, var=None):
         """
         Reloads the service item
 
@@ -1607,6 +1610,12 @@ class PreviewController(RegistryBase, SlideController):
         self.category = 'Preview Toolbar'
 
     def bootstrap_initialise(self):
+        """
+        process the bootstrap initialise request
+        """
+        self.initialise()
+
+    def bootstrap_post_set_up(self):
         """
         process the bootstrap post setup request
         """
@@ -1640,6 +1649,12 @@ class LiveController(RegistryBase, SlideController):
         ActionList.get_instance().add_category(str(self.category), CategoryOrder.standard_toolbar)
 
     def bootstrap_initialise(self):
+        """
+        process the bootstrap initialise request
+        """
+        self.initialise()
+
+    def bootstrap_post_set_up(self):
         """
         process the bootstrap post setup request
         """

openlp/core/ui/thememanager.py

@@ -136,7 +136,9 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
     """
     Manages the orders of Theme.
     """
+    # These signals are used by the web api to update the theme on the ui thread
     theme_update_global = QtCore.pyqtSignal()
+    theme_level_updated = QtCore.pyqtSignal()
 
     def __init__(self, parent=None):
         """
@@ -169,7 +171,10 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.setup_ui(self)
         self.global_theme = self.settings.value('themes/global theme')
         self.build_theme_path()
-        self.theme_update_global.connect(self.change_global_from_tab)
+        Registry().register_function('reload_global_theme', self.on_update_global_theme)
+        # These signals are used by the web api to update the theme on the ui thread
+        self.theme_level_updated.connect(self.on_theme_level_updated)
+        self.theme_update_global.connect(self.on_update_global_theme)
 
     def bootstrap_post_set_up(self):
         """
@@ -179,14 +184,8 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.theme_form = ThemeForm(self)
         self.theme_form.path = self.theme_path
         self.file_rename_form = FileRenameForm()
-
-    def bootstrap_completion(self):
-        """
-        process the bootstrap completion request
-        """
         self.upgrade_themes()  # TODO: Can be removed when upgrade path from OpenLP 2.4 no longer needed
         self.load_themes()
-        Registry().register_function('theme_update_global', self.change_global_from_tab)
 
     def screen_changed(self):
         """
@@ -259,12 +258,12 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
         self.global_action.setVisible(visible)
         self.menu.exec(self.theme_list_widget.mapToGlobal(point))
 
-    def change_global_from_tab(self):
+    def on_update_global_theme(self):
         """
-        Change the global theme when it is changed through the Themes settings tab
+        Update the global theme to the theme set in the settings.
         """
         self.global_theme = self.settings.value('themes/global theme')
-        self.log_debug('change_global_from_tab {text}'.format(text=self.global_theme))
+        self.log_debug('on_update_global_theme {text}'.format(text=self.global_theme))
         for count in range(0, self.theme_list_widget.count()):
             # reset the old name
             item = self.theme_list_widget.item(count)
@@ -277,6 +276,13 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
                 name = translate('OpenLP.ThemeManager', '{text} (default)').format(text=new_name)
                 self.theme_list_widget.item(count).setText(name)
                 self.delete_toolbar_action.setVisible(item not in self.theme_list_widget.selectedItems())
+        Registry().execute('theme_change_global')
+
+    def on_theme_level_updated(self):
+        """
+        Update the theme level, called from web controller.
+        """
+        Registry().execute('theme_level_changed')
 
     def change_global_from_screen(self, index=-1):
         """
@@ -297,8 +303,7 @@ class ThemeManager(QtWidgets.QWidget, RegistryBase, Ui_ThemeManager, LogMixin, R
                 name = translate('OpenLP.ThemeManager', '{text} (default)').format(text=self.global_theme)
                 self.theme_list_widget.item(count).setText(name)
                 self.settings.setValue('themes/global theme', self.global_theme)
-                Registry().execute('theme_update_global')
+        Registry().execute('theme_change_global')
-                self._push_themes()
 
     def on_add_theme(self, field=None):
         """

openlp/core/ui/themestab.py

@@ -154,13 +154,19 @@ class ThemesTab(SettingsTab):
         """
         Save the settings
         """
+        theme_level = self.settings.value('themes/theme level')
+        global_theme = self.settings.value('themes/global theme')
         self.settings.setValue('themes/theme level', self.theme_level)
         self.settings.setValue('themes/global theme', self.global_theme)
         self.settings.setValue('themes/item transitions', self.item_transitions_check_box.isChecked())
         self.settings.setValue('themes/hot reload', self.theme_hot_reload.isChecked())
         self.renderer.set_theme_level(self.theme_level)
+        if theme_level != self.theme_level:
+            self.settings_form.register_post_process('theme_level_changed')
+        if global_theme != self.global_theme:
+            self.settings_form.register_post_process('reload_global_theme')
         if self.tab_visited:
-            self.settings_form.register_post_process('theme_update_global')
+            self.settings_form.register_post_process('theme_update_list')
         self.tab_visited = False
 
     def on_song_level_button_clicked(self):

openlp/core/widgets/views.py

@@ -105,6 +105,7 @@ class ListPreviewWidget(QtWidgets.QTableWidget, RegistryProperties):
         """
         Overloaded method from QTableWidget. Will recalculate the layout.
         """
+        super().resizeEvent(event)
         self.__recalculate_layout()
         self.resize_event.emit()
 

tests/openlp_core/api/v2/test_controller.py

@@ -122,26 +122,38 @@ def test_controller_set_theme_level_aborts_if_no_theme_level(flask_client, setti
 
 
 def test_controller_set_theme_level_aborts_if_invalid_theme_level(flask_client, settings):
+    fake_theme_manager = MagicMock()
+    Registry().register('theme_manager', fake_theme_manager)
     res = flask_client.post('/api/v2/controller/theme-level', json=dict(level='foo'))
     assert res.status_code == 400
+    fake_theme_manager.theme_level_updated.emit.assert_not_called()
 
 
 def test_controller_set_theme_level_sets_theme_level_global(flask_client, settings):
+    fake_theme_manager = MagicMock()
+    Registry().register('theme_manager', fake_theme_manager)
     res = flask_client.post('/api/v2/controller/theme-level', json=dict(level='global'))
     assert res.status_code == 204
     assert Registry().get('settings').value('themes/theme level') == 1
+    fake_theme_manager.theme_level_updated.emit.assert_called_once()
 
 
 def test_controller_set_theme_level_sets_theme_level_service(flask_client, settings):
+    fake_theme_manager = MagicMock()
+    Registry().register('theme_manager', fake_theme_manager)
     res = flask_client.post('/api/v2/controller/theme-level', json=dict(level='service'))
     assert res.status_code == 204
     assert Registry().get('settings').value('themes/theme level') == 2
+    fake_theme_manager.theme_level_updated.emit.assert_called_once()
 
 
 def test_controller_set_theme_level_sets_theme_level_song(flask_client, settings):
+    fake_theme_manager = MagicMock()
+    Registry().register('theme_manager', fake_theme_manager)
     res = flask_client.post('/api/v2/controller/theme-level', json=dict(level='song'))
     assert res.status_code == 204
     assert Registry().get('settings').value('themes/theme level') == 3
+    fake_theme_manager.theme_level_updated.emit.assert_called_once()
 
 
 def test_controller_get_themes_retrieves_themes_list(flask_client, settings):

tests/openlp_core/ui/test_mainwindow.py

@@ -252,7 +252,7 @@ def test_mainwindow_configuration(main_window):
                              'authentication_token', 'settings_form', 'service_manager', 'theme_manager',
                              'projector_manager']
     expected_functions_list = ['bootstrap_initialise', 'bootstrap_post_set_up', 'bootstrap_completion',
-                               'config_screen_changed', 'theme_update_global']
+                               'config_screen_changed', 'theme_change_global']
     assert list(Registry().service_list.keys()) == expected_service_list, \
         'The service list should have been {}'.format(Registry().service_list.keys())
     assert list(Registry().functions_list.keys()) == expected_functions_list, \

tests/openlp_core/ui/test_servicemanager.py

@@ -38,7 +38,7 @@ from openlp.core.widgets.toolbar import OpenLPToolbar
 from tests.helpers.testmixin import TestMixin
 
 
-def test_initial_service_manager(registry):
+def test_initial_service_manager(settings):
     """
     Test the initial of service manager.
     """
@@ -81,7 +81,7 @@ def test_new_file(registry):
     assert mocked_slide_controller.slide_count == 0, 'Slide count should be zero'
 
 
-def test_create_basic_service(registry):
+def test_create_basic_service(settings):
     """
     Test the create basic service array
     """
@@ -97,7 +97,7 @@ def test_create_basic_service(registry):
     assert service['openlp_core']['lite-service'] is False, 'The lite service should be saved'
 
 
-def test_is_modified(registry):
+def test_is_modified(settings):
     """
     Test the is_modified() method
     """
@@ -112,7 +112,7 @@ def test_is_modified(registry):
     assert result is True, 'is_modified should return True'
 
 
-def test_supported_suffixes(registry):
+def test_supported_suffixes(settings):
     """
     Test the create basic service array
     """
@@ -127,7 +127,7 @@ def test_supported_suffixes(registry):
     assert 'pptx' in service_manager.suffixes, 'The suffix pptx should be in the list'
 
 
-def test_reset_supported_suffixes(registry):
+def test_reset_supported_suffixes(settings):
     """
     Test the create basic service array
     """
@@ -142,7 +142,7 @@ def test_reset_supported_suffixes(registry):
     assert service_manager.suffixes == [], 'There should not be any suffixes'
 
 
-def test_build_context_menu(registry):
+def test_build_context_menu(settings):
     """
     Test the creation of a context menu from a null service item.
     """
@@ -187,7 +187,7 @@ def test_build_context_menu(registry):
         'Should have been called once'
 
 
-def test_build_song_context_menu(registry, state):
+def test_build_song_context_menu(settings, state):
     """
     Test the creation of a context menu from service item of type text from Songs.
     """
@@ -253,7 +253,7 @@ def test_build_song_context_menu(registry, state):
     assert service_manager.timed_slide_interval.setChecked.call_count == 1, 'Should have be called once'
 
 
-def test_build_bible_context_menu(registry, state):
+def test_build_bible_context_menu(settings, state):
     """
     Test the creation of a context menu from service item of type text from Bibles.
     """
@@ -319,7 +319,7 @@ def test_build_bible_context_menu(registry, state):
     assert service_manager.timed_slide_interval.setChecked.call_count == 1, 'Should have be called once'
 
 
-def test_build_custom_context_menu(registry, state):
+def test_build_custom_context_menu(settings, state):
     """
     Test the creation of a context menu from service item of type text from Custom.
     """
@@ -386,7 +386,7 @@ def test_build_custom_context_menu(registry, state):
     assert service_manager.timed_slide_interval.setChecked.call_count == 1, 'Should have be called once'
 
 
-def test_build_image_context_menu(registry):
+def test_build_image_context_menu(settings):
     """
     Test the creation of a context menu from service item of type Image from Image.
     """
@@ -450,7 +450,7 @@ def test_build_image_context_menu(registry):
     assert service_manager.timed_slide_interval.setChecked.call_count == 1, 'Should have be called once'
 
 
-def test_build_media_context_menu(registry):
+def test_build_media_context_menu(settings):
     """
     Test the creation of a context menu from service item of type Command from Media.
     """
@@ -508,7 +508,7 @@ def test_build_media_context_menu(registry):
     assert service_manager.time_action.setVisible.call_count == 3, 'Should have be called three times'
 
 
-def test_build_presentation_pdf_context_menu(registry):
+def test_build_presentation_pdf_context_menu(settings):
     """
     Test the creation of a context menu from service item of type Command with PDF from Presentation.
     """
@@ -562,7 +562,7 @@ def test_build_presentation_pdf_context_menu(registry):
         'Should have be called once'
 
 
-def test_build_presentation_non_pdf_context_menu(registry):
+def test_build_presentation_non_pdf_context_menu(settings):
     """
     Test the creation of a context menu from service item of type Command with Impress from Presentation.
     """
@@ -614,14 +614,12 @@ def test_build_presentation_non_pdf_context_menu(registry):
 
 
 @patch('openlp.core.ui.servicemanager.QtCore.QTimer.singleShot')
-def test_single_click_preview_true(mocked_singleShot, registry):
+def test_single_click_preview_true(mocked_singleShot, settings):
     """
     Test that when "Preview items when clicked in Service Manager" enabled the preview timer starts
     """
     # GIVEN: A setting to enable "Preview items when clicked in Service Manager" and a service manager.
-    mocked_settings = MagicMock()
+    settings.setValue('advanced/single click service preview', True)
-    mocked_settings.value.return_value = True
-    Registry().register('settings', mocked_settings)
     service_manager = ServiceManager(None)
     # WHEN: on_single_click_preview() is called
     service_manager.on_single_click_preview()
@@ -631,14 +629,12 @@ def test_single_click_preview_true(mocked_singleShot, registry):
 
 
 @patch('openlp.core.ui.servicemanager.QtCore.QTimer.singleShot')
-def test_single_click_preview_false(mocked_singleShot, registry):
+def test_single_click_preview_false(mocked_singleShot, settings):
     """
     Test that when "Preview items when clicked in Service Manager" disabled the preview timer doesn't start
     """
     # GIVEN: A setting to enable "Preview items when clicked in Service Manager" and a service manager.
-    mocked_settings = MagicMock()
+    settings.setValue('advanced/single click service preview', False)
-    mocked_settings.value.return_value = False
-    Registry().register('settings', mocked_settings)
     service_manager = ServiceManager(None)
     # WHEN: on_single_click_preview() is called
     service_manager.on_single_click_preview()
@@ -648,14 +644,12 @@ def test_single_click_preview_false(mocked_singleShot, registry):
 
 @patch('openlp.core.ui.servicemanager.QtCore.QTimer.singleShot')
 @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
-def test_single_click_preview_double(mocked_make_live, mocked_singleShot, registry):
+def test_single_click_preview_double(mocked_make_live, mocked_singleShot, settings):
     """
     Test that when a double click has registered the preview timer doesn't start
     """
     # GIVEN: A setting to enable "Preview items when clicked in Service Manager" and a service manager.
-    mocked_settings = MagicMock()
+    settings.setValue('advanced/single click service preview', True)
-    mocked_settings.value.return_value = True
-    Registry().register('settings', mocked_settings)
     service_manager = ServiceManager(None)
     # WHEN: on_single_click_preview() is called following a double click
     service_manager.on_double_click_live()
@@ -666,7 +660,7 @@ def test_single_click_preview_double(mocked_make_live, mocked_singleShot, regist
 
 
 @patch('openlp.core.ui.servicemanager.ServiceManager.make_preview')
-def test_single_click_timeout_single(mocked_make_preview, registry):
+def test_single_click_timeout_single(mocked_make_preview, settings):
     """
     Test that when a single click has been registered, the item is sent to preview
     """
@@ -680,7 +674,7 @@ def test_single_click_timeout_single(mocked_make_preview, registry):
 
 @patch('openlp.core.ui.servicemanager.ServiceManager.make_preview')
 @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
-def test_single_click_timeout_double(mocked_make_live, mocked_make_preview, registry):
+def test_single_click_timeout_double(mocked_make_live, mocked_make_preview, settings):
     """
     Test that when a double click has been registered, the item does not goes to preview
     """
@@ -699,15 +693,13 @@ def test_single_click_timeout_double(mocked_make_live, mocked_make_preview, regi
 @patch('openlp.core.ui.servicemanager.shutil')
 @patch('openlp.core.ui.servicemanager.NamedTemporaryFile')
 def test_save_file_raises_permission_error(mocked_temp_file, mocked_shutil, mocked_os, mocked_save_file_as,
-                                           mocked_zipfile, registry):
+                                           mocked_zipfile, settings):
     """
     Test that when a PermissionError is raised when trying to save a file, it is handled correctly
     """
     # GIVEN: A service manager, a service to save
     mocked_main_window = MagicMock()
     Registry().register('main_window', mocked_main_window)
-    Registry().register('application', MagicMock())
-    Registry().register('settings', MagicMock())
     service_manager = ServiceManager(None)
     service_manager._service_path = MagicMock()
     service_manager._save_lite = False
@@ -800,7 +792,7 @@ def test_save_file_falls_back_to_shutil(mocked_temp_file, mocked_shutil, mocked_
 
 
 @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
-def test_theme_change_global(mocked_regenerate_service_items, registry):
+def test_theme_change_global(mocked_regenerate_service_items, settings):
     """
     Test that when a Toolbar theme combobox displays correctly when the theme is set to Global
     """
@@ -809,12 +801,10 @@ def test_theme_change_global(mocked_regenerate_service_items, registry):
     service_manager.toolbar = OpenLPToolbar(None)
     service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
     service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
-    mocked_settings = MagicMock()
+    settings.setValue('themes/theme level', ThemeLevel.Global)
-    mocked_settings.value.return_value = ThemeLevel.Global
-    Registry().register('settings', mocked_settings)
 
     # WHEN: theme_change is called
-    service_manager.theme_change()
+    service_manager.on_theme_level_changed()
 
     # THEN: The the theme toolbar should not be visible
     assert service_manager.toolbar.actions['theme_combo_box'].isVisible() is False, \
@@ -822,7 +812,7 @@ def test_theme_change_global(mocked_regenerate_service_items, registry):
 
 
 @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
-def test_theme_change_service(mocked_regenerate_service_items, registry):
+def test_theme_change_service(mocked_regenerate_service_items, settings):
     """
     Test that when a Toolbar theme combobox displays correctly when the theme is set to Theme
     """
@@ -831,12 +821,10 @@ def test_theme_change_service(mocked_regenerate_service_items, registry):
     service_manager.toolbar = OpenLPToolbar(None)
     service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
     service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
-    mocked_settings = MagicMock()
+    settings.setValue('themes/theme level', ThemeLevel.Service)
-    mocked_settings.value.return_value = ThemeLevel.Service
-    Registry().register('settings', mocked_settings)
 
     # WHEN: theme_change is called
-    service_manager.theme_change()
+    service_manager.on_theme_level_changed()
 
     # THEN: The the theme toolbar should be visible
     assert service_manager.toolbar.actions['theme_combo_box'].isVisible() is True, \
@@ -844,7 +832,7 @@ def test_theme_change_service(mocked_regenerate_service_items, registry):
 
 
 @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
-def test_theme_change_song(mocked_regenerate_service_items, registry):
+def test_theme_change_song(mocked_regenerate_service_items, settings):
     """
     Test that when a Toolbar theme combobox displays correctly when the theme is set to Song
     """
@@ -853,18 +841,100 @@ def test_theme_change_song(mocked_regenerate_service_items, registry):
     service_manager.toolbar = OpenLPToolbar(None)
     service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
     service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
-    mocked_settings = MagicMock()
+    settings.setValue('themes/theme level', ThemeLevel.Song)
-    mocked_settings.value.return_value = ThemeLevel.Song
-    Registry().register('settings', mocked_settings)
 
     # WHEN: theme_change is called
-    service_manager.theme_change()
+    service_manager.on_theme_level_changed()
 
     # THEN: The the theme toolbar should be visible
     assert service_manager.toolbar.actions['theme_combo_box'].isVisible() is True, \
         'The visibility should be True'
 
 
+@patch('PyQt5.QtWidgets.QTreeWidgetItemIterator')
+def test_regenerate_service_items(mocked_tree, settings):
+    """
+    test that an unmodified service item that is regenerated is still unmodified
+    """
+    # GIVEN: A service manager and a service item
+    mocked_main_window = MagicMock()
+    Registry().register('main_window', mocked_main_window)
+    service_manager = ServiceManager(None)
+    service_item = ServiceItem(None)
+    service_item.service_item_type = ServiceItemType.Command
+    service_item.edit_id = 1
+    service_item.icon = MagicMock(pixmap=MagicMock())
+    service_item.slides.append(MagicMock())
+    service_manager.service_items.insert(1, {'service_item': service_item, 'expanded': False})
+    service_manager._modified = False
+    service_manager.service_manager_list = MagicMock()
+    service_manager.repaint_service_list = MagicMock()
+    mocked_tree.return_value = MagicMock(value=MagicMock(return_value=None))
+
+    # WHEN: regenerate_service_items is called
+    service_manager.regenerate_service_items()
+
+    # THEN: The the service should be repainted and not be marked as modified
+    assert service_manager.is_modified() is False
+    service_manager.repaint_service_list.assert_called_once()
+
+
+@patch('PyQt5.QtWidgets.QTreeWidgetItemIterator')
+def test_regenerate_service_items_modified(mocked_tree, settings):
+    """
+    test that an unmodified service item that is regenerated is still unmodified
+    """
+    # GIVEN: A service manager and a service item
+    mocked_main_window = MagicMock()
+    Registry().register('main_window', mocked_main_window)
+    service_manager = ServiceManager(None)
+    service_item = ServiceItem(None)
+    service_item.service_item_type = ServiceItemType.Command
+    service_item.edit_id = 1
+    service_item.icon = MagicMock(pixmap=MagicMock())
+    service_item.slides.append(MagicMock())
+    service_manager.service_items.insert(1, {'service_item': service_item, 'expanded': False})
+    service_manager._modified = True
+    service_manager.service_manager_list = MagicMock()
+    service_manager.repaint_service_list = MagicMock()
+    mocked_tree.return_value = MagicMock(value=MagicMock(return_value=None))
+
+    # WHEN: regenerate_service_items is called
+    service_manager.regenerate_service_items()
+
+    # THEN: The the service should be repainted and still be marked as modified
+    assert service_manager.is_modified() is True
+    service_manager.repaint_service_list.assert_called_once()
+
+
+@patch('PyQt5.QtWidgets.QTreeWidgetItemIterator')
+def test_regenerate_service_items_set_modified(mocked_tree, settings):
+    """
+    test that a service item that is regenerated with the modified argument becomes modified
+    """
+    # GIVEN: A service manager and a service item
+    mocked_main_window = MagicMock()
+    Registry().register('main_window', mocked_main_window)
+    service_manager = ServiceManager(None)
+    service_item = ServiceItem(None)
+    service_item.service_item_type = ServiceItemType.Command
+    service_item.edit_id = 1
+    service_item.icon = MagicMock(pixmap=MagicMock())
+    service_item.slides.append(MagicMock())
+    service_manager.service_items.insert(1, {'service_item': service_item, 'expanded': False})
+    service_manager._modified = False
+    service_manager.service_manager_list = MagicMock()
+    service_manager.repaint_service_list = MagicMock()
+    mocked_tree.return_value = MagicMock(value=MagicMock(return_value=None))
+
+    # WHEN: regenerate_service_items is called
+    service_manager.regenerate_service_items(True)
+
+    # THEN: The the service should be repainted and now be marked as modified
+    assert service_manager.is_modified() is True
+    service_manager.repaint_service_list.assert_called_once()
+
+
 def test_service_manager_list_drag_enter_event():
     """
     Test that the ServiceManagerList.dragEnterEvent slot accepts the event
@@ -1162,7 +1232,7 @@ class TestServiceManager(TestCase, TestMixin):
         self.service_manager.servicemanager_new_file.connect.assert_called_once_with(
             self.service_manager.new_file)
         self.service_manager.theme_update_service.connect.assert_called_once_with(
-            self.service_manager.service_theme_change)
+            self.service_manager.on_service_theme_change)
 
     @patch('openlp.core.ui.servicemanager.ServiceNoteForm')
     @patch('openlp.core.ui.servicemanager.ServiceItemEditForm')

tests/openlp_core/ui/test_slidecontroller.py

@@ -820,7 +820,7 @@ def test_theme_updated(mock_settings):
     mock_settings.value.return_value = True
 
     # WHEN: theme_updated is called
-    slide_controller.theme_updated()
+    slide_controller.on_theme_changed()
 
     # THEN: process_item is called with the current service_item and slide number
     slide_controller._process_item.assert_called_once_with(sentinel.service_item, 14)
@@ -838,7 +838,7 @@ def test_theme_updated_no_reload(mock_settings):
     mock_settings.value.return_value = False
 
     # WHEN: theme_updated is called
-    slide_controller.theme_updated()
+    slide_controller.on_theme_changed()
 
     # THEN: process_item is not called
     assert slide_controller._process_item.call_count == 0

tests/openlp_core/ui/test_thememanager.py

@@ -492,6 +492,8 @@ def test_bootstrap_post(mocked_rename_form, mocked_theme_form, theme_manager):
     """
     # GIVEN:
     theme_manager.theme_path = MagicMock()
+    theme_manager.load_themes = MagicMock()
+    theme_manager.upgrade_themes = MagicMock()
 
     # WHEN:
     with patch('openlp.core.ui.thememanager.ThemeProgressForm'):
@@ -501,19 +503,5 @@ def test_bootstrap_post(mocked_rename_form, mocked_theme_form, theme_manager):
     assert theme_manager.progress_form is not None
     assert theme_manager.theme_form is not None
     assert theme_manager.file_rename_form is not None
-
-
-def test_bootstrap_completion(theme_manager):
-    """
-    Test the functions of bootstrap_post_setup are called.
-    """
-    # GIVEN:
-    theme_manager.load_themes = MagicMock()
-    theme_manager.upgrade_themes = MagicMock()
-
-    # WHEN:
-    theme_manager.bootstrap_completion()
-
-    # THEN:
     theme_manager.upgrade_themes.assert_called_once()
     theme_manager.load_themes.assert_called_once()