Merge lp:~raoul-snyman/openlp/bug-1533246 into lp:openlp

[Raoul Snyman]

Fixed a problem with the shortcuts.
Also tried to make the alternate clear button on the shortcuts dialog a little less weird.

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1533246 (revision 2616)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1275/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1199/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1138/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/974/
[FAILURE] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/566/
Stopping after failure

[David Wales (daviewales)]

Starting on line 197, there is a repeated comment which says:

# WHEN: The even is handled

But should say

# WHEN: The event is handled

Running this search and replace on the code should fix it if you use vim:

%s/ even / event /g

Alternatively, I'm sure you know how to use the search and replace in your favourite editor.

review: Needs Fixing

[Tim Bentley (trb143)]

review: Approve

[David Wales (daviewales)]

review: Approve

=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py	2016-01-10 16:01:43 +0000
+++ openlp/core/common/settings.py	2016-02-05 19:02:39 +0000
@@ -252,68 +252,56 @@
             'shortcuts/blankScreen': [QtGui.QKeySequence(QtCore.Qt.Key_Period)],
             'shortcuts/collapse': [QtGui.QKeySequence(QtCore.Qt.Key_Minus)],
             'shortcuts/desktopScreen': [QtGui.QKeySequence(QtCore.Qt.Key_D)],
-            'shortcuts/delete': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                 QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/delete': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/down': [QtGui.QKeySequence(QtCore.Qt.Key_Down)],
             'shortcuts/editSong': [],
             'shortcuts/escapeItem': [QtGui.QKeySequence(QtCore.Qt.Key_Escape)],
             'shortcuts/expand': [QtGui.QKeySequence(QtCore.Qt.Key_Plus)],
             'shortcuts/exportThemeItem': [],
-            'shortcuts/fileNewItem': [QtGui.QKeySequence(QtGui.QKeySequence.New),
-                                      QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.Key_N)],
-            'shortcuts/fileSaveAsItem': [QtGui.QKeySequence(QtGui.QKeySequence.SaveAs),
-                                         QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.SHIFT + QtCore.Qt.Key_S)],
-            'shortcuts/fileExitItem': [QtGui.QKeySequence(QtGui.QKeySequence.Quit),
-                                       QtGui.QKeySequence(QtCore.Qt.ALT + QtCore.Qt.Key_F4)],
-            'shortcuts/fileSaveItem': [QtGui.QKeySequence(QtGui.QKeySequence.Save),
-                                       QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.Key_S)],
-            'shortcuts/fileOpenItem': [QtGui.QKeySequence(QtGui.QKeySequence.Open),
-                                       QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.Key_O)],
+            'shortcuts/fileNewItem': [QtGui.QKeySequence(QtGui.QKeySequence.New)],
+            'shortcuts/fileSaveAsItem': [QtGui.QKeySequence(QtGui.QKeySequence.SaveAs)],
+            'shortcuts/fileExitItem': [QtGui.QKeySequence(QtGui.QKeySequence.Quit)],
+            'shortcuts/fileSaveItem': [QtGui.QKeySequence(QtGui.QKeySequence.Save)],
+            'shortcuts/fileOpenItem': [QtGui.QKeySequence(QtGui.QKeySequence.Open)],
             'shortcuts/goLive': [],
             'shortcuts/importThemeItem': [],
             'shortcuts/importBibleItem': [],
-            'shortcuts/listViewBiblesDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                   QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewBiblesDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewBiblesPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewBiblesLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
                                                  QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Enter)],
             'shortcuts/listViewBiblesServiceItem': [QtGui.QKeySequence(QtCore.Qt.Key_Plus),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Equal)],
-            'shortcuts/listViewCustomDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                   QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewCustomDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewCustomPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewCustomLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
                                                  QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Enter)],
             'shortcuts/listViewCustomServiceItem': [QtGui.QKeySequence(QtCore.Qt.Key_Plus),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Equal)],
-            'shortcuts/listViewImagesDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                   QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewImagesDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewImagesPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewImagesLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
                                                  QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Enter)],
             'shortcuts/listViewImagesServiceItem': [QtGui.QKeySequence(QtCore.Qt.Key_Plus),
                                                     QtGui.QKeySequence(QtCore.Qt.Key_Equal)],
