From b27aae52cfbf9a91b56c81299b0fe633f456342f Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sat, 16 May 2015 23:24:57 +0200 Subject: [PATCH 1/2] Fix bug#1451237 by overriding the create_item_from_id() method in the child MediaItem class to build a proper QTreeWidgetItem. --- openlp/plugins/images/lib/mediaitem.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/openlp/plugins/images/lib/mediaitem.py b/openlp/plugins/images/lib/mediaitem.py index 8c8bfe811..5c28de338 100644 --- a/openlp/plugins/images/lib/mediaitem.py +++ b/openlp/plugins/images/lib/mediaitem.py @@ -551,6 +551,7 @@ class ImageMediaItem(MediaManagerItem): 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 @@ class ImageMediaItem(MediaManagerItem): 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 From d354bcf36590e677f2fcddd4cb46e56f6ef8230c Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Sun, 17 May 2015 23:13:05 +0200 Subject: [PATCH 2/2] Test that the overridden method does the right thing --- .../openlp_plugins/images/test_lib.py | 219 ++++++++++-------- 1 file changed, 121 insertions(+), 98 deletions(-) diff --git a/tests/functional/openlp_plugins/images/test_lib.py b/tests/functional/openlp_plugins/images/test_lib.py index 070b4b40d..9d187af9e 100644 --- a/tests/functional/openlp_plugins/images/test_lib.py +++ b/tests/functional/openlp_plugins/images/test_lib.py @@ -24,6 +24,8 @@ This module contains tests for the lib submodule of the Images plugin. """ 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 @@ class TestImageMediaItem(TestCase): 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) - # 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') - # 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): + @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') - # 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') - # 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): + @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() + 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) + # 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') + # 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): + @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() + 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) + # 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') + # 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') + # CLEANUP: Remove added attribute from ImageFilenames + delattr(ImageFilenames, 'filename') - def save_new_images_list_single_image_without_reload_test(self): + @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() + 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) + # 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') + # 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): + @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() + 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) + # 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') + # 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): + @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() + 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) + # 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') + # 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 @@ class TestImageMediaItem(TestCase): 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 + 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) + # 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') + # 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') + # 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 @@ class TestImageMediaItem(TestCase): 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] + 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() + # 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') + # 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)