Merge branch 'issue-881-overwriting-service-items' into 'master'

Change the `replace` argument in `add_service_item` to an integer value (fixes #881)

Closes #881

See merge request openlp/openlp!351

openlp/core/lib/mediamanageritem.py

@@ -611,7 +611,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties, LogMixin):
         """
         self.add_to_service(message[0], remote=message[1])
 
-    def add_to_service(self, item=None, replace=None, remote=False, position=-1):
+    def add_to_service(self, item=None, replace=-1, remote=False, position=-1):
         """
         Add this item to the current service.
 
@@ -635,6 +635,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties, LogMixin):
                                                         'You must select one or more items.'))
         else:
             self.log_debug('{plugin} Add requested'.format(plugin=self.plugin.name))
+            item = self.service_manager.find_service_item()[0]
             service_item = self.service_manager.get_service_item()
             if not service_item:
                 QtWidgets.QMessageBox.information(self, UiStrings().NISs,
@@ -642,7 +643,7 @@ class MediaManagerItem(QtWidgets.QWidget, RegistryProperties, LogMixin):
                                                             'You must select an existing service item to add to.'))
             elif self.plugin.name == service_item.name:
                 self.generate_slide_data(service_item)
-                self.service_manager.add_service_item(service_item, replace=True)
+                self.service_manager.add_service_item(service_item, replace=item)
             else:
                 # Turn off the remote edit update message indicator
                 QtWidgets.QMessageBox.information(self, translate('OpenLP.MediaManagerItem', 'Invalid Service Item'),

openlp/core/ui/servicemanager.py

@@ -1018,7 +1018,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         self.service_item_edit_form.set_service_item(self.service_items[item]['service_item'])
         if self.service_item_edit_form.exec():
             self.add_service_item(self.service_item_edit_form.get_service_item(),
-                                  replace=True, expand=self.service_items[item]['expanded'])
+                                  replace=item, expand=self.service_items[item]['expanded'])
 
     def preview_live(self, unique_identifier, row):
         """
@@ -1441,7 +1441,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
                 self.live_controller.replace_service_manager_item(new_item)
                 self.set_modified()
 
-    def add_service_item(self, item, rebuild=False, expand=None, replace=False, repaint=True, selected=False,
+    def add_service_item(self, item, rebuild=False, expand=None, replace=-1, repaint=True, selected=False,
                          position=-1):
         """
         Add a Service item to the list
@@ -1449,7 +1449,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         :param item: Service Item to be added
         :param rebuild: Do we need to rebuild the live display (Default False)
         :param expand: Override the default expand settings. (Tristate)
-        :param replace: Is the service item a replacement (Default False)
+        :param replace: The service item to be replaced
         :param repaint: Do we need to repaint the service item list (Default True)
         :param selected: Has the item been selected (Default False)
         :param position: The position where the item is dropped (Default -1)
@@ -1460,11 +1460,10 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
         item.from_service = True
         if position != -1:
             self.drop_position = position
-        if replace:
+        if replace > -1:
-            s_item, child = self.find_service_item()
+            item.merge(self.service_items[replace]['service_item'])
-            item.merge(self.service_items[s_item]['service_item'])
+            self.service_items[replace]['service_item'] = item
-            self.service_items[s_item]['service_item'] = item
+            self.repaint_service_list(replace, -1)
-            self.repaint_service_list(s_item, child)
             self.live_controller.replace_service_manager_item(item)
         else:
             item.render_text_items()
@@ -1582,7 +1581,7 @@ class ServiceManager(QtWidgets.QWidget, RegistryBase, Ui_ServiceManager, LogMixi
             new_item = Registry().get(self.service_items[item]['service_item'].name). \
                 on_remote_edit(self.service_items[item]['service_item'].edit_id)
             if new_item:
-                self.add_service_item(new_item, replace=True)
+                self.add_service_item(new_item, replace=item)
 
     def on_service_item_rename(self):
         """

tests/openlp_core/ui/test_servicemanager.py

@@ -21,8 +21,7 @@
 """
 Package to test the openlp.core.ui.slidecontroller package.
 """
-import PyQt5
+from pathlib import Path
-
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
@@ -33,7 +32,7 @@ from openlp.core.common.registry import Registry
 from openlp.core.common.settings import Settings
 from openlp.core.common.enum import ServiceItemType
 from openlp.core.lib.serviceitem import ItemCapabilities, ServiceItem
-from openlp.core.ui.servicemanager import ServiceManager
+from openlp.core.ui.servicemanager import ServiceManager, ServiceManagerList
 from openlp.core.widgets.toolbar import OpenLPToolbar
 
 from tests.helpers.testmixin import TestMixin
@@ -552,7 +551,7 @@ def test_build_presentation_non_pdf_context_menu(registry):
         'Should have be called once'
 
 
-@patch('PyQt5.QtCore.QTimer.singleShot')
+@patch('openlp.core.ui.servicemanager.QtCore.QTimer.singleShot')
 def test_single_click_preview_true(mocked_singleShot, registry):
     """
     Test that when "Preview items when clicked in Service Manager" enabled the preview timer starts
@@ -565,11 +564,11 @@ def test_single_click_preview_true(mocked_singleShot, registry):
     # WHEN: on_single_click_preview() is called
     service_manager.on_single_click_preview()
     # THEN: timer should have been started
-    mocked_singleShot.assert_called_with(PyQt5.QtWidgets.QApplication.instance().doubleClickInterval(),
+    mocked_singleShot.assert_called_with(QtWidgets.QApplication.instance().doubleClickInterval(),
                                          service_manager.on_single_click_preview_timeout)
 
 
-@patch('PyQt5.QtCore.QTimer.singleShot')
+@patch('openlp.core.ui.servicemanager.QtCore.QTimer.singleShot')
 def test_single_click_preview_false(mocked_singleShot, registry):
     """
     Test that when "Preview items when clicked in Service Manager" disabled the preview timer doesn't start
@@ -585,7 +584,7 @@ def test_single_click_preview_false(mocked_singleShot, registry):
     assert mocked_singleShot.call_count == 0, 'Should not be called'
 
 
-@patch('PyQt5.QtCore.QTimer.singleShot')
+@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):
     """
@@ -804,6 +803,113 @@ def test_theme_change_song(mocked_regenerate_service_items, registry):
         'The visibility should be True'
 
 
+def test_service_manager_list_drag_enter_event():
+    """
+    Test that the ServiceManagerList.dragEnterEvent slot accepts the event
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    mocked_event = MagicMock()
+
+    # WHEN: The dragEnterEvent method is called
+    service_manager_list.dragEnterEvent(mocked_event)
+
+    # THEN: The accept() method on the event is called
+    mocked_event.accept.assert_called_once_with()
+
+
+def test_service_manager_list_drag_move_event():
+    """
+    Test that the ServiceManagerList.dragMoveEvent slot accepts the event
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    mocked_event = MagicMock()
+
+    # WHEN: The dragMoveEvent method is called
+    service_manager_list.dragMoveEvent(mocked_event)
+
+    # THEN: The accept() method on the event is called
+    mocked_event.accept.assert_called_once_with()
+
+
+def test_service_manager_list_key_press_event():
+    """
+    Test that the ServiceManagerList.keyPressEvent slot ignores the event
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    mocked_event = MagicMock()
+
+    # WHEN: The keyPressEvent method is called
+    service_manager_list.keyPressEvent(mocked_event)
+
+    # THEN: The ignore() method on the event is called
+    mocked_event.ignore.assert_called_once_with()
+
+
+def test_service_manager_list_mouse_move_event_not_left_button():
+    """
+    Test that the ServiceManagerList.mouseMoveEvent slot ignores the event if it's not a left-click
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    mocked_event = MagicMock()
+    mocked_event.buttons.return_value = QtCore.Qt.RightButton
+
+    # WHEN: The mouseMoveEvent method is called
+    service_manager_list.mouseMoveEvent(mocked_event)
+
+    # THEN: The ignore() method on the event is called
+    mocked_event.ignore.assert_called_once_with()
+
+
+@patch('openlp.core.ui.servicemanager.QtGui.QCursor')
+def test_service_manager_list_mouse_move_event_no_item(MockQCursor):
+    """
+    Test that the ServiceManagerList.mouseMoveEvent slot ignores the event if it's not over an item
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    service_manager_list.itemAt = MagicMock(return_value=None)
+    service_manager_list.mapFromGlobal = MagicMock()
+    mocked_event = MagicMock()
+    mocked_event.buttons.return_value = QtCore.Qt.LeftButton
+
+    # WHEN: The mouseMoveEvent method is called
+    service_manager_list.mouseMoveEvent(mocked_event)
+
+    # THEN: The ignore() method on the event is called
+    mocked_event.ignore.assert_called_once_with()
+
+
+@patch('openlp.core.ui.servicemanager.QtGui.QCursor')
+@patch('openlp.core.ui.servicemanager.QtGui.QDrag')
+@patch('openlp.core.ui.servicemanager.QtCore.QMimeData')
+def test_service_manager_list_mouse_move_event(MockQMimeData, MockQDrag, MockQCursor):
+    """
+    Test that the ServiceManagerList.mouseMoveEvent slot ignores the event if it's not over an item
+    """
+    # GIVEN: A service manager and a mocked event
+    service_manager_list = ServiceManagerList(None)
+    service_manager_list.itemAt = MagicMock(return_value=True)
+    service_manager_list.mapFromGlobal = MagicMock()
+    mocked_event = MagicMock()
+    mocked_event.buttons.return_value = QtCore.Qt.LeftButton
+    mocked_drag = MagicMock()
+    MockQDrag.return_value = mocked_drag
+    mocked_mime_data = MagicMock()
+    MockQMimeData.return_value = mocked_mime_data
+
+    # WHEN: The mouseMoveEvent method is called
+    service_manager_list.mouseMoveEvent(mocked_event)
+
+    # THEN: The ignore() method on the event is called
+    mocked_drag.setMimeData.assert_called_once_with(mocked_mime_data)
+    mocked_mime_data.setText.assert_called_once_with('ServiceManager')
+    mocked_drag.exec.assert_called_once_with(QtCore.Qt.CopyAction)
+
+
 class TestServiceManager(TestCase, TestMixin):
     """
     Test the service manager
@@ -841,6 +947,122 @@ class TestServiceManager(TestCase, TestMixin):
         self.add_toolbar_action_patcher.stop()
         del self.service_manager
 
+    def test_bootstrap_initialise(self):
+        """
+        Test the bootstrap_initialise method
+        """
+        # GIVEN: Mocked out stuff
+        self.service_manager.setup_ui = MagicMock()
+        self.service_manager.servicemanager_set_item = MagicMock()
+        self.service_manager.servicemanager_set_item_by_uuid = MagicMock()
+        self.service_manager.servicemanager_next_item = MagicMock()
+        self.service_manager.servicemanager_previous_item = MagicMock()
+        self.service_manager.servicemanager_new_file = MagicMock()
+        self.service_manager.theme_update_service = MagicMock()
+
+        # WHEN: bootstrap_initialise is called
+        self.service_manager.bootstrap_initialise()
+
+        # THEN: The correct calls should have been made
+        self.service_manager.setup_ui.assert_called_once_with(self.service_manager)
+        self.service_manager.servicemanager_set_item.connect.assert_called_once_with(
+            self.service_manager.on_set_item)
+        self.service_manager.servicemanager_set_item_by_uuid.connect.assert_called_once_with(
+            self.service_manager.set_item_by_uuid)
+        self.service_manager.servicemanager_next_item.connect.assert_called_once_with(
+            self.service_manager.next_item)
+        self.service_manager.servicemanager_previous_item.connect.assert_called_once_with(
+            self.service_manager.previous_item)
+        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)
+
+    @patch('openlp.core.ui.servicemanager.ServiceNoteForm')
+    @patch('openlp.core.ui.servicemanager.ServiceItemEditForm')
+    @patch('openlp.core.ui.servicemanager.StartTimeForm')
+    def test_bootstrap_post_set_up(self, MockStartTimeForm, MockServiceItemEditForm, MockServiceNoteForm):
+        """
+        Test the post bootstrap setup
+        """
+        # GIVEN: Mocked forms and a ServiceManager object
+        mocked_service_note_form = MagicMock()
+        MockServiceNoteForm.return_value = mocked_service_note_form
+        mocked_service_item_edit_form = MagicMock()
+        MockServiceItemEditForm.return_value = mocked_service_item_edit_form
+        mocked_start_time_form = MagicMock()
+        MockStartTimeForm.return_value = mocked_start_time_form
+
+        # WHEN: bootstrap_post_set_up is run
+        self.service_manager.bootstrap_post_set_up()
+
+        # THEN: The forms should have been created
+        assert self.service_manager.service_note_form == mocked_service_note_form
+        assert self.service_manager.service_item_edit_form == mocked_service_item_edit_form
+        assert self.service_manager.start_time_form == mocked_start_time_form
+
+    def test_is_modified(self):
+        """
+        Test the is_modified() method
+        """
+        # GIVEN: A service manager with the self._modified set
+        self.service_manager._modified = True
+
+        # WHEN: is_modified is called
+        result = self.service_manager.is_modified()
+
+        # THEN: The result should be True
+        assert result is True, 'is_modified should return True'
+
+    def test_set_file_name_osz(self):
+        """
+        Test setting the file name
+        """
+        # GIVEN: A service manager, some mocks and a file name
+        self.service_manager.set_modified = MagicMock()
+        self.service_manager.settings.setValue = MagicMock()
+        file_path = Path('servicefile.osz')
+
+        # WHEN: The filename is set
+        self.service_manager.set_file_name(file_path)
+
+        # THEN: Various things should have been called and set
+        assert self.service_manager._service_path == file_path
+        self.service_manager.set_modified.assert_called_once_with(False)
+        self.service_manager.settings.setValue.assert_called_once_with('servicemanager/last file', file_path)
+        assert self.service_manager._save_lite is False
+
+    def test_set_file_name_oszl(self):
+        """
+        Test setting the file name
+        """
+        # GIVEN: A service manager, some mocks and a file name
+        self.service_manager.set_modified = MagicMock()
+        self.service_manager.settings.setValue = MagicMock()
+        file_path = Path('servicefile.oszl')
+
+        # WHEN: The filename is set
+        self.service_manager.set_file_name(file_path)
+
+        # THEN: Various things should have been called and set
+        assert self.service_manager._service_path == file_path
+        self.service_manager.set_modified.assert_called_once_with(False)
+        self.service_manager.settings.setValue.assert_called_once_with('servicemanager/last file', file_path)
+        assert self.service_manager._save_lite is True
+
+    def test_short_file_name(self):
+        """
+        Test the short_file_name method
+        """
+        # GIVEN: A service manager and a file name
+        self.service_manager._service_path = Path('/home/user/service.osz')
+
+        # WHEN: short_file_name is called
+        result = self.service_manager.short_file_name()
+
+        # THEN: The result should be correct
+        assert result == 'service.osz'
+
     def test_basic_service_manager(self):
         """
         Test the Service Manager UI Functionality
@@ -900,9 +1122,9 @@ class TestServiceManager(TestCase, TestMixin):
         """
         # GIVEN: A service item added
         self.service_manager.setup_ui(self.service_manager)
-        with patch('PyQt5.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
+        with patch('openlp.core.ui.servicemanager.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
-                patch('PyQt5.QtWidgets.QWidget.mapToGlobal'), \
+                patch('openlp.core.ui.servicemanager.QtWidgets.QWidget.mapToGlobal'), \
-                patch('PyQt5.QtWidgets.QMenu.exec'):
+                patch('openlp.core.ui.servicemanager.QtWidgets.QMenu.exec'):
             mocked_item = MagicMock()
             mocked_item.parent.return_value = None
             mocked_item_at_method.return_value = mocked_item
@@ -944,9 +1166,9 @@ class TestServiceManager(TestCase, TestMixin):
         """
         # GIVEN: A service item added
         self.service_manager.setup_ui(self.service_manager)
-        with patch('PyQt5.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
+        with patch('openlp.core.ui.servicemanager.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
-                patch('PyQt5.QtWidgets.QWidget.mapToGlobal'), \
+                patch('openlp.core.ui.servicemanager.QtWidgets.QWidget.mapToGlobal'), \
-                patch('PyQt5.QtWidgets.QMenu.exec'):
+                patch('openlp.core.ui.servicemanager.QtWidgets.QMenu.exec'):
             mocked_item = MagicMock()
             mocked_item.parent.return_value = None
             mocked_item_at_method.return_value = mocked_item
@@ -987,9 +1209,9 @@ class TestServiceManager(TestCase, TestMixin):
         """
         # GIVEN: A service item added
         self.service_manager.setup_ui(self.service_manager)
-        with patch('PyQt5.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
+        with patch('openlp.core.ui.servicemanager.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
-                patch('PyQt5.QtWidgets.QWidget.mapToGlobal'), \
+                patch('openlp.core.ui.servicemanager.QtWidgets.QWidget.mapToGlobal'), \
-                patch('PyQt5.QtWidgets.QMenu.exec'):
+                patch('openlp.core.ui.servicemanager.QtWidgets.QMenu.exec'):
             mocked_item = MagicMock()
             mocked_item.parent.return_value = None
             mocked_item_at_method.return_value = mocked_item
@@ -1032,9 +1254,9 @@ class TestServiceManager(TestCase, TestMixin):
         """
         # GIVEN: A service item added
         self.service_manager.setup_ui(self.service_manager)
-        with patch('PyQt5.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
+        with patch('openlp.core.ui.servicemanager.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
-                patch('PyQt5.QtWidgets.QWidget.mapToGlobal'), \
+                patch('openlp.core.ui.servicemanager.QtWidgets.QWidget.mapToGlobal'), \
-                patch('PyQt5.QtWidgets.QMenu.exec'):
+                patch('openlp.core.ui.servicemanager.QtWidgets.QMenu.exec'):
             mocked_item = MagicMock()
             mocked_item.parent.return_value = None
             mocked_item_at_method.return_value = mocked_item
@@ -1075,9 +1297,9 @@ class TestServiceManager(TestCase, TestMixin):
         """
         # GIVEN: A service item added
         self.service_manager.setup_ui(self.service_manager)
-        with patch('PyQt5.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
+        with patch('openlp.core.ui.servicemanager.QtWidgets.QTreeWidget.itemAt') as mocked_item_at_method, \
-                patch('PyQt5.QtWidgets.QWidget.mapToGlobal'), \
+                patch('openlp.core.ui.servicemanager.QtWidgets.QWidget.mapToGlobal'), \
-                patch('PyQt5.QtWidgets.QMenu.exec'):
+                patch('openlp.core.ui.servicemanager.QtWidgets.QMenu.exec'):
             mocked_item = MagicMock()
             mocked_item.parent.return_value = None
             mocked_item_at_method.return_value = mocked_item