diff --git a/openlp/plugins/songs/lib/songselect.py b/openlp/plugins/songs/lib/songselect.py index af9c1870f..9f1de0ac8 100644 --- a/openlp/plugins/songs/lib/songselect.py +++ b/openlp/plugins/songs/lib/songselect.py @@ -52,6 +52,7 @@ LOGIN_PAGE = 'https://profile.ccli.com/account/signin?appContext=SongSelect&retu LOGIN_URL = 'https://profile.ccli.com' LOGOUT_URL = BASE_URL + '/account/logout' SEARCH_URL = BASE_URL + '/search/results' +SONG_PAGE = BASE_URL + '/Songs/' log = logging.getLogger(__name__) @@ -123,9 +124,10 @@ class SongSelectImport(object): elif posted_page.find('div', id="select-organization") is not None: try: home_page = BeautifulSoup(self.opener.open(BASE_URL).read(), 'lxml') + self.subscription_level = self.find_subscription_level(home_page) except (TypeError, URLError) as error: log.exception('Could not reach SongSelect, {error}'.format(error=error)) - self.subscription_level = self.find_subscription_level(home_page) + self.subscription_level = None return self.subscription_level else: log.debug(posted_page) @@ -160,6 +162,7 @@ class SongSelectImport(object): :return: List of songs """ self.run_search = True + search_text = search_text.strip() params = { 'SongContent': '', 'PrimaryLanguage': '', @@ -179,8 +182,19 @@ class SongSelectImport(object): search_results = results_page.find_all('div', 'song-result') except (TypeError, URLError) as error: log.exception('Could not search SongSelect, {error}'.format(error=error)) + results_page = None search_results = None if not search_results: + if results_page and re.compile('^[0-9]+$').match(search_text): + author_elements = results_page.find('ul', class_='authors').find_all('li') + song = { + 'link': SONG_PAGE + search_text, + 'authors': [unescape(li.find('a').string).strip() for li in author_elements], + 'title': unescape(results_page.find('div', 'content-title').find('h1').string).strip() + } + if callback: + callback(song) + songs.append(song) break for result in search_results: song = { @@ -212,10 +226,11 @@ class SongSelectImport(object): except (TypeError, URLError) as error: log.exception('Could not get song from SongSelect, {error}'.format(error=error)) return None + lyrics_link = song_page.find('a', title='View song lyrics')['href'] if callback: callback() try: - lyrics_page = BeautifulSoup(self.opener.open(song['link'] + '/viewlyrics').read(), 'lxml') + lyrics_page = BeautifulSoup(self.opener.open(BASE_URL + lyrics_link).read(), 'lxml') except (TypeError, URLError): log.exception('Could not get lyrics from SongSelect') return None diff --git a/tests/functional/openlp_plugins/songs/test_editverseform.py b/tests/functional/openlp_plugins/songs/test_editverseform.py index 82cb6a98e..2aa7ff78b 100644 --- a/tests/functional/openlp_plugins/songs/test_editverseform.py +++ b/tests/functional/openlp_plugins/songs/test_editverseform.py @@ -57,7 +57,7 @@ class TestEditVerseForm(TestCase, TestMixin): """ self.destroy_settings() - def test_update_suggested_verse_number(self): + def test_update_suggested_verse_number_has_no_effect(self): """ Test that update_suggested_verse_number() has no effect when editing a single verse """ @@ -73,6 +73,22 @@ class TestEditVerseForm(TestCase, TestMixin): # THEN the verse number must not be changed assert 3 == self.edit_verse_form.verse_number_box.value(), 'The verse number should be 3' + def test_update_suggested_verse_number_different_type(self): + """ + Test that update_suggested_verse_number() returns 0 when editing a second verse of a different type + """ + # GIVEN some input values + self.edit_verse_form.has_single_verse = False + self.edit_verse_form.verse_type_combo_box.currentIndex = MagicMock(return_value=2) + self.edit_verse_form.verse_text_edit.toPlainText = MagicMock(return_value='Text') + self.edit_verse_form.verse_number_box.setValue(3) + + # WHEN the method is called + self.edit_verse_form.update_suggested_verse_number() + + # THEN the verse number must be changed to 1 + assert 1 == self.edit_verse_form.verse_number_box.value(), 'The verse number should be 1' + def test_on_divide_split_button_clicked(self): """ Test that divide adds text at the correct position diff --git a/tests/functional/openlp_plugins/songs/test_songselect.py b/tests/functional/openlp_plugins/songs/test_songselect.py index 405c25865..b93d913cd 100644 --- a/tests/functional/openlp_plugins/songs/test_songselect.py +++ b/tests/functional/openlp_plugins/songs/test_songselect.py @@ -22,6 +22,10 @@ """ This module contains tests for the CCLI SongSelect importer. +It needs re-writing at some point to load real HTML pages from disk and +then test the behaviour based on those. That way if and when CCLI change +their page layout, changing the tests would just be a case of +re-downloading the HTML pages and changing the code to use the new layout. """ from unittest import TestCase from unittest.mock import MagicMock, call, patch @@ -204,6 +208,35 @@ class TestSongSelectImport(TestCase, TestMixin): mocked_results_page.find_all.assert_called_with('div', 'song-result') assert [] == results, 'The search method should have returned an empty list' + @patch('openlp.plugins.songs.lib.songselect.build_opener') + @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') + def test_search_returns_ccli_song_number_result(self, MockedBeautifulSoup, mocked_build_opener): + """ + Test that search can find a single song by CCLI number + """ + # GIVEN: A bunch of mocked out stuff and an importer object + mocked_opener = MagicMock() + mocked_build_opener.return_value = mocked_opener + mocked_results_page = MagicMock() + mocked_results_page.find_all.return_value = [] + MockedBeautifulSoup.return_value = mocked_results_page + mock_callback = MagicMock() + importer = SongSelectImport(None) + importer.subscription_level = 'premium' + + # WHEN: The search is performed + results = importer.search('1234567', 1000, mock_callback) + + # THEN: callback was called once and the results are as expected + assert 1 == mock_callback.call_count, 'callback should not have been called' + assert 1 == mocked_opener.open.call_count, 'open should have been called once' + assert 1 == mocked_results_page.find_all.call_count, 'find_all should have been called once' + mocked_results_page.find_all.assert_called_with('div', 'song-result') + + assert 1 == len(results), 'The search method should have returned an single song in a list' + assert 'https://songselect.ccli.com/Songs/1234567' == results[0]['link'],\ + 'The correct link should have been returned' + @patch('openlp.plugins.songs.lib.songselect.build_opener') @patch('openlp.plugins.songs.lib.songselect.BeautifulSoup') def test_search_returns_two_results(self, MockedBeautifulSoup, mocked_build_opener): @@ -341,7 +374,8 @@ class TestSongSelectImport(TestCase, TestMixin): Test that when BeautifulSoup gets a bad lyrics page the get_song() method returns None """ # GIVEN: A bunch of mocked out stuff and an importer object - MockedBeautifulSoup.side_effect = [None, TypeError('Test Error')] + song_page = MagicMock(return_value={'href': '/lyricpage'}) + MockedBeautifulSoup.side_effect = [song_page, TypeError('Test Error')] mocked_callback = MagicMock() importer = SongSelectImport(None)