openlp-core team mailing list archive

[Bob Luursema <bob.luursema@xxxxxxxxxxx>]

Bob Luursema has proposed merging lp:~bob-luursema/openlp/bug-1800761 into lp:openlp.

Commit message:
Fix bug when using SongSelect import with a free account.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1800761 in OpenLP: "SongSelect Importer Crashes on Import"
  https://bugs.launchpad.net/openlp/+bug/1800761

For more details, see:
https://code.launchpad.net/~bob-luursema/openlp/bug-1800761/+merge/363996

This change checks the subscription level of the login that the user has from a piece of JavaScript on the SongSelect page. If it is a free subscription it will display a messagebox informing the user that their searches are limited to songs in the public domain (as that is what a free user can access).
-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~bob-luursema/openlp/bug-1800761 into lp:openlp.

=== modified file '.bzrignore'
--- .bzrignore	2018-09-07 06:43:01 +0000
+++ .bzrignore	2019-03-05 21:04:35 +0000
@@ -7,6 +7,7 @@
 .coverage
 coverage
 .directory
+.vscode
 dist
 *.dll
 documentation/build/doctrees

=== modified file 'openlp/plugins/songs/forms/songselectform.py'
--- openlp/plugins/songs/forms/songselectform.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/forms/songselectform.py	2019-03-05 21:04:35 +0000
@@ -263,8 +263,9 @@
         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,13 @@
                           '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())

=== modified file 'openlp/plugins/songs/lib/songselect.py'
--- openlp/plugins/songs/lib/songselect.py	2019-02-14 15:09:09 +0000
+++ openlp/plugins/songs/lib/songselect.py	2019-03-05 21:04:35 +0000
@@ -81,7 +81,7 @@
         :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 @@
             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 'unkown'
 
     def logout(self):
         """
@@ -128,7 +148,7 @@
         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 @@
             'PrimaryLanguage': '',
             'Keys': '',
             'Themes': '',
-            'List': '',
+            'List': 'publicdomain' if self.subscription_level == 'Free' else '',
             'Sort': '',
             'SearchText': search_text
         }

=== modified file 'tests/functional/openlp_plugins/songs/test_songselect.py'
--- tests/functional/openlp_plugins/songs/test_songselect.py	2019-02-14 15:09:09 +0000
+++ tests/functional/openlp_plugins/songs/test_songselect.py	2019-03-05 21:04:35 +0000
@@ -64,7 +64,7 @@
     @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 @@
         # 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 @@
         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 'unkown', '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 @@
         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 @@
         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 'unkown', '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 @@
         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 @@
         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 @@
         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)