-            'shortcuts/listViewMediaDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                  QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewMediaDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewMediaPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                    QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewMediaLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
                                                 QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Enter)],
             'shortcuts/listViewMediaServiceItem': [QtGui.QKeySequence(QtCore.Qt.Key_Plus),
                                                    QtGui.QKeySequence(QtCore.Qt.Key_Equal)],
-            'shortcuts/listViewPresentationsDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                          QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewPresentationsDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewPresentationsPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                            QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewPresentationsLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
                                                         QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Enter)],
             'shortcuts/listViewPresentationsServiceItem': [QtGui.QKeySequence(QtCore.Qt.Key_Plus),
                                                            QtGui.QKeySequence(QtCore.Qt.Key_Equal)],
-            'shortcuts/listViewSongsDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete),
-                                                  QtGui.QKeySequence(QtCore.Qt.Key_Delete)],
+            'shortcuts/listViewSongsDeleteItem': [QtGui.QKeySequence(QtGui.QKeySequence.Delete)],
             'shortcuts/listViewSongsPreviewItem': [QtGui.QKeySequence(QtCore.Qt.Key_Return),
                                                    QtGui.QKeySequence(QtCore.Qt.Key_Enter)],
             'shortcuts/listViewSongsLiveItem': [QtGui.QKeySequence(QtCore.Qt.SHIFT + QtCore.Qt.Key_Return),
@@ -337,8 +325,7 @@
             'shortcuts/nextService': [QtGui.QKeySequence(QtCore.Qt.Key_Right)],
             'shortcuts/newService': [],
             'shortcuts/offlineHelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents)],
-            'shortcuts/onlineHelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents),
-                                         QtGui.QKeySequence(QtCore.Qt.ALT + QtCore.Qt.Key_F1)],
+            'shortcuts/onlineHelpItem': [QtGui.QKeySequence(QtGui.QKeySequence.HelpContents)],
             'shortcuts/openService': [],
             'shortcuts/saveService': [],
             'shortcuts/previousItem_live': [QtGui.QKeySequence(QtCore.Qt.Key_Up),
@@ -351,12 +338,10 @@
             'shortcuts/previousService': [QtGui.QKeySequence(QtCore.Qt.Key_Left)],
             'shortcuts/previousItem_preview': [QtGui.QKeySequence(QtCore.Qt.Key_Up),
                                                QtGui.QKeySequence(QtCore.Qt.Key_PageUp)],
-            'shortcuts/printServiceItem': [QtGui.QKeySequence(QtGui.QKeySequence.Print),
-                                           QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.Key_P)],
+            'shortcuts/printServiceItem': [QtGui.QKeySequence(QtGui.QKeySequence.Print)],
             'shortcuts/songExportItem': [],
             'shortcuts/songUsageStatus': [QtGui.QKeySequence(QtCore.Qt.Key_F4)],
-            'shortcuts/searchShortcut': [QtGui.QKeySequence(QtGui.QKeySequence.Find),
-                                         QtGui.QKeySequence(QtCore.Qt.CTRL + QtCore.Qt.Key_F)],
+            'shortcuts/searchShortcut': [QtGui.QKeySequence(QtGui.QKeySequence.Find)],
             'shortcuts/settingsShortcutsItem': [],
             'shortcuts/settingsImportItem': [],
             'shortcuts/settingsPluginListItem': [QtGui.QKeySequence(QtCore.Qt.ALT + QtCore.Qt.Key_F7)],

=== modified file 'openlp/core/ui/shortcutlistform.py'
--- openlp/core/ui/shortcutlistform.py	2016-01-09 16:26:14 +0000
+++ openlp/core/ui/shortcutlistform.py	2016-02-05 19:02:39 +0000
@@ -320,9 +320,17 @@
         """
         if not toggled:
             return
-        self.on_primary_push_button_clicked(False)
-        self.on_alternate_push_button_clicked(False)
+        action = self._current_item_action()
+        shortcuts = self._action_shortcuts(action)
         self.refresh_shortcut_list()
+        primary_button_text = ''
+        alternate_button_text = ''
+        if shortcuts:
+            primary_button_text = self.get_shortcut_string(shortcuts[0], for_display=True)
+        if len(shortcuts) == 2:
+            alternate_button_text = self.get_shortcut_string(shortcuts[1], for_display=True)
+        self.primary_push_button.setText(primary_button_text)
+        self.alternate_push_button.setText(alternate_button_text)
 
     def save(self):
         """

