openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/bug-1642684 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1642684 in OpenLP: "Auto-completed topics are off-by-1 when added to the list"
  https://bugs.launchpad.net/openlp/+bug/1642684

For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/bug-1642684/+merge/313366

Fix bug #1642684 by removing the blank item and clearing the text box instead.

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1642684 (revision 2710)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1869/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1780/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1718/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1458/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1048/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1116/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/984/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/135/
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1642684 into lp:openlp.

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2016-10-23 19:24:53 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-12-15 14:37:24 +0000
@@ -118,13 +118,13 @@
         objects = self.manager.get_all_objects(cls)
         objects.sort(key=get_key)
         combo.clear()
-        combo.addItem('')
         for obj in objects:
             row = combo.count()
             combo.addItem(obj.name)
             cache.append(obj.name)
             combo.setItemData(row, obj.id)
         set_case_insensitive_completer(cache, combo)
+        combo.setEditText('')
 
     def _add_author_to_list(self, author, author_type):
         """
@@ -360,7 +360,6 @@
         authors = self.manager.get_all_objects(Author)
         authors.sort(key=get_author_key)
         self.authors_combo_box.clear()
-        self.authors_combo_box.addItem('')
         self.authors = []
         for author in authors:
             row = self.authors_combo_box.count()
@@ -368,6 +367,7 @@
             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('')
 
         # Types
         self.author_types_combo_box.clear()
@@ -398,11 +398,11 @@
             return get_natural_key(theme)
 
         self.theme_combo_box.clear()
-        self.theme_combo_box.addItem('')
         self.themes = theme_list
         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('')
 
     def load_media_files(self):
         """
@@ -442,7 +442,6 @@
         self.load_songbooks()
         self.load_media_files()
         self.theme_combo_box.setEditText('')
-        self.theme_combo_box.setCurrentIndex(0)
         # it's a new song to preview is not possible
         self.preview_button.setVisible(False)
 
@@ -591,7 +590,7 @@
                 self.manager.save_object(author)
                 self._add_author_to_list(author, author_type)
                 self.load_authors()
-                self.authors_combo_box.setCurrentIndex(0)
+                self.authors_combo_box.setEditText('')
             else:
                 return
         elif item > 0:
@@ -602,7 +601,7 @@
                     message=translate('SongsPlugin.EditSongForm', 'This author is already in the list.'))
             else:
                 self._add_author_to_list(author, author_type)
-            self.authors_combo_box.setCurrentIndex(0)
+            self.authors_combo_box.setEditText('')
         else:
             QtWidgets.QMessageBox.warning(
                 self, UiStrings().NISs,
@@ -666,7 +665,7 @@
                 topic_item.setData(QtCore.Qt.UserRole, topic.id)
                 self.topics_list_view.addItem(topic_item)
                 self.load_topics()
-                self.topics_combo_box.setCurrentIndex(0)
+                self.topics_combo_box.setEditText('')
             else:
                 return
         elif item > 0:
@@ -679,7 +678,7 @@
                 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.setCurrentIndex(0)
+            self.topics_combo_box.setEditText('')
         else:
             QtWidgets.QMessageBox.warning(
                 self, UiStrings().NISs,
@@ -709,7 +708,7 @@
                 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.setCurrentIndex(0)
+                self.songbooks_combo_box.setEditText('')
                 self.songbook_entry_edit.clear()
             else:
                 return
@@ -721,7 +720,7 @@
                     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.setCurrentIndex(0)
+            self.songbooks_combo_box.setEditText('')
             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-05-31 21:40:13 +0000
+++ tests/functional/openlp_plugins/songs/test_editsongform.py	2016-12-15 14:37:24 +0000
@@ -76,3 +76,34 @@
 
         # THEN they should be valid
         self.assertTrue(valid, "The tags list should be valid")
+
+    @patch('openlp.plugins.songs.forms.editsongform.set_case_insensitive_completer')
+    def test_load_objects(self, mocked_set_case_insensitive_completer):
+        """
+        Test the _load_objects() method
+        """
+        # GIVEN: A song edit form and some mocked stuff
+        mocked_class = MagicMock()
+        mocked_class.name = 'Author'
+        mocked_combo = MagicMock()
+        mocked_combo.count.return_value = 0
+        mocked_cache = MagicMock()
+        mocked_object = MagicMock()
+        mocked_object.name = 'Charles'
+        mocked_object.id = 1
+        mocked_manager = MagicMock()
+        mocked_manager.get_all_objects.return_value = [mocked_object]
+        self.edit_song_form.manager = mocked_manager
+
+        # WHEN: _load_objects() is called
+        self.edit_song_form._load_objects(mocked_class, mocked_combo, mocked_cache)
+
+        # THEN: All the correct methods should have been called
+        self.edit_song_form.manager.get_all_objects.assert_called_once_with(mocked_class)
+        mocked_combo.clear.assert_called_once_with()
+        mocked_combo.count.assert_called_once_with()
+        mocked_combo.addItem.assert_called_once_with('Charles')
+        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('')