forked from openlp/openlp
Fix merging of song books
This commit is contained in:
parent
54ad7496cd
commit
80a70d5b63
@ -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):
|
||||
|
@ -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
|
||||
|
@ -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'
|
||||
|
Loading…
Reference in New Issue
Block a user