diff --git a/openlp/core/__init__.py b/openlp/core/__init__.py index dfd710523..862345e68 100644 --- a/openlp/core/__init__.py +++ b/openlp/core/__init__.py @@ -317,7 +317,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 diff --git a/openlp/plugins/songs/lib/importers/openlp.py b/openlp/plugins/songs/lib/importers/openlp.py index 7fd87cacb..4a7a847a3 100644 --- a/openlp/plugins/songs/lib/importers/openlp.py +++ b/openlp/plugins/songs/lib/importers/openlp.py @@ -221,11 +221,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 58f4587ce..fe7a26335 100644 --- a/openlp/plugins/songs/lib/importers/presentationmanager.py +++ b/openlp/plugins/songs/lib/importers/presentationmanager.py @@ -23,7 +23,7 @@ The :mod:`presentationmanager` module provides the functionality for importing Presentationmanager song files into the current database. """ - +import logging import os import re import chardet @@ -32,6 +32,8 @@ from lxml import objectify, etree from openlp.core.ui.wizard import WizardStrings from .songimport import SongImport +log = logging.getLogger(__name__) + class PresentationManagerImport(SongImport): """ @@ -62,15 +64,36 @@ 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): + log.debug('%s: %s', name, getattr(elem, name)) + return str(getattr(elem, name)) + name = name[0].upper() + name[1:] + if hasattr(elem, name): + log.debug('%s: %s', name, getattr(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 = [] @@ -102,4 +125,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 3d43eed30..a34d89c9e 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, %s', 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/tests/functional/openlp_core/__init__.py b/tests/functional/openlp_core/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/functional/openlp_core/test_init.py b/tests/functional/openlp_core/test_init.py index 3b5f10139..0aef7d8cb 100644 --- a/tests/functional/openlp_core/test_init.py +++ b/tests/functional/openlp_core/test_init.py @@ -22,12 +22,12 @@ import sys from unittest import TestCase +from unittest.mock import MagicMock, patch -from openlp.core import parse_options -from tests.helpers.testmixin import TestMixin +from openlp.core import OpenLP, parse_options -class TestInitFunctions(TestMixin, TestCase): +class TestInitFunctions(TestCase): def parse_options_basic_test(self): """ @@ -116,7 +116,7 @@ class TestInitFunctions(TestMixin, TestCase): def parse_options_file_and_debug_test(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. @@ -131,14 +131,28 @@ class TestInitFunctions(TestMixin, TestCase): 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') - def parse_options_two_files_test(self): - """ - Test the parse options process works with a file +class TestOpenLP(TestCase): + """ + Test the OpenLP app class + """ + @patch('openlp.core.QtWidgets.QApplication.exec') + def test_exec(self, mocked_exec): """ - # GIVEN: a a set of system arguments. - sys.argv[1:] = ['dummy_temp', 'dummy_temp2'] - # WHEN: We we parse them to expand to options - args = parse_options() - # THEN: the following fields will have been extracted. - self.assertEquals(args, None, 'The args should be None') + 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 + + del app diff --git a/tests/functional/openlp_plugins/remotes/test_router.py b/tests/functional/openlp_plugins/remotes/test_router.py index 872c7c788..a9e21c2fe 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__ = { @@ -313,11 +313,13 @@ 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() + # Not sure why this is here, it doesn't make sense in the test + # self.app.processEvents() # WHEN: Remote next is received self.router.service(action='next') - self.app.processEvents() + # Not sure why this is here, it doesn't make sense in the test + # 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') @@ -334,11 +336,13 @@ 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() + # Not sure why this is here, it doesn't make sense in the test + # self.app.processEvents() # WHEN: Remote next is received self.router.service(action='previous') - self.app.processEvents() + # Not sure why this is here, it doesn't make sense in the test + # 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 cbcade105..f52ce1046 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -32,7 +32,7 @@ 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 @@ -81,7 +81,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(2, mock_callback.call_count, 'callback should have been called 3 times') - self.assertEqual(1, 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') self.assertEqual(2, mocked_opener.open.call_count, 'opener should have been called twice') self.assertFalse(result, 'The login method should have returned False') @@ -96,7 +96,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) @@ -105,7 +107,8 @@ 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(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.assertFalse(result, 'The login method should have returned False') @@ -136,8 +139,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) @@ -146,10 +151,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 logout_test(self, mocked_build_opener): """ diff --git a/tests/interfaces/openlp_plugins/bibles/forms/__init__.py b/tests/interfaces/openlp_plugins/bibles/forms/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py b/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py index 8f07ac387..bf1a27d0b 100644 --- a/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py +++ b/tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py @@ -22,7 +22,7 @@ """ Package to test the openlp.plugins.bibles.forms.bibleimportform package. """ -from unittest import TestCase +from unittest import TestCase, skip from PyQt5 import QtWidgets @@ -48,12 +48,12 @@ class TestBibleImportForm(TestCase, TestMixin): Registry().register('main_window', self.main_window) self.form = BibleImportForm(self.main_window, MagicMock(), MagicMock()) - 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 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 @patch('openlp.plugins.bibles.forms.bibleimportform.CWExtract.get_bibles_from_http') @patch('openlp.plugins.bibles.forms.bibleimportform.BGExtract.get_bibles_from_http') diff --git a/tests/interfaces/openlp_plugins/songusage/__init__.py b/tests/interfaces/openlp_plugins/songusage/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/resources/presentationmanagersongs/Agnus Dei.sng b/tests/resources/presentationmanagersongs/Agnus Dei.sng index ce4385c55..6a0f7e6f7 100644 --- a/tests/resources/presentationmanagersongs/Agnus Dei.sng +++ b/tests/resources/presentationmanagersongs/Agnus Dei.sng @@ -1,7 +1,7 @@ -Agnus Dei +Agnus Dei