Merge pull request #92 from marwoodandrew/fix-null-take-key

fix(formatters) Cope with null byline, anpa_take_key and abstract
superdesk · Aug 16, 2016 · ce74fdf · ce74fdf
2 parents a2d57d3 + 231183f
commit ce74fdf
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 4 deletions.
diff --git a/server/aap/publish/formatters/aap_bulletinbuilder_formatter.py b/server/aap/publish/formatters/aap_bulletinbuilder_formatter.py
@@ -45,11 +45,11 @@ def format(self, article, subscriber, codes=None):
             body_html = to_ascii(self.append_body_footer(formatted_article)).strip('\r\n')
             formatted_article['body_text'] = self.get_text_content(body_html)
             formatted_article['abstract'] = self.get_text_content(
-                to_ascii(formatted_article.get('abstract', ''))).strip()
+                to_ascii(formatted_article.get('abstract', '') or '')).strip()
             formatted_article['headline'] = self.get_text_content(
                 to_ascii(formatted_article.get('headline', ''))).strip()
             formatted_article['byline'] = self.get_text_content(
-                to_ascii(formatted_article.get('byline', ''))).strip()
+                to_ascii(formatted_article.get('byline', '') or '')).strip()
 
             # get the first category and derive the locator
             category = next((iter(formatted_article.get('anpa_category', []))), None)

diff --git a/server/aap/publish/formatters/aap_bulletinbuilder_formatter_tests.py b/server/aap/publish/formatters/aap_bulletinbuilder_formatter_tests.py
@@ -258,3 +258,23 @@ def test_takes_package(self):
                      'Para4:Tropical refers to the geographical origin of these systems\r\n\r\n')
 
         self.assertEqual(formatted_content, body_text)
+
+    def test_null_abstract_byline_key(self):
+        article = {
+            config.ID_FIELD: '123',
+            config.VERSION: 2,
+            'source': 'AAP',
+            'headline': 'This is a test headline',
+            'type': 'text',
+            'anpa_take_key': None,
+            'byline': None,
+            'abstract': None,
+            'body_html': ('<p>Hi</p>')
+        }
+        subscriber = self.app.data.find('subscribers', None, None)[0]
+        seq, item = self._formatter.format(article, subscriber)[0]
+        item = json.loads(item)
+        self.assertGreater(int(seq), 0)
+        test_article = json.loads(item.get('data'))
+        self.assertEqual(test_article['byline'], '')
+        self.assertEqual(test_article['abstract'], '')
diff --git a/server/aap/publish/formatters/aap_ipnews_formatter_test.py b/server/aap/publish/formatters/aap_ipnews_formatter_test.py
@@ -363,6 +363,35 @@ def testSpacesContent(self):
         expected = '   a b c d e f g\r\n\r\nAAP'
         self.assertEqual(item['article_text'], expected)
 
+    def testNullTakeKeyContent(self):
+        article = {
+            '_id': '3',
+            'source': 'AAP',
+            'anpa_category': [{'qcode': 'a'}],
+            'headline': 'This is a test headline',
+            'byline': None,
+            'slugline': 'slugline',
+            'subject': [{'qcode': '02011001'}],
+            'anpa_take_key': None,
+            'unique_id': '1',
+            'type': 'text',
+            'body_html': '<p>Nothing</p>',
+            'word_count': '1',
+            'priority': 1,
+            "linked_in_packages": [
+                {
+                    "package": "package",
+                    "package_type": "takes"
+                }
+            ],
+        }
+        subscriber = self.app.data.find('subscribers', None, None)[0]
+
+        f = AAPIpNewsFormatter()
+        seq, item = f.format(article, subscriber)[0]
+        item = json.loads(item)
+        self.assertEqual(item['take_key'], '')
+
     def testControlCharsContent(self):
         article = {
             '_id': '3',

diff --git a/server/aap/publish/formatters/aap_nitf_formatter.py b/server/aap/publish/formatters/aap_nitf_formatter.py
@@ -41,7 +41,8 @@ def _append_meta(self, article, head, destination, pub_seq_num):
 
         SubElement(head, 'meta', {'name': 'anpa-sequence', 'content': str(pub_seq_num)})
         SubElement(head, 'meta', {'name': 'anpa-keyword', 'content': self.append_legal(article)})
-        SubElement(head, 'meta', {'name': 'anpa-takekey', 'content': article.get('anpa_take_key', '')})
+        if article.get('anpa_take_key'):
+            SubElement(head, 'meta', {'name': 'anpa-takekey', 'content': article.get('anpa_take_key', '')})
 
         original_creator = superdesk.get_resource_service('users').find_one(req=None,
                                                                             _id=article.get('original_creator', ''))

diff --git a/server/aap/publish/formatters/aap_nitf_formatter_test.py b/server/aap/publish/formatters/aap_nitf_formatter_test.py
@@ -323,3 +323,30 @@ def testControlCharsContent(self):
         seq, doc = self.formatter.format(article, {'name': 'Test Subscriber'})[0]
         nitf_xml = etree.fromstring(doc)
         self.assertEqual(nitf_xml.find('body/body.content/p').text, ' ')
+
+    def testNullTakeKeyContent(self):
+        article = {
+            '_id': '3',
+            'source': 'AAP',
+            'anpa_category': [{'qcode': 'a'}],
+            'headline': 'This is a test headline',
+            'byline': None,
+            'slugline': 'slugline',
+            'subject': [{'qcode': '02011001'}],
+            'anpa_take_key': None,
+            'unique_id': '1',
+            'type': 'text',
+            'body_html': '<p>no body</p>',
+            'word_count': '1',
+            'priority': 1,
+            'abstract': None,
+            "linked_in_packages": [
+                {
+                    "package": "package",
+                    "package_type": "takes"
+                }
+            ],
+        }
+        seq, doc = self.formatter.format(article, {'name': 'Test Subscriber'})[0]
+        nitf_xml = etree.fromstring(doc)
+        self.assertIsNone(nitf_xml.find('head/meta[@name="anpa-takekey"]'))
diff --git a/server/aap/publish/formatters/anpa_formatter.py b/server/aap/publish/formatters/anpa_formatter.py
@@ -107,7 +107,7 @@ def format(self, article, subscriber, codes=None):
                     ednote = '{}\r\n'.format(to_ascii(formatted_article.get('ednote')))
                     anpa.append(ednote.encode('ascii', 'replace'))
 
-                if BYLINE in formatted_article:
+                if formatted_article.get(BYLINE):
                     anpa.append(BeautifulSoup(formatted_article.get(BYLINE), 'html.parser').text.encode
                                 ('ascii', 'ignore'))
                     anpa.append(b'\x0D\x0A')

diff --git a/server/aap/publish/formatters/anpa_formatter_test.py b/server/aap/publish/formatters/anpa_formatter_test.py
@@ -327,3 +327,11 @@ def test_br_body(self):
         lines = io.StringIO(out.decode())
         self.assertTrue(lines.getvalue().split('\n')[4].find('   Dental materials maker and marketer SDI') == 0)
         self.assertTrue(lines.getvalue().split('\n')[5].find(' has boosted its shares after reporting') == 0)
+
+    def test_none_body(self):
+        f = AAPAnpaFormatter()
+        subscriber = self.app.data.find('subscribers', None, None)[0]
+        item = self.article.copy()
+        item.update({
+            'anpa_take_key': None, 'byline': None, 'abstract': None})
+        seq, out = f.format(item, subscriber)[0]