forked from openlp/openlp
fix for CCLI song select when the user's account has no access to the song
This commit is contained in:
parent
6ff9c2041b
commit
5ed56e388a
@ -32,7 +32,6 @@ from openlp.plugins.songs.forms.songselectdialog import Ui_SongSelectDialog
|
||||
from openlp.plugins.songs.lib.db import Song
|
||||
from openlp.plugins.songs.lib.songselect import SongSelectImport, Pages
|
||||
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@ -100,8 +99,15 @@ class SongSelectForm(QtWidgets.QDialog, Ui_SongSelectDialog, RegistryProperties)
|
||||
self.song_progress_bar.setMaximum(3)
|
||||
self.song_progress_bar.setValue(0)
|
||||
self.song = self.song_select_importer.get_song(self._update_song_progress)
|
||||
if self.song:
|
||||
self.import_button.setEnabled(True)
|
||||
self.view_button.setEnabled(True)
|
||||
else:
|
||||
QtWidgets.QMessageBox.critical(
|
||||
self, translate('SongsPlugin.SongSelectForm', 'No access to song'),
|
||||
translate('SongsPlugin.SongSelectForm', 'This song cannot be read. Perhaps your CCLI account '
|
||||
'does not give you access to this song.'),
|
||||
QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok), QtWidgets.QMessageBox.Ok)
|
||||
self.back_button.setEnabled(True)
|
||||
if page_type == Pages.Other:
|
||||
self.back_button.setEnabled(True)
|
||||
|
@ -182,7 +182,11 @@ class SongSelectImport(object):
|
||||
except (TypeError, URLError) as error:
|
||||
log.exception('Could not get song from SongSelect, {error}'.format(error=error))
|
||||
return None
|
||||
try:
|
||||
lyrics_link = song_page.find('section', 'page-section').find('a')['href']
|
||||
except KeyError:
|
||||
# can't find a link to the song - most likely the user account has no access to it
|
||||
return None
|
||||
if callback:
|
||||
callback()
|
||||
try:
|
||||
|
@ -299,6 +299,52 @@ class TestSongSelectImport(TestCase, TestMixin):
|
||||
assert 2 == mocked_callback.call_count, 'The callback should have been called twice'
|
||||
assert result is None, 'The get_song() method should have returned None'
|
||||
|
||||
@patch('openlp.plugins.songs.lib.songselect.log.exception')
|
||||
@patch('openlp.plugins.songs.lib.songselect.SongSelectImport.get_song_number_from_url')
|
||||
@patch('openlp.plugins.songs.lib.songselect.SongSelectImport.get_page')
|
||||
def test_get_song_no_access(self, mocked_get_page, mock_get_num, mock_log_exception):
|
||||
"""
|
||||
Test that the get_song() handles the case when the user's CCLI account has no access to the song
|
||||
"""
|
||||
fake_song_page = '''<!DOCTYPE html><html><body>
|
||||
<div class="content-title">
|
||||
<h1>Song Title</h1>
|
||||
<ul class="authors">
|
||||
<li><a>Author 1</a></li>
|
||||
<li><a>Author 2</a></li>
|
||||
</ul>
|
||||
</div>
|
||||
<div class="song-content-data"><ul><li><strong>1234_cclinumber_5678</strong></li></ul></div>
|
||||
<section class="page-section">
|
||||
<a title="View song lyrics" data-open="ssUpgradeModal"></a>
|
||||
</section>
|
||||
<ul class="song-meta-list">
|
||||
<li>Themes</li><li><a>theme1</a></li><li><a>theme2</a></li>
|
||||
</ul>
|
||||
</body></html>
|
||||
'''
|
||||
fake_lyrics_page = '''<!DOCTYPE html><html><body>
|
||||
<div class="song-viewer lyrics">
|
||||
<h3>Verse 1</h3>
|
||||
<p>verse thing 1<br>line 2</p>
|
||||
<h3>Verse 2</h3>
|
||||
<p>verse thing 2</p>
|
||||
</div>
|
||||
<ul class="copyright">
|
||||
<li>Copy thing</li><li>Copy thing 2</li>
|
||||
</ul>
|
||||
</body></html>
|
||||
'''
|
||||
mocked_get_page.side_effect = [fake_song_page, fake_lyrics_page]
|
||||
mocked_callback = MagicMock()
|
||||
importer = SongSelectImport(None, MagicMock())
|
||||
|
||||
# WHEN: get_song is called
|
||||
result = importer.get_song(callback=mocked_callback)
|
||||
|
||||
# THEN: None should be returned
|
||||
assert result is None, 'The get_song() method should have returned None'
|
||||
|
||||
@patch('openlp.plugins.songs.lib.songselect.SongSelectImport.get_song_number_from_url')
|
||||
@patch('openlp.plugins.songs.lib.songselect.SongSelectImport.get_page')
|
||||
def test_get_song(self, mocked_get_page, mock_get_num):
|
||||
|
Loading…
Reference in New Issue
Block a user