Fix bug when using SongSelect import with a free account.

bzr-revno: 2847
This commit is contained in:
Bob Luursema 2019-03-06 11:29:36 -07:00 committed by Raoul Snyman
commit f04e261e0c
4 changed files with 48 additions and 14 deletions

View File

@ -7,6 +7,7 @@ cover
.coverage
coverage
.directory
.vscode
dist
*.dll
documentation/build/doctrees

View File

@ -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'),
@ -272,6 +273,14 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties)
'There was a problem logging in, perhaps your username or password is incorrect?')
)
else:
if subscription_level == 'Free':
QtWidgets.QMessageBox.information(
self,
translate('SongsPlugin.SongSelectForm', 'Free user'),
translate('SongsPlugin.SongSelectForm', 'You logged in with a free account, '
'the search will be limited to songs '
'in the public domain.')
)
if self.save_password_checkbox.isChecked():
Settings().setValue(self.plugin.settings_section + '/songselect username', self.username_edit.text())
Settings().setValue(self.plugin.settings_section + '/songselect password', self.password_edit.text())

View File

@ -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,31 @@ class SongSelectImport(object):
return False
if callback:
callback()
# Page if user is in an organization
if posted_page.find('input', id='SearchText') is not None:
return True
self.subscription_level = self.find_subscription_level(posted_page)
return self.subscription_level
# Page if user is not in an organization
elif 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))
self.subscription_level = self.find_subscription_level(home_page)
return self.subscription_level
else:
log.debug(posted_page)
return False
return None
def find_subscription_level(self, page):
scripts = page.find_all('script')
for tag in scripts:
if tag.string:
match = re.search("'Subscription': '(?P<subscription_level>[^']+)", tag.string)
if match:
return match.group('subscription_level')
log.error('Could not determine SongSelect subscription level')
return None
def logout(self):
"""
@ -128,7 +148,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 +165,7 @@ class SongSelectImport(object):
'PrimaryLanguage': '',
'Keys': '',
'Themes': '',
'List': '',
'List': 'publicdomain' if self.subscription_level == 'Free' else '',
'Sort': '',
'SearchText': search_text
}

View File

@ -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):
@ -129,7 +129,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 2 == mocked_opener.open.call_count, 'opener should have been called twice'
assert result is True, 'The login method should have returned True'
assert result is None, '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 +146,8 @@ 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()
MockedBeautifulSoup.side_effect = [mocked_login_page, mocked_posted_page, mocked_home_page]
mock_callback = MagicMock()
importer = SongSelectImport(None)
@ -158,7 +159,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 None, '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 +192,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 +233,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 +285,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)