Merge branch 'allow-search-by-song-number' into 'master'

Allow searching by Song Number

See merge request openlp/openlp!26

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

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

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)