openlp-core team mailing list archive

[Tomas Groth <second@xxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/24bugfix-backport4 into lp:openlp/2.4.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1562384 in OpenLP: "Biblegateway Downloads Amplified Classic instead of Amplified 2015"
  https://bugs.launchpad.net/openlp/+bug/1562384
  Bug #1599999 in OpenLP: "CrossWalk bible list is empty"
  https://bugs.launchpad.net/openlp/+bug/1599999

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/24bugfix-backport4/+merge/301319

Use BibleGateway standard site instead of the legacy site. Fixes bug 1562384.
Update Crosswalk webpage parser to match new layout. Fixes bug 1599999.

-- 
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/24bugfix-backport4 into lp:openlp/2.4.

=== modified file 'openlp/plugins/bibles/lib/http.py'
--- openlp/plugins/bibles/lib/http.py	2016-05-09 19:41:23 +0000
+++ openlp/plugins/bibles/lib/http.py	2016-07-27 19:32:31 +0000
@@ -248,7 +248,7 @@
         url_book_name = urllib.parse.quote(book_name.encode("utf-8"))
         url_params = 'search=%s+%s&version=%s' % (url_book_name, chapter, version)
         soup = get_soup_for_bible_ref(
-            'http://legacy.biblegateway.com/passage/?%s' % url_params,
+            'http://biblegateway.com/passage/?%s' % url_params,
             pre_parse_regex=r'<meta name.*?/>', pre_parse_substitute='')
         if not soup:
             return None
@@ -277,7 +277,7 @@
         """
         log.debug('BGExtract.get_books_from_http("%s")', version)
         url_params = urllib.parse.urlencode({'action': 'getVersionInfo', 'vid': '%s' % version})
-        reference_url = 'http://legacy.biblegateway.com/versions/?%s#books' % url_params
+        reference_url = 'http://biblegateway.com/versions/?%s#books' % url_params
         page = get_web_page(reference_url)
         if not page:
             send_error_message('download')
@@ -308,7 +308,7 @@
         for book in content:
             book = book.find('td')
             if book:
-                books.append(book.contents[0])
+                books.append(book.contents[1])
         return books
 
     def get_bibles_from_http(self):
@@ -318,11 +318,11 @@
         returns a list in the form [(biblename, biblekey, language_code)]
         """
         log.debug('BGExtract.get_bibles_from_http')
-        bible_url = 'https://legacy.biblegateway.com/versions/'
+        bible_url = 'https://biblegateway.com/versions/'
         soup = get_soup_for_bible_ref(bible_url)
         if not soup:
             return None
-        bible_select = soup.find('select', {'class': 'translation-dropdown'})
+        bible_select = soup.find('select', {'class': 'search-translation-select'})
         if not bible_select:
             log.debug('No select tags found - did site change?')
             return None
@@ -520,28 +520,26 @@
         returns a list in the form [(biblename, biblekey, language_code)]
         """
         log.debug('CWExtract.get_bibles_from_http')
-        bible_url = 'http://www.biblestudytools.com/'
+        bible_url = 'http://www.biblestudytools.com/bible-versions/'
         soup = get_soup_for_bible_ref(bible_url)
         if not soup:
             return None
-        bible_select = soup.find('select')
-        if not bible_select:
-            log.debug('No select tags found - did site change?')
-            return None
-        option_tags = bible_select.find_all('option', {'class': 'log-translation'})
-        if not option_tags:
-            log.debug('No option tags found - did site change?')
+        h4_tags = soup.find_all('h4', {'class': 'small-header'})
+        if not h4_tags:
+            log.debug('No h4 tags found - did site change?')
             return None
         bibles = []
-        for ot in option_tags:
-            tag_text = ot.get_text().strip()
-            try:
-                tag_value = ot['value']
-            except KeyError:
-                log.exception('No value attribute found - did site change?')
+        for h4t in h4_tags:
+            short_name = None
+            if h4t.span:
+                short_name = h4t.span.get_text().strip().lower()
+            else:
+                log.error('No span tag found - did site change?')
                 return None
-            if not tag_value:
+            if not short_name:
                 continue
+            h4t.span.extract()
+            tag_text = h4t.get_text().strip()
             # The names of non-english bibles has their language in parentheses at the end
             if tag_text.endswith(')'):
                 language = tag_text[tag_text.rfind('(') + 1:-1]
@@ -549,12 +547,20 @@
                     language_code = CROSSWALK_LANGUAGES[language]
                 else:
                     language_code = ''
-            # ... except for the latin vulgate
+            # ... except for those that don't...
             elif 'latin' in tag_text.lower():
                 language_code = 'la'
+            elif 'la biblia' in tag_text.lower() or 'nueva' in tag_text.lower():
+                language_code = 'es'
+            elif 'chinese' in tag_text.lower():
+                language_code = 'zh'
+            elif 'greek' in tag_text.lower():
+                language_code = 'el'
+            elif 'nova' in tag_text.lower():
+                language_code = 'pt'
             else:
                 language_code = 'en'
-            bibles.append((tag_text, tag_value, language_code))
+            bibles.append((tag_text, short_name, language_code))
         return bibles
 
 

=== modified file 'tests/interfaces/openlp_plugins/bibles/test_lib_http.py'
--- tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2016-01-07 21:38:13 +0000
+++ tests/interfaces/openlp_plugins/bibles/test_lib_http.py	2016-07-27 19:32:31 +0000
@@ -50,7 +50,8 @@
         books = handler.get_books_from_http('NIV')
 
         # THEN: We should get back a valid service item
-        assert len(books) == 66, 'The bible should not have had any books added or removed'
+        self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed')
+        self.assertEqual(books[0], 'Genesis', 'The first bible book should be Genesis')
 
     def bible_gateway_extract_books_support_redirect_test(self):
         """
@@ -63,7 +64,7 @@
         books = handler.get_books_from_http('DN1933')
 
         # THEN: We should get back a valid service item
-        assert len(books) == 66, 'This bible should have 66 books'
+        self.assertEqual(len(books), 66, 'This bible should have 66 books')
 
     def bible_gateway_extract_verse_test(self):
         """
@@ -76,7 +77,8 @@
         results = handler.get_bible_chapter('NIV', 'John', 3)
 
         # THEN: We should get back a valid service item
-        assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed'
+        self.assertEqual(len(results.verse_list), 36,
+                         'The book of John should not have had any verses added or removed')
 
     def bible_gateway_extract_verse_nkjv_test(self):
         """
@@ -89,7 +91,8 @@
         results = handler.get_bible_chapter('NKJV', 'John', 3)
 
         # THEN: We should get back a valid service item
-        assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed'
+        self.assertEqual(len(results.verse_list), 36,
+                         'The book of John should not have had any verses added or removed')
 
     def crosswalk_extract_books_test(self):
         """
@@ -102,7 +105,7 @@
         books = handler.get_books_from_http('niv')
 
         # THEN: We should get back a valid service item
-        assert len(books) == 66, 'The bible should not have had any books added or removed'
+        self.assertEqual(len(books), 66, 'The bible should not have had any books added or removed')
 
     def crosswalk_extract_verse_test(self):
         """
@@ -115,7 +118,8 @@
         results = handler.get_bible_chapter('niv', 'john', 3)
 
         # THEN: We should get back a valid service item
-        assert len(results.verse_list) == 36, 'The book of John should not have had any verses added or removed'
+        self.assertEqual(len(results.verse_list), 36,
+                         'The book of John should not have had any verses added or removed')
 
     def bibleserver_get_bibles_test(self):
         """
@@ -144,7 +148,7 @@
 
         # THEN: The list should not be None, and some known bibles should be there
         self.assertIsNotNone(bibles)
-        self.assertIn(('Holman Christian Standard Bible', 'HCSB', 'en'), bibles)
+        self.assertIn(('Holman Christian Standard Bible (HCSB)', 'HCSB', 'en'), bibles)
 
     def crosswalk_get_bibles_test(self):
         """