openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #23364
[Merge] lp:~googol/openlp/bug-1240037 into lp:openlp
Andreas Preikschat has proposed merging lp:~googol/openlp/bug-1240037 into lp:openlp.
Requested reviews:
Tim Bentley (trb143)
Related bugs:
Bug #1240037 in OpenLP: "Error occured when moving directory"
https://bugs.launchpad.net/openlp/+bug/1240037
For more details, see:
https://code.launchpad.net/~googol/openlp/bug-1240037/+merge/217343
Hello,
- Fixed bug #1240037
- Fixed a few bugs in the CategoryActionList class (recursion, ...)
- Removed depreciated has_key() method.
- Changed behaviour of the remove() method to match the behaviour of the list.remove() method
- refactored defaultShortcuts to default_shortcuts
Note: It seems that the CategoryActionList class provides methods we never used. I thought about removing these, but as I wrote tests now, I decided against removing them.
lp:~googol/openlp/bug-1240037 (revision 2380)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/407/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/363/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/308/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/269/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/186/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/61/
--
https://code.launchpad.net/~googol/openlp/bug-1240037/+merge/217343
Your team OpenLP Core is subscribed to branch lp:openlp.
=== modified file 'openlp/core/ui/mainwindow.py'
--- openlp/core/ui/mainwindow.py 2014-04-12 20:19:22 +0000
+++ openlp/core/ui/mainwindow.py 2014-04-26 17:47:16 +0000
@@ -1334,7 +1334,7 @@
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)
@@ -1364,8 +1364,7 @@
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')):
=== modified file 'openlp/core/ui/shortcutlistform.py'
--- openlp/core/ui/shortcutlistform.py 2014-03-20 19:10:31 +0000
+++ openlp/core/ui/shortcutlistform.py 2014-04-26 17:47:16 +0000
@@ -244,10 +244,10 @@
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 @@
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 @@
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 @@
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 @@
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
=== modified file 'openlp/core/utils/actions.py'
--- openlp/core/utils/actions.py 2014-03-20 19:10:31 +0000
+++ openlp/core/utils/actions.py 2014-04-26 17:47:16 +0000
@@ -65,20 +65,14 @@
self.index = 0
self.actions = []
- def __getitem__(self, key):
- """
- Implement the __getitem__() method to make this class a dictionary type
- """
- for weight, action in self.actions:
- if action.text() == key:
- return action
- raise KeyError('Action "%s" does not exist.' % key)
-
- def __contains__(self, item):
+ def __contains__(self, key):
"""
Implement the __contains__() method to make this class a dictionary type
"""
- return item in self
+ for weight, action in self.actions:
+ if action == key:
+ return True
+ return False
def __len__(self):
"""
@@ -103,23 +97,14 @@
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
- """
- for weight, action in self.actions:
- if action.text() == key:
- return True
- return False
-
- 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):
"""
@@ -128,14 +113,15 @@
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.' % action)
class CategoryList(object):
@@ -184,9 +170,9 @@
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:
@@ -200,10 +186,7 @@
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):
"""
@@ -226,6 +209,8 @@
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):
@@ -270,7 +255,7 @@
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:
=== modified file 'tests/functional/openlp_core_utils/test_actions.py'
--- tests/functional/openlp_core_utils/test_actions.py 2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_core_utils/test_actions.py 2014-04-26 17:47:16 +0000
@@ -35,9 +35,107 @@
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
+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: The actions should (not) be in the list.
+ 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: Do nothing.
+ # 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.actions[0], (0, self.action1))
+ self.assertEqual(self.list.actions[1], (1, self.action2))
+
+ def add_test(self):
+ """
+ Test the add() method
+ """
+ # 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)
+ # Now check if action1 is second and action2 is first (due to their weights).
+ self.assertEqual(self.list.actions[0], (41, self.action2))
+ self.assertEqual(self.list.actions[1], (42, self.action1))
+
+ def remove_test(self):
+ """
+ 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):
"""
Test the ActionList class
Follow ups
