openlp-core team mailing list archive

[Raoul Snyman <raoul@xxxxxxxxxxx>]

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

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-2.4/+merge/313348

Fix an off-by-one problem when using the autocompletion on macOS

Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/bug-1642684-2.4 (revision 2660)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1868/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1779/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1717/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1457/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1047/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1115/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/983/
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1642684-2.4 into lp:openlp/2.4.

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2016-04-28 21:12:50 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2016-12-15 12:17:02 +0000
@@ -112,13 +112,13 @@
         """
         objects = self.manager.get_all_objects(cls, order_by_ref=cls.name)
         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):
         """
@@ -345,7 +345,6 @@
         """
         authors = self.manager.get_all_objects(Author, order_by_ref=Author.display_name)
         self.authors_combo_box.clear()
-        self.authors_combo_box.addItem('')
         self.authors = []
         for author in authors:
             row = self.authors_combo_box.count()
@@ -353,6 +352,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()
@@ -379,10 +379,10 @@
         Load the themes into a combobox.
         """
         self.theme_combo_box.clear()
-        self.theme_combo_box.addItem('')
         self.themes = theme_list
         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):
         """
@@ -422,7 +422,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)
 
@@ -571,7 +570,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:
@@ -582,7 +581,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,
@@ -646,7 +645,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:
@@ -659,7 +658,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,
@@ -689,7 +688,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
@@ -701,7 +700,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	2015-12-31 22:46:06 +0000
+++ tests/functional/openlp_plugins/songs/test_editsongform.py	2016-12-15 12:17:02 +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, order_by_ref='Author')
+        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('')