diff --git a/CHANGELOG.rst b/CHANGELOG.rst new file mode 100644 index 000000000..13787ccea --- /dev/null +++ b/CHANGELOG.rst @@ -0,0 +1,12 @@ +OpenLP 2.5.1 +============ + +* 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 +* Sometimes the timer goes off as OpenLP is shutting down, and the application has already been deleted (reported via support system) +* Fix opening the data folder (KDE thought the old way was an SMB share) +* Fix a problem with the new QMediaPlayer not controlling the playlist anymore +* Added importing of author types to the OpenLP 2 song importer +* Refactored the merge script and gave it some options +* Fix a problem with loading Qt's translation files, bug #1676163 diff --git a/openlp/core/__init__.py b/openlp/core/__init__.py index e035c6dda..aa44d1d1b 100644 --- a/openlp/core/__init__.py +++ b/openlp/core/__init__.py @@ -319,7 +319,7 @@ class OpenLP(OpenLPMixin, QtWidgets.QApplication): return QtWidgets.QApplication.event(self, event) -def parse_options(args): +def parse_options(args=None): """ Parse the command line arguments @@ -428,13 +428,12 @@ def main(args=None): sys.exit() # i18n Set Language language = LanguageManager.get_language() - application_translator, default_translator = LanguageManager.get_translator(language) - if not application_translator.isEmpty(): - application.installTranslator(application_translator) - if not default_translator.isEmpty(): - application.installTranslator(default_translator) - else: - log.debug('Could not find default_translator.') + translators = LanguageManager.get_translators(language) + for translator in translators: + if not translator.isEmpty(): + application.installTranslator(translator) + if not translators: + log.debug('Could not find translators.') if args and not args.no_error_form: sys.excepthook = application.hook_exception sys.exit(application.run(qt_args)) diff --git a/openlp/core/common/languagemanager.py b/openlp/core/common/languagemanager.py index 0e1787d2e..5023f7383 100644 --- a/openlp/core/common/languagemanager.py +++ b/openlp/core/common/languagemanager.py @@ -45,7 +45,7 @@ class LanguageManager(object): auto_language = False @staticmethod - def get_translator(language): + def get_translators(language): """ Set up a translator to use in this instance of OpenLP @@ -59,9 +59,12 @@ class LanguageManager(object): # A translator for buttons and other default strings provided by Qt. if not is_win() and not is_macosx(): lang_path = QtCore.QLibraryInfo.location(QtCore.QLibraryInfo.TranslationsPath) + # As of Qt5, the core translations come in 2 files per language default_translator = QtCore.QTranslator() default_translator.load('qt_%s' % language, lang_path) - return app_translator, default_translator + base_translator = QtCore.QTranslator() + base_translator.load('qtbase_%s' % language, lang_path) + return app_translator, default_translator, base_translator @staticmethod def find_qm_files(): @@ -71,8 +74,8 @@ class LanguageManager(object): log.debug('Translation files: {files}'.format(files=AppLocation.get_directory(AppLocation.LanguageDir))) trans_dir = QtCore.QDir(AppLocation.get_directory(AppLocation.LanguageDir)) file_names = trans_dir.entryList(['*.qm'], QtCore.QDir.Files, QtCore.QDir.Name) - # Remove qm files from the list which start with "qt_". - file_names = [file_ for file_ in file_names if not file_.startswith('qt_')] + # Remove qm files from the list which start with "qt". + file_names = [file_ for file_ in file_names if not file_.startswith('qt')] return list(map(trans_dir.filePath, file_names)) @staticmethod diff --git a/openlp/core/ui/maindisplay.py b/openlp/core/ui/maindisplay.py index 6b4359b74..a40ade826 100644 --- a/openlp/core/ui/maindisplay.py +++ b/openlp/core/ui/maindisplay.py @@ -689,7 +689,7 @@ class AudioPlayer(OpenLPMixin, QtCore.QObject): """ Skip forward to the next track in the list """ - self.player.next() + self.playerlist.next() def go_to(self, index): """ diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 5a8029746..fb7add70f 100644 --- a/openlp/core/ui/mainwindow.py +++ b/openlp/core/ui/mainwindow.py @@ -766,7 +766,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Use the Online manual in other cases. (Linux) """ if is_macosx() or is_win(): - QtGui.QDesktopServices.openUrl(QtCore.QUrl("file:///" + self.local_help_file)) + QtGui.QDesktopServices.openUrl(QtCore.QUrl.fromLocalFile(self.local_help_file)) else: import webbrowser webbrowser.open_new('http://manual.openlp.org/') @@ -789,7 +789,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): Open data folder """ path = AppLocation.get_data_path() - QtGui.QDesktopServices.openUrl(QtCore.QUrl("file:///" + path)) + QtGui.QDesktopServices.openUrl(QtCore.QUrl.fromLocalFile(path)) def on_update_theme_images(self): """ @@ -1393,7 +1393,9 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, RegistryProperties): if event.timerId() == self.timer_id: self.timer_id = 0 self.load_progress_bar.hide() - self.application.process_events() + # Sometimes the timer goes off as OpenLP is shutting down, and the application has already been deleted + if self.application: + self.application.process_events() def set_new_data_path(self, new_data_path): """ diff --git a/openlp/core/ui/slidecontroller.py b/openlp/core/ui/slidecontroller.py index 163cea658..68df50b2f 100644 --- a/openlp/core/ui/slidecontroller.py +++ b/openlp/core/ui/slidecontroller.py @@ -921,7 +921,8 @@ class SlideController(DisplayController, RegistryProperties): Registry().execute('{name}_stop'.format(name=old_item.name.lower()), [old_item, self.is_live]) if old_item.is_media() and not self.service_item.is_media(): self.on_media_close() - Registry().execute('slidecontroller_{item}_started'.format(item=self.type_prefix), [self.service_item]) + if self.is_live: + Registry().execute('slidecontroller_{item}_started'.format(item=self.type_prefix), [self.service_item]) def on_slide_selected_index(self, message): """ diff --git a/openlp/plugins/songs/forms/songmaintenanceform.py b/openlp/plugins/songs/forms/songmaintenanceform.py index 5ee6f561a..a29c048e2 100644 --- a/openlp/plugins/songs/forms/songmaintenanceform.py +++ b/openlp/plugins/songs/forms/songmaintenanceform.py @@ -20,7 +20,6 @@ # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### import logging -import os from PyQt5 import QtCore, QtWidgets from sqlalchemy.sql import and_ @@ -184,7 +183,7 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP Author.display_name == new_author.display_name ) ) - return self.__check_object_exists(authors, new_author, edit) + return self._check_object_exists(authors, new_author, edit) def check_topic_exists(self, new_topic, edit=False): """ @@ -194,7 +193,7 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP :param edit: Are we editing the song? """ topics = self.manager.get_all_objects(Topic, Topic.name == new_topic.name) - return self.__check_object_exists(topics, new_topic, edit) + return self._check_object_exists(topics, new_topic, edit) def check_song_book_exists(self, new_book, edit=False): """ @@ -205,9 +204,9 @@ class SongMaintenanceForm(QtWidgets.QDialog, Ui_SongMaintenanceDialog, RegistryP """ books = self.manager.get_all_objects( Book, and_(Book.name == new_book.name, Book.publisher == new_book.publisher)) - return self.__check_object_exists(books, new_book, edit) + return self._check_object_exists(books, new_book, edit) - def __check_object_exists(self, existing_objects, new_object, edit): + def _check_object_exists(self, existing_objects, new_object, edit): """ Utility method to check for an existing object. diff --git a/openlp/plugins/songs/lib/importers/openlp.py b/openlp/plugins/songs/lib/importers/openlp.py index 1a3a62112..c0506bf6a 100644 --- a/openlp/plugins/songs/lib/importers/openlp.py +++ b/openlp/plugins/songs/lib/importers/openlp.py @@ -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_songs table + """ + pass + class OldAuthor(BaseModel): """ Maps to the authors table @@ -109,14 +115,19 @@ class OpenLPSongImport(SongImport): source_meta.reflect(engine) self.source_session = scoped_session(sessionmaker(bind=engine)) # Run some checks to see which version of the database we have - if 'media_files' in list(source_meta.tables.keys()): + table_list = list(source_meta.tables.keys()) + if 'media_files' in table_list: has_media_files = True else: has_media_files = False - if 'songs_songbooks' in list(source_meta.tables.keys()): + if 'songs_songbooks' in table_list: has_songs_books = True else: has_songs_books = False + if 'authors_songs' in table_list: + 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 +150,15 @@ class OpenLPSongImport(SongImport): class_mapper(OldSongBookEntry) except UnmappedClassError: mapper(OldSongBookEntry, source_songs_songbooks_table, properties={'songbook': relation(OldBook)}) + if has_authors_songs: + try: + class_mapper(OldAuthorSong) + except UnmappedClassError: + mapper(OldAuthorSong, source_authors_songs_table) + if has_authors_songs and 'author_type' in source_authors_songs_table.c.keys(): + 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 +177,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) @@ -205,8 +227,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: @@ -221,11 +251,17 @@ class OpenLPSongImport(SongImport): if not existing_book: existing_book = Book.populate(name=entry.songbook.name, publisher=entry.songbook.publisher) new_song.add_songbook_entry(existing_book, entry.entry) - elif song.book: + elif hasattr(song, 'book') and song.book: existing_book = self.manager.get_object_filtered(Book, Book.name == song.book.name) if not existing_book: existing_book = Book.populate(name=song.book.name, publisher=song.book.publisher) - new_song.add_songbook_entry(existing_book, '') + # Get the song_number from "songs" table "song_number" field. (This is db structure from 2.2.1) + # If there's a number, add it to the song, otherwise it will be "". + existing_number = song.song_number if hasattr(song, 'song_number') else '' + if existing_number: + new_song.add_songbook_entry(existing_book, existing_number) + else: + new_song.add_songbook_entry(existing_book, '') # Find or create all the media files and add them to the new song object if has_media_files and song.media_files: for media_file in song.media_files: diff --git a/openlp/plugins/songs/lib/importers/presentationmanager.py b/openlp/plugins/songs/lib/importers/presentationmanager.py index 8d43e4633..7113842c4 100644 --- a/openlp/plugins/songs/lib/importers/presentationmanager.py +++ b/openlp/plugins/songs/lib/importers/presentationmanager.py @@ -23,7 +23,6 @@ The :mod:`presentationmanager` module provides the functionality for importing Presentationmanager song files into the current database. """ - import os import re @@ -65,15 +64,34 @@ class PresentationManagerImport(SongImport): 'File is not in XML-format, which is the only format supported.')) continue root = objectify.fromstring(etree.tostring(tree)) - self.process_song(root) + self.process_song(root, file_path) - def process_song(self, root): + def _get_attr(self, elem, name): + """ + Due to PresentationManager's habit of sometimes capitilising the first letter of an element, we have to do + some gymnastics. + """ + if hasattr(elem, name): + return str(getattr(elem, name)) + name = name[0].upper() + name[1:] + if hasattr(elem, name): + return str(getattr(elem, name)) + else: + return '' + + def process_song(self, root, file_path): self.set_defaults() - self.title = str(root.attributes.title) - self.add_author(str(root.attributes.author)) - self.copyright = str(root.attributes.copyright) - self.ccli_number = str(root.attributes.ccli_number) - self.comments = str(root.attributes.comments) + attrs = None + if hasattr(root, 'attributes'): + attrs = root.attributes + elif hasattr(root, 'Attributes'): + attrs = root.Attributes + if attrs is not None: + self.title = self._get_attr(root.attributes, 'title') + self.add_author(self._get_attr(root.attributes, 'author')) + self.copyright = self._get_attr(root.attributes, 'copyright') + self.ccli_number = self._get_attr(root.attributes, 'ccli_number') + self.comments = str(root.attributes.comments) if hasattr(root.attributes, 'comments') else None verse_order_list = [] verse_count = {} duplicates = [] @@ -105,4 +123,4 @@ class PresentationManagerImport(SongImport): self.verse_order_list = verse_order_list if not self.finish(): - self.log_error(self.import_source) + self.log_error(os.path.basename(file_path)) diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index 6fc8d35ef..85ada339f 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -47,7 +47,7 @@ USER_AGENTS = [ BASE_URL = 'https://songselect.ccli.com' LOGIN_PAGE = 'https://profile.ccli.com/account/signin?appContext=SongSelect&returnUrl='\ 'https%3a%2f%2fsongselect.ccli.com%2f' -LOGIN_URL = 'https://profile.ccli.com/' +LOGIN_URL = 'https://profile.ccli.com' LOGOUT_URL = BASE_URL + '/account/logout' SEARCH_URL = BASE_URL + '/search/results' @@ -97,14 +97,27 @@ class SongSelectImport(object): 'password': password, 'RememberMe': 'false' }) + login_form = login_page.find('form') + if login_form: + login_url = login_form.attrs['action'] + else: + login_url = '/Account/SignIn' + if not login_url.startswith('http'): + if login_url[0] != '/': + login_url = '/' + login_url + login_url = LOGIN_URL + login_url try: - posted_page = BeautifulSoup(self.opener.open(LOGIN_URL, data.encode('utf-8')).read(), 'lxml') + posted_page = BeautifulSoup(self.opener.open(login_url, data.encode('utf-8')).read(), 'lxml') except (TypeError, URLError) as error: log.exception('Could not login to SongSelect, {error}'.format(error=error)) return False if callback: callback() - return posted_page.find('input', id='SearchText') is not None + if posted_page.find('input', id='SearchText') is not None: + return True + else: + log.debug(posted_page) + return False def logout(self): """ diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py index c8fbd8d02..1871ca718 100644 --- a/openlp/plugins/songs/lib/upgrade.py +++ b/openlp/plugins/songs/lib/upgrade.py @@ -32,7 +32,7 @@ from openlp.core.common.db import drop_columns from openlp.core.lib.db import get_upgrade_op log = logging.getLogger(__name__) -__version__ = 5 +__version__ = 6 # TODO: When removing an upgrade path the ftw-data needs updating to the minimum supported version @@ -52,7 +52,6 @@ def upgrade_1(session, metadata): :param metadata: """ op = get_upgrade_op(session) - songs_table = Table('songs', metadata, autoload=True) if 'media_files_songs' in [t.name for t in metadata.tables.values()]: op.drop_table('media_files_songs') op.add_column('media_files', Column('song_id', types.Integer(), server_default=null())) @@ -102,21 +101,8 @@ def upgrade_4(session, metadata): This upgrade adds a column for author type to the authors_songs table """ - # 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('""'))) - 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') - else: - log.warning('Skipping upgrade_4 step of upgrading the song db') + # This is now empty due to a bug in the upgrade + pass def upgrade_5(session, metadata): @@ -125,26 +111,52 @@ def upgrade_5(session, metadata): This upgrade adds support for multiple songbooks """ + # This is now empty due to a bug in the upgrade + pass + + +def upgrade_6(session, metadata): + """ + Version 6 upgrade + + This version corrects the errors in upgrades 4 and 5 + """ op = get_upgrade_op(session) - songs_table = Table('songs', metadata) - if 'song_book_id' in [col.name for col in songs_table.c.values()]: - log.warning('Skipping upgrade_5 step of upgrading the song db') - return + # Move upgrade 4 to here and correct it (authors_songs table, not songs table) + authors_songs = Table('authors_songs', metadata, autoload=True) + if 'author_type' not in [col.name for col in authors_songs.c.values()]: + # Since SQLite doesn't support changing the primary key of a table, we need to recreate the table + # and copy the old 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('""')) + ) + 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') + # Move upgrade 5 here to correct it + if 'songs_songbooks' not in [t.name for t in metadata.tables.values()]: + # 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) + ) - # 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)) + # 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 AND song_book_id <> 0') - # 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') - - # Drop old columns - if metadata.bind.url.get_dialect().name == 'sqlite': - drop_columns(op, 'songs', ['song_book_id', 'song_number']) - else: - op.drop_constraint('songs_ibfk_1', 'songs', 'foreignkey') - op.drop_column('songs', 'song_book_id') - op.drop_column('songs', 'song_number') + # Drop old columns + if metadata.bind.url.get_dialect().name == 'sqlite': + drop_columns(op, 'songs', ['song_book_id', 'song_number']) + else: + op.drop_constraint('songs_ibfk_1', 'songs', 'foreignkey') + op.drop_column('songs', 'song_book_id') + op.drop_column('songs', 'song_number') + # Finally, clean up our mess in people's databases + op.execute('DELETE FROM songs_songbooks WHERE songbook_id = 0') diff --git a/scripts/lp-merge.py b/scripts/lp-merge.py index 1743c4a14..c59e03705 100755 --- a/scripts/lp-merge.py +++ b/scripts/lp-merge.py @@ -51,101 +51,173 @@ will print the detected bugs + author for easy copying into the GUI. """ import subprocess import re -import sys import os -import urllib.request +from urllib.request import urlopen +from urllib.error import HTTPError +from argparse import ArgumentParser from bs4 import BeautifulSoup -# Check that the argument count is correct -if len(sys.argv) != 2: - print('\n\tUsage: ' + sys.argv[0] + ' \n') - exit() -url = sys.argv[1] -pattern = re.compile('.+?/\+merge/\d+') -match = pattern.match(url) +def parse_args(): + """ + Parse command line arguments and return them + """ + parser = ArgumentParser(prog='lp-merge', description='A helper script to merge proposals on Launchpad') + parser.add_argument('url', help='URL to the merge proposal') + parser.add_argument('-y', '--yes-to-all', action='store_true', help='Presume yes for all queries') + parser.add_argument('-q', '--use-qcommit', action='store_true', help='Use qcommit for committing') + return parser.parse_args() -# Check that the given argument is an url in the right format -if not url.startswith('https://code.launchpad.net/~') or match is None: - print('The url is not valid! It should look like this:\n ' - 'https://code.launchpad.net/~tomasgroth/openlp/doc22update4/+merge/271874') -page = urllib.request.urlopen(url) -soup = BeautifulSoup(page.read(), 'lxml') +def is_merge_url_valid(url): + """ + Determine if the merge URL is valid + """ + match = re.match(r'.+?/\+merge/\d+', url) + if not url.startswith('https://code.launchpad.net/~') or match is None: + print('The url is not valid! It should look like this:\n ' + 'https://code.launchpad.net/~myusername/openlp/mybranch/+merge/271874') + return False + return True -# Find this span tag that contains the branch url -# -span_branch_url = soup.find('span', class_='branch-url') -branch_url = span_branch_url.contents[0] -# Find this tag that describes the branch. We'll use that for commit message -# -meta = soup.find('meta', attrs={"name": "description"}) +def get_merge_info(url): + """ + Get all the merge information and return it as a dictionary + """ + merge_info = {} + print('Fetching merge information...') + # Try to load the page + try: + page = urlopen(url) + except HTTPError: + print('Unable to load merge URL: {}'.format(url)) + return None + soup = BeautifulSoup(page.read(), 'lxml') + # Find this span tag that contains the branch url + # + span_branch_url = soup.find('span', class_='branch-url') + if not span_branch_url: + print('Unable to find merge details on URL: {}'.format(url)) + return None + merge_info['branch_url'] = span_branch_url.contents[0] + # Find this tag that describes the branch. We'll use that for commit message + # + meta = soup.find('meta', attrs={"name": "description"}) + merge_info['commit_message'] = meta.get('content') + # Find all tr-tags with this class. Makes it possible to get bug numbers. + #

