From 2a6378f489b03fa58640babfbc9292812b1b226e Mon Sep 17 00:00:00 2001 From: Bob Luursema Date: Mon, 4 Mar 2019 21:37:11 +0100 Subject: [PATCH] Almost fixed bug 1800761 --- .bzrignore | 1 + openlp/plugins/songs/forms/songselectform.py | 5 ++-- openlp/plugins/songs/lib/songselect.py | 21 ++++++++++---- .../openlp_plugins/songs/test_songselect.py | 29 +++++++++++++------ 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/.bzrignore b/.bzrignore index f60d6cfff..2b43ce13a 100644 --- a/.bzrignore +++ b/.bzrignore @@ -7,6 +7,7 @@ cover .coverage coverage .directory +.vscode dist *.dll documentation/build/doctrees diff --git a/openlp/plugins/songs/forms/songselectform.py b/openlp/plugins/songs/forms/songselectform.py index d8b4db300..423f1f17c 100644 --- a/openlp/plugins/songs/forms/songselectform.py +++ b/openlp/plugins/songs/forms/songselectform.py @@ -263,8 +263,9 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties) self.login_progress_bar.setVisible(True) self.application.process_events() # Log the user in - if not self.song_select_importer.login( - self.username_edit.text(), self.password_edit.text(), self._update_login_progress): + subscription_level = self.song_select_importer.login( + self.username_edit.text(), self.password_edit.text(), self._update_login_progress) + if not subscription_level: QtWidgets.QMessageBox.critical( self, translate('SongsPlugin.SongSelectForm', 'Error Logging In'), diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index bde9b7bf9..01b9a635b 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -81,7 +81,7 @@ class SongSelectImport(object): :param username: SongSelect username :param password: SongSelect password :param callback: Method to notify of progress. - :return: True on success, False on failure. + :return: subscription level on success, None on failure. """ if callback: callback() @@ -115,11 +115,20 @@ class SongSelectImport(object): return False if callback: callback() - if posted_page.find('input', id='SearchText') is not None: - return True + if posted_page.find('input', id='SearchText') is not None or posted_page.find('div', id="select-organization") is not None: + try: + home_page = BeautifulSoup(self.opener.open(BASE_URL).read(), 'lxml') + except (TypeError, URLError) as error: + log.exception('Could not reach SongSelect, {error}'.format(error=error)) + subscription_element = home_page.find('div', {'class': 'subscriptionlevel'}) + if subscription_element is not None: + self.subscription_level = subscription_element.string.strip() + else: + self.subscription_level = 'premium' + return self.subscription_level else: log.debug(posted_page) - return False + return None def logout(self): """ @@ -128,7 +137,7 @@ class SongSelectImport(object): try: self.opener.open(LOGOUT_URL) except (TypeError, URLError) as error: - log.exception('Could not log of SongSelect, {error}'.format(error=error)) + log.exception('Could not log out of SongSelect, {error}'.format(error=error)) def search(self, search_text, max_results, callback=None): """ @@ -145,7 +154,7 @@ class SongSelectImport(object): 'PrimaryLanguage': '', 'Keys': '', 'Themes': '', - 'List': '', + 'List': '' if self.subscription_level == 'premium' else 'publicdomain', 'Sort': '', 'SearchText': search_text } diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 1351faf51..4ef122c40 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -64,7 +64,7 @@ class TestSongSelectImport(TestCase, TestMixin): @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') def test_login_fails(self, MockedBeautifulSoup, mocked_build_opener): """ - Test that when logging in to SongSelect fails, the login method returns False + Test that when logging in to SongSelect fails, the login method returns None """ # GIVEN: A bunch of mocked out stuff and an importer object mocked_opener = MagicMock() @@ -80,12 +80,12 @@ class TestSongSelectImport(TestCase, TestMixin): # 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 False was returned + # THEN: callback was called 3 times, open was called twice, find was called twice, and None was returned assert 3 == mock_callback.call_count, 'callback should have been called 3 times' assert 2 == mocked_login_page.find.call_count, 'find should have been called twice' - assert 1 == mocked_posted_page.find.call_count, 'find should have been called once' + assert 2 == mocked_posted_page.find.call_count, 'find should have been called twice' assert 2 == mocked_opener.open.call_count, 'opener should have been called twice' - assert result is False, 'The login method should have returned False' + assert result is None, 'The login method should have returned None' @patch('openlp.plugins.songs.lib.songselect.build_opener') def test_login_except(self, mocked_build_opener): @@ -117,7 +117,11 @@ class TestSongSelectImport(TestCase, TestMixin): 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] + mocked_home_page = MagicMock() + mocked_return = MagicMock() + mocked_return.string = 'premium' + mocked_home_page.find.return_value = mocked_return + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -128,8 +132,8 @@ class TestSongSelectImport(TestCase, TestMixin): assert 3 == mock_callback.call_count, 'callback should have been called 3 times' assert 2 == mocked_login_page.find.call_count, 'find should have been called twice on the login page' assert 1 == mocked_posted_page.find.call_count, 'find should have been called once on the posted page' - assert 2 == mocked_opener.open.call_count, 'opener should have been called twice' - assert result is True, 'The login method should have returned True' + assert 3 == mocked_opener.open.call_count, 'opener should have been called 3 times' + assert result is 'premium', 'The login method should have returned the subscription level' @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') @@ -146,7 +150,11 @@ class TestSongSelectImport(TestCase, TestMixin): 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] + mocked_home_page = MagicMock() + mocked_return = MagicMock() + mocked_return.string = 'premium' + mocked_home_page.find.return_value = mocked_return + MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page] mock_callback = MagicMock() importer = SongSelectImport(None) @@ -158,7 +166,7 @@ class TestSongSelectImport(TestCase, TestMixin): assert 2 == mocked_login_page.find.call_count, 'find should have been called twice on the login page' assert 1 == mocked_posted_page.find.call_count, 'find should have been called once on the posted page' assert 'https://profile.ccli.com/do/login', mocked_opener.open.call_args_list[1][0][0] - assert result is True, 'The login method should have returned True' + assert result is 'premium', 'The login method should have returned the subscription level' @patch('openlp.plugins.songs.lib.songselect.build_opener') def test_logout(self, mocked_build_opener): @@ -191,6 +199,7 @@ class TestSongSelectImport(TestCase, TestMixin): MockedBeautifulSoup.return_value = mocked_results_page mock_callback = MagicMock() importer = SongSelectImport(None) + importer.subscription_level = 'premium' # WHEN: The login method is called after being rigged to fail results = importer.search('text', 1000, mock_callback) @@ -231,6 +240,7 @@ class TestSongSelectImport(TestCase, TestMixin): MockedBeautifulSoup.return_value = mocked_results_page mock_callback = MagicMock() importer = SongSelectImport(None) + importer.subscription_level = 'premium' # WHEN: The search method is called results = importer.search('text', 1000, mock_callback) @@ -282,6 +292,7 @@ class TestSongSelectImport(TestCase, TestMixin): MockedBeautifulSoup.return_value = mocked_results_page mock_callback = MagicMock() importer = SongSelectImport(None) + importer.subscription_level = 'premium' # WHEN: The search method is called results = importer.search('text', 2, mock_callback)