From 3bc6fc0b77e7b9fbafc6ed09810514b5d288262d Mon Sep 17 00:00:00 2001 From: Raoul Snyman Date: Wed, 1 Mar 2017 21:34:24 -0700 Subject: [PATCH] Fix bug #1666005 and bug #1668994 --- openlp/plugins/songs/forms/editsongform.py | 51 +++-- .../songs/forms/test_authorsform.py | 215 ++++++++++++++++++ 2 files changed, 243 insertions(+), 23 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index ba79654f4..d7f614245 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -124,7 +124,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): cache.append(obj.name) combo.setItemData(row, obj.id) set_case_insensitive_completer(cache, combo) - combo.setEditText('') + combo.setCurrentIndex(-1) + combo.setCurrentText('') def _add_author_to_list(self, author, author_type): """ @@ -367,7 +368,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.authors_combo_box.setItemData(row, author.id) self.authors.append(author.display_name) set_case_insensitive_completer(self.authors, self.authors_combo_box) - self.authors_combo_box.setEditText('') + self.authors_combo_box.setCurrentIndex(-1) + self.authors_combo_box.setCurrentText('') # Types self.author_types_combo_box.clear() @@ -402,7 +404,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.themes.sort(key=get_theme_key) self.theme_combo_box.addItems(theme_list) set_case_insensitive_completer(self.themes, self.theme_combo_box) - self.theme_combo_box.setEditText('') + self.theme_combo_box.setCurrentIndex(-1) + self.theme_combo_box.setCurrentText('') def load_media_files(self): """ @@ -441,7 +444,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.load_topics() self.load_songbooks() self.load_media_files() - self.theme_combo_box.setEditText('') + self.theme_combo_box.setCurrentIndex(-1) + self.theme_combo_box.setCurrentText('') # it's a new song to preview is not possible self.preview_button.setVisible(False) @@ -466,8 +470,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): find_and_set_in_combo_box(self.theme_combo_box, str(self.song.theme_name)) else: # Clear the theme combo box in case it was previously set (bug #1212801) - self.theme_combo_box.setEditText('') - self.theme_combo_box.setCurrentIndex(0) + self.theme_combo_box.setCurrentIndex(-1) + self.theme_combo_box.setCurrentText('') self.copyright_edit.setText(self.song.copyright if self.song.copyright else '') self.comments_edit.setPlainText(self.song.comments if self.song.comments else '') self.ccli_number_edit.setText(self.song.ccli_number if self.song.ccli_number else '') @@ -570,12 +574,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): item = int(self.authors_combo_box.currentIndex()) text = self.authors_combo_box.currentText().strip(' \r\n\t') author_type = self.author_types_combo_box.itemData(self.author_types_combo_box.currentIndex()) - # This if statement is for OS X, which doesn't seem to work well with - # the QCompleter auto-completion class. See bug #812628. - if text in self.authors: - # Index 0 is a blank string, so add 1 - item = self.authors.index(text) + 1 - if item == 0 and text: + if item == -1 and text: if QtWidgets.QMessageBox.question( self, translate('SongsPlugin.EditSongForm', 'Add Author'), @@ -590,10 +589,11 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.manager.save_object(author) self._add_author_to_list(author, author_type) self.load_authors() - self.authors_combo_box.setEditText('') + self.authors_combo_box.setCurrentIndex(-1) + self.authors_combo_box.setCurrentText('') else: return - elif item > 0: + elif item >= 0: item_id = (self.authors_combo_box.itemData(item)) author = self.manager.get_object(Author, item_id) if self.authors_list_view.findItems(author.get_display_name(author_type), QtCore.Qt.MatchExactly): @@ -601,7 +601,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): message=translate('SongsPlugin.EditSongForm', 'This author is already in the list.')) else: self._add_author_to_list(author, author_type) - self.authors_combo_box.setEditText('') + self.authors_combo_box.setCurrentIndex(-1) + self.authors_combo_box.setCurrentText('') else: QtWidgets.QMessageBox.warning( self, UiStrings().NISs, @@ -653,7 +654,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): def on_topic_add_button_clicked(self): item = int(self.topics_combo_box.currentIndex()) text = self.topics_combo_box.currentText() - if item == 0 and text: + if item == -1 and text: if QtWidgets.QMessageBox.question( self, translate('SongsPlugin.EditSongForm', 'Add Topic'), translate('SongsPlugin.EditSongForm', 'This topic does not exist, do you want to add it?'), @@ -665,10 +666,11 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): topic_item.setData(QtCore.Qt.UserRole, topic.id) self.topics_list_view.addItem(topic_item) self.load_topics() - self.topics_combo_box.setEditText('') + self.topics_combo_box.setCurrentIndex(-1) + self.topics_combo_box.setCurrentText('') else: return - elif item > 0: + elif item >= 0: item_id = (self.topics_combo_box.itemData(item)) topic = self.manager.get_object(Topic, item_id) if self.topics_list_view.findItems(str(topic.name), QtCore.Qt.MatchExactly): @@ -678,7 +680,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): topic_item = QtWidgets.QListWidgetItem(str(topic.name)) topic_item.setData(QtCore.Qt.UserRole, topic.id) self.topics_list_view.addItem(topic_item) - self.topics_combo_box.setEditText('') + self.topics_combo_box.setCurrentIndex(-1) + self.topics_combo_box.setCurrentText('') else: QtWidgets.QMessageBox.warning( self, UiStrings().NISs, @@ -698,7 +701,7 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): def on_songbook_add_button_clicked(self): item = int(self.songbooks_combo_box.currentIndex()) text = self.songbooks_combo_box.currentText() - if item == 0 and text: + if item == -1 and text: if QtWidgets.QMessageBox.question( self, translate('SongsPlugin.EditSongForm', 'Add Songbook'), translate('SongsPlugin.EditSongForm', 'This Songbook does not exist, do you want to add it?'), @@ -708,11 +711,12 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): self.manager.save_object(songbook) self.add_songbook_entry_to_list(songbook.id, songbook.name, self.songbook_entry_edit.text()) self.load_songbooks() - self.songbooks_combo_box.setEditText('') + self.songbooks_combo_box.setCurrentIndex(-1) + self.songbooks_combo_box.setCurrentText('') self.songbook_entry_edit.clear() else: return - elif item > 0: + elif item >= 0: item_id = (self.songbooks_combo_box.itemData(item)) songbook = self.manager.get_object(Book, item_id) if self.songbooks_list_view.findItems(str(songbook.name), QtCore.Qt.MatchExactly): @@ -720,7 +724,8 @@ class EditSongForm(QtWidgets.QDialog, Ui_EditSongDialog, RegistryProperties): message=translate('SongsPlugin.EditSongForm', 'This Songbook is already in the list.')) else: self.add_songbook_entry_to_list(songbook.id, songbook.name, self.songbook_entry_edit.text()) - self.songbooks_combo_box.setEditText('') + self.songbooks_combo_box.setCurrentIndex(-1) + self.songbooks_combo_box.setCurrentText('') self.songbook_entry_edit.clear() else: QtWidgets.QMessageBox.warning( diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py b/tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py index 871f79deb..ca6f976f2 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py @@ -23,6 +23,7 @@ Package to test the openlp.plugins.songs.forms.authorsform package. """ from unittest import TestCase +from unittest.mock import patch from PyQt5 import QtWidgets @@ -138,3 +139,217 @@ class TestAuthorsForm(TestCase, TestMixin): # THEN: The display_name_edit should have the correct value self.assertEqual(self.form.display_edit.text(), display_name, 'The display name should be set correctly') + + @patch('openlp.plugins.songs.forms.authorsform.QtWidgets.QDialog.exec') + def test_exec(self, mocked_exec): + """ + Test the exec() method + """ + # GIVEN: An authors for and various mocked objects + with patch.object(self.form.first_name_edit, 'clear') as mocked_first_name_edit_clear, \ + patch.object(self.form.last_name_edit, 'clear') as mocked_last_name_edit_clear, \ + patch.object(self.form.display_edit, 'clear') as mocked_display_edit_clear, \ + patch.object(self.form.first_name_edit, 'setFocus') as mocked_first_name_edit_setFocus: + # WHEN: The exec() method is called + self.form.exec(clear=True) + + # THEN: The clear and exec() methods should have been called + mocked_first_name_edit_clear.assert_called_once_with() + mocked_last_name_edit_clear.assert_called_once_with() + mocked_display_edit_clear.assert_called_once_with() + mocked_first_name_edit_setFocus.assert_called_once_with() + mocked_exec.assert_called_once_with(self.form) + + def test_first_name_edited(self): + """ + Test the on_first_name_edited() method + """ + # GIVEN: An author form + self.form.auto_display_name = True + + with patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.display_edit, 'setText') as mocked_display_edit_setText: + mocked_last_name_edit_text.return_value = 'Newton' + + # WHEN: on_first_name_edited() is called + self.form.on_first_name_edited('John') + + # THEN: The display name should be updated + assert mocked_last_name_edit_text.call_count == 2 + mocked_display_edit_setText.assert_called_once_with('John Newton') + + def test_first_name_edited_no_auto(self): + """ + Test the on_first_name_edited() method without auto_display_name + """ + # GIVEN: An author form + self.form.auto_display_name = False + + with patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.display_edit, 'setText') as mocked_display_edit_setText: + + # WHEN: on_first_name_edited() is called + self.form.on_first_name_edited('John') + + # THEN: The display name should not be updated + assert mocked_last_name_edit_text.call_count == 0 + assert mocked_display_edit_setText.call_count == 0 + + def test_last_name_edited(self): + """ + Test the on_last_name_edited() method + """ + # GIVEN: An author form + self.form.auto_display_name = True + + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.display_edit, 'setText') as mocked_display_edit_setText: + mocked_first_name_edit_text.return_value = 'John' + + # WHEN: on_last_name_edited() is called + self.form.on_last_name_edited('Newton') + + # THEN: The display name should be updated + assert mocked_first_name_edit_text.call_count == 2 + mocked_display_edit_setText.assert_called_once_with('John Newton') + + def test_last_name_edited_no_auto(self): + """ + Test the on_last_name_edited() method without auto_display_name + """ + # GIVEN: An author form + self.form.auto_display_name = False + + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.display_edit, 'setText') as mocked_display_edit_setText: + + # WHEN: on_last_name_edited() is called + self.form.on_last_name_edited('Newton') + + # THEN: The display name should not be updated + assert mocked_first_name_edit_text.call_count == 0 + assert mocked_display_edit_setText.call_count == 0 + + @patch('openlp.plugins.songs.forms.authorsform.critical_error_message_box') + def test_accept_no_first_name(self, mocked_critical_error): + """ + Test the accept() method with no first name + """ + # GIVEN: A form and no text in thefirst name edit + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.first_name_edit, 'setFocus') as mocked_first_name_edit_setFocus: + mocked_first_name_edit_text.return_value = '' + + # WHEN: accept() is called + result = self.form.accept() + + # THEN: The result should be false and a critical error displayed + assert result is False + mocked_critical_error.assert_called_once_with(message='You need to type in the first name of the author.') + mocked_first_name_edit_text.assert_called_once_with() + mocked_first_name_edit_setFocus.assert_called_once_with() + + @patch('openlp.plugins.songs.forms.authorsform.critical_error_message_box') + def test_accept_no_last_name(self, mocked_critical_error): + """ + Test the accept() method with no last name + """ + # GIVEN: A form and no text in the last name edit + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.last_name_edit, 'setFocus') as mocked_last_name_edit_setFocus: + mocked_first_name_edit_text.return_value = 'John' + mocked_last_name_edit_text.return_value = '' + + # WHEN: accept() is called + result = self.form.accept() + + # THEN: The result should be false and a critical error displayed + assert result is False + mocked_critical_error.assert_called_once_with(message='You need to type in the last name of the author.') + mocked_first_name_edit_text.assert_called_once_with() + mocked_last_name_edit_text.assert_called_once_with() + mocked_last_name_edit_setFocus.assert_called_once_with() + + @patch('openlp.plugins.songs.forms.authorsform.critical_error_message_box') + def test_accept_no_display_name_no_combine(self, mocked_critical_error): + """ + Test the accept() method with no display name and no combining + """ + # GIVEN: A form and no text in the display name edit + mocked_critical_error.return_value = QtWidgets.QMessageBox.No + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.display_edit, 'text') as mocked_display_edit_text, \ + patch.object(self.form.display_edit, 'setFocus') as mocked_display_edit_setFocus: + mocked_first_name_edit_text.return_value = 'John' + mocked_last_name_edit_text.return_value = 'Newton' + mocked_display_edit_text.return_value = '' + + # WHEN: accept() is called + result = self.form.accept() + + # THEN: The result should be false and a critical error displayed + assert result is False + mocked_critical_error.assert_called_once_with( + message='You have not set a display name for the author, combine the first and last names?', + parent=self.form, question=True) + mocked_first_name_edit_text.assert_called_once_with() + mocked_last_name_edit_text.assert_called_once_with() + mocked_display_edit_text.assert_called_once_with() + mocked_display_edit_setFocus.assert_called_once_with() + + @patch('openlp.plugins.songs.forms.authorsform.critical_error_message_box') + @patch('openlp.plugins.songs.forms.authorsform.QtWidgets.QDialog.accept') + def test_accept_no_display_name(self, mocked_accept, mocked_critical_error): + """ + Test the accept() method with no display name and auto-combine + """ + # GIVEN: A form and no text in the display name edit + mocked_accept.return_value = True + mocked_critical_error.return_value = QtWidgets.QMessageBox.Yes + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.display_edit, 'text') as mocked_display_edit_text, \ + patch.object(self.form.display_edit, 'setText') as mocked_display_edit_setText: + mocked_first_name_edit_text.return_value = 'John' + mocked_last_name_edit_text.return_value = 'Newton' + mocked_display_edit_text.return_value = '' + + # WHEN: accept() is called + result = self.form.accept() + + # THEN: The result should be false and a critical error displayed + assert result is True + mocked_critical_error.assert_called_once_with( + message='You have not set a display name for the author, combine the first and last names?', + parent=self.form, question=True) + assert mocked_first_name_edit_text.call_count == 2 + assert mocked_last_name_edit_text.call_count == 2 + mocked_display_edit_text.assert_called_once_with() + mocked_display_edit_setText.assert_called_once_with('John Newton') + mocked_accept.assert_called_once_with(self.form) + + @patch('openlp.plugins.songs.forms.authorsform.QtWidgets.QDialog.accept') + def test_accept(self, mocked_accept): + """ + Test the accept() method + """ + # GIVEN: A form and text in the right places + mocked_accept.return_value = True + with patch.object(self.form.first_name_edit, 'text') as mocked_first_name_edit_text, \ + patch.object(self.form.last_name_edit, 'text') as mocked_last_name_edit_text, \ + patch.object(self.form.display_edit, 'text') as mocked_display_edit_text: + mocked_first_name_edit_text.return_value = 'John' + mocked_last_name_edit_text.return_value = 'Newton' + mocked_display_edit_text.return_value = 'John Newton' + + # WHEN: accept() is called + result = self.form.accept() + + # THEN: The result should be false and a critical error displayed + assert result is True + mocked_first_name_edit_text.assert_called_once_with() + mocked_last_name_edit_text.assert_called_once_with() + mocked_display_edit_text.assert_called_once_with() + mocked_accept.assert_called_once_with(self.form)