raoul/openlp
1
0
Fork 0
forked from openlp/openlp
Code Issues Pull Requests Projects Releases Wiki Activity

Add this to your merge proposal:

Browse Source 
--------------------------------
lp:~raoul-snyman/openlp/db-upgrades-2.4 (revision 2681)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1929/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1840/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1781/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1511/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1101/
[SUCCESS] https://ci.openlp.io/...

bzr-revno: 2676
This commit is contained in:
Raoul Snyman 2017-03-11 20:20:26 -07:00
commit c4162fdf1e
4 changed files with 120 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG.rst Normal file
View File

@ -0,0 +1,8 @@
OpenLP 2.4.6
============
* Fixed a bug where the author type upgrade was being ignore because it was looking at the wrong table
* Fixed a bug where the songs_songbooks table was not being created because the if expression was the wrong way round
* Changed the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed
* Add another upgrade step to remove erroneous songs_songbooks entries due to the previous bug
* Added importing of author types to the OpenLP 2 song importer

33
openlp/plugins/songs/lib/importers/openlp.py
View File

@ -61,6 +61,12 @@ class OpenLPSongImport(SongImport):
:param progress_dialog: The QProgressDialog used when importing songs from the FRW.
"""
class OldAuthorSong(BaseModel):
"""
Maps to the authors table
"""
pass
class OldAuthor(BaseModel):
"""
Maps to the authors table
@ -117,6 +123,10 @@ class OpenLPSongImport(SongImport):
has_songs_books = True
else:
has_songs_books = False
if 'authors_songs' in list(source_meta.tables.keys()):
has_authors_songs = True
else:
has_authors_songs = False
# Load up the tabls and map them out
source_authors_table = source_meta.tables['authors']
source_song_books_table = source_meta.tables['song_books']
@ -139,6 +149,10 @@ class OpenLPSongImport(SongImport):
class_mapper(OldSongBookEntry)
except UnmappedClassError:
mapper(OldSongBookEntry, source_songs_songbooks_table, properties={'songbook': relation(OldBook)})
if has_authors_songs and 'author_type' in source_authors_songs_table.c.values():
has_author_type = True
else:
has_author_type = False
# Set up the songs relationships
song_props = {
'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),
@ -157,6 +171,8 @@ class OpenLPSongImport(SongImport):
song_props['songbook_entries'] = relation(OldSongBookEntry, backref='song', cascade='all, delete-orphan')
else:
song_props['book'] = relation(OldBook, backref='songs')
if has_authors_songs:
song_props['authors_songs'] = relation(OldAuthorSong)
# Map the rest of the tables
try:
class_mapper(OldAuthor)
@ -174,6 +190,11 @@ class OpenLPSongImport(SongImport):
class_mapper(OldTopic)
except UnmappedClassError:
mapper(OldTopic, source_topics_table)
if has_authors_songs:
try:
class_mapper(OldAuthorSong)
except UnmappedClassError:
mapper(OldAuthorSong, source_authors_songs_table)
source_songs = self.source_session.query(OldSong).all()
if self.import_wizard:
@ -205,8 +226,16 @@ class OpenLPSongImport(SongImport):
existing_author = Author.populate(
first_name=author.first_name,
last_name=author.last_name,
display_name=author.display_name)
new_song.add_author(existing_author)
display_name=author.display_name
)
# if this is a new database, we need to import the author type too
author_type = None
if has_author_type:
for author_song in song.authors_songs:
if author_song.author_id == author.id:
author_type = author_song.author_type
break
new_song.add_author(existing_author, author_type)
# Find or create all the topics and add them to the new song object
if song.topics:
for topic in song.topics:

48
openlp/plugins/songs/lib/upgrade.py
View File

