openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #31169
[Merge] lp:~raoul-snyman/openlp/off-by-one into lp:openlp
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/off-by-one into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/off-by-one/+merge/318719
Fix bug #1666005 and bug #1668994
Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/off-by-one (revision 2725)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1914/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1825/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1766/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1499/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1089/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1157/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/1025/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/167/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/off-by-one into lp:openlp.
=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py 2016-12-31 11:01:36 +0000
+++ openlp/plugins/songs/forms/editsongform.py 2017-03-02 04:54:00 +0000
@@ -124,7 +124,8 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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(
=== modified file 'tests/functional/openlp_plugins/songs/test_editsongform.py'
--- tests/functional/openlp_plugins/songs/test_editsongform.py 2016-12-31 11:01:36 +0000
+++ tests/functional/openlp_plugins/songs/test_editsongform.py 2017-03-02 04:54:00 +0000
@@ -106,4 +106,5 @@
mocked_cache.append.assert_called_once_with('Charles')
mocked_combo.setItemData.assert_called_once_with(0, 1)
mocked_set_case_insensitive_completer.assert_called_once_with(mocked_cache, mocked_combo)
- mocked_combo.setEditText.assert_called_once_with('')
+ mocked_combo.setCurrentIndex.assert_called_once_with(-1)
+ mocked_combo.setCurrentText.assert_called_once_with('')
=== modified file 'tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py'
--- tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py 2016-12-31 11:01:36 +0000
+++ tests/interfaces/openlp_plugins/songs/forms/test_authorsform.py 2017-03-02 04:54:00 +0000
@@ -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 @@
# 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)
Follow ups
