← Back to team overview

openlp-core team mailing list archive

[Merge] lp:~arjan-i/openlp/images_groups_bugfix into lp:openlp

 

Arjan Schrijver has proposed merging lp:~arjan-i/openlp/images_groups_bugfix into lp:openlp.

Requested reviews:
  Raoul Snyman (raoul-snyman)
  Tim Bentley (trb143)
  Andreas Preikschat (googol)

For more details, see:
https://code.launchpad.net/~arjan-i/openlp/images_groups_bugfix/+merge/165295

- Fixed traceback on canceling the 'choose group' dialog
- Clear the 'new group' lineedit in the 'choose group' dialog
- Fixed traceback on adding multiple image groups to the service in one go
- Fixed a bug where new image groups added from the 'choose group' popup weren't added to the 'add group' and 'choose group' popup selectboxes
- Cleaned up obsolete line in imagetab
- Added a simple test for onResetClick()
- Added a test for recursively_delete_group()


NOTE: this merge request is now only a bugfix merge for what's already in trunk. The 'adding multiple service items' functionality is completely gone. I'll work on the changes as discussed on the mailinglist in a different branch.
-- 
https://code.launchpad.net/~arjan-i/openlp/images_groups_bugfix/+merge/165295
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/plugins/images/forms/choosegroupform.py'
--- openlp/plugins/images/forms/choosegroupform.py	2013-04-19 19:03:16 +0000
+++ openlp/plugins/images/forms/choosegroupform.py	2013-05-23 07:35:33 +0000
@@ -50,6 +50,7 @@
         ``selected_group``
             The ID of the group that should be selected by default when showing the dialog.
         """
+        self.new_group_edit.clear()
         if selected_group is not None:
             for index in range(self.group_combobox.count()):
                 if self.group_combobox.itemData(index) == selected_group:

=== modified file 'openlp/plugins/images/lib/imagetab.py'
--- openlp/plugins/images/lib/imagetab.py	2013-03-18 14:11:58 +0000
+++ openlp/plugins/images/lib/imagetab.py	2013-05-23 07:35:33 +0000
@@ -32,7 +32,6 @@
 from openlp.core.lib import Registry, SettingsTab, Settings, UiStrings, translate
 
 
-
 class ImageTab(SettingsTab):
     """
     ImageTab is the images settings tab in the settings dialog.

