openlp-core team mailing list archive

[Samuel Mehrbrodt <s.mehrbrodt@xxxxxxxxx>]

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1316979 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1316979 in OpenLP: "Crash when trying to delete a duplicated song"
  https://bugs.launchpad.net/openlp/+bug/1316979

For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1316979/+merge/218628

Fix bug 1316979 by changing the 'authors' relation to readonly.

The main change is in line 133, the rest is just removing the usage of that relationship.

lp:~sam92/openlp/bug-1316979 (revision 2380)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/435/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/391/
[SUCCESS] http://ci.openlp.org/job/Branch-03-Interface-Tests/336/
[SUCCESS] http://ci.openlp.org/job/Branch-04-Windows_Tests/297/
[SUCCESS] http://ci.openlp.org/job/Branch-05a-Code_Analysis/200/
[SUCCESS] http://ci.openlp.org/job/Branch-05b-Test_Coverage/75/
-- 
https://code.launchpad.net/~sam92/openlp/bug-1316979/+merge/218628
Your team OpenLP Core is requested to review the proposed merge of lp:~sam92/openlp/bug-1316979 into lp:openlp.

=== modified file 'openlp/core/lib/ui.py'
--- openlp/core/lib/ui.py	2014-04-08 14:08:07 +0000
+++ openlp/core/lib/ui.py	2014-05-07 14:00:12 +0000
@@ -295,7 +295,7 @@
     Sets a case insensitive text completer for a widget.
 
     :param cache: The list of items to use as suggestions.
-    :param widget: A widget to set the completer (QComboBox or QTextEdit instance)
+    :param widget: A widget to set the completer (QComboBox or QLineEdit instance)
     """
     completer = QtGui.QCompleter(cache)
     completer.setCaseSensitivity(QtCore.Qt.CaseInsensitive)

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2014-04-15 08:30:04 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2014-05-07 14:00:12 +0000
@@ -42,7 +42,7 @@
 from openlp.core.lib import FileDialog, PluginStatus, MediaType, create_separated_list
 from openlp.core.lib.ui import set_case_insensitive_completer, critical_error_message_box, find_and_set_in_combo_box
 from openlp.plugins.songs.lib import VerseType, clean_song
-from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorSong, AuthorType, Topic, MediaFile
+from openlp.plugins.songs.lib.db import Book, Song, Author, AuthorType, Topic, MediaFile
 from openlp.plugins.songs.lib.ui import SongStrings
 from openlp.plugins.songs.lib.xml import SongXML
 from openlp.plugins.songs.forms.editsongdialog import Ui_EditSongDialog
@@ -916,10 +916,8 @@
         self.song.authors_songs = []
         for row in range(self.authors_list_view.count()):
             item = self.authors_list_view.item(row)
-            author_song = AuthorSong()
-            author_song.author_id = item.data(QtCore.Qt.UserRole)[0]
-            author_song.author_type = item.data(QtCore.Qt.UserRole)[1]
-            self.song.authors_songs.append(author_song)
+            self.song.add_author(self.manager.get_object(Author, item.data(QtCore.Qt.UserRole)[0]),
+                                 item.data(QtCore.Qt.UserRole)[1])
         self.song.topics = []
         for row in range(self.topics_list_view.count()):
             item = self.topics_list_view.item(row)

=== modified file 'openlp/plugins/songs/forms/songmaintenanceform.py'
--- openlp/plugins/songs/forms/songmaintenanceform.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/forms/songmaintenanceform.py	2014-05-07 14:00:12 +0000
@@ -400,7 +400,7 @@
         """
         Merges two authors into one author.
 
