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

Merge branch 'fix-songbook-merge' into 'master'

Browse Source 
Fix merging of song books

Closes #899

See merge request openlp/openlp!347
This commit is contained in:
Tim Bentley 2021-09-02 06:46:10 +00:00
commit a3ff2b1d0b
3 changed files with 175 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
openlp/plugins/songs/forms/songmaintenanceform.py
View File

@ -30,7 +30,7 @@ from openlp.core.lib.ui import critical_error_message_box
from openlp.plugins.songs.forms.authorsform import AuthorsForm
from openlp.plugins.songs.forms.songbookform import SongBookForm
from openlp.plugins.songs.forms.topicsform import TopicsForm
from openlp.plugins.songs.lib.db import Author, Book, Song, Topic
from openlp.plugins.songs.lib.db import Author, Book, Song, Topic, SongBookEntry
from .songmaintenancedialog import Ui_SongMaintenanceDialog
@ -473,11 +473,41 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP
Book.id != old_song_book.id
)
)
# Find the songs, which have the old_song_book as book.
songs = self.manager.get_all_objects(Song, Song.song_book_id == old_song_book.id)
for song in songs:
song.song_book_id = existing_book.id
if existing_book is None:
return False
# Find the songs which have the old_song_book as book, via matching entries in the songs_songbooks table
songbook_entries = self.manager.get_all_objects(SongBookEntry, SongBookEntry.songbook_id == old_song_book.id)
affected_songs = []
for songbook_entry in songbook_entries:
affected_songs.append(songbook_entry.song_id)
for song_id in affected_songs:
song = self.manager.get_object(Song, song_id)
# Go through the song's song/songbook link records to look for the link to the old songbook.
# Store the song number in that book so we can apply it in the link record to the existing songbook.
old_book_song_number = ''
for index, record in enumerate(song.songbook_entries):
if record.songbook_id == old_song_book.id:
old_book_song_number = record.entry
break
# Look through the same link records to see if there's a link to the duplicate (existing) songbook
# If so, transfer the song number in 'entry' field (if appropriate) ...
found = False
for record in song.songbook_entries:
if record.songbook_id == existing_book.id:
if not record.entry and old_book_song_number:
record.entry = old_book_song_number
found = True
break
# ... otherwise create a new link record
if not found:
new_songbook_entry = SongBookEntry()
new_songbook_entry.songbook_id = existing_book.id
new_songbook_entry.song_id = song.id
new_songbook_entry.entry = old_book_song_number
song.songbook_entries.append(new_songbook_entry)
self.manager.save_object(song)
self.manager.delete_object(SongBookEntry, (old_song_book.id, song_id, old_book_song_number))
self.manager.delete_object(Book, old_song_book.id)
def on_delete_author_button_clicked(self):

74
openlp/plugins/songs/lib/db.py
View File

