Almost fixed bug 1800761

.bzrignore

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

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(
+        subscription_level = self.song_select_importer.login(
-                self.username_edit.text(), self.password_edit.text(), self._update_login_progress):
+                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'),

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:
+        if posted_page.find('input', id='SearchText') is not None or posted_page.find('div', id="select-organization") is not None:
-            return True
+            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
         }

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 3 == mocked_opener.open.call_count, 'opener should have been called 3 times'
-        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')
     @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)