-        :param old_author:  The object, which was edited, that will be deleted
+        :param old_author: The object, which was edited, that will be deleted
         """
         # Find the duplicate.
         existing_author = self.manager.get_object_filtered(
@@ -415,11 +415,9 @@
         # Find the songs, which have the old_author as author.
         songs = self.manager.get_all_objects(Song, Song.authors.contains(old_author))
         for song in songs:
-            # We check if the song has already existing_author as author. If
-            # that is not the case we add it.
-            if existing_author not in song.authors:
-                song.authors.append(existing_author)
-            song.authors.remove(old_author)
+            for author_song in song.authors_songs:
+                song.add_author(existing_author, author_song.author_type)
+                song.remove_author(old_author, author_song.author_type)
             self.manager.save_object(song)
         self.manager.delete_object(Author, old_author.id)
 

=== modified file 'openlp/plugins/songs/lib/__init__.py'
--- openlp/plugins/songs/lib/__init__.py	2014-05-02 06:42:17 +0000
+++ openlp/plugins/songs/lib/__init__.py	2014-05-07 14:00:12 +0000
@@ -390,12 +390,12 @@
     verses = SongXML().get_verses(song.lyrics)
     song.search_lyrics = ' '.join([clean_string(verse[1]) for verse in verses])
     # The song does not have any author, add one.
-    if not song.authors and not song.authors_songs:  # Need to check both relations
+    if not song.authors_songs:
         name = SongStrings.AuthorUnknown
         author = manager.get_object_filtered(Author, Author.display_name == name)
         if author is None:
             author = Author.populate(display_name=name, last_name='', first_name='')
-        song.authors.append(author)
+        song.add_author(author)
     if song.copyright:
         song.copyright = CONTROL_CHARS.sub('', song.copyright).strip()
 

=== modified file 'openlp/plugins/songs/lib/db.py'
--- openlp/plugins/songs/lib/db.py	2014-04-21 14:40:54 +0000
+++ openlp/plugins/songs/lib/db.py	2014-05-07 14:00:12 +0000
@@ -114,6 +114,33 @@
         """
         self.sort_key = get_natural_key(self.title)
 
+    def add_author(self, author, author_type=None):
+        """
+        Add an author to the song if it not yet exists
+
+        :param author: Author object
+        :param author_type: AuthorType constant or None
+        """
+        for author_song in self.authors_songs:
+            if author_song.author == author and author_song.author_type == author_type:
+                return
+        new_author_song = AuthorSong()
+        new_author_song.author = author
+        new_author_song.author_type = author_type
+        self.authors_songs.append(new_author_song)
+
+    def remove_author(self, author, author_type=None):
+        """
+        Remove an existing author from the song
+
+        :param author: Author object
+        :param author_type: AuthorType constant or None
+        """
+        for author_song in self.authors_songs:
+            if author_song.author == author and author_song.author_type == author_type:
+                self.authors_songs.remove(author_song)
+                return
+
 
 class Topic(BaseModel):
     """
@@ -283,9 +310,10 @@
     mapper(Book, song_books_table)
     mapper(MediaFile, media_files_table)
     mapper(Song, songs_table, properties={
-        # Use the authors_songs relation when you need access to the 'author_type' attribute.
+        # Use the authors_songs relation when you need access to the 'author_type' attribute
+        # or when creating new relations
         'authors_songs': relation(AuthorSong, cascade="all, delete-orphan"),
-        'authors': relation(Author, secondary=authors_songs_table),
+        'authors': relation(Author, secondary=authors_songs_table, viewonly=True),
         'book': relation(Book, backref='songs'),
         'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight),
         'topics': relation(Topic, backref='songs', secondary=songs_topics_table)

=== modified file 'openlp/plugins/songs/lib/foilpresenterimport.py'
--- openlp/plugins/songs/lib/foilpresenterimport.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/foilpresenterimport.py	2014-05-07 14:00:12 +0000
@@ -343,7 +343,7 @@
                 author = Author.populate(display_name=display_name, last_name=display_name.split(' ')[-1],
                                          first_name=' '.join(display_name.split(' ')[:-1]))
                 self.manager.save_object(author)
-            song.authors.append(author)
+            song.add_author(author)
 
     def _process_cclinumber(self, foilpresenterfolie, song):
         """

=== modified file 'openlp/plugins/songs/lib/olpimport.py'
--- openlp/plugins/songs/lib/olpimport.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/olpimport.py	2014-05-07 14:00:12 +0000
@@ -187,7 +187,7 @@
                         first_name=author.first_name,
                         last_name=author.last_name,
                         display_name=author.display_name)
-                new_song.authors.append(existing_author)
+                new_song.add_author(existing_author)
             if song.book:
                 existing_song_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
                 if existing_song_book is None:

=== modified file 'openlp/plugins/songs/lib/songimport.py'
--- openlp/plugins/songs/lib/songimport.py	2014-03-21 21:38:08 +0000
+++ openlp/plugins/songs/lib/songimport.py	2014-05-07 14:00:12 +0000
@@ -325,7 +325,7 @@
                 author = Author.populate(display_name=author_text,
                                          last_name=author_text.split(' ')[-1],
                                          first_name=' '.join(author_text.split(' ')[:-1]))
