From df800850e1cee6a43955e35a7886d2d0950dc26e Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sun, 20 Apr 2014 18:00:24 +0200 Subject: [PATCH 01/13] fixed bug 1240037 (Error occured when moving directory) Fixes: https://launchpad.net/bugs/1240037 --- openlp/core/ui/mainwindow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 81e822c16..e17b1a976 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1334,7 +1334,7 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties): if self.copy_data: log.info('Copying data to new path') try: - self.showStatusMessage( + self.show_status_message( translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - %s ' '- Please wait for copy to finish').replace('%s', self.new_data_path)) dir_util.copy_tree(old_data_path, self.new_data_path) From cf798e73d592f43285eb9b1654dc808ee5f71ebf Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sun, 20 Apr 2014 18:02:50 +0200 Subject: [PATCH 02/13] removed not needed code --- openlp/core/ui/mainwindow.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index e17b1a976..9c193b079 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -1364,8 +1364,7 @@ class MainWindow(QtGui.QMainWindow, Ui_MainWindow, RegistryProperties): args = [] for a in self.arguments: args.extend([a]) - for arg in args: - filename = arg + for filename in args: if not isinstance(filename, str): filename = str(filename, sys.getfilesystemencoding()) if filename.endswith(('.osz', '.oszl')): From 6c08a80bfda00504312ec54f82cf51a320fbb17e Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Tue, 22 Apr 2014 12:29:15 +0200 Subject: [PATCH 03/13] added test; refactor --- openlp/core/ui/shortcutlistform.py | 20 +++--- openlp/core/utils/actions.py | 13 ++-- .../openlp_core_utils/test_actions.py | 62 +++++++++++++++++++ 3 files changed, 79 insertions(+), 16 deletions(-) diff --git a/openlp/core/ui/shortcutlistform.py b/openlp/core/ui/shortcutlistform.py index 4b64c3b54..dbfbbd439 100644 --- a/openlp/core/ui/shortcutlistform.py +++ b/openlp/core/ui/shortcutlistform.py @@ -244,10 +244,10 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog, RegistryProperties) self.primary_push_button.setChecked(False) self.alternate_push_button.setChecked(False) else: - if action.defaultShortcuts: - primary_label_text = action.defaultShortcuts[0].toString() - if len(action.defaultShortcuts) == 2: - alternate_label_text = action.defaultShortcuts[1].toString() + if action.default_shortcuts: + primary_label_text = action.default_shortcuts[0].toString() + if len(action.default_shortcuts) == 2: + alternate_label_text = action.default_shortcuts[1].toString() shortcuts = self._action_shortcuts(action) # We do not want to loose pending changes, that is why we have to keep the text when, this function has not # been triggered by a signal. @@ -292,7 +292,7 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog, RegistryProperties) self._adjust_button(self.alternate_push_button, False, text='') for category in self.action_list.categories: for action in category.actions: - self.changed_actions[action] = action.defaultShortcuts + self.changed_actions[action] = action.default_shortcuts self.refresh_shortcut_list() def on_default_radio_button_clicked(self, toggled): @@ -306,7 +306,7 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog, RegistryProperties) if action is None: return temp_shortcuts = self._action_shortcuts(action) - self.changed_actions[action] = action.defaultShortcuts + self.changed_actions[action] = action.default_shortcuts self.refresh_shortcut_list() primary_button_text = '' alternate_button_text = '' @@ -357,8 +357,8 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog, RegistryProperties) return shortcuts = self._action_shortcuts(action) new_shortcuts = [] - if action.defaultShortcuts: - new_shortcuts.append(action.defaultShortcuts[0]) + if action.default_shortcuts: + new_shortcuts.append(action.default_shortcuts[0]) # We have to check if the primary default shortcut is available. But we only have to check, if the action # has a default primary shortcut (an "empty" shortcut is always valid and if the action does not have a # default primary shortcut, then the alternative shortcut (not the default one) will become primary @@ -383,8 +383,8 @@ class ShortcutListForm(QtGui.QDialog, Ui_ShortcutListDialog, RegistryProperties) new_shortcuts = [] if shortcuts: new_shortcuts.append(shortcuts[0]) - if len(action.defaultShortcuts) == 2: - new_shortcuts.append(action.defaultShortcuts[1]) + if len(action.default_shortcuts) == 2: + new_shortcuts.append(action.default_shortcuts[1]) if len(new_shortcuts) == 2: if not self._validiate_shortcut(action, new_shortcuts[1]): return diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 29f2d279b..b4efc561f 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -69,6 +69,7 @@ class CategoryActionList(object): """ Implement the __getitem__() method to make this class a dictionary type """ + assert False for weight, action in self.actions: if action.text() == key: return action @@ -78,7 +79,10 @@ class CategoryActionList(object): """ Implement the __contains__() method to make this class a dictionary type """ - return item in self + for weight, action in self.actions: + if action.text() == item: + return True + return False def __len__(self): """ @@ -107,10 +111,7 @@ class CategoryActionList(object): """ Implement the has_key() method to make this class a dictionary type """ - for weight, action in self.actions: - if action.text() == key: - return True - return False + return key in self def append(self, name): """ @@ -270,7 +271,7 @@ class ActionList(object): settings = Settings() settings.beginGroup('shortcuts') # Get the default shortcut from the config. - action.defaultShortcuts = settings.get_default_value(action.objectName()) + action.default_shortcuts = settings.get_default_value(action.objectName()) if weight is None: self.categories[category].actions.append(action) else: diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 2868f8555..566b020fe 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -32,9 +32,11 @@ Package to test the openlp.core.utils.actions package. from unittest import TestCase from PyQt4 import QtGui, QtCore +from mock import MagicMock from openlp.core.common import Settings from openlp.core.utils import ActionList +from openlp.core.utils.actions import CategoryActionList from tests.helpers.testmixin import TestMixin @@ -149,3 +151,63 @@ class TestActionList(TestCase, TestMixin): # THEN: Both action should keep their shortcuts. assert len(action3.shortcuts()) == 2, 'The action should have two shortcut assigned.' assert len(action_with_same_shortcuts3.shortcuts()) == 2, 'The action should have two shortcuts assigned.' + + +class TestCategoryActionList(TestCase): + def setUp(self): + """ + """ + self.added_action = MagicMock() + self.added_action.text = MagicMock('first') + self.not_added_action = MagicMock('second') + self.not_added_action.text = MagicMock() + self.list = CategoryActionList() + self.list.add(self.added_action, 10) + + def tearDown(self): + """ + + """ + del self.list + + def len_test(self): + """ + Test the __len__ method + """ + # GIVEN: The list. + + # WHEN: Check the length + length = len(self.list) + + # THEN: + self.assertEqual(length, 1, "The length should be 1.") + + # GIVEN: A list with an item. + self.list.append(self.not_added_action) + + # WHEN: Check the length. + length = len(self.list) + + # THEN: + self.assertEqual(length, 2, "The length should be 2.") + + def remove_test(self): + """ + Test the remove() method + """ + # GIVEN: The list + + # WHEN: Delete an item from the list. + self.list.remove(self.added_action) + + # THEN: Now the element should not be in the list anymore. + self.assertFalse(self.added_action in self.list) + + def contains_test(self): + """ + Test the __contains__() method + """ + # GIVEN: The list. + # WHEN: Do nothing. + # THEN: A not added item should not be in the list. + self.assertFalse(self.not_added_action in self.list) \ No newline at end of file From 1ae7c32adb435428193f05a39980d403712f9d2a Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Tue, 22 Apr 2014 12:32:02 +0200 Subject: [PATCH 04/13] fixed import --- tests/functional/openlp_core_utils/test_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 566b020fe..9ee0f5c6a 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -32,11 +32,11 @@ Package to test the openlp.core.utils.actions package. from unittest import TestCase from PyQt4 import QtGui, QtCore -from mock import MagicMock from openlp.core.common import Settings from openlp.core.utils import ActionList from openlp.core.utils.actions import CategoryActionList +from tests.functional import MagicMock from tests.helpers.testmixin import TestMixin From 5a3aecf997040b0705fd7319f8d8fb39a3455eaa Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Tue, 22 Apr 2014 17:06:21 +0200 Subject: [PATCH 05/13] fixed bugs in class --- openlp/core/utils/actions.py | 15 +- .../openlp_core_utils/test_actions.py | 147 +++++++++++------- 2 files changed, 94 insertions(+), 68 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index b4efc561f..98eaced5b 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -69,18 +69,17 @@ class CategoryActionList(object): """ Implement the __getitem__() method to make this class a dictionary type """ - assert False - for weight, action in self.actions: - if action.text() == key: - return action - raise KeyError('Action "%s" does not exist.' % key) + try: + return self.actions[key] + except: + raise KeyError('Action "%s" does not exist.' % key) def __contains__(self, item): """ Implement the __contains__() method to make this class a dictionary type """ for weight, action in self.actions: - if action.text() == item: + if action == item: return True return False @@ -113,14 +112,14 @@ class CategoryActionList(object): """ return key in self - def append(self, name): + def append(self, action): """ Append an action """ weight = 0 if self.actions: weight = self.actions[-1][0] + 1 - self.add(name, weight) + self.add(action, weight) def add(self, action, weight=0): """ diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 9ee0f5c6a..d79617037 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -36,10 +36,96 @@ from PyQt4 import QtGui, QtCore from openlp.core.common import Settings from openlp.core.utils import ActionList from openlp.core.utils.actions import CategoryActionList -from tests.functional import MagicMock +from tests.functional import MagicMock, patch from tests.helpers.testmixin import TestMixin +class TestCategoryActionList(TestCase): + def setUp(self): + """ + Create an instance and a few example actions. + """ + self.action1 = MagicMock() + self.action1.text.return_value = 'first' + self.action2 = MagicMock() + self.action2.text.return_value = 'second' + self.list = CategoryActionList() + + def tearDown(self): + """ + Clean up + """ + del self.list + + def contains_test(self): + """ + Test the __contains__() method + """ + # GIVEN: The list. + # WHEN: Add an action + self.list.append(self.action1) + # THEN: + self.assertTrue(self.action1 in self.list) + self.assertFalse(self.action2 in self.list) + + def len_test(self): + """ + Test the __len__ method + """ + # GIVEN: The list. + # WHEN: + # THEN: Check the length. + self.assertEqual(len(self.list), 0, "The length should be 0.") + + # GIVEN: The list. + # WHEN: Append an action. + self.list.append(self.action1) + + # THEN: Check the length. + self.assertEqual(len(self.list), 1, "The length should be 1.") + + def append_test(self): + """ + Test the append() method + """ + # GIVEN: The list. + # WHEN: Append an action. + self.list.append(self.action1) + self.list.append(self.action2) + + # THEN: Check if the actions are in the list and check if they have the correct weights. + self.assertTrue(self.action1 in self.list) + self.assertTrue(self.action2 in self.list) + self.assertEqual(self.list[0], (0, self.action1)) + self.assertEqual(self.list[1], (1, self.action2)) + + def add_test(self): + """ + Test the add() method + """ + # GIVEN: The list. + # WHEN: Append actions. + self.list.add(self.action1, 42) + self.list.add(self.action2, 99) + + # THEN: Check if they were added and have the specified weights. + self.assertTrue(self.action1 in self.list) + self.assertTrue(self.action2 in self.list) + self.assertEqual(self.list[0], (42, self.action1)) + self.assertEqual(self.list[1], (99, self.action2)) + + def remove_test(self): + """ + Test the remove() method + """ + # GIVEN: The list + # WHEN: Delete an item from the list. + self.list.remove(self.action1) + + # THEN: Now the element should not be in the list anymore. + self.assertFalse(self.action1 in self.list) + + class TestActionList(TestCase, TestMixin): """ Test the ActionList class @@ -152,62 +238,3 @@ class TestActionList(TestCase, TestMixin): assert len(action3.shortcuts()) == 2, 'The action should have two shortcut assigned.' assert len(action_with_same_shortcuts3.shortcuts()) == 2, 'The action should have two shortcuts assigned.' - -class TestCategoryActionList(TestCase): - def setUp(self): - """ - """ - self.added_action = MagicMock() - self.added_action.text = MagicMock('first') - self.not_added_action = MagicMock('second') - self.not_added_action.text = MagicMock() - self.list = CategoryActionList() - self.list.add(self.added_action, 10) - - def tearDown(self): - """ - - """ - del self.list - - def len_test(self): - """ - Test the __len__ method - """ - # GIVEN: The list. - - # WHEN: Check the length - length = len(self.list) - - # THEN: - self.assertEqual(length, 1, "The length should be 1.") - - # GIVEN: A list with an item. - self.list.append(self.not_added_action) - - # WHEN: Check the length. - length = len(self.list) - - # THEN: - self.assertEqual(length, 2, "The length should be 2.") - - def remove_test(self): - """ - Test the remove() method - """ - # GIVEN: The list - - # WHEN: Delete an item from the list. - self.list.remove(self.added_action) - - # THEN: Now the element should not be in the list anymore. - self.assertFalse(self.added_action in self.list) - - def contains_test(self): - """ - Test the __contains__() method - """ - # GIVEN: The list. - # WHEN: Do nothing. - # THEN: A not added item should not be in the list. - self.assertFalse(self.not_added_action in self.list) \ No newline at end of file From aa899bfde0868e8a241c358398b12110b68890d2 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 10:53:36 +0200 Subject: [PATCH 06/13] fixes --- openlp/core/utils/actions.py | 2 +- .../openlp_core_utils/test_actions.py | 44 ++++++++++++++----- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 98eaced5b..66da3f256 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -70,7 +70,7 @@ class CategoryActionList(object): Implement the __getitem__() method to make this class a dictionary type """ try: - return self.actions[key] + return self.actions[key][1] except: raise KeyError('Action "%s" does not exist.' % key) diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index d79617037..8cc6df5da 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -36,7 +36,7 @@ from PyQt4 import QtGui, QtCore from openlp.core.common import Settings from openlp.core.utils import ActionList from openlp.core.utils.actions import CategoryActionList -from tests.functional import MagicMock, patch +from tests.functional import MagicMock from tests.helpers.testmixin import TestMixin @@ -57,6 +57,23 @@ class TestCategoryActionList(TestCase): """ del self.list + def get_test(self): + """ + Test the __getitem__() method + """ + # GIVEN: The list. + self.list.append(self.action1) + + # WHEN: Add an action. + returned_action = self.list[0] + print(returned_action) + + # THEN: Check if the correct action was returned. + self.assertEqual(self.action1, returned_action) + + # THEN: Test if an exception is raised when trying to access a non existing item. + self.assertRaises(KeyError, self.list.__getitem__, 1) + def contains_test(self): """ Test the __contains__() method @@ -64,7 +81,8 @@ class TestCategoryActionList(TestCase): # GIVEN: The list. # WHEN: Add an action self.list.append(self.action1) - # THEN: + + # THEN: The actions should (not) be in the list. self.assertTrue(self.action1 in self.list) self.assertFalse(self.action2 in self.list) @@ -73,7 +91,7 @@ class TestCategoryActionList(TestCase): Test the __len__ method """ # GIVEN: The list. - # WHEN: + # WHEN: Do nothing. # THEN: Check the length. self.assertEqual(len(self.list), 0, "The length should be 0.") @@ -96,23 +114,27 @@ class TestCategoryActionList(TestCase): # THEN: Check if the actions are in the list and check if they have the correct weights. self.assertTrue(self.action1 in self.list) self.assertTrue(self.action2 in self.list) - self.assertEqual(self.list[0], (0, self.action1)) - self.assertEqual(self.list[1], (1, self.action2)) + self.assertEqual(self.list[0], self.action1) + self.assertEqual(self.list[1], self.action2) def add_test(self): """ Test the add() method """ - # GIVEN: The list. - # WHEN: Append actions. - self.list.add(self.action1, 42) - self.list.add(self.action2, 99) + # GIVEN: The list and weights. + action1_weight = 42 + action2_weight = 41 + + # WHEN: Add actions and their weights. + self.list.add(self.action1, action1_weight) + self.list.add(self.action2, action2_weight) # THEN: Check if they were added and have the specified weights. self.assertTrue(self.action1 in self.list) self.assertTrue(self.action2 in self.list) - self.assertEqual(self.list[0], (42, self.action1)) - self.assertEqual(self.list[1], (99, self.action2)) + # Now check if action1 is second and action2 is first (due to their weights). + self.assertEqual(self.list[0], self.action2) + self.assertEqual(self.list[1], self.action1) def remove_test(self): """ From 6bdadff334e6ac67571f2c80d8a4c86261928414 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 11:01:12 +0200 Subject: [PATCH 07/13] raise exception when item not in list --- openlp/core/utils/actions.py | 16 ++++------------ .../functional/openlp_core_utils/test_actions.py | 7 ++++++- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 66da3f256..d37a17ffe 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -69,17 +69,14 @@ class CategoryActionList(object): """ Implement the __getitem__() method to make this class a dictionary type """ - try: - return self.actions[key][1] - except: - raise KeyError('Action "%s" does not exist.' % key) + return self.actions[key][1] - def __contains__(self, item): + def __contains__(self, key): """ Implement the __contains__() method to make this class a dictionary type """ for weight, action in self.actions: - if action == item: + if action == key: return True return False @@ -106,12 +103,6 @@ class CategoryActionList(object): self.index += 1 return self.actions[self.index - 1][1] - def has_key(self, key): - """ - Implement the has_key() method to make this class a dictionary type - """ - return key in self - def append(self, action): """ Append an action @@ -136,6 +127,7 @@ class CategoryActionList(object): if action[1] == remove_action: self.actions.remove(action) return + raise ValueError('Action "%s" does not exist.' % remove_action) class CategoryList(object): diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 8cc6df5da..890bde2ba 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -72,7 +72,7 @@ class TestCategoryActionList(TestCase): self.assertEqual(self.action1, returned_action) # THEN: Test if an exception is raised when trying to access a non existing item. - self.assertRaises(KeyError, self.list.__getitem__, 1) + self.assertRaises(IndexError, self.list.__getitem__, 1) def contains_test(self): """ @@ -141,12 +141,17 @@ class TestCategoryActionList(TestCase): Test the remove() method """ # GIVEN: The list + self.list.append(self.action1) + # WHEN: Delete an item from the list. self.list.remove(self.action1) # THEN: Now the element should not be in the list anymore. self.assertFalse(self.action1 in self.list) + # THEN: Check if an exception is raised when trying to remove a not present action. + self.assertRaises(ValueError, self.list.remove, self.action2) + class TestActionList(TestCase, TestMixin): """ From 533710fc5b389aefe804559eca1e6e95b8fec83f Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 11:02:04 +0200 Subject: [PATCH 08/13] blank line --- tests/functional/openlp_core_utils/test_actions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 890bde2ba..1d815a94b 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -264,4 +264,3 @@ class TestActionList(TestCase, TestMixin): # THEN: Both action should keep their shortcuts. assert len(action3.shortcuts()) == 2, 'The action should have two shortcut assigned.' assert len(action_with_same_shortcuts3.shortcuts()) == 2, 'The action should have two shortcuts assigned.' - From 53c584ede4a4ccddd5f226a5150c571caadc0d62 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 11:26:20 +0200 Subject: [PATCH 09/13] removed print --- tests/functional/openlp_core_utils/test_actions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index 1d815a94b..c608ce0b1 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -66,7 +66,6 @@ class TestCategoryActionList(TestCase): # WHEN: Add an action. returned_action = self.list[0] - print(returned_action) # THEN: Check if the correct action was returned. self.assertEqual(self.action1, returned_action) From 66c1f7c5da14b69e3069ec7f436d0348ba354150 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 11:35:50 +0200 Subject: [PATCH 10/13] renamed variable and parameter --- openlp/core/utils/actions.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index d37a17ffe..3d679ac02 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -119,15 +119,15 @@ class CategoryActionList(object): self.actions.append((weight, action)) self.actions.sort(key=lambda act: act[0]) - def remove(self, remove_action): + def remove(self, action): """ Remove an action """ - for action in self.actions: - if action[1] == remove_action: - self.actions.remove(action) + for item in self.actions: + if item[1] == action: + self.actions.remove(item) return - raise ValueError('Action "%s" does not exist.' % remove_action) + raise ValueError('Action "%s" does not exist.' % action) class CategoryList(object): From 911864a4424acb62fc3e7b2384b7a2f59c1e396a Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 19:31:59 +0200 Subject: [PATCH 11/13] removed useless code --- openlp/core/utils/actions.py | 6 ----- .../openlp_core_utils/test_actions.py | 24 ++++--------------- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 3d679ac02..256d36a1e 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -65,12 +65,6 @@ class CategoryActionList(object): self.index = 0 self.actions = [] - def __getitem__(self, key): - """ - Implement the __getitem__() method to make this class a dictionary type - """ - return self.actions[key][1] - def __contains__(self, key): """ Implement the __contains__() method to make this class a dictionary type diff --git a/tests/functional/openlp_core_utils/test_actions.py b/tests/functional/openlp_core_utils/test_actions.py index c608ce0b1..4958c4677 100644 --- a/tests/functional/openlp_core_utils/test_actions.py +++ b/tests/functional/openlp_core_utils/test_actions.py @@ -57,22 +57,6 @@ class TestCategoryActionList(TestCase): """ del self.list - def get_test(self): - """ - Test the __getitem__() method - """ - # GIVEN: The list. - self.list.append(self.action1) - - # WHEN: Add an action. - returned_action = self.list[0] - - # THEN: Check if the correct action was returned. - self.assertEqual(self.action1, returned_action) - - # THEN: Test if an exception is raised when trying to access a non existing item. - self.assertRaises(IndexError, self.list.__getitem__, 1) - def contains_test(self): """ Test the __contains__() method @@ -113,8 +97,8 @@ class TestCategoryActionList(TestCase): # THEN: Check if the actions are in the list and check if they have the correct weights. self.assertTrue(self.action1 in self.list) self.assertTrue(self.action2 in self.list) - self.assertEqual(self.list[0], self.action1) - self.assertEqual(self.list[1], self.action2) + self.assertEqual(self.list.actions[0], (0, self.action1)) + self.assertEqual(self.list.actions[1], (1, self.action2)) def add_test(self): """ @@ -132,8 +116,8 @@ class TestCategoryActionList(TestCase): self.assertTrue(self.action1 in self.list) self.assertTrue(self.action2 in self.list) # Now check if action1 is second and action2 is first (due to their weights). - self.assertEqual(self.list[0], self.action2) - self.assertEqual(self.list[1], self.action1) + self.assertEqual(self.list.actions[0], (41, self.action2)) + self.assertEqual(self.list.actions[1], (42, self.action1)) def remove_test(self): """ From 3f1e7f1912ce54c475f4e18353177e1e046a9160 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 19:36:47 +0200 Subject: [PATCH 12/13] fixes --- openlp/core/utils/actions.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 256d36a1e..924a5a2f9 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -170,9 +170,9 @@ class CategoryList(object): self.index += 1 return self.categories[self.index - 1] - def has_key(self, key): + def __contains__(self, key): """ - Implement the has_key() method to make this class like a dictionary + Implement the __contains__() method to make this class like a dictionary """ for category in self.categories: if category.name == key: @@ -186,10 +186,7 @@ class CategoryList(object): weight = 0 if self.categories: weight = self.categories[-1].weight + 1 - if actions: - self.add(name, weight, actions) - else: - self.add(name, weight) + self.add(name, weight, actions) def add(self, name, weight=0, actions=None): """ From 88a3ac62160bac2a23bb87f48ef07768b671e6ca Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sat, 26 Apr 2014 19:40:26 +0200 Subject: [PATCH 13/13] match list behaviour --- openlp/core/utils/actions.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/openlp/core/utils/actions.py b/openlp/core/utils/actions.py index 924a5a2f9..d81e16b2e 100644 --- a/openlp/core/utils/actions.py +++ b/openlp/core/utils/actions.py @@ -209,6 +209,8 @@ class CategoryList(object): for category in self.categories: if category.name == name: self.categories.remove(category) + return + raise ValueError('Category "%s" does not exist.' % name) class ActionList(object):