Merge ... into... + div_branches = soup.find('div', class_='context-publication') + branches = div_branches.h1.contents[0] + merge_info['target_branch'] = '+branch/' + branches[(branches.find(' into lp:') + 9):] + # Find the authors email address. It is hidden in a javascript line like this: + # conf = {"status_value": "Needs review", "source_revid": "tomasgroth@yahoo.dk-20160921204550-gxduegmcmty9rljf", + # "user_can_edit_status": false, ... + script_tag = soup.find('script', attrs={"id": "codereview-script"}) + content = script_tag.contents[0] + start_pos = content.find('source_revid') + 16 + pattern = re.compile('.*\w-\d\d\d\d\d+') + match = pattern.match(content[start_pos:]) + merge_info['author_email'] = match.group()[:-15] + # Launchpad doesn't supply the author's true name, so we'll just grab whatever they use for display on LP + a_person = soup.find('div', id='registration').find('a', 'person') + merge_info['author_name'] = a_person.contents[0] + return merge_info -commit_message = meta.get('content') -# Find all tr-tags with this class. Makes it possible to get bug numbers. -#

Merge ... into... -div_branches = soup.find('div', class_='context-publication') -branches = div_branches.h1.contents[0] -target_branch = '+branch/' + branches[(branches.find(' into lp:') + 9):] -# Check that we are in the right branch -bzr_info_output = subprocess.check_output(['bzr', 'info']) -if target_branch not in bzr_info_output.decode(): - print('ERROR: It seems you are not in the right folder...') - exit() - -# Find the authors email address. It is hidden in a javascript line like this: -# conf = {"status_value": "Needs review", "source_revid": "tomasgroth@yahoo.dk-20160921204550-gxduegmcmty9rljf", -# "user_can_edit_status": false, ... -script_tag = soup.find('script', attrs={"id": "codereview-script"}) -content = script_tag.contents[0] -start_pos = content.find('source_revid') + 16 -pattern = re.compile('.*\w-\d\d\d\d\d+') -match = pattern.match(content[start_pos:]) -author_email = match.group()[:-15] - -# Merge the branch -do_merge = input('Merge ' + branch_url + ' into local branch? (y/N/q): ').lower() -if do_merge == 'y': - subprocess.call(['bzr', 'merge', branch_url]) -elif do_merge == 'q': - exit() - -# Create commit command -commit_command = ['bzr', 'commit'] - -for bug in bugs: - commit_command.append('--fixes') - commit_command.append('lp:' + bug) - -commit_command.append('-m') -commit_command.append(commit_message) - -commit_command.append('--author') -commit_command.append('"' + author_email + '"') - -print('About to run the bzr command below:\n') -print(' '.join(commit_command)) -do_commit = input('Run the command (y), use qcommit (qcommit) or cancel (C): ').lower() - -if do_commit == 'y': - subprocess.call(commit_command) -elif do_commit == 'qcommit': +def qcommit(commit_message, author_name, author_email, bugs=[]): + """ + Use qcommit to make the commit + """ # Setup QT workaround to make qbzr look right on my box my_env = os.environ.copy() my_env['QT_GRAPHICSSYSTEM'] = 'native' # Print stuff that kan be copy/pasted into qbzr GUI - print('These bugs can be copy/pasted in: lp:' + ' lp:'.join(bugs)) - print('The authors email is: ' + author_email) + if bugs: + print('These bugs can be copy/pasted in: ' + ' '.join(['lp:{}'.format(bug) for bug in bugs])) + print('The authors email is: {} <{}>'.format(author_name, author_email)) # Run qcommit subprocess.call(['bzr', 'qcommit', '-m', commit_message], env=my_env) + + +def do_commit(merge_info, yes_to_all=False, use_qcommit=False): + """ + Actually do the commit + """ + if use_qcommit: + qcommit(merge_info['commit_message'], merge_info['author_name'], merge_info['author_email'], + merge_info.get('bugs')) + return True + # Create commit command + commit_command = ['bzr', 'commit'] + if 'bugs' in merge_info: + commit_command.extend(['--fixes=lp:{}'.format(bug) for bug in merge_info['bugs']]) + commit_command.extend(['-m', merge_info['commit_message'], + '--author', '{author_name} <{author_email}>'.format(**merge_info)]) + if yes_to_all: + can_commit = True + else: + print('About to run the bzr command below:\n') + print(' '.join(commit_command)) + user_choice = input('Run the command (y), use qcommit (q) or cancel (C): ').lower() + if user_choice == 'q': + qcommit(merge_info['commit_message'], merge_info['author_name'], merge_info['author_email'], + merge_info.get('bugs')) + return True + can_commit = user_choice == 'y' + if can_commit: + print('Committing...') + subprocess.call(commit_command) + return True + else: + return False + + +def main(): + """ + Run the script + """ + args = parse_args() + if not is_merge_url_valid(args.url): + exit(1) + merge_info = get_merge_info(args.url) + if not merge_info: + exit(2) + if not do_merge(merge_info, args.yes_to_all): + exit(3) + if not do_commit(merge_info, args.yes_to_all, args.use_qcommit): + exit(4) + + +if __name__ == '__main__': + main() diff --git a/tests/functional/openlp_core/test_init.py b/tests/functional/openlp_core/test_init.py index 2d833772f..e18d0f376 100644 --- a/tests/functional/openlp_core/test_init.py +++ b/tests/functional/openlp_core/test_init.py @@ -19,15 +19,16 @@ # with this program; if not, write to the Free Software Foundation, Inc., 59 # # Temple Place, Suite 330, Boston, MA 02111-1307 USA # ############################################################################### - import sys -from unittest import TestCase +from unittest import TestCase, skip +from unittest.mock import MagicMock, patch -from openlp.core import parse_options -from tests.helpers.testmixin import TestMixin +from PyQt5 import QtWidgets + +from openlp.core import OpenLP, parse_options -class TestInitFunctions(TestMixin, TestCase): +class TestInitFunctions(TestCase): def test_parse_options_basic(self): """ @@ -116,8 +117,7 @@ class TestInitFunctions(TestMixin, TestCase): def test_parse_options_file_and_debug(self): """ - Test the parse options process works with a file - + Test the parse options process works with a file and the debug log level """ # GIVEN: a a set of system arguments. sys.argv[1:] = ['-l debug', 'dummy_temp'] @@ -130,3 +130,98 @@ class TestInitFunctions(TestMixin, TestCase): self.assertFalse(args.portable, 'The portable flag should be set to false') self.assertEquals(args.style, None, 'There are no style flags to be processed') self.assertEquals(args.rargs, 'dummy_temp', 'The service file should not be blank') + + +@skip('Figure out why this is causing a segfault') +class TestOpenLP(TestCase): + """ + Test the OpenLP app class + """ + @patch('openlp.core.QtWidgets.QApplication.exec') + def test_exec(self, mocked_exec): + """ + Test the exec method + """ + # GIVEN: An app + app = OpenLP([]) + app.shared_memory = MagicMock() + mocked_exec.return_value = False + + # WHEN: exec() is called + result = app.exec() + + # THEN: The right things should be called + assert app.is_event_loop_active is True + mocked_exec.assert_called_once_with() + app.shared_memory.detach.assert_called_once_with() + assert result is False + + @patch('openlp.core.QtCore.QSharedMemory') + def test_is_already_running_not_running(self, MockedSharedMemory): + """ + Test the is_already_running() method when OpenLP is NOT running + """ + # GIVEN: An OpenLP app and some mocks + mocked_shared_memory = MagicMock() + mocked_shared_memory.attach.return_value = False + MockedSharedMemory.return_value = mocked_shared_memory + app = OpenLP([]) + + # WHEN: is_already_running() is called + result = app.is_already_running() + + # THEN: The result should be false + MockedSharedMemory.assert_called_once_with('OpenLP') + mocked_shared_memory.attach.assert_called_once_with() + mocked_shared_memory.create.assert_called_once_with(1) + assert result is False + + @patch('openlp.core.QtWidgets.QMessageBox.critical') + @patch('openlp.core.QtWidgets.QMessageBox.StandardButtons') + @patch('openlp.core.QtCore.QSharedMemory') + def test_is_already_running_is_running_continue(self, MockedSharedMemory, MockedStandardButtons, mocked_critical): + """ + Test the is_already_running() method when OpenLP IS running and the user chooses to continue + """ + # GIVEN: An OpenLP app and some mocks + mocked_shared_memory = MagicMock() + mocked_shared_memory.attach.return_value = True + MockedSharedMemory.return_value = mocked_shared_memory + MockedStandardButtons.return_value = 0 + mocked_critical.return_value = QtWidgets.QMessageBox.Yes + app = OpenLP([]) + + # WHEN: is_already_running() is called + result = app.is_already_running() + + # THEN: The result should be false + MockedSharedMemory.assert_called_once_with('OpenLP') + mocked_shared_memory.attach.assert_called_once_with() + MockedStandardButtons.assert_called_once_with(QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No) + mocked_critical.assert_called_once_with(None, 'Error', 'OpenLP is already running. Do you wish to continue?', 0) + assert result is False + + @patch('openlp.core.QtWidgets.QMessageBox.critical') + @patch('openlp.core.QtWidgets.QMessageBox.StandardButtons') + @patch('openlp.core.QtCore.QSharedMemory') + def test_is_already_running_is_running_stop(self, MockedSharedMemory, MockedStandardButtons, mocked_critical): + """ + Test the is_already_running() method when OpenLP IS running and the user chooses to stop + """ + # GIVEN: An OpenLP app and some mocks + mocked_shared_memory = MagicMock() + mocked_shared_memory.attach.return_value = True + MockedSharedMemory.return_value = mocked_shared_memory + MockedStandardButtons.return_value = 0 + mocked_critical.return_value = QtWidgets.QMessageBox.No + app = OpenLP([]) + + # WHEN: is_already_running() is called + result = app.is_already_running() + + # THEN: The result should be false + MockedSharedMemory.assert_called_once_with('OpenLP') + mocked_shared_memory.attach.assert_called_once_with() + MockedStandardButtons.assert_called_once_with(QtWidgets.QMessageBox.Yes | QtWidgets.QMessageBox.No) + mocked_critical.assert_called_once_with(None, 'Error', 'OpenLP is already running. Do you wish to continue?', 0) + assert result is True diff --git a/tests/functional/openlp_core_ui/test_slidecontroller.py b/tests/functional/openlp_core_ui/test_slidecontroller.py index 7cc6559e3..7b0fdfd55 100644 --- a/tests/functional/openlp_core_ui/test_slidecontroller.py +++ b/tests/functional/openlp_core_ui/test_slidecontroller.py @@ -658,7 +658,7 @@ class TestSlideController(TestCase): slide_controller._process_item(mocked_media_item, 0) # THEN: Registry.execute should have been called to stop the presentation - self.assertEqual(3, mocked_execute.call_count, 'Execute should have been called 3 times') + self.assertEqual(2, mocked_execute.call_count, 'Execute should have been called 2 times') self.assertEqual('mocked_presentation_item_stop', mocked_execute.call_args_list[1][0][0], 'The presentation should have been stopped.') diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 670f100de..c06e9b7e9 100644 --- a/tests/functional/openlp_plugins/remotes/test_router.py +++ b/tests/functional/openlp_plugins/remotes/test_router.py @@ -25,11 +25,11 @@ This module contains tests for the lib submodule of the Remotes plugin. import os import urllib.request from unittest import TestCase +from unittest.mock import MagicMock, patch, mock_open from openlp.core.common import Settings, Registry from openlp.core.ui import ServiceManager from openlp.plugins.remotes.lib.httpserver import HttpRouter -from tests.functional import MagicMock, patch, mock_open from tests.helpers.testmixin import TestMixin __default_settings__ = { @@ -336,11 +336,9 @@ class TestRouter(TestCase, TestMixin): with patch.object(self.service_manager, 'setup_ui'), \ patch.object(self.router, 'do_json_header'): self.service_manager.bootstrap_initialise() - self.app.processEvents() # WHEN: Remote next is received self.router.service(action='next') - self.app.processEvents() # THEN: service_manager.next_item() should have been called self.assertTrue(mocked_next_item.called, 'next_item() should have been called in service_manager') @@ -357,11 +355,9 @@ class TestRouter(TestCase, TestMixin): with patch.object(self.service_manager, 'setup_ui'), \ patch.object(self.router, 'do_json_header'): self.service_manager.bootstrap_initialise() - self.app.processEvents() # WHEN: Remote next is received self.router.service(action='previous') - self.app.processEvents() # THEN: service_manager.next_item() should have been called self.assertTrue(mocked_previous_item.called, 'previous_item() should have been called in service_manager') diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 28ec24fe8..b367b7c41 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -25,6 +25,7 @@ This module contains tests for the CCLI SongSelect importer. """ import os from unittest import TestCase +from unittest.mock import MagicMock, patch, call from urllib.error import URLError from PyQt5 import QtWidgets @@ -32,9 +33,8 @@ from PyQt5 import QtWidgets from openlp.core import Registry from openlp.plugins.songs.forms.songselectform import SongSelectForm, SearchWorker from openlp.plugins.songs.lib import Song -from openlp.plugins.songs.lib.songselect import SongSelectImport, LOGOUT_URL, BASE_URL +from openlp.plugins.songs.lib.songselect import SongSelectImport, LOGIN_PAGE, LOGOUT_URL, BASE_URL -from tests.functional import MagicMock, patch, call from tests.helpers.songfileimport import SongImportTestHelper from tests.helpers.testmixin import TestMixin @@ -72,7 +72,9 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_build_opener.return_value = mocked_opener mocked_login_page = MagicMock() mocked_login_page.find.side_effect = [{'value': 'blah'}, None] - MockedBeautifulSoup.return_value = mocked_login_page + mocked_posted_page = MagicMock() + mocked_posted_page.find.return_value = None + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -82,6 +84,7 @@ class TestSongSelectImport(TestCase, TestMixin): # THEN: callback was called 3 times, open was called twice, find was called twice, and False was returned self.assertEqual(3, mock_callback.call_count, 'callback should have been called 3 times') self.assertEqual(2, mocked_login_page.find.call_count, 'find should have been called twice') + self.assertEqual(1, mocked_posted_page.find.call_count, 'find should have been called once') self.assertEqual(2, mocked_opener.open.call_count, 'opener should have been called twice') self.assertFalse(result, 'The login method should have returned False') @@ -112,8 +115,10 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_opener = MagicMock() mocked_build_opener.return_value = mocked_opener mocked_login_page = MagicMock() - mocked_login_page.find.side_effect = [{'value': 'blah'}, MagicMock()] - MockedBeautifulSoup.return_value = mocked_login_page + mocked_login_page.find.side_effect = [{'value': 'blah'}, None] + mocked_posted_page = MagicMock() + mocked_posted_page.find.return_value = MagicMock() + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -122,10 +127,40 @@ class TestSongSelectImport(TestCase, TestMixin): # THEN: callback was called 3 times, open was called twice, find was called twice, and True was returned self.assertEqual(3, mock_callback.call_count, 'callback should have been called 3 times') - self.assertEqual(2, mocked_login_page.find.call_count, 'find should have been called twice') + self.assertEqual(2, mocked_login_page.find.call_count, 'find should have been called twice on the login page') + self.assertEqual(1, mocked_posted_page.find.call_count, 'find should have been called once on the posted page') self.assertEqual(2, mocked_opener.open.call_count, 'opener should have been called twice') self.assertTrue(result, 'The login method should have returned True') + @patch('openlp.plugins.songs.lib.songselect.build_opener') + @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') + def login_url_from_form_test(self, MockedBeautifulSoup, mocked_build_opener): + """ + Test that the login URL is from the form + """ + # GIVEN: A bunch of mocked out stuff and an importer object + mocked_opener = MagicMock() + mocked_build_opener.return_value = mocked_opener + mocked_form = MagicMock() + mocked_form.attrs = {'action': 'do/login'} + mocked_login_page = MagicMock() + mocked_login_page.find.side_effect = [{'value': 'blah'}, mocked_form] + mocked_posted_page = MagicMock() + mocked_posted_page.find.return_value = MagicMock() + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page] + mock_callback = MagicMock() + importer = SongSelectImport(None) + + # WHEN: The login method is called after being rigged to fail + result = importer.login('username', 'password', mock_callback) + + # THEN: callback was called 3 times, open was called twice, find was called twice, and True was returned + self.assertEqual(3, mock_callback.call_count, 'callback should have been called 3 times') + self.assertEqual(2, mocked_login_page.find.call_count, 'find should have been called twice on the login page') + self.assertEqual(1, mocked_posted_page.find.call_count, 'find should have been called once on the posted page') + self.assertEqual('https://profile.ccli.com/do/login', mocked_opener.open.call_args_list[1][0][0]) + self.assertTrue(result, 'The login method should have returned True') + @patch('openlp.plugins.songs.lib.songselect.build_opener') def test_logout(self, mocked_build_opener): """ diff --git a/tests/interfaces/openlp_plugins/bibles/forms/__init__.py b/tests/interfaces/openlp_plugins/bibles/forms/__init__.py index ea62548f4..e69de29bb 100644 --- a/tests/interfaces/openlp_plugins/bibles/forms/__init__.py +++ b/tests/interfaces/openlp_plugins/bibles/forms/__init__.py @@ -1,21 +0,0 @@ -# -*- coding: utf-8 -*- -# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 - -############################################################################### -# OpenLP - Open Source Lyrics Projection # -# --------------------------------------------------------------------------- # -# Copyright (c) 2008-2017 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 # -############################################################################### diff --git a/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py b/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py index fe1b18291..a7bf3c562 100644 --- a/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py +++ b/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py @@ -22,17 +22,18 @@ """ Package to test the openlp.plugins.bibles.forms.bibleimportform package. """ -from unittest import TestCase +from unittest import TestCase, skip +from unittest.mock import MagicMock, patch from PyQt5 import QtWidgets from openlp.core.common import Registry -import openlp.plugins.bibles.forms.bibleimportform as bibleimportform +from openlp.plugins.bibles.forms.bibleimportform import BibleImportForm, PYSWORD_AVAILABLE from tests.helpers.testmixin import TestMixin -from tests.functional import MagicMock, patch +@skip('One of the QFormLayouts in the BibleImportForm is causing a segfault') class TestBibleImportForm(TestCase, TestMixin): """ Test the BibleImportForm class @@ -46,8 +47,9 @@ class TestBibleImportForm(TestCase, TestMixin): self.setup_application() self.main_window = QtWidgets.QMainWindow() Registry().register('main_window', self.main_window) - bibleimportform.PYSWORD_AVAILABLE = False - self.form = bibleimportform.BibleImportForm(None, MagicMock(), MagicMock()) + PYSWORD_AVAILABLE = False + self.mocked_manager = MagicMock() + self.form = BibleImportForm(self.main_window, self.mocked_manager, MagicMock()) def tearDown(self): """ diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py b/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py new file mode 100644 index 000000000..09c12c136 --- /dev/null +++ b/tests/interfaces/openlp_plugins/songs/forms/test_songmaintenanceform.py @@ -0,0 +1,421 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 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 the openlp.plugins.songs.forms.songmaintenanceform package. +""" +from unittest import TestCase +from unittest.mock import MagicMock, patch, call + +from PyQt5 import QtCore, QtWidgets + +from openlp.core.common import Registry, UiStrings +from openlp.plugins.songs.forms.songmaintenanceform import SongMaintenanceForm + +from tests.helpers.testmixin import TestMixin + + +class TestSongMaintenanceForm(TestCase, TestMixin): + """ + Test the SongMaintenanceForm class + """ + + def setUp(self): + """ + Create the UI + """ + Registry.create() + self.setup_application() + self.main_window = QtWidgets.QMainWindow() + Registry().register('main_window', self.main_window) + self.mocked_manager = MagicMock() + self.form = SongMaintenanceForm(self.mocked_manager) + + def tearDown(self): + """ + Delete all the C++ objects at the end so that we don't have a segfault + """ + del self.form + del self.main_window + + def test_constructor(self): + """ + Test that a SongMaintenanceForm is created successfully + """ + # GIVEN: A SongMaintenanceForm + # WHEN: The form is created + # THEN: It should have some of the right components + assert self.form is not None + assert self.form.manager is self.mocked_manager + + @patch.object(QtWidgets.QDialog, 'exec') + def test_exect(self, mocked_exec): + """ + Test the song maintenance form being executed + """ + # GIVEN: A song maintenance form + mocked_exec.return_value = True + + # WHEN: The song mainetnance form is executed + with patch.object(self.form, 'type_list_widget') as mocked_type_list_widget, \ + patch.object(self.form, 'reset_authors') as mocked_reset_authors, \ + patch.object(self.form, 'reset_topics') as mocked_reset_topics, \ + patch.object(self.form, 'reset_song_books') as mocked_reset_song_books: + result = self.form.exec(from_song_edit=True) + + # THEN: The correct methods should have been called + assert self.form.from_song_edit is True + mocked_type_list_widget.setCurrentRow.assert_called_once_with(0) + mocked_reset_authors.assert_called_once_with() + mocked_reset_topics.assert_called_once_with() + mocked_reset_song_books.assert_called_once_with() + mocked_type_list_widget.setFocus.assert_called_once_with() + mocked_exec.assert_called_once_with(self.form) + assert result is True + + def test_get_current_item_id_no_item(self): + """ + Test _get_current_item_id() when there's no item + """ + # GIVEN: A song maintenance form without a selected item + mocked_list_widget = MagicMock() + mocked_list_widget.currentItem.return_value = None + + # WHEN: _get_current_item_id() is called + result = self.form._get_current_item_id(mocked_list_widget) + + # THEN: The result should be -1 + mocked_list_widget.currentItem.assert_called_once_with() + assert result == -1 + + def test_get_current_item_id(self): + """ + Test _get_current_item_id() when there's a valid item + """ + # GIVEN: A song maintenance form with a selected item + mocked_item = MagicMock() + mocked_item.data.return_value = 7 + mocked_list_widget = MagicMock() + mocked_list_widget.currentItem.return_value = mocked_item + + # WHEN: _get_current_item_id() is called + result = self.form._get_current_item_id(mocked_list_widget) + + # THEN: The result should be -1 + mocked_list_widget.currentItem.assert_called_once_with() + mocked_item.data.assert_called_once_with(QtCore.Qt.UserRole) + assert result == 7 + + @patch('openlp.plugins.songs.forms.songmaintenanceform.critical_error_message_box') + def test_delete_item_no_item_id(self, mocked_critical_error_message_box): + """ + Test the _delete_item() method when there is no item selected + """ + # GIVEN: Some mocked items + mocked_item_class = MagicMock() + mocked_list_widget = MagicMock() + mocked_reset_func = MagicMock() + dialog_title = 'Delete Item' + delete_text = 'Are you sure you want to delete this item?' + error_text = 'There was a problem deleting this item' + + # WHEN: _delete_item() is called + with patch.object(self.form, '_get_current_item_id') as mocked_get_current_item_id: + mocked_get_current_item_id.return_value = -1 + self.form._delete_item(mocked_item_class, mocked_list_widget, mocked_reset_func, dialog_title, delete_text, + error_text) + + # THEN: The right things should have been called + mocked_get_current_item_id.assert_called_once_with(mocked_list_widget) + mocked_critical_error_message_box.assert_called_once_with(dialog_title, UiStrings().NISs) + + @patch('openlp.plugins.songs.forms.songmaintenanceform.critical_error_message_box') + def test_delete_item_invalid_item(self, mocked_critical_error_message_box): + """ + Test the _delete_item() method when the item doesn't exist in the database + """ + # GIVEN: Some mocked items + self.mocked_manager.get_object.return_value = None + mocked_item_class = MagicMock() + mocked_list_widget = MagicMock() + mocked_reset_func = MagicMock() + dialog_title = 'Delete Item' + delete_text = 'Are you sure you want to delete this item?' + error_text = 'There was a problem deleting this item' + + # WHEN: _delete_item() is called + with patch.object(self.form, '_get_current_item_id') as mocked_get_current_item_id: + mocked_get_current_item_id.return_value = 1 + self.form._delete_item(mocked_item_class, mocked_list_widget, mocked_reset_func, dialog_title, delete_text, + error_text) + + # THEN: The right things should have been called + mocked_get_current_item_id.assert_called_once_with(mocked_list_widget) + self.mocked_manager.get_object.assert_called_once_with(mocked_item_class, 1) + mocked_critical_error_message_box.assert_called_once_with(dialog_title, error_text) + + @patch('openlp.plugins.songs.forms.songmaintenanceform.critical_error_message_box') + def test_delete_item(self, mocked_critical_error_message_box): + """ + Test the _delete_item() method + """ + # GIVEN: Some mocked items + mocked_item = MagicMock() + mocked_item.songs = [] + mocked_item.id = 1 + self.mocked_manager.get_object.return_value = mocked_item + mocked_critical_error_message_box.return_value = QtWidgets.QMessageBox.Yes + mocked_item_class = MagicMock() + mocked_list_widget = MagicMock() + mocked_reset_func = MagicMock() + dialog_title = 'Delete Item' + delete_text = 'Are you sure you want to delete this item?' + error_text = 'There was a problem deleting this item' + + # WHEN: _delete_item() is called + with patch.object(self.form, '_get_current_item_id') as mocked_get_current_item_id: + mocked_get_current_item_id.return_value = 1 + self.form._delete_item(mocked_item_class, mocked_list_widget, mocked_reset_func, dialog_title, delete_text, + error_text) + + # THEN: The right things should have been called + mocked_get_current_item_id.assert_called_once_with(mocked_list_widget) + self.mocked_manager.get_object.assert_called_once_with(mocked_item_class, 1) + mocked_critical_error_message_box.assert_called_once_with(dialog_title, delete_text, self.form, True) + self.mocked_manager.delete_object(mocked_item_class, 1) + mocked_reset_func.assert_called_once_with() + + @patch('openlp.plugins.songs.forms.songmaintenanceform.QtWidgets.QListWidgetItem') + @patch('openlp.plugins.songs.forms.songmaintenanceform.Author') + def test_reset_authors(self, MockedAuthor, MockedQListWidgetItem): + """ + Test the reset_authors() method + """ + # GIVEN: A mocked authors_list_widget and a few other mocks + mocked_author1 = MagicMock() + mocked_author1.display_name = 'John Newton' + mocked_author1.id = 1 + mocked_author2 = MagicMock() + mocked_author2.display_name = '' + mocked_author2.first_name = 'John' + mocked_author2.last_name = 'Wesley' + mocked_author2.id = 2 + mocked_authors = [mocked_author1, mocked_author2] + mocked_author_item1 = MagicMock() + mocked_author_item2 = MagicMock() + MockedQListWidgetItem.side_effect = [mocked_author_item1, mocked_author_item2] + MockedAuthor.display_name = None + self.mocked_manager.get_all_objects.return_value = mocked_authors + + # WHEN: reset_authors() is called + with patch.object(self.form, 'authors_list_widget') as mocked_authors_list_widget: + self.form.reset_authors() + + # THEN: The authors list should be reset + expected_widget_item_calls = [call('John Wesley'), call('John Newton')] + mocked_authors_list_widget.clear.assert_called_once_with() + self.mocked_manager.get_all_objects.assert_called_once_with(MockedAuthor) + assert MockedQListWidgetItem.call_args_list == expected_widget_item_calls, MockedQListWidgetItem.call_args_list + mocked_author_item1.setData.assert_called_once_with(QtCore.Qt.UserRole, 2) + mocked_author_item2.setData.assert_called_once_with(QtCore.Qt.UserRole, 1) + mocked_authors_list_widget.addItem.call_args_list == [ + call(mocked_author_item1), call(mocked_author_item2)] + + @patch('openlp.plugins.songs.forms.songmaintenanceform.QtWidgets.QListWidgetItem') + @patch('openlp.plugins.songs.forms.songmaintenanceform.Topic') + def test_reset_topics(self, MockedTopic, MockedQListWidgetItem): + """ + Test the reset_topics() method + """ + # GIVEN: Some mocked out objects and methods + MockedTopic.name = 'Grace' + mocked_topic = MagicMock() + mocked_topic.id = 1 + mocked_topic.name = 'Grace' + self.mocked_manager.get_all_objects.return_value = [mocked_topic] + mocked_topic_item = MagicMock() + MockedQListWidgetItem.return_value = mocked_topic_item + + # WHEN: reset_topics() is called + with patch.object(self.form, 'topics_list_widget') as mocked_topic_list_widget: + self.form.reset_topics() + + # THEN: The topics list should be reset correctly + mocked_topic_list_widget.clear.assert_called_once_with() + self.mocked_manager.get_all_objects.assert_called_once_with(MockedTopic) + MockedQListWidgetItem.assert_called_once_with('Grace') + mocked_topic_item.setData.assert_called_once_with(QtCore.Qt.UserRole, 1) + mocked_topic_list_widget.addItem.assert_called_once_with(mocked_topic_item) + + @patch('openlp.plugins.songs.forms.songmaintenanceform.QtWidgets.QListWidgetItem') + @patch('openlp.plugins.songs.forms.songmaintenanceform.Book') + def test_reset_song_books(self, MockedBook, MockedQListWidgetItem): + """ + Test the reset_song_books() method + """ + # GIVEN: Some mocked out objects and methods + MockedBook.name = 'Hymnal' + mocked_song_book = MagicMock() + mocked_song_book.id = 1 + mocked_song_book.name = 'Hymnal' + mocked_song_book.publisher = 'Hymns and Psalms, Inc.' + self.mocked_manager.get_all_objects.return_value = [mocked_song_book] + mocked_song_book_item = MagicMock() + MockedQListWidgetItem.return_value = mocked_song_book_item + + # WHEN: reset_song_books() is called + with patch.object(self.form, 'song_books_list_widget') as mocked_song_book_list_widget: + self.form.reset_song_books() + + # THEN: The song_books list should be reset correctly + mocked_song_book_list_widget.clear.assert_called_once_with() + self.mocked_manager.get_all_objects.assert_called_once_with(MockedBook) + MockedQListWidgetItem.assert_called_once_with('Hymnal (Hymns and Psalms, Inc.)') + mocked_song_book_item.setData.assert_called_once_with(QtCore.Qt.UserRole, 1) + mocked_song_book_list_widget.addItem.assert_called_once_with(mocked_song_book_item) + + @patch('openlp.plugins.songs.forms.songmaintenanceform.and_') + @patch('openlp.plugins.songs.forms.songmaintenanceform.Author') + def test_check_author_exists(self, MockedAuthor, mocked_and): + """ + Test the check_author_exists() method + """ + # GIVEN: A bunch of mocked out stuff + MockedAuthor.first_name = 'John' + MockedAuthor.last_name = 'Newton' + MockedAuthor.display_name = 'John Newton' + mocked_new_author = MagicMock() + mocked_new_author.first_name = 'John' + mocked_new_author.last_name = 'Newton' + mocked_new_author.display_name = 'John Newton' + mocked_and.return_value = True + mocked_authors = [MagicMock(), MagicMock()] + self.mocked_manager.get_all_objects.return_value = mocked_authors + + # WHEN: check_author_exists() is called + with patch.object(self.form, '_check_object_exists') as mocked_check_object_exists: + mocked_check_object_exists.return_value = True + result = self.form.check_author_exists(mocked_new_author, edit=True) + + # THEN: The correct result is returned + mocked_and.assert_called_once_with(True, True, True) + self.mocked_manager.get_all_objects.assert_called_once_with(MockedAuthor, True) + mocked_check_object_exists.assert_called_once_with(mocked_authors, mocked_new_author, True) + assert result is True + + @patch('openlp.plugins.songs.forms.songmaintenanceform.Topic') + def test_check_topic_exists(self, MockedTopic): + """ + Test the check_topic_exists() method + """ + # GIVEN: Some mocked stuff + MockedTopic.name = 'Grace' + mocked_new_topic = MagicMock() + mocked_new_topic.name = 'Grace' + mocked_topics = [MagicMock(), MagicMock()] + self.mocked_manager.get_all_objects.return_value = mocked_topics + + # WHEN: check_topic_exists() is run + with patch.object(self.form, '_check_object_exists') as mocked_check_object_exists: + mocked_check_object_exists.return_value = True + result = self.form.check_topic_exists(mocked_new_topic, True) + + # THEN: The correct things should have been called + self.mocked_manager.get_all_objects.assert_called_once_with(MockedTopic, True) + mocked_check_object_exists.assert_called_once_with(mocked_topics, mocked_new_topic, True) + assert result is True + + @patch('openlp.plugins.songs.forms.songmaintenanceform.and_') + @patch('openlp.plugins.songs.forms.songmaintenanceform.Book') + def test_check_song_book_exists(self, MockedBook, mocked_and): + """ + Test the check_song_book_exists() method + """ + # GIVEN: Some mocked stuff + MockedBook.name = 'Hymns' + MockedBook.publisher = 'Christian Songs' + mocked_new_book = MagicMock() + mocked_new_book.name = 'Hymns' + mocked_new_book.publisher = 'Christian Songs' + mocked_and.return_value = True + mocked_books = [MagicMock(), MagicMock()] + self.mocked_manager.get_all_objects.return_value = mocked_books + + # WHEN: check_book_exists() is run + with patch.object(self.form, '_check_object_exists') as mocked_check_object_exists: + mocked_check_object_exists.return_value = True + result = self.form.check_song_book_exists(mocked_new_book, True) + + # THEN: The correct things should have been called + mocked_and.assert_called_once_with(True, True) + self.mocked_manager.get_all_objects.assert_called_once_with(MockedBook, True) + mocked_check_object_exists.assert_called_once_with(mocked_books, mocked_new_book, True) + assert result is True + + def test_check_object_exists_no_existing_objects(self): + """ + Test the _check_object_exists() method when there are no existing objects + """ + # GIVEN: A SongMaintenanceForm instance + # WHEN: _check_object_exists() is called without existing objects + result = self.form._check_object_exists([], None, False) + + # THEN: The result should be True + assert result is True + + def test_check_object_exists_without_edit(self): + """ + Test the _check_object_exists() method when edit is false + """ + # GIVEN: A SongMaintenanceForm instance + # WHEN: _check_object_exists() is called with edit set to false + result = self.form._check_object_exists([MagicMock()], None, False) + + # THEN: The result should be False + assert result is False + + def test_check_object_exists_not_found(self): + """ + Test the _check_object_exists() method when the object is not found + """ + # GIVEN: A SongMaintenanceForm instance and some mocked objects + mocked_existing_objects = [MagicMock(id=1)] + mocked_new_object = MagicMock(id=2) + + # WHEN: _check_object_exists() is called with edit set to false + result = self.form._check_object_exists(mocked_existing_objects, mocked_new_object, True) + + # THEN: The result should be False + assert result is False + + def test_check_object_exists(self): + """ + Test the _check_object_exists() method + """ + # GIVEN: A SongMaintenanceForm instance and some mocked objects + mocked_existing_objects = [MagicMock(id=1)] + mocked_new_object = MagicMock(id=1) + + # WHEN: _check_object_exists() is called with edit set to false + result = self.form._check_object_exists(mocked_existing_objects, mocked_new_object, True) + + # THEN: The result should be False + assert result is True diff --git a/tests/resources/presentationmanagersongs/Amazing Grace.sng b/tests/resources/presentationmanagersongs/Amazing Grace.sng index 47b5ff3b3..4d7b8ccc6 100644 Binary files a/tests/resources/presentationmanagersongs/Amazing Grace.sng and b/tests/resources/presentationmanagersongs/Amazing Grace.sng differ