openlp-core team mailing list archive
-
openlp-core team
-
Mailing list archive
-
Message #31215
[Merge] lp:~raoul-snyman/openlp/db-upgrades-2.6 into lp:openlp
Raoul Snyman has proposed merging lp:~raoul-snyman/openlp/db-upgrades-2.6 into lp:openlp.
Requested reviews:
OpenLP Core (openlp-core)
For more details, see:
https://code.launchpad.net/~raoul-snyman/openlp/db-upgrades-2.6/+merge/319632
Fix a songselect bug, a bunch of database upgrade bugs, and a few other issues.
Add this to your merge proposal:
--------------------------------
lp:~raoul-snyman/openlp/db-upgrades-2.6 (revision 2730)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1934/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1845/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1786/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1512/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/1102/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1170/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/1038/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/175/
--
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/db-upgrades-2.6 into lp:openlp.
=== modified file 'openlp/core/__init__.py'
--- openlp/core/__init__.py 2016-12-31 11:01:36 +0000
+++ openlp/core/__init__.py 2017-03-11 00:02:53 +0000
@@ -319,7 +319,7 @@
return QtWidgets.QApplication.event(self, event)
-def parse_options(args):
+def parse_options(args=None):
"""
Parse the command line arguments
=== modified file 'openlp/plugins/songs/lib/importers/openlp.py'
--- openlp/plugins/songs/lib/importers/openlp.py 2016-12-31 11:01:36 +0000
+++ openlp/plugins/songs/lib/importers/openlp.py 2017-03-11 00:02:53 +0000
@@ -221,11 +221,17 @@
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:
=== modified file 'openlp/plugins/songs/lib/importers/presentationmanager.py'
--- openlp/plugins/songs/lib/importers/presentationmanager.py 2016-12-31 11:01:36 +0000
+++ openlp/plugins/songs/lib/importers/presentationmanager.py 2017-03-11 00:02:53 +0000
@@ -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
@@ -34,6 +34,8 @@
from openlp.core.ui.lib.wizard import WizardStrings
from .songimport import SongImport
+log = logging.getLogger(__name__)
+
class PresentationManagerImport(SongImport):
"""
@@ -65,15 +67,36 @@
'File is not in XML-format, which is the only format supported.'))
continue
root = objectify.fromstring(etree.tostring(tree))
- self.process_song(root)
-
- def process_song(self, root):
+ self.process_song(root, file_path)
+
+ 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 = []
@@ -105,4 +128,4 @@
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))
=== modified file 'openlp/plugins/songs/lib/songselect.py'
--- openlp/plugins/songs/lib/songselect.py 2016-12-31 11:01:36 +0000
+++ openlp/plugins/songs/lib/songselect.py 2017-03-11 00:02:53 +0000
@@ -47,7 +47,7 @@
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 @@
'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):
"""
=== modified file 'tests/functional/openlp_core/test_init.py'
--- tests/functional/openlp_core/test_init.py 2016-12-31 11:01:36 +0000
+++ tests/functional/openlp_core/test_init.py 2017-03-11 00:02:53 +0000
@@ -21,13 +21,13 @@
###############################################################################
import sys
-from unittest import TestCase
-
-from openlp.core import parse_options
-from tests.helpers.testmixin import TestMixin
-
-
-class TestInitFunctions(TestMixin, TestCase):
+from unittest import TestCase, skip
+from unittest.mock import MagicMock, patch
+
+from openlp.core import OpenLP, parse_options
+
+
+class TestInitFunctions(TestCase):
def test_parse_options_basic(self):
"""
@@ -116,8 +116,7 @@
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 +129,30 @@
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')
+
+
+class TestOpenLP(TestCase):
+ """
+ Test the OpenLP app class
+ """
+ @skip('Figure out why this is causing a segfault')
+ @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
+
+ del app
=== modified file 'tests/functional/openlp_plugins/remotes/test_router.py'
--- tests/functional/openlp_plugins/remotes/test_router.py 2017-03-04 16:51:51 +0000
+++ tests/functional/openlp_plugins/remotes/test_router.py 2017-03-11 00:02:53 +0000
@@ -25,11 +25,11 @@
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,13 @@
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 and causes them to hang
+ # 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 and causes them to hang
+ # 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 +359,13 @@
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 and causes them to hang
+ # 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 and causes them to hang
+ # 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')
=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py 2016-12-31 11:01:36 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py 2017-03-11 00:02:53 +0000
@@ -25,6 +25,7 @@
"""
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 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 @@
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 @@
# 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 @@
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,11 +127,41 @@
# 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):
"""
Test that when the logout method is called, it logs the user out of SongSelect
=== added file 'tests/interfaces/openlp_plugins/bibles/forms/__init__.py'
=== modified file 'tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py'
--- tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py 2016-12-31 11:01:36 +0000
+++ tests/interfaces/openlp_plugins/bibles/forms/test_bibleimportform.py 2017-03-11 00:02:53 +0000
@@ -49,12 +49,12 @@
bibleimportform.PYSWORD_AVAILABLE = False
self.form = bibleimportform.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')
=== modified file 'tests/resources/presentationmanagersongs/Amazing Grace.sng'
Binary files tests/resources/presentationmanagersongs/Amazing Grace.sng 2015-01-29 20:54:06 +0000 and tests/resources/presentationmanagersongs/Amazing Grace.sng 2017-03-11 00:02:53 +0000 differ
Follow ups
