Merge lp:~raoul-snyman/openlp/db-upgrades-2.4 into lp:openlp/2.4

Proposed by Raoul Snyman
Status: Merged
Merged at revision: 2676
Proposed branch: lp:~raoul-snyman/openlp/db-upgrades-2.4
Merge into: lp:openlp/2.4
Diff against target: 251 lines (+120/-20)
4 files modified
CHANGELOG.rst (+8/-0)
openlp/plugins/songs/lib/importers/openlp.py (+31/-2)
openlp/plugins/songs/lib/upgrade.py (+32/-16)
tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py (+49/-2)
To merge this branch: bzr merge lp:~raoul-snyman/openlp/db-upgrades-2.4
Reviewer Review Type Date Requested Status
Tim Bentley Approve
Review via email: mp+319385@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Tim Bentley (trb143) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'CHANGELOG.rst'
2--- CHANGELOG.rst 1970-01-01 00:00:00 +0000
3+++ CHANGELOG.rst 2017-03-09 05:46:09 +0000
4@@ -0,0 +1,8 @@
5+OpenLP 2.4.6
6+============
7+
8+* Fixed a bug where the author type upgrade was being ignore because it was looking at the wrong table
9+* Fixed a bug where the songs_songbooks table was not being created because the if expression was the wrong way round
10+* Changed the songs_songbooks migration SQL slightly to take into account a bug that has (hopefully) been fixed
11+* Add another upgrade step to remove erroneous songs_songbooks entries due to the previous bug
12+* Added importing of author types to the OpenLP 2 song importer
13
14=== modified file 'openlp/plugins/songs/lib/importers/openlp.py'
15--- openlp/plugins/songs/lib/importers/openlp.py 2017-03-01 18:24:27 +0000
16+++ openlp/plugins/songs/lib/importers/openlp.py 2017-03-09 05:46:09 +0000
17@@ -61,6 +61,12 @@
18 :param progress_dialog: The QProgressDialog used when importing songs from the FRW.
19 """
20
21+ class OldAuthorSong(BaseModel):
22+ """
23+ Maps to the authors table
24+ """
25+ pass
26+
27 class OldAuthor(BaseModel):
28 """
29 Maps to the authors table
30@@ -117,6 +123,10 @@
31 has_songs_books = True
32 else:
33 has_songs_books = False
34+ if 'authors_songs' in list(source_meta.tables.keys()):
35+ has_authors_songs = True
36+ else:
37+ has_authors_songs = False
38 # Load up the tabls and map them out
39 source_authors_table = source_meta.tables['authors']
40 source_song_books_table = source_meta.tables['song_books']
41@@ -139,6 +149,10 @@
42 class_mapper(OldSongBookEntry)
43 except UnmappedClassError:
44 mapper(OldSongBookEntry, source_songs_songbooks_table, properties={'songbook': relation(OldBook)})
45+ if has_authors_songs and 'author_type' in source_authors_songs_table.c.values():
46+ has_author_type = True
47+ else:
48+ has_author_type = False
49 # Set up the songs relationships
50 song_props = {
51 'authors': relation(OldAuthor, backref='songs', secondary=source_authors_songs_table),
52@@ -157,6 +171,8 @@
53 song_props['songbook_entries'] = relation(OldSongBookEntry, backref='song', cascade='all, delete-orphan')
54 else:
55 song_props['book'] = relation(OldBook, backref='songs')
56+ if has_authors_songs:
57+ song_props['authors_songs'] = relation(OldAuthorSong)
58 # Map the rest of the tables
59 try:
60 class_mapper(OldAuthor)
61@@ -174,6 +190,11 @@
62 class_mapper(OldTopic)
63 except UnmappedClassError:
64 mapper(OldTopic, source_topics_table)
65+ if has_authors_songs:
66+ try:
67+ class_mapper(OldAuthorSong)
68+ except UnmappedClassError:
69+ mapper(OldAuthorSong, source_authors_songs_table)
70
71 source_songs = self.source_session.query(OldSong).all()
72 if self.import_wizard:
73@@ -205,8 +226,16 @@
74 existing_author = Author.populate(
75 first_name=author.first_name,
76 last_name=author.last_name,
77- display_name=author.display_name)
78- new_song.add_author(existing_author)
79+ display_name=author.display_name
80+ )
81+ # if this is a new database, we need to import the author type too
82+ author_type = None
83+ if has_author_type:
84+ for author_song in song.authors_songs:
85+ if author_song.author_id == author.id:
86+ author_type = author_song.author_type
87+ break
88+ new_song.add_author(existing_author, author_type)
89 # Find or create all the topics and add them to the new song object
90 if song.topics:
91 for topic in song.topics:
92
93=== modified file 'openlp/plugins/songs/lib/upgrade.py'
94--- openlp/plugins/songs/lib/upgrade.py 2016-12-31 11:05:48 +0000
95+++ openlp/plugins/songs/lib/upgrade.py 2017-03-09 05:46:09 +0000
96@@ -26,13 +26,13 @@
97 import logging
98
99 from sqlalchemy import Table, Column, ForeignKey, types
100-from sqlalchemy.sql.expression import func, false, null, text
101+from sqlalchemy.sql.expression import func, false, null, text, and_, select
102
103 from openlp.core.lib.db import get_upgrade_op
104 from openlp.core.utils.db import drop_columns
105
106 log = logging.getLogger(__name__)
107-__version__ = 5
108+__version__ = 6
109
110
111 # TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version
112@@ -105,13 +105,15 @@
113 # Since SQLite doesn't support changing the primary key of a table, we need to recreate the table
114 # and copy the old values
115 op = get_upgrade_op(session)
116- songs_table = Table('songs', metadata)
117- if 'author_type' not in [col.name for col in songs_table.c.values()]:
118- op.create_table('authors_songs_tmp',
119- Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
120- Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
121- Column('author_type', types.Unicode(255), primary_key=True,
122- nullable=False, server_default=text('""')))
123+ authors_songs = Table('authors_songs', metadata, autoload=True)
124+ if 'author_type' not in [col.name for col in authors_songs.c.values()]:
125+ authors_songs_tmp = op.create_table(
126+ 'authors_songs_tmp',
127+ Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
128+ Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
129+ Column('author_type', types.Unicode(255), primary_key=True,
130+ nullable=False, server_default=text('""'))
131+ )
132 op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs')
133 op.drop_table('authors_songs')
134 op.rename_table('authors_songs_tmp', 'authors_songs')
135@@ -126,20 +128,22 @@
136 This upgrade adds support for multiple songbooks
137 """
138 op = get_upgrade_op(session)
139- songs_table = Table('songs', metadata)
140- if 'song_book_id' in [col.name for col in songs_table.c.values()]:
141+ songs_table = Table('songs', metadata, autoload=True)
142+ if 'song_book_id' not in [col.name for col in songs_table.c.values()]:
143 log.warning('Skipping upgrade_5 step of upgrading the song db')
144 return
145
146 # Create the mapping table (songs <-> songbooks)
147- op.create_table('songs_songbooks',
148- Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
149- Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
150- Column('entry', types.Unicode(255), primary_key=True, nullable=False))
151+ songs_songbooks_table = op.create_table(
152+ 'songs_songbooks',
153+ Column('songbook_id', types.Integer(), ForeignKey('song_books.id'), primary_key=True),
154+ Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
155+ Column('entry', types.Unicode(255), primary_key=True, nullable=False)
156+ )
157
158 # Migrate old data
159 op.execute('INSERT INTO songs_songbooks SELECT song_book_id, id, song_number FROM songs\
160- WHERE song_book_id IS NOT NULL AND song_number IS NOT NULL')
161+ WHERE song_book_id IS NOT NULL AND song_book_id <> 0 AND song_number IS NOT NULL')
162
163 # Drop old columns
164 if metadata.bind.url.get_dialect().name == 'sqlite':
165@@ -148,3 +152,15 @@
166 op.drop_constraint('songs_ibfk_1', 'songs', 'foreignkey')
167 op.drop_column('songs', 'song_book_id')
168 op.drop_column('songs', 'song_number')
169+
170+
171+def upgrade_6(session, metadata):
172+ """
173+ Version 6 upgrade.
174+
175+ This is to fix an issue we had with songbooks with an id of "0" being imported in the previous upgrade.
176+ """
177+ op = get_upgrade_op(session)
178+ songs_songbooks = Table('songs_songbooks', metadata, autoload=True)
179+ del_query = songs_songbooks.delete().where(songs_songbooks.c.songbook_id == 0)
180+ op.execute(del_query)
181
182=== modified file 'tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py'
183--- tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py 2016-12-31 11:05:48 +0000
184+++ tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py 2017-03-09 05:46:09 +0000
185@@ -23,13 +23,13 @@
186 Package to test the openlp.plugins.songs.forms.editsongform package.
187 """
188 from unittest import TestCase
189+from unittest.mock import MagicMock, patch
190
191-from PyQt5 import QtWidgets
192+from PyQt5 import QtCore, QtWidgets
193
194 from openlp.core.common import Registry
195 from openlp.core.common.uistrings import UiStrings
196 from openlp.plugins.songs.forms.editsongform import EditSongForm
197-from tests.interfaces import MagicMock
198 from tests.helpers.testmixin import TestMixin
199
200
201@@ -157,3 +157,50 @@
202
203 # THEN: The verse order should be converted to uppercase
204 self.assertEqual(form.verse_order_edit.text(), 'V1 V2 C1 V3 C1 V4 C1')
205+
206+ @patch('openlp.plugins.songs.forms.editsongform.QtWidgets.QListWidgetItem')
207+ def test_add_author_to_list(self, MockedQListWidgetItem):
208+ """
209+ Test the _add_author_to_list() method
210+ """
211+ # GIVEN: A song edit form and some mocked stuff
212+ mocked_author = MagicMock()
213+ mocked_author.id = 1
214+ mocked_author.get_display_name.return_value = 'John Newton'
215+ mocked_author_type = 'words'
216+ mocked_widget_item = MagicMock()
217+ MockedQListWidgetItem.return_value = mocked_widget_item
218+
219+ # WHEN: _add_author_to_list() is called
220+ with patch.object(self.form.authors_list_view, 'addItem') as mocked_add_item:
221+ self.form._add_author_to_list(mocked_author, mocked_author_type)
222+
223+ # THEN: All the correct methods should have been called
224+ mocked_author.get_display_name.assert_called_once_with('words')
225+ MockedQListWidgetItem.assert_called_once_with('John Newton')
226+ mocked_widget_item.setData.assert_called_once_with(QtCore.Qt.UserRole, (1, mocked_author_type))
227+ mocked_add_item.assert_called_once_with(mocked_widget_item)
228+
229+ @patch('openlp.plugins.songs.forms.editsongform.SongBookEntry')
230+ @patch('openlp.plugins.songs.forms.editsongform.QtWidgets.QListWidgetItem')
231+ def test_add_songbook_entry_to_list(self, MockedQListWidgetItem, MockedSongbookEntry):
232+ """
233+ Test the add_songbook_entry_to_list() method
234+ """
235+ # GIVEN: A song edit form and some mocked stuff
236+ songbook_id = 1
237+ songbook_name = 'Hymnal'
238+ entry = '546'
239+ MockedSongbookEntry.get_display_name.return_value = 'Hymnal #546'
240+ mocked_widget_item = MagicMock()
241+ MockedQListWidgetItem.return_value = mocked_widget_item
242+
243+ # WHEN: _add_author_to_list() is called
244+ with patch.object(self.form.songbooks_list_view, 'addItem') as mocked_add_item:
245+ self.form.add_songbook_entry_to_list(songbook_id, songbook_name, entry)
246+
247+ # THEN: All the correct methods should have been called
248+ MockedSongbookEntry.get_display_name.assert_called_once_with(songbook_name, entry)
249+ MockedQListWidgetItem.assert_called_once_with('Hymnal #546')
250+ mocked_widget_item.setData.assert_called_once_with(QtCore.Qt.UserRole, (songbook_id, entry))
251+ mocked_add_item.assert_called_once_with(mocked_widget_item)

Subscribers

People subscribed via source and target branches

to all changes: