openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #26908
[Merge] lp:~raoul-snyman/openlp/bug-1451237 into lp:openlp
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/bug-1451237 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1451237 in OpenLP: "Remote view: sending image or video live from search results causes traceback"
https://bugs.launchpad.net/openlp/+bug/1451237
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/bug-1451237/+merge/259336
Fix bug#1451237 by overriding the create_item_from_id() method in the Images MediaItem.
Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1451237 (revision 2537)
[SUCCESS] https//ci.openlp.io/job/Branch-01-Pull/1019/
[SUCCESS] https//ci.openlp.io/job/Branch-02-Functional-Tests/942/
[SUCCESS] https//ci.openlp.io/job/Branch-03-Interface-Tests/884/
[SUCCESS] https//ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/771/
[SUCCESS] https//ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/369/
[SUCCESS] https//ci.openlp.io/job/Branch-05a-Code_Analysis/501/
[SUCCESS] https//ci.openlp.io/job/Branch-05b-Test_Coverage/372/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1451237 into lp:openlp.
=== modified file 'openlp/plugins/images/lib/mediaitem.py'
--- openlp/plugins/images/lib/mediaitem.py 2015-04-22 19:37:32 +0000
+++ openlp/plugins/images/lib/mediaitem.py 2015-05-17 21:27:07 +0000
@@ -551,6 +551,7 @@
service_item.title = items[0].text(0)
else:
service_item.title = str(self.plugin.name_strings['plural'])
+
service_item.add_capability(ItemCapabilities.CanMaintain)
service_item.add_capability(ItemCapabilities.CanPreview)
service_item.add_capability(ItemCapabilities.CanLoop)
@@ -697,3 +698,15 @@
filename = os.path.split(str(file_object.filename))[1]
results.append([file_object.filename, filename])
return results
+
+ def create_item_from_id(self, item_id):
+ """
+ Create a media item from an item id. Overridden from the parent method to change the item type.
+
+ :param item_id: Id to make live
+ """
+ item = QtGui.QTreeWidgetItem()
+ item_data = self.manager.get_object_filtered(ImageFilenames, ImageFilenames.filename == item_id)
+ item.setText(0, os.path.basename(item_data.filename))
+ item.setData(0, QtCore.Qt.UserRole, item_data)
+ return item
=== modified file 'tests/functional/openlp_plugins/images/test_lib.py'
--- tests/functional/openlp_plugins/images/test_lib.py 2015-04-22 21:05:46 +0000
+++ tests/functional/openlp_plugins/images/test_lib.py 2015-05-17 21:27:07 +0000
@@ -24,6 +24,8 @@
"""
from unittest import TestCase
+from PyQt4 import QtCore, QtGui
+
from openlp.core.common import Registry
from openlp.plugins.images.lib.db import ImageFilenames, ImageGroups
from openlp.plugins.images.lib.mediaitem import ImageMediaItem
@@ -48,123 +50,121 @@
self.media_item = ImageMediaItem(None, mocked_plugin)
self.media_item.settings_section = 'images'
- def validate_and_load_test(self):
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list')
+ @patch('openlp.plugins.images.lib.mediaitem.Settings')
+ def validate_and_load_test(self, mocked_settings, mocked_load_list):
"""
Test that the validate_and_load_test() method when called without a group
"""
# GIVEN: A list of files
file_list = ['/path1/image1.jpg', '/path2/image2.jpg']
- with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list') as mocked_load_list, \
- patch('openlp.plugins.images.lib.mediaitem.Settings') as mocked_settings:
-
- # WHEN: Calling validate_and_load with the list of files
- self.media_item.validate_and_load(file_list)
-
- # THEN: load_list should have been called with the file list and None,
- # the directory should have been saved to the settings
- mocked_load_list.assert_called_once_with(file_list, None)
- mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
-
- def validate_and_load_group_test(self):
+ # WHEN: Calling validate_and_load with the list of files
+ self.media_item.validate_and_load(file_list)
+
+ # THEN: load_list should have been called with the file list and None,
+ # the directory should have been saved to the settings
+ mocked_load_list.assert_called_once_with(file_list, None)
+ mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list')
+ @patch('openlp.plugins.images.lib.mediaitem.Settings')
+ def validate_and_load_group_test(self, mocked_settings, mocked_load_list):
"""
Test that the validate_and_load_test() method when called with a group
"""
# GIVEN: A list of files
file_list = ['/path1/image1.jpg', '/path2/image2.jpg']
- with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_list') as mocked_load_list, \
- patch('openlp.plugins.images.lib.mediaitem.Settings') as mocked_settings:
-
- # WHEN: Calling validate_and_load with the list of files and a group
- self.media_item.validate_and_load(file_list, 'group')
-
- # THEN: load_list should have been called with the file list and the group name,
- # the directory should have been saved to the settings
- mocked_load_list.assert_called_once_with(file_list, 'group')
- mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
-
- def save_new_images_list_empty_list_test(self):
+ # WHEN: Calling validate_and_load with the list of files and a group
+ self.media_item.validate_and_load(file_list, 'group')
+
+ # THEN: load_list should have been called with the file list and the group name,
+ # the directory should have been saved to the settings
+ mocked_load_list.assert_called_once_with(file_list, 'group')
+ mocked_settings().setValue.assert_called_once_with(ANY, '/path1')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list')
+ def save_new_images_list_empty_list_test(self, mocked_load_full_list):
"""
Test that the save_new_images_list() method handles empty lists gracefully
"""
# GIVEN: An empty image_list
image_list = []
- with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list'):
- self.media_item.manager = MagicMock()
-
- # WHEN: We run save_new_images_list with the empty list
- self.media_item.save_new_images_list(image_list)
-
- # THEN: The save_object() method should not have been called
- self.assertEquals(self.media_item.manager.save_object.call_count, 0,
- 'The save_object() method should not have been called')
-
- def save_new_images_list_single_image_with_reload_test(self):
+ self.media_item.manager = MagicMock()
+
+ # WHEN: We run save_new_images_list with the empty list
+ self.media_item.save_new_images_list(image_list)
+
+ # THEN: The save_object() method should not have been called
+ self.assertEquals(self.media_item.manager.save_object.call_count, 0,
+ 'The save_object() method should not have been called')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list')
+ def save_new_images_list_single_image_with_reload_test(self, mocked_load_full_list):
"""
Test that the save_new_images_list() calls load_full_list() when reload_list is set to True
"""
# GIVEN: A list with 1 image and a mocked out manager
image_list = ['test_image.jpg']
- with patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list') as mocked_load_full_list:
- ImageFilenames.filename = ''
- self.media_item.manager = MagicMock()
-
- # WHEN: We run save_new_images_list with reload_list=True
- self.media_item.save_new_images_list(image_list, reload_list=True)
-
- # THEN: load_full_list() should have been called
- self.assertEquals(mocked_load_full_list.call_count, 1, 'load_full_list() should have been called')
-
- # CLEANUP: Remove added attribute from ImageFilenames
- delattr(ImageFilenames, 'filename')
-
- def save_new_images_list_single_image_without_reload_test(self):
+ ImageFilenames.filename = ''
+ self.media_item.manager = MagicMock()
+
+ # WHEN: We run save_new_images_list with reload_list=True
+ self.media_item.save_new_images_list(image_list, reload_list=True)
+
+ # THEN: load_full_list() should have been called
+ self.assertEquals(mocked_load_full_list.call_count, 1, 'load_full_list() should have been called')
+
+ # CLEANUP: Remove added attribute from ImageFilenames
+ delattr(ImageFilenames, 'filename')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list')
+ def save_new_images_list_single_image_without_reload_test(self, mocked_load_full_list):
"""
Test that the save_new_images_list() doesn't call load_full_list() when reload_list is set to False
"""
# GIVEN: A list with 1 image and a mocked out manager
image_list = ['test_image.jpg']
- with patch('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
- self.assertEquals(mocked_load_full_list.call_count, 0, 'load_full_list() should not have been called')
-
- def save_new_images_list_multiple_images_test(self):
+ 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
+ self.assertEquals(mocked_load_full_list.call_count, 0, 'load_full_list() should not have been called')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list')
+ def save_new_images_list_multiple_images_test(self, mocked_load_full_list):
"""
Test that the save_new_images_list() saves all images in the list
"""
# GIVEN: A list with 3 images
image_list = ['test_image_1.jpg', 'test_image_2.jpg', 'test_image_3.jpg']
- with patch('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
- self.media_item.save_new_images_list(image_list, reload_list=False)
-
- # THEN: load_full_list() should not have been called
- self.assertEquals(self.media_item.manager.save_object.call_count, 3,
- 'load_full_list() should have been called three times')
-
- def save_new_images_list_other_objects_in_list_test(self):
+ self.media_item.manager = MagicMock()
+
+ # WHEN: We run save_new_images_list with the list of 3 images
+ self.media_item.save_new_images_list(image_list, reload_list=False)
+
+ # THEN: load_full_list() should not have been called
+ self.assertEquals(self.media_item.manager.save_object.call_count, 3,
+ 'load_full_list() should have been called three times')
+
+ @patch('openlp.plugins.images.lib.mediaitem.ImageMediaItem.load_full_list')
+ def save_new_images_list_other_objects_in_list_test(self, mocked_load_full_list):
"""
Test that the save_new_images_list() ignores everything in the provided list except strings
"""
# GIVEN: A list with images and objects
image_list = ['test_image_1.jpg', None, True, ImageFilenames(), 'test_image_2.jpg']
- with patch('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
- self.media_item.save_new_images_list(image_list, reload_list=False)
-
- # THEN: load_full_list() should not have been called
- self.assertEquals(self.media_item.manager.save_object.call_count, 2,
- 'load_full_list() should have been called only once')
+ self.media_item.manager = MagicMock()
+
+ # WHEN: We run save_new_images_list with the list of images and objects
+ self.media_item.save_new_images_list(image_list, reload_list=False)
+
+ # THEN: load_full_list() should not have been called
+ self.assertEquals(self.media_item.manager.save_object.call_count, 2,
+ 'load_full_list() should have been called only once')
def on_reset_click_test(self):
"""
@@ -180,31 +180,31 @@
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_test(self):
+ @patch('openlp.plugins.images.lib.mediaitem.delete_file')
+ def recursively_delete_group_test(self, mocked_delete_file):
"""
Test that recursively_delete_group() works
"""
# GIVEN: An ImageGroups object and mocked functions
- with patch('openlp.plugins.images.lib.mediaitem.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.service_path = ''
- test_group = ImageGroups()
- test_group.id = 1
-
- # WHEN: recursively_delete_group() is called
- self.media_item.recursively_delete_group(test_group)
-
- # THEN: delete_file() should have been called 12 times and manager.delete_object() 7 times.
- self.assertEquals(mocked_delete_file.call_count, 12, 'delete_file() should have been called 12 times')
- self.assertEquals(self.media_item.manager.delete_object.call_count, 7,
- 'manager.delete_object() should be called exactly 7 times')
-
- # CLEANUP: Remove added attribute from Image Filenames and ImageGroups
- delattr(ImageFilenames, 'group_id')
- delattr(ImageGroups, 'parent_id')
+ 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.service_path = ''
+ test_group = ImageGroups()
+ test_group.id = 1
+
+ # WHEN: recursively_delete_group() is called
+ self.media_item.recursively_delete_group(test_group)
+
+ # THEN: delete_file() should have been called 12 times and manager.delete_object() 7 times.
+ self.assertEquals(mocked_delete_file.call_count, 12, 'delete_file() should have been called 12 times')
+ self.assertEquals(self.media_item.manager.delete_object.call_count, 7,
+ 'manager.delete_object() should be called exactly 7 times')
+
+ # CLEANUP: Remove added attribute from Image Filenames and ImageGroups
+ delattr(ImageFilenames, 'group_id')
+ delattr(ImageGroups, 'parent_id')
def _recursively_delete_group_side_effect(*args, **kwargs):
"""
@@ -231,28 +231,51 @@
return [returned_object1]
return []
- def on_delete_click_test(self):
+ @patch('openlp.plugins.images.lib.mediaitem.delete_file')
+ @patch('openlp.plugins.images.lib.mediaitem.check_item_selected')
+ def on_delete_click_test(self, mocked_check_item_selected, mocked_delete_file):
"""
Test that on_delete_click() works
"""
# GIVEN: An ImageGroups object and mocked functions
- with patch('openlp.plugins.images.lib.mediaitem.delete_file') as mocked_delete_file, \
- patch('openlp.plugins.images.lib.mediaitem.check_item_selected') as mocked_check_item_selected:
- mocked_check_item_selected.return_value = True
- test_image = ImageFilenames()
- test_image.id = 1
- test_image.group_id = 1
- test_image.filename = 'imagefile.png'
- self.media_item.manager = MagicMock()
- self.media_item.service_path = ''
- self.media_item.list_view = MagicMock()
- mocked_row_item = MagicMock()
- mocked_row_item.data.return_value = test_image
- mocked_row_item.text.return_value = ''
- self.media_item.list_view.selectedItems.return_value = [mocked_row_item]
-
- # WHEN: Calling on_delete_click
- self.media_item.on_delete_click()
-
- # THEN: delete_file should have been called twice
- self.assertEquals(mocked_delete_file.call_count, 2, 'delete_file() should have been called twice')
+ mocked_check_item_selected.return_value = True
+ test_image = ImageFilenames()
+ test_image.id = 1
+ test_image.group_id = 1
+ test_image.filename = 'imagefile.png'
+ self.media_item.manager = MagicMock()
+ self.media_item.service_path = ''
+ self.media_item.list_view = MagicMock()
+ mocked_row_item = MagicMock()
+ mocked_row_item.data.return_value = test_image
+ mocked_row_item.text.return_value = ''
+ self.media_item.list_view.selectedItems.return_value = [mocked_row_item]
+
+ # WHEN: Calling on_delete_click
+ self.media_item.on_delete_click()
+
+ # THEN: delete_file should have been called twice
+ self.assertEquals(mocked_delete_file.call_count, 2, 'delete_file() should have been called twice')
+
+ def create_item_from_id_test(self):
+ """
+ Test that the create_item_from_id() method returns a valid QTreeWidgetItem with a pre-created ImageFilenames
+ """
+ # GIVEN: An ImageFilenames that already exists in the database
+ image_file = ImageFilenames()
+ image_file.id = 1
+ image_file.filename = '/tmp/test_file_1.jpg'
+ self.media_item.manager = MagicMock()
+ self.media_item.manager.get_object_filtered.return_value = image_file
+ ImageFilenames.filename = ''
+
+ # WHEN: create_item_from_id() is called
+ item = self.media_item.create_item_from_id(1)
+
+ # THEN: A QTreeWidgetItem should be created with the above model object as it's data
+ self.assertIsInstance(item, QtGui.QTreeWidgetItem)
+ self.assertEqual('test_file_1.jpg', item.text(0))
+ item_data = item.data(0, QtCore.Qt.UserRole)
+ self.assertIsInstance(item_data, ImageFilenames)
+ self.assertEqual(1, item_data.id)
+ self.assertEqual('/tmp/test_file_1.jpg', item_data.filename)
Follow ups
