Merge lp:~sam92/openlp/bug-1533081 into lp:openlp

[Samuel Mehrbrodt]

* Fix importing older song dbs by migrating them first
* Fix comparing songs without title

lp:~sam92/openlp/bug-1533081 (revision 2615)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1260/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1184/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1123/
[FAILURE] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/959/
Stopping after failure

[Tim Bentley (trb143)]

review: Approve

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/core/utils/__init__.py'
--- openlp/core/utils/__init__.py	2015-12-31 22:46:06 +0000
+++ openlp/core/utils/__init__.py	2016-01-15 21:49:07 +0000
@@ -529,7 +529,7 @@
     key = [int(part) if part.isdigit() else get_locale_key(part) for part in key]
     # Python 3 does not support comparison of different types anymore. So make sure, that we do not compare str
     # and int.
-    if string[0].isdigit():
+    if string and string[0].isdigit():
         return [b''] + key
     return key
 

=== modified file 'openlp/plugins/songs/lib/importers/openlp.py'
--- openlp/plugins/songs/lib/importers/openlp.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/importers/openlp.py	2016-01-15 21:49:07 +0000
@@ -24,15 +24,18 @@
 song databases into the current installation database.
 """
 import logging
+import os
+import shutil
+import tempfile
 
 from sqlalchemy import create_engine, MetaData, Table
 from sqlalchemy.orm import class_mapper, mapper, relation, scoped_session, sessionmaker
 from sqlalchemy.orm.exc import UnmappedClassError
 
 from openlp.core.common import translate
-from openlp.core.lib.db import BaseModel
+from openlp.core.lib.db import BaseModel, upgrade_db
 from openlp.core.ui.wizard import WizardStrings
-from openlp.plugins.songs.lib import clean_song
+from openlp.plugins.songs.lib import clean_song, upgrade
 from openlp.plugins.songs.lib.db import Author, Book, Song, Topic, MediaFile
 from .songimport import SongImport
 
@@ -67,12 +70,6 @@
             """
             pass
 
-        class OldBook(BaseModel):
-            """
-            Book model
-            """
-            pass
-
         class OldMediaFile(BaseModel):
             """
             MediaFile model
@@ -91,26 +88,38 @@
             """
             pass
 
+        class OldSongBookEntry(BaseModel):
+            """
+            SongbookEntry model
+            """
+            pass
+
         # Check the file type
         if not self.import_source.endswith('.sqlite'):
             self.log_error(self.import_source, translate('SongsPlugin.OpenLPSongImport',
                                                          'Not a valid OpenLP 2 song database.'))
             return
-        self.import_source = 'sqlite:///%s' % self.import_source
+        # Operate on a copy - We might need to apply migrations to the db and don't want to change the existing db
+        import_db = os.path.join(tempfile.gettempdir(), os.path.basename(self.import_source))
+        shutil.copy(self.import_source, import_db)
+        import_db_url = 'sqlite:///%s' % import_db
+        # Apply migrations
+        upgrade_db(import_db_url, upgrade)
         # Load the db file
-        engine = create_engine(self.import_source)
+        engine = create_engine(import_db_url)
         source_meta = MetaData()
         source_meta.reflect(engine)
         self.source_session = scoped_session(sessionmaker(bind=engine))
+
         if 'media_files' in list(source_meta.tables.keys()):
             has_media_files = True
         else:
             has_media_files = False
         source_authors_table = source_meta.tables['authors']
-        source_song_books_table = source_meta.tables['song_books']
         source_songs_table = source_meta.tables['songs']
         source_topics_table = source_meta.tables['topics']
         source_authors_songs_table = source_meta.tables['authors_songs']
+        source_songs_songbooks_table = source_meta.tables['songs_songbooks']
         source_songs_topics_table = source_meta.tables['songs_topics']
         source_media_files_songs_table = None
         if has_media_files:
@@ -122,8 +131,8 @@
                 mapper(OldMediaFile, source_media_files_table)
         song_props = {
             'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),
-            'book': relation(OldBook, backref='songs'),
-            'topics': relation(OldTopic, backref='songs', secondary=source_songs_topics_table)
+            'topics': relation(OldTopic, backref='songs', secondary=source_songs_topics_table),
+            'songbook_entries': relation(OldSongBookEntry, backref='song'),
         }
         if has_media_files:
             if isinstance(source_media_files_songs_table, Table):
@@ -139,10 +148,6 @@
         except UnmappedClassError:
             mapper(OldAuthor, source_authors_table)
         try:
-            class_mapper(OldBook)
-        except UnmappedClassError:
-            mapper(OldBook, source_song_books_table)
-        try:
             class_mapper(OldSong)
         except UnmappedClassError:
             mapper(OldSong, source_songs_table, properties=song_props)
@@ -150,6 +155,10 @@
             class_mapper(OldTopic)
         except UnmappedClassError:
             mapper(OldTopic, source_topics_table)
+        try:
+            class_mapper(OldSongBookEntry)
+        except UnmappedClassError:
+            mapper(OldSongBookEntry, source_songs_songbooks_table)
 
         source_songs = self.source_session.query(OldSong).all()
         if self.import_wizard:
@@ -166,7 +175,6 @@
             # Values will be set when cleaning the song.
             new_song.search_title = ''
             new_song.search_lyrics = ''
-            new_song.song_number = song.song_number
             new_song.lyrics = song.lyrics
             new_song.verse_order = song.verse_order
             new_song.copyright = song.copyright
@@ -181,11 +189,12 @@
                         last_name=author.last_name,
                         display_name=author.display_name)
                 new_song.add_author(existing_author)
-            if song.book:
-                existing_song_book = self.manager.get_object_filtered(Book, Book.name == song.book.name)
+            for songbook_entry in song.songbook_entries:
+                existing_song_book = self.manager.get_object_filtered(Book, Book.name == songbook_entry.book.name)
                 if existing_song_book is None:
-                    existing_song_book = Book.populate(name=song.book.name, publisher=song.book.publisher)
-                new_song.book = existing_song_book
+                    existing_song_book = Book.populate(name=songbook_entry.book.name,
+                                                       publisher=songbook_entry.book.publisher)
+                new_song.add_songbook_entry(existing_song_book, songbook_entry.entry)
             if song.topics:
                 for topic in song.topics:
                     existing_topic = self.manager.get_object_filtered(Topic, Topic.name == topic.name)
@@ -212,3 +221,4 @@
                 break
         self.source_session.close()
         engine.dispose()
+        os.remove(import_db)

=== added file 'tests/utils/test_pep8.py'
--- tests/utils/test_pep8.py	1970-01-01 00:00:00 +0000
+++ tests/utils/test_pep8.py	2016-01-15 21:49:07 +0000
@@ -0,0 +1,89 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 OpenLP Developers                                   #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+
+"""
+Package to test for pep8 errors
+Inspired by http://blog.jameskyle.org/2014/05/pep8-pylint-tests-with-nose-xunit/
+"""
+
+import os
+import pep8
+import re
+
+from unittest import TestCase
+
+from tests.utils.constants import OPENLP_PATH
+from tests.utils import is_running_on_ci
+
+PROJ_ROOT = 'openlp'
+
+
+class TestPep8(TestCase):
+
+    def pep8_test(self):
+        """
+        Test for pep8 conformance
+        """
+        pattern = re.compile(r'.*({0}.*\.py)'.format(PROJ_ROOT))
+        pep8style = pep8.StyleGuide(reporter=CustomReport, config_file=os.path.join(OPENLP_PATH, 'setup.cfg'))
+
+        report = pep8style.init_report()
+        pep8style.input_dir(OPENLP_PATH)
+
+        for error in report.results:
+            msg = "{path}: {code} {row}, {col} - {text}"
+
+            match = pattern.match(error['path'])
+            if match:
+                rel_path = match.group(1)
+            else:
+                rel_path = error['path']
+
+            print(msg.format(
+                path=rel_path,
+                code=error['code'],
+                row=error['row'],
+                col=error['col'],
+                text=error['text']
+            ))
+        if report.results:
+            self.fail("Please fix the PEP8 warnings.")
+
+
+class CustomReport(pep8.StandardReport):
+    """
+    Collect report into an array of results.
+    """
+    results = []
+
+    def get_file_results(self):
+        if self._deferred_print:
+            self._deferred_print.sort()
+            for line_number, offset, code, text, _ in self._deferred_print:
+                self.results.append({
+                    'path': self.filename,
+                    'row': self.line_offset + line_number,
+                    'col': offset + 1,
+                    'code': code,
+                    'text': text,
+                })
+        return self.file_errors