=== modified file 'tests/interfaces/openlp_core_ui/test_shortcutlistform.py'
--- tests/interfaces/openlp_core_ui/test_shortcutlistform.py	2015-12-31 22:46:06 +0000
+++ tests/interfaces/openlp_core_ui/test_shortcutlistform.py	2016-02-05 19:02:39 +0000
@@ -24,11 +24,12 @@
 """
 from unittest import TestCase
 
-from PyQt5 import QtWidgets
+from PyQt5 import QtCore, QtGui, QtWidgets
 
 from openlp.core.common import Registry
 from openlp.core.ui.shortcutlistform import ShortcutListForm
-from tests.interfaces import patch
+
+from tests.interfaces import MagicMock, patch
 from tests.helpers.testmixin import TestMixin
 
 
@@ -59,13 +60,171 @@
         button = QtWidgets.QPushButton()
         checked = True
         enabled = True
-        text = "new!"
+        text = 'new!'
 
         # WHEN: Call the method.
         with patch('PyQt5.QtWidgets.QPushButton.setChecked') as mocked_check_method:
             self.form._adjust_button(button, checked, enabled, text)
 
             # THEN: The button should be changed.
-            self.assertEqual(button.text(), text, "The text should match.")
+            self.assertEqual(button.text(), text, 'The text should match.')
             mocked_check_method.assert_called_once_with(True)
-            self.assertEqual(button.isEnabled(), enabled, "The button should be disabled.")
+            self.assertEqual(button.isEnabled(), enabled, 'The button should be disabled.')
+
+    def space_key_press_event_test(self):
+        """
+        Test the keyPressEvent when the spacebar was pressed
+        """
+        # GIVEN: A key event that is a space
+        mocked_event = MagicMock()
+        mocked_event.key.return_value = QtCore.Qt.Key_Space
+
+        # WHEN: The event is handled
+        with patch.object(self.form, 'keyReleaseEvent') as mocked_key_release_event:
+            self.form.keyPressEvent(mocked_event)
+
+            # THEN: The key should be released
+            mocked_key_release_event.assert_called_with(mocked_event)
+            self.assertEqual(0, mocked_event.accept.call_count)
+
+    def primary_push_button_checked_key_press_event_test(self):
+        """
+        Test the keyPressEvent when the primary push button is checked
+        """
+        # GIVEN: The primary push button is checked
+        with patch.object(self.form, 'keyReleaseEvent') as mocked_key_release_event, \
+                patch.object(self.form.primary_push_button, 'isChecked') as mocked_is_checked:
+            mocked_is_checked.return_value = True
+            mocked_event = MagicMock()
+
+            # WHEN: The event is handled
+            self.form.keyPressEvent(mocked_event)
+
+            # THEN: The key should be released
+            mocked_key_release_event.assert_called_with(mocked_event)
+            self.assertEqual(0, mocked_event.accept.call_count)
+
+    def alternate_push_button_checked_key_press_event_test(self):
+        """
+        Test the keyPressEvent when the alternate push button is checked
+        """
+        # GIVEN: The primary push button is checked
+        with patch.object(self.form, 'keyReleaseEvent') as mocked_key_release_event, \
+                patch.object(self.form.alternate_push_button, 'isChecked') as mocked_is_checked:
+            mocked_is_checked.return_value = True
+            mocked_event = MagicMock()
+
+            # WHEN: The event is handled
+            self.form.keyPressEvent(mocked_event)
+
+            # THEN: The key should be released
+            mocked_key_release_event.assert_called_with(mocked_event)
+            self.assertEqual(0, mocked_event.accept.call_count)
+
+    def escape_key_press_event_test(self):
+        """
+        Test the keyPressEvent when the escape key was pressed
+        """
+        # GIVEN: A key event that is an escape
+        mocked_event = MagicMock()
+        mocked_event.key.return_value = QtCore.Qt.Key_Escape
+
+        # WHEN: The event is handled
+        with patch.object(self.form, 'close') as mocked_close:
+            self.form.keyPressEvent(mocked_event)
+
+            # THEN: The key should be released
+            mocked_event.accept.assert_called_with()
+            mocked_close.assert_called_with()
+
+    def on_default_radio_button_not_toggled_test(self):
+        """
+        Test that the default radio button method exits early when the button is not toggled
+        """
+        # GIVEN: A not-toggled custom radio button
+        with patch.object(self.form, '_current_item_action') as mocked_current_item_action:
+
+            # WHEN: The clicked method is called
+            self.form.on_default_radio_button_clicked(False)
+
+            # THEN: The method should exit early (i.e. the rest of the methods are not called)
+            self.assertEqual(0, mocked_current_item_action.call_count)
+
+    def on_default_radio_button_clicked_no_action_test(self):
+        """
+        Test that nothing happens when an action hasn't been selected and you click the default radio button
+        """
+        # GIVEN: Some mocked out methods, a current action, and some shortcuts
+        with patch.object(self.form, '_current_item_action') as mocked_current_item_action, \
+                patch.object(self.form, '_action_shortcuts') as mocked_action_shortcuts:
+            mocked_current_item_action.return_value = None
+
+            # WHEN: The default radio button is clicked
+            self.form.on_default_radio_button_clicked(True)
+
+            # THEN: The method should exit early (i.e. the rest of the methods are not called)
+            mocked_current_item_action.assert_called_with()
+            self.assertEqual(0, mocked_action_shortcuts.call_count)
+
+    def on_default_radio_button_clicked_test(self):
+        """
+        Test that the values are copied across correctly when the default radio button is selected
+        """
+        # GIVEN: Some mocked out methods, a current action, and some shortcuts
+        with patch.object(self.form, '_current_item_action') as mocked_current_item_action, \
+                patch.object(self.form, '_action_shortcuts') as mocked_action_shortcuts, \
+                patch.object(self.form, 'refresh_shortcut_list') as mocked_refresh_shortcut_list, \
+                patch.object(self.form, 'get_shortcut_string') as mocked_get_shortcut_string, \
+                patch.object(self.form.primary_push_button, 'setText') as mocked_set_text:
+            mocked_action = MagicMock()
+            mocked_action.default_shortcuts = [QtCore.Qt.Key_Escape]
+            mocked_current_item_action.return_value = mocked_action
+            mocked_action_shortcuts.return_value = [QtCore.Qt.Key_Escape]
+            mocked_get_shortcut_string.return_value = 'Esc'
+
+            # WHEN: The default radio button is clicked
+            self.form.on_default_radio_button_clicked(True)
+
+            # THEN: The shorcuts should be copied across
+            mocked_current_item_action.assert_called_with()
+            mocked_action_shortcuts.assert_called_with(mocked_action)
+            mocked_refresh_shortcut_list.assert_called_with()
+            mocked_set_text.assert_called_with('Esc')
+
+    def on_custom_radio_button_not_toggled_test(self):
+        """
+        Test that the custom radio button method exits early when the button is not toggled
+        """
+        # GIVEN: A not-toggled custom radio button
+        with patch.object(self.form, '_current_item_action') as mocked_current_item_action:
+
+            # WHEN: The clicked method is called
+            self.form.on_custom_radio_button_clicked(False)
+
+            # THEN: The method should exit early (i.e. the rest of the methods are not called)
+            self.assertEqual(0, mocked_current_item_action.call_count)
+
+    def on_custom_radio_button_clicked_test(self):
+        """
+        Test that the values are copied across correctly when the custom radio button is selected
+        """
+        # GIVEN: Some mocked out methods, a current action, and some shortcuts
+        with patch.object(self.form, '_current_item_action') as mocked_current_item_action, \
+                patch.object(self.form, '_action_shortcuts') as mocked_action_shortcuts, \
+                patch.object(self.form, 'refresh_shortcut_list') as mocked_refresh_shortcut_list, \
+                patch.object(self.form, 'get_shortcut_string') as mocked_get_shortcut_string, \
+                patch.object(self.form.primary_push_button, 'setText') as mocked_set_text:
+            mocked_action = MagicMock()
+            mocked_current_item_action.return_value = mocked_action
+            mocked_action_shortcuts.return_value = [QtCore.Qt.Key_Escape]
+            mocked_get_shortcut_string.return_value = 'Esc'
+
+            # WHEN: The custom radio button is clicked
+            self.form.on_custom_radio_button_clicked(True)
+
+            # THEN: The shorcuts should be copied across
+            mocked_current_item_action.assert_called_with()
+            mocked_action_shortcuts.assert_called_with(mocked_action)
+            mocked_refresh_shortcut_list.assert_called_with()
+            mocked_set_text.assert_called_with('Esc')
+