@ -23,8 +23,10 @@ The :mod:`db` module provides the database and schema that is the backend for
the Songs plugin
"""
from sqlalchemy import Column, ForeignKey, Table, types
from sqlalchemy.orm import mapper, reconstructor, relation
from sqlalchemy.orm import class_mapper, mapper, reconstructor, relation
from sqlalchemy.sql.expression import func, text
from sqlalchemy.orm.exc import UnmappedClassError
from openlp.core.common.i18n import get_natural_key, translate
from openlp.core.lib.db import BaseModel, PathType, init_db
@ -364,30 +366,52 @@ def init_schema(url):
Column('topic_id', types.Integer(), ForeignKey('topics.id'), primary_key=True)
)
mapper(Author, authors_table, properties={
'songs': relation(Song, secondary=authors_songs_table, viewonly=True)
})
mapper(AuthorSong, authors_songs_table, properties={
'author': relation(Author)
})
mapper(SongBookEntry, songs_songbooks_table, properties={
'songbook': relation(Book)
})
mapper(Book, song_books_table, properties={
'songs': relation(Song, secondary=songs_songbooks_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
# or when creating new relations
'authors_songs': relation(AuthorSong, cascade="all, delete-orphan"),
# Use lazy='joined' to always load authors when the song is fetched from the database (bug 1366198)
'authors': relation(Author, secondary=authors_songs_table, viewonly=True, lazy='joined'),
'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight),
'songbook_entries': relation(SongBookEntry, backref='song', cascade='all, delete-orphan'),
'topics': relation(Topic, backref='songs', secondary=songs_topics_table)
})
mapper(Topic, topics_table)
# try/except blocks are for the purposes of tests - the mappers could have been defined in a previous test
try:
class_mapper(Author)
except UnmappedClassError:
mapper(Author, authors_table, properties={
'songs': relation(Song, secondary=authors_songs_table, viewonly=True)
})
try:
class_mapper(AuthorSong)
except UnmappedClassError:
mapper(AuthorSong, authors_songs_table, properties={
'author': relation(Author)
})
try:
class_mapper(SongBookEntry)
except UnmappedClassError:
mapper(SongBookEntry, songs_songbooks_table, properties={
'songbook': relation(Book)
})
try:
class_mapper(Book)
except UnmappedClassError:
mapper(Book, song_books_table, properties={
'songs': relation(Song, secondary=songs_songbooks_table)
})
try:
class_mapper(MediaFile)
except UnmappedClassError:
mapper(MediaFile, media_files_table)
try:
class_mapper(Song)
except UnmappedClassError:
mapper(Song, songs_table, properties={
# 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"),
# Use lazy='joined' to always load authors when the song is fetched from the database (bug 1366198)
'authors': relation(Author, secondary=authors_songs_table, viewonly=True, lazy='joined'),
'media_files': relation(MediaFile, backref='songs', order_by=media_files_table.c.weight),
'songbook_entries': relation(SongBookEntry, backref='song', cascade='all, delete-orphan'),
'topics': relation(Topic, backref='songs', secondary=songs_topics_table)
})
try:
class_mapper(Topic)
except UnmappedClassError:
mapper(Topic, topics_table)
metadata.create_all(checkfirst=True)
return session

91
tests/openlp_plugins/songs/forms/test_songmaintenanceform.py
View File

@ -22,14 +22,20 @@
Package to test the openlp.plugins.songs.forms.songmaintenanceform package.
"""
import pytest
import os
from unittest.mock import MagicMock, call, patch, ANY
from PyQt5 import QtCore, QtWidgets
from openlp.core.common.i18n import UiStrings
from openlp.core.common.registry import Registry
from openlp.core.lib.db import Manager
from openlp.plugins.songs.lib.db import init_schema, Book, Song, SongBookEntry
from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm
from sqlalchemy.sql import and_
@pytest.fixture()
def form_env(settings):
@ -448,3 +454,88 @@ def test_check_object_exists(form_env):
# THEN: The result should be False
assert result is True
def test_merge_song_books(registry, settings, temp_folder):
"""
Test the functionality of merging 2 song books.
"""
# GIVEN a test database populated with test data, and a song maintenance form
db_tmp_path = os.path.join(temp_folder, 'test-songs-2.9.2.sqlite')
manager = Manager('songs', init_schema, db_file_path=db_tmp_path)
# create 2 song books, both with the same name
book1 = Book()
book1.name = 'test book1'
book1.publisher = ''
manager.save_object(book1)
book2 = Book()
book2.name = 'test book1'
book2.publisher = ''
manager.save_object(book2)
# create 3 songs, all with same search_title
song1 = Song()
song1.title = 'test song1'
song1.lyrics = 'lyrics1'
song1.search_title = 'test song'
song1.search_lyrics = 'lyrics1'
manager.save_object(song1)
song2 = Song()
song2.title = 'test song2'
song2.lyrics = 'lyrics2'
song2.search_title = 'test song'
song2.search_lyrics = 'lyrics2'
manager.save_object(song2)
song3 = Song()
song3.title = 'test song3'
song3.lyrics = 'lyrics3'
song3.search_title = 'test song'
song3.search_lyrics = 'lyrics3'
manager.save_object(song3)
# associate songs with song books
song1.add_songbook_entry(book1, '10')
song2.add_songbook_entry(book1, '20')
song2.add_songbook_entry(book2, '30')
song3.add_songbook_entry(book1, '40')
song3.add_songbook_entry(book2, '')
song_maintenance_form = SongMaintenanceForm(manager)
# WHEN the song books are merged, getting rid of book1
song_maintenance_form.merge_song_books(book1)
# THEN the database should reflect correctly the merge
songs = manager.get_all_objects(Song, Song.search_title == 'test song')
songbook1_entries = manager.get_all_objects(SongBookEntry, SongBookEntry.songbook_id == book1.id)
songbook2_entries = manager.get_all_objects(SongBookEntry, SongBookEntry.songbook_id == book2.id)
song1_book2_entry = manager.get_all_objects(SongBookEntry, and_(SongBookEntry.songbook_id == book2.id,
SongBookEntry.song_id == song1.id))
song2_book2_entry = manager.get_all_objects(SongBookEntry, and_(SongBookEntry.songbook_id == book2.id,
SongBookEntry.song_id == song2.id))
song3_book2_entry = manager.get_all_objects(SongBookEntry, and_(SongBookEntry.songbook_id == book2.id,
SongBookEntry.song_id == song3.id))
books = manager.get_all_objects(Book, Book.name == 'test book1')
# song records should not be deleted
assert len(songs) == 3
# the old book should have been deleted, with its songs_songbooks records
assert len(books) == 1
assert len(songbook1_entries) == 0
# each of the 3 songs should be associated with book2
assert len(songbook2_entries) == 3
# the individual SongBookEntry records should be correct
assert len(song1_book2_entry) == 1
assert song1_book2_entry[0].entry == '10'
assert len(song2_book2_entry) == 1
# entry field should not be overridden, as it was set previously for book2
assert song2_book2_entry[0].entry == '30'
assert len(song3_book2_entry) == 1
# entry field should be overridden, as it was not set previously
assert song3_book2_entry[0].entry == '40'