From b71d3e9e5378025126870ce8afd60c0a75fb7c0c Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Thu, 18 Apr 2013 19:50:39 +0200 Subject: [PATCH 1/7] fixed bug-1170435 Fixes: https://launchpad.net/bugs/1170435 --- openlp/plugins/songs/forms/editsongform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 49a20762a..beddee475 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -697,7 +697,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): for verse in verses: if not verse in order: verses_not_used.append(verse) - self.warning_label.setVisible(len(verses_not_used) > 0) + self.warning_label.setVisible(len(verses_not_used) > 0 and bool(text)) def on_copyright_insert_button_triggered(self): text = self.copyright_edit.text() From e7f553afb1fbe9c64b8dd71e34f9043e1c9ed4fa Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Thu, 18 Apr 2013 20:27:22 +0200 Subject: [PATCH 2/7] added test --- openlp/plugins/songs/forms/editsongform.py | 1 + .../songs/forms/test_editsongform.py | 59 ++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index beddee475..818ed1b87 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -697,6 +697,7 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): for verse in verses: if not verse in order: verses_not_used.append(verse) + print len(verses_not_used) self.warning_label.setVisible(len(verses_not_used) > 0 and bool(text)) def on_copyright_insert_button_triggered(self): diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index 2c5de6535..9c74c38c0 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -34,7 +34,7 @@ class TestEditSongForm(TestCase): del self.main_window del self.app - def ui_defaults_test(self): + def ui_defaults_(self): """ Test that the EditSongForm defaults are correct """ @@ -43,5 +43,58 @@ class TestEditSongForm(TestCase): self.assertFalse(self.form.author_remove_button.isEnabled(), u'The author remove button should not be enabled') self.assertFalse(self.form.topic_remove_button.isEnabled(), u'The topic remove button should not be enabled') - def is_verse_edit_form_executed_test(self): - pass \ No newline at end of file + def is_verse_edit_form_executed_t(self): + pass + + def verse_order_warning_hidden_(self): + """ + Test if the verse order warning lable is visible, when a verse order is specified + """ + # GIVEN: Mocked methods. + mocked_row_count = MagicMock() + mocked_row_count.return_value = 1 + self.form.verse_list_widget.rowCount = mocked_row_count + # Mock out the verse. + mocked_verse = MagicMock() + self.form.verse_list_widget.item = mocked_verse + mocked_verse_data_method = MagicMock() + mocked_verse_data_method.return_value = u'V1' + mocked_verse.data = mocked_verse_data_method + mocked_item_method = MagicMock() + mocked_item_method.return_value = mocked_verse + mocked_extract_verse_order_method = MagicMock() + mocked_extract_verse_order_method.return_value = [u'V1'] + self.form._extract_verse_order = mocked_extract_verse_order_method + + # WHEN: Call the method. + self.form.on_verse_order_text_changed(u'V1') + + # THEN: The warning lable should be hidden. + assert not self.form.warning_label.isVisible(), u'The warning lable should be hidden.' + + def bug_1170435_no_text_test(self): + """ + Regression test for bug 1170435 (test if lable hidden, when no verse order is specified) + """ + # GIVEN: Mocked methods. + mocked_row_count = MagicMock() + mocked_row_count.return_value = 0 + self.form.verse_list_widget.rowCount = mocked_row_count + # Mock out the verse. (We want a verse type to be returned). + mocked_verse = MagicMock() + self.form.verse_list_widget.item = mocked_verse + mocked_verse_data_method = MagicMock() + mocked_verse_data_method.return_value = u'V1' + mocked_verse.data = mocked_verse_data_method + mocked_item_method = MagicMock() + mocked_item_method.return_value = mocked_verse + mocked_extract_verse_order_method = MagicMock() + mocked_extract_verse_order_method.return_value = [] + self.form._extract_verse_order = mocked_extract_verse_order_method + + # WHEN: Call the method. + self.form.on_verse_order_text_changed(u'') + + # THEN: The warning lable should be hidden. + assert not self.form.warning_label.isVisible(), \ + u'The lable should be visible because the verse order was left empty.' From 31aaafa254c5c14f14adcbe035423a0cd6d75ae6 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Thu, 18 Apr 2013 20:27:54 +0200 Subject: [PATCH 3/7] clean ups --- openlp/plugins/songs/forms/editsongform.py | 1 - .../interfaces/openlp_plugins/songs/forms/test_editsongform.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 818ed1b87..beddee475 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -697,7 +697,6 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): for verse in verses: if not verse in order: verses_not_used.append(verse) - print len(verses_not_used) self.warning_label.setVisible(len(verses_not_used) > 0 and bool(text)) def on_copyright_insert_button_triggered(self): diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index 9c74c38c0..829bc1fbc 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -46,7 +46,7 @@ class TestEditSongForm(TestCase): def is_verse_edit_form_executed_t(self): pass - def verse_order_warning_hidden_(self): + def verse_order_warning_hidden_test(self): """ Test if the verse order warning lable is visible, when a verse order is specified """ From c0d67ab45f7c14bbfffe2318dc4e19b11254d56d Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Thu, 18 Apr 2013 20:28:11 +0200 Subject: [PATCH 4/7] fixed method name --- .../interfaces/openlp_plugins/songs/forms/test_editsongform.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index 829bc1fbc..6c90fa0c0 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -72,7 +72,7 @@ class TestEditSongForm(TestCase): # THEN: The warning lable should be hidden. assert not self.form.warning_label.isVisible(), u'The warning lable should be hidden.' - def bug_1170435_no_text_test(self): + def bug_1170435_test(self): """ Regression test for bug 1170435 (test if lable hidden, when no verse order is specified) """ From 6b881e312fa0097b0a03af1c78976095c73cbd10 Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Sun, 16 Jun 2013 19:28:44 +0200 Subject: [PATCH 5/7] added second error message --- openlp/plugins/songs/forms/editsongdialog.py | 7 ++++--- openlp/plugins/songs/forms/editsongform.py | 9 ++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongdialog.py b/openlp/plugins/songs/forms/editsongdialog.py index 4bf2c91d9..19dad3eae 100644 --- a/openlp/plugins/songs/forms/editsongdialog.py +++ b/openlp/plugins/songs/forms/editsongdialog.py @@ -275,7 +275,6 @@ class Ui_EditSongDialog(object): self.bottom_layout.setObjectName(u'bottom_layout') self.warning_label = QtGui.QLabel(edit_song_dialog) self.warning_label.setObjectName(u'warning_label') - self.warning_label.setVisible(False) self.bottom_layout.addWidget(self.warning_label) self.button_box = create_button_box(edit_song_dialog, u'button_box', [u'cancel', u'save']) self.bottom_layout.addWidget(self.button_box) @@ -323,8 +322,10 @@ class Ui_EditSongDialog(object): self.from_media_button.setText(translate('SongsPlugin.EditSongForm', 'Add &Media')) self.audio_remove_button.setText(translate('SongsPlugin.EditSongForm', '&Remove')) self.audio_remove_all_button.setText(translate('SongsPlugin.EditSongForm', 'Remove &All')) - self.warning_label.setText( - translate('SongsPlugin.EditSongForm', 'Warning: Not all of the verses are in use.')) + self.not_all_verses_used_warning = \ + translate('SongsPlugin.EditSongForm', 'Warning: Not all of the verses are in use.') + self.no_verse_order_entered_warning = \ + translate('SongsPlugin.EditSongForm', 'Warning: You have not entered a verse order.') def create_combo_box(parent, name): diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 81cfce2fd..79d6bdd76 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -456,6 +456,8 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): self.title_edit.setFocus() # Hide or show the preview button. self.preview_button.setVisible(preview) + # Check if all verse tags are used. + self.on_verse_order_text_changed(self.verse_order_edit.text()) def tag_rows(self): """ @@ -697,7 +699,12 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): for verse in verses: if not verse in order: verses_not_used.append(verse) - self.warning_label.setVisible(len(verses_not_used) > 0 and bool(text)) + label_text = u'' + if not self.verse_order_edit.text(): + label_text = self.no_verse_order_entered_warning + elif verses_not_used: + label_text = self.not_all_verses_used_warning + self.warning_label.setText(label_text) def on_copyright_insert_button_triggered(self): text = self.copyright_edit.text() From 6265ce93de7ab59643c7f7f20cacce0b43e0a25d Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Fri, 21 Jun 2013 12:16:50 +0200 Subject: [PATCH 6/7] fixed tests --- openlp/plugins/songs/forms/editsongform.py | 28 ++++--- .../songs/forms/test_editsongform.py | 83 +++++++++++-------- 2 files changed, 65 insertions(+), 46 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 79d6bdd76..9ad8809b7 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -685,23 +685,31 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): self.verse_edit_button.setEnabled(False) self.verse_delete_button.setEnabled(False) + def on_verse_order_text_changed(self, text): - verses = [] - verse_names = [] - order = self._extract_verse_order(text) + """ + Checks if the verse order is complete or missing. Shows a error message according to the state of the verse + order. + + ``text`` + The text of the verse order edit (ignored). + """ + # Extract all verses which were used in the order. + verses_in_order = self._extract_verse_order(self.verse_order_edit.text()) + # Find the verses which were not used in the order. + verses_not_used = [] for index in range(self.verse_list_widget.rowCount()): verse = self.verse_list_widget.item(index, 0) verse = verse.data(QtCore.Qt.UserRole) - if verse not in verse_names: - verses.append(verse) - verse_names.append(u'%s%s' % (VerseType.translated_tag(verse[0]), verse[1:])) - verses_not_used = [] - for verse in verses: - if not verse in order: + print(verse) + if verse not in verses_in_order: verses_not_used.append(verse) + # Set the label text. label_text = u'' - if not self.verse_order_edit.text(): + # No verse order was entered. + if not verses_in_order: label_text = self.no_verse_order_entered_warning + # The verse order does not contain all verses. elif verses_not_used: label_text = self.not_all_verses_used_warning self.warning_label.setText(label_text) diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index 6c90fa0c0..9e560c5e6 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -34,7 +34,7 @@ class TestEditSongForm(TestCase): del self.main_window del self.app - def ui_defaults_(self): + def ui_defaults_test(self): """ Test that the EditSongForm defaults are correct """ @@ -43,58 +43,69 @@ class TestEditSongForm(TestCase): self.assertFalse(self.form.author_remove_button.isEnabled(), u'The author remove button should not be enabled') self.assertFalse(self.form.topic_remove_button.isEnabled(), u'The topic remove button should not be enabled') - def is_verse_edit_form_executed_t(self): + def is_verse_edit_form_executed_test(self): pass - def verse_order_warning_hidden_test(self): + def verse_order_no_warning_test(self): """ - Test if the verse order warning lable is visible, when a verse order is specified + Test if the verse order warning is not shown """ # GIVEN: Mocked methods. - mocked_row_count = MagicMock() - mocked_row_count.return_value = 1 - self.form.verse_list_widget.rowCount = mocked_row_count + given_verse_order = u'V1 V2' + self.form.verse_list_widget.rowCount = MagicMock(return_value=2) # Mock out the verse. - mocked_verse = MagicMock() - self.form.verse_list_widget.item = mocked_verse - mocked_verse_data_method = MagicMock() - mocked_verse_data_method.return_value = u'V1' - mocked_verse.data = mocked_verse_data_method - mocked_item_method = MagicMock() - mocked_item_method.return_value = mocked_verse - mocked_extract_verse_order_method = MagicMock() - mocked_extract_verse_order_method.return_value = [u'V1'] - self.form._extract_verse_order = mocked_extract_verse_order_method + first_verse = MagicMock() + first_verse.data = MagicMock(return_value=u'V1') + second_verse = MagicMock() + second_verse.data = MagicMock(return_value= u'V2') + self.form.verse_list_widget.item = MagicMock(side_effect=[first_verse, second_verse]) + self.form._extract_verse_order = MagicMock(return_value=given_verse_order.split()) # WHEN: Call the method. - self.form.on_verse_order_text_changed(u'V1') + self.form.on_verse_order_text_changed(given_verse_order) # THEN: The warning lable should be hidden. - assert not self.form.warning_label.isVisible(), u'The warning lable should be hidden.' + print self.form.warning_label.text() + assert self.form.warning_label.text() == u'', u'There should be no warning.' + + def verse_order_incomplete_warning_test(self): + """ + Test if the verse-order-incomple warning is shown + """ + # GIVEN: Mocked methods. + given_verse_order = u'V1' + self.form.verse_list_widget.rowCount = MagicMock(return_value=2) + # Mock out the verse. + first_verse = MagicMock() + first_verse.data = MagicMock(return_value=u'V1') + second_verse = MagicMock() + second_verse.data = MagicMock(return_value= u'V2') + self.form.verse_list_widget.item = MagicMock(side_effect=[first_verse, second_verse]) + self.form._extract_verse_order = MagicMock(return_value=[given_verse_order]) + + # WHEN: Call the method. + self.form.on_verse_order_text_changed(given_verse_order) + + # THEN: The warning lable should be hidden. + assert self.form.warning_label.text() == u'Warning: Not all of the verses are in use.', \ + u'The verse-order-incomplete warning should be shown.' def bug_1170435_test(self): """ - Regression test for bug 1170435 (test if lable hidden, when no verse order is specified) + Regression test for bug 1170435 (test if "no verse order" message is shown) """ # GIVEN: Mocked methods. - mocked_row_count = MagicMock() - mocked_row_count.return_value = 0 - self.form.verse_list_widget.rowCount = mocked_row_count + given_verse_order = u'' + self.form.verse_list_widget.rowCount = MagicMock(return_value=1) # Mock out the verse. (We want a verse type to be returned). mocked_verse = MagicMock() - self.form.verse_list_widget.item = mocked_verse - mocked_verse_data_method = MagicMock() - mocked_verse_data_method.return_value = u'V1' - mocked_verse.data = mocked_verse_data_method - mocked_item_method = MagicMock() - mocked_item_method.return_value = mocked_verse - mocked_extract_verse_order_method = MagicMock() - mocked_extract_verse_order_method.return_value = [] - self.form._extract_verse_order = mocked_extract_verse_order_method - + mocked_verse.data = MagicMock(return_value=u'V1') + self.form.verse_list_widget.item = MagicMock(return_value=mocked_verse) + self.form._extract_verse_order = MagicMock(return_value=[given_verse_order]) + self.form.verse_order_edit.text = MagicMock(return_value=given_verse_order) # WHEN: Call the method. - self.form.on_verse_order_text_changed(u'') + self.form.on_verse_order_text_changed(given_verse_order) # THEN: The warning lable should be hidden. - assert not self.form.warning_label.isVisible(), \ - u'The lable should be visible because the verse order was left empty.' + assert self.form.warning_label.text() == u'Warning: You have not entered a verse order.', \ + u'The no-verse-order message should be shown.' From 4f8d93638ba44678ebcff8306acb40be1abf942c Mon Sep 17 00:00:00 2001 From: Andreas Preikschat Date: Fri, 21 Jun 2013 20:13:59 +0200 Subject: [PATCH 7/7] fixes --- openlp/plugins/songs/forms/editsongform.py | 1 - .../openlp_plugins/songs/forms/test_editsongform.py | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/openlp/plugins/songs/forms/editsongform.py b/openlp/plugins/songs/forms/editsongform.py index 9ad8809b7..24d0d3024 100644 --- a/openlp/plugins/songs/forms/editsongform.py +++ b/openlp/plugins/songs/forms/editsongform.py @@ -701,7 +701,6 @@ class EditSongForm(QtGui.QDialog, Ui_EditSongDialog): for index in range(self.verse_list_widget.rowCount()): verse = self.verse_list_widget.item(index, 0) verse = verse.data(QtCore.Qt.UserRole) - print(verse) if verse not in verses_in_order: verses_not_used.append(verse) # Set the label text. diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index 9e560c5e6..013e91bc1 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -64,8 +64,7 @@ class TestEditSongForm(TestCase): # WHEN: Call the method. self.form.on_verse_order_text_changed(given_verse_order) - # THEN: The warning lable should be hidden. - print self.form.warning_label.text() + # THEN: No text should be shown. assert self.form.warning_label.text() == u'', u'There should be no warning.' def verse_order_incomplete_warning_test(self): @@ -86,8 +85,8 @@ class TestEditSongForm(TestCase): # WHEN: Call the method. self.form.on_verse_order_text_changed(given_verse_order) - # THEN: The warning lable should be hidden. - assert self.form.warning_label.text() == u'Warning: Not all of the verses are in use.', \ + # THEN: The verse-order-incomplete text should be shown. + assert self.form.warning_label.text() == self.form.not_all_verses_used_warning, \ u'The verse-order-incomplete warning should be shown.' def bug_1170435_test(self): @@ -101,11 +100,11 @@ class TestEditSongForm(TestCase): mocked_verse = MagicMock() mocked_verse.data = MagicMock(return_value=u'V1') self.form.verse_list_widget.item = MagicMock(return_value=mocked_verse) - self.form._extract_verse_order = MagicMock(return_value=[given_verse_order]) + self.form._extract_verse_order = MagicMock(return_value=[]) self.form.verse_order_edit.text = MagicMock(return_value=given_verse_order) # WHEN: Call the method. self.form.on_verse_order_text_changed(given_verse_order) - # THEN: The warning lable should be hidden. - assert self.form.warning_label.text() == u'Warning: You have not entered a verse order.', \ + # THEN: The no-verse-order message should be shown. + assert self.form.warning_label.text() == self.form.no_verse_order_entered_warning, \ u'The no-verse-order message should be shown.'