@ -26,13 +26,13 @@ backend for the Songs plugin
import logging
from sqlalchemy import Table, Column, ForeignKey, types
from sqlalchemy.sql.expression import func, false, null, text
from sqlalchemy.sql.expression import func, false, null, text, and_, select
from openlp.core.lib.db import get_upgrade_op
from openlp.core.utils.db import drop_columns
log = logging.getLogger(__name__)
__version__ = 5
__version__ = 6
# TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version
@ -105,13 +105,15 @@ def upgrade_4(session, metadata):
# Since SQLite doesn't support changing the primary key of a table, we need to recreate the table
# and copy the old values
op = get_upgrade_op(session)
songs_table = Table('songs', metadata)
if 'author_type' not in [col.name for col in songs_table.c.values()]:
op.create_table('authors_songs_tmp',
Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
Column('author_type', types.Unicode(255), primary_key=True,
nullable=False, server_default=text('""')))
authors_songs = Table('authors_songs', metadata, autoload=True)
if 'author_type' not in [col.name for col in authors_songs.c.values()]:
authors_songs_tmp = op.create_table(
'authors_songs_tmp',
Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
Column('author_type', types.Unicode(255), primary_key=True,
nullable=False, server_default=text('""'))
)
op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs')
op.drop_table('authors_songs')
op.rename_table('authors_songs_tmp', 'authors_songs')
@ -126,20 +128,22 @@ def upgrade_5(session, metadata):
This upgrade adds support for multiple songbooks
"""
op = get_upgrade_op(session)
songs_table = Table('songs', metadata)
if 'song_book_id' in [col.name for col in songs_table.c.values()]:
songs_table = Table('songs', metadata, autoload=True)
if 'song_book_id' not in [col.name for col in songs_table.c.values()]:
log.warning('Skipping upgrade_5 step of upgrading the song db')
return
# Create the mapping table (songs <-> songbooks)
op.create_table('songs_songbooks',
Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
Column('entry', types.Unicode(255), primary_key=True, nullable=False))
songs_songbooks_table = op.create_table(
'songs_songbooks',
Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
Column('entry', types.Unicode(255), primary_key=True, nullable=False)
)
# Migrate old data
op.execute('INSERT INTO songs_songbooks SELECT song_book_id, id, song_number FROM songs\
WHERE song_book_id IS NOT NULL AND song_number IS NOT NULL')
WHERE song_book_id IS NOT NULL AND song_book_id <> 0 AND song_number IS NOT NULL')
# Drop old columns
if metadata.bind.url.get_dialect().name == 'sqlite':
@ -148,3 +152,15 @@ def upgrade_5(session, metadata):
op.drop_constraint('songs_ibfk_1', 'songs', 'foreignkey')
op.drop_column('songs', 'song_book_id')
op.drop_column('songs', 'song_number')
def upgrade_6(session, metadata):
"""
Version 6 upgrade.
This is to fix an issue we had with songbooks with an id of "0" being imported in the previous upgrade.
"""
op = get_upgrade_op(session)
songs_songbooks = Table('songs_songbooks', metadata, autoload=True)
del_query = songs_songbooks.delete().where(songs_songbooks.c.songbook_id == 0)
op.execute(del_query)

51
tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py
View File

@ -23,13 +23,13 @@
Package to test the openlp.plugins.songs.forms.editsongform package.
"""
from unittest import TestCase
from unittest.mock import MagicMock, patch
from PyQt5 import QtWidgets
from PyQt5 import QtCore, QtWidgets
from openlp.core.common import Registry
from openlp.core.common.uistrings import UiStrings
from openlp.plugins.songs.forms.editsongform import EditSongForm
from tests.interfaces import MagicMock
from tests.helpers.testmixin import TestMixin
@ -157,3 +157,50 @@ class TestEditSongForm(TestCase, TestMixin):
# THEN: The verse order should be converted to uppercase
self.assertEqual(form.verse_order_edit.text(), 'V1 V2 C1 V3 C1 V4 C1')
@patch('openlp.plugins.songs.forms.editsongform.QtWidgets.QListWidgetItem')
def test_add_author_to_list(self, MockedQListWidgetItem):
"""
Test the _add_author_to_list() method
"""
# GIVEN: A song edit form and some mocked stuff
mocked_author = MagicMock()
mocked_author.id = 1
mocked_author.get_display_name.return_value = 'John Newton'
mocked_author_type = 'words'
mocked_widget_item = MagicMock()
MockedQListWidgetItem.return_value = mocked_widget_item
# WHEN: _add_author_to_list() is called
with patch.object(self.form.authors_list_view, 'addItem') as mocked_add_item:
self.form._add_author_to_list(mocked_author, mocked_author_type)
# THEN: All the correct methods should have been called
mocked_author.get_display_name.assert_called_once_with('words')
MockedQListWidgetItem.assert_called_once_with('John Newton')
mocked_widget_item.setData.assert_called_once_with(QtCore.Qt.UserRole, (1, mocked_author_type))
mocked_add_item.assert_called_once_with(mocked_widget_item)
@patch('openlp.plugins.songs.forms.editsongform.SongBookEntry')
@patch('openlp.plugins.songs.forms.editsongform.QtWidgets.QListWidgetItem')
def test_add_songbook_entry_to_list(self, MockedQListWidgetItem, MockedSongbookEntry):
"""
Test the add_songbook_entry_to_list() method
"""
# GIVEN: A song edit form and some mocked stuff
songbook_id = 1
songbook_name = 'Hymnal'
entry = '546'
MockedSongbookEntry.get_display_name.return_value = 'Hymnal #546'
mocked_widget_item = MagicMock()
MockedQListWidgetItem.return_value = mocked_widget_item
# WHEN: _add_author_to_list() is called
with patch.object(self.form.songbooks_list_view, 'addItem') as mocked_add_item:
self.form.add_songbook_entry_to_list(songbook_id, songbook_name, entry)
# THEN: All the correct methods should have been called
MockedSongbookEntry.get_display_name.assert_called_once_with(songbook_name, entry)
MockedQListWidgetItem.assert_called_once_with('Hymnal #546')
mocked_widget_item.setData.assert_called_once_with(QtCore.Qt.UserRole, (songbook_id, entry))
mocked_add_item.assert_called_once_with(mocked_widget_item)