openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #28444
[Merge] lp:~sam92/openlp/bug-1533081 into lp:openlp
Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1533081 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
Related bugs:
Bug #1533081 in OpenLP: "Importing Song DB fails"
https://bugs.launchpad.net/openlp/+bug/1533081
Bug #1534306 in OpenLP: "Empty song-titles causes tracebacks on update from 2.0 to 2.2"
https://bugs.launchpad.net/openlp/+bug/1534306
For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1533081/+merge/282817
* 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
--
Your team OpenLP Core is requested to review the proposed merge of lp:~sam92/openlp/bug-1533081 into lp:openlp.
=== 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
Follow ups