-            song.authors.append(author)
+            song.add_author(author)
         if self.song_book_name:
             song_book = self.manager.get_object_filtered(Book, Book.name == self.song_book_name)
             if song_book is None:

=== modified file 'openlp/plugins/songs/lib/songselect.py'
--- openlp/plugins/songs/lib/songselect.py	2014-05-02 06:42:17 +0000
+++ openlp/plugins/songs/lib/songselect.py	2014-05-07 14:00:12 +0000
@@ -196,13 +196,13 @@
         db_song.lyrics = song_xml.extract_xml()
         clean_song(self.db_manager, db_song)
         self.db_manager.save_object(db_song)
-        db_song.authors = []
+        db_song.authors_songs = []
         for author_name in song['authors']:
             author = self.db_manager.get_object_filtered(Author, Author.display_name == author_name)
             if not author:
                 author = Author.populate(first_name=author_name.rsplit(' ', 1)[0],
                                          last_name=author_name.rsplit(' ', 1)[1],
                                          display_name=author_name)
-            db_song.authors.append(author)
+            db_song.add_author(author)
         self.db_manager.save_object(db_song)
         return db_song

=== modified file 'openlp/plugins/songs/lib/xml.py'
--- openlp/plugins/songs/lib/xml.py	2014-05-02 06:42:17 +0000
+++ openlp/plugins/songs/lib/xml.py	2014-05-07 14:00:12 +0000
@@ -71,7 +71,7 @@
 from openlp.core.common import translate
 from openlp.core.lib import FormattingTags
 from openlp.plugins.songs.lib import VerseType, clean_song
-from openlp.plugins.songs.lib.db import Author, AuthorSong, AuthorType, Book, Song, Topic
+from openlp.plugins.songs.lib.db import Author, AuthorType, Book, Song, Topic
 from openlp.core.utils import get_application_version
 
 log = logging.getLogger(__name__)
@@ -519,10 +519,7 @@
                 author = Author.populate(display_name=display_name,
                                          last_name=display_name.split(' ')[-1],
                                          first_name=' '.join(display_name.split(' ')[:-1]))
-            author_song = AuthorSong()
-            author_song.author = author
-            author_song.author_type = author_type
-            song.authors_songs.append(author_song)
+            song.add_author(author, author_type)
 
     def _process_cclinumber(self, properties, song):
         """

=== modified file 'tests/functional/openlp_core_lib/test_ui.py'
--- tests/functional/openlp_core_lib/test_ui.py	2014-04-20 20:19:21 +0000
+++ tests/functional/openlp_core_lib/test_ui.py	2014-05-07 14:00:12 +0000
@@ -170,3 +170,19 @@
         self.assertIsInstance(action.icon(), QtGui.QIcon)
         self.assertEqual('my tooltip', action.toolTip())
         self.assertEqual('my statustip', action.statusTip())
+
+    def test_set_case_insensitive_completer(self):
+        """
+        Test setting a case insensitive completer on a widget
+        """
+        # GIVEN: A QComboBox and a list of completion items
+        line_edit = QtGui.QLineEdit()
+        suggestions = ['one', 'Two', 'THRee', 'FOUR']
+
+        # WHEN: We call the function
+        set_case_insensitive_completer(suggestions, line_edit)
+
+        # THEN: The Combobox should have a completer which is case insensitive
+        completer = line_edit.completer()
+        self.assertIsInstance(completer, QtGui.QCompleter)
+        self.assertEqual(completer.caseSensitivity(), QtCore.Qt.CaseInsensitive)

=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py	2014-04-02 19:35:09 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py	2014-05-07 14:00:12 +0000
@@ -344,7 +344,7 @@
             mocked_db_manager.get_object_filtered.assert_called_with(MockedAuthor, False)
             MockedAuthor.populate.assert_called_with(first_name='Public', last_name='Domain',
                                                      display_name='Public Domain')
-            self.assertEqual(1, len(result.authors), 'There should only be one author')
+            self.assertEqual(1, len(result.authors_songs), 'There should only be one author')
 
     def save_song_existing_author_test(self):
         """
@@ -379,4 +379,4 @@
                              'The save_object() method should have been called twice')
             mocked_db_manager.get_object_filtered.assert_called_with(MockedAuthor, False)
             self.assertEqual(0, MockedAuthor.populate.call_count, 'A new author should not have been instantiated')
-            self.assertEqual(1, len(result.authors), 'There should only be one author')
+            self.assertEqual(1, len(result.authors_songs), 'There should only be one author')