=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py	2013-04-19 19:15:12 +0000
+++ openlp/plugins/images/lib/mediaitem.py	2013-05-23 07:35:33 +0000
@@ -391,6 +391,7 @@
         ``initial_load``
             When set to False, the busy cursor and progressbar will be shown while loading images
         """
+        parent_group = None
         if target_group is None:
             # Find out if a group must be pre-selected
             preselect_group = None
@@ -436,6 +437,8 @@
                     parent_group.parent_id = 0
                     parent_group.group_name = self.choose_group_form.new_group_edit.text()
                     self.manager.save_object(parent_group)
+                    self.fill_groups_combobox(self.choose_group_form.group_combobox)
+                    self.fill_groups_combobox(self.add_group_form.parent_group_combobox)
         else:
             parent_group = target_group.data(0, QtCore.Qt.UserRole)
             if isinstance(parent_group, ImageFilenames):
@@ -550,28 +553,25 @@
         service_item.add_capability(ItemCapabilities.CanAppend)
         # force a nonexistent theme
         service_item.theme = -1
-        missing_items = []
         missing_items_filenames = []
+        images_filenames = []
         # Expand groups to images
         for bitem in items:
             if isinstance(bitem.data(0, QtCore.Qt.UserRole), ImageGroups) or bitem.data(0, QtCore.Qt.UserRole) is None:
                 for index in range(0, bitem.childCount()):
                     if isinstance(bitem.child(index).data(0, QtCore.Qt.UserRole), ImageFilenames):
-                        items.append(bitem.child(index))
-                items.remove(bitem)
+                        images_filenames.append(bitem.child(index).data(0, QtCore.Qt.UserRole).filename)
+            elif isinstance(bitem.data(0, QtCore.Qt.UserRole), ImageFilenames):
+                images_filenames.append(bitem.data(0, QtCore.Qt.UserRole).filename)
         # Don't try to display empty groups
-        if not items:
+        if not images_filenames:
             return False
         # Find missing files
-        for bitem in items:
-            filename = bitem.data(0, QtCore.Qt.UserRole).filename
+        for filename in images_filenames:
             if not os.path.exists(filename):
-                missing_items.append(bitem)
                 missing_items_filenames.append(filename)
-        for item in missing_items:
-            items.remove(item)
         # We cannot continue, as all images do not exist.
-        if not items:
+        if not images_filenames:
             if not remote:
                 critical_error_message_box(
                     translate('ImagePlugin.MediaItem', 'Missing Image(s)'),
@@ -579,15 +579,14 @@
                         u'\n'.join(missing_items_filenames))
             return False
         # We have missing as well as existing images. We ask what to do.
-        elif missing_items and QtGui.QMessageBox.question(self,
+        elif missing_items_filenames and QtGui.QMessageBox.question(self,
             translate('ImagePlugin.MediaItem', 'Missing Image(s)'),
             translate('ImagePlugin.MediaItem', 'The following image(s) no longer exist: %s\n'
                 'Do you want to add the other images anyway?') % u'\n'.join(missing_items_filenames),
             QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.No | QtGui.QMessageBox.Yes)) == QtGui.QMessageBox.No:
             return False
         # Continue with the existing images.
-        for bitem in items:
-            filename = bitem.data(0, QtCore.Qt.UserRole).filename
+        for filename in images_filenames:
             name = os.path.split(filename)[1]
             service_item.add_from_image(filename, name, background)
         return True

=== modified file 'tests/functional/openlp_plugins/images/test_lib.py'
--- tests/functional/openlp_plugins/images/test_lib.py	2013-04-19 19:07:25 +0000
+++ tests/functional/openlp_plugins/images/test_lib.py	2013-05-23 07:35:33 +0000
@@ -9,7 +9,7 @@
 from mock import MagicMock, patch
 
 from openlp.core.lib import Registry
-from openlp.plugins.images.lib.db import ImageFilenames
+from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups
 from openlp.plugins.images.lib.mediaitem import ImageMediaItem
 
 
@@ -23,6 +23,7 @@
         Registry.create()
         Registry().register(u'service_list', MagicMock())
         Registry().register(u'main_window', self.mocked_main_window)
+        Registry().register(u'live_controller', MagicMock())
         mocked_parent = MagicMock()
         mocked_plugin = MagicMock()
         with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.__init__') as mocked_init:
@@ -35,7 +36,7 @@
         """
         # GIVEN: An empty image_list
         image_list = []
-        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_loadFullList:
+        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
             self.media_item.manager = MagicMock()
 
             # WHEN: We run save_new_images_list with the empty list
@@ -51,7 +52,7 @@
         """
         # GIVEN: A list with 1 image
         image_list = [ u'test_image.jpg' ]
-        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_loadFullList:
+        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
             ImageFilenames.filename = ''
             self.media_item.manager = MagicMock()
 
@@ -59,7 +60,7 @@
             self.media_item.save_new_images_list(image_list, reload_list=True)
 
             # THEN: load_full_list() should have been called
-            assert mocked_loadFullList.call_count == 1, u'load_full_list() should have been called'
+            assert mocked_load_full_list.call_count == 1, u'load_full_list() should have been called'
 
             # CLEANUP: Remove added attribute from ImageFilenames
             delattr(ImageFilenames, 'filename')
@@ -70,14 +71,14 @@
         """
         # GIVEN: A list with 1 image
         image_list = [ u'test_image.jpg' ]
-        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_loadFullList:
+        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
             self.media_item.manager = MagicMock()
 
             # WHEN: We run save_new_images_list with reload_list=False
             self.media_item.save_new_images_list(image_list, reload_list=False)
 
             # THEN: load_full_list() should not have been called
-            assert mocked_loadFullList.call_count == 0, u'load_full_list() should not have been called'
+            assert mocked_load_full_list.call_count == 0, u'load_full_list() should not have been called'
 
     def save_new_images_list_multiple_images_test(self):
         """
@@ -85,7 +86,7 @@
         """
         # GIVEN: A list with 3 images
         image_list = [ u'test_image_1.jpg', u'test_image_2.jpg', u'test_image_3.jpg' ]
-        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_loadFullList:
+        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
             self.media_item.manager = MagicMock()
 
             # WHEN: We run save_new_images_list with the list of 3 images
@@ -101,7 +102,7 @@
         """
         # GIVEN: A list with images and objects
         image_list = [ u'test_image_1.jpg', None, True, ImageFilenames(), 'test_image_2.jpg' ]
-        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_loadFullList:
+        with patch(u'openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
             self.media_item.manager = MagicMock()
 
             # WHEN: We run save_new_images_list with the list of images and objects
@@ -110,3 +111,68 @@
             # THEN: load_full_list() should not have been called
             assert self.media_item.manager.save_object.call_count == 2, \
                 u'load_full_list() should have been called only once'
+
+    def on_reset_click_test(self):
+        """
+        Test that on_reset_click() actually resets the background
+        """
+        # GIVEN: A mocked version of reset_action
+        self.media_item.reset_action = MagicMock()
+
+        # WHEN: on_reset_click is called
+        self.media_item.on_reset_click()
+
+        # THEN: the reset_action should be set visible, and the image should be reset
+        self.media_item.reset_action.setVisible.assert_called_with(False)
+        self.media_item.live_controller.display.reset_image.assert_called_with()
+
+    def _recursively_delete_group_side_effect(*args, **kwargs):
+        """
+        Side effect method that creates custom retun values for the recursively_delete_group method
+        """
+        if args[1] == ImageFilenames and args[2]:
+            # Create some fake objects that should be removed
+            returned_object1 = ImageFilenames()
+            returned_object1.id = 1
+            returned_object1.filename = u'/tmp/test_file_1.jpg'
+            returned_object2 = ImageFilenames()
+            returned_object2.id = 2
+            returned_object2.filename = u'/tmp/test_file_2.jpg'
+            returned_object3 = ImageFilenames()
+            returned_object3.id = 3
+            returned_object3.filename = u'/tmp/test_file_3.jpg'
+            return [returned_object1, returned_object2, returned_object3]
+        if args[1] == ImageGroups and args[2]:
+            # Change the parent_id that is matched so we don't get into an endless loop
+            ImageGroups.parent_id = 0
+            # Create a fake group that will be used in the next run
+            returned_object1 = ImageGroups()
+            returned_object1.id = 1
+            return [returned_object1]
+        return []
+
+    def recursively_delete_group_test(self):
+        """
+        Test that recursively_delete_group() works
+        """
+        # GIVEN: An ImageGroups object and mocked functions
+        with patch(u'openlp.core.utils.delete_file') as mocked_delete_file:
+            ImageFilenames.group_id = 1
+            ImageGroups.parent_id = 1
+            self.media_item.manager = MagicMock()
+            self.media_item.manager.get_all_objects.side_effect = self._recursively_delete_group_side_effect
+            self.media_item.servicePath = ""
+            test_group = ImageGroups()
+            test_group.id = 1
+
+            # WHEN: recursively_delete_group() is called
+            self.media_item.recursively_delete_group(test_group)
+
+            # THEN:
+            assert mocked_delete_file.call_count == 0, u'delete_file() should not be called'
+            assert self.media_item.manager.delete_object.call_count == 7, \
+                u'manager.delete_object() should be called exactly 7 times'
+
+            # CLEANUP: Remove added attribute from ImageFilenames and ImageGroups
+            delattr(ImageFilenames, 'group_id')
+            delattr(ImageGroups, 'parent_id')


Follow ups