Prepend a real summary in mail digest

CHANGELOG.rst

@@ -2,6 +2,16 @@
 Changelog
 =========
 
+Master
+======
+
+Bug fixes
+---------
+
+* Restore summary in digest email when reporting verbose messages
+  (`#278 <https://github.com/fedora-infra/fmn/pull/278>`_).
+
+
 v2.0.2
 ======
 

fmn/formatters.py

@@ -438,50 +438,76 @@ def email_batch(messages, recipient):
     email_message = _base_email(recipient=recipient, messages=messages)
     email_message.add_header(
         'Subject', u'Fedora Notifications Digest ({n} updates)'.format(n=len(messages)))
-
-    content = u'Digest Summary:\n'
-    message_template = u'{number}.\t{summary} ({ts})\n\t- {link}\n{details}{separator}'
     separator = u'\n\n' + '-' * 79 + '\n\n'
-    for message_number, message in enumerate(messages, start=1):
-        try:
-            summary = fedmsg.meta.msg2subtitle(message, **config.app_conf) or u''
-        except Exception:
-            _log.exception('fedmsg.meta.msg2subtitle failed to handle %r', message)
-            summary = u'Unparsable message subtitle'
+    formatted_messages = []
+
+    if recipient.get('verbose', True):
+        summary_template = u'{number}.\t{short_title}'
+        message_template = u'({ts}) {short_title}\n- {link}\n\n{details}'
+        summary = [u'Digest Summary:', ]
+        for message_number, message in enumerate(messages, start=1):
+            try:
+                shortform = fedmsg.meta.msg2subtitle(message, **config.app_conf) or u''
+            except Exception:
+                _log.exception('fedmsg.meta.msg2subtitle failed to handle %r', message)
+                shortform = u'Unparsable message subtitle'
 
-        if recipient.get('verbose', True):
             try:
                 longform = fedmsg.meta.msg2long_form(message, **config.app_conf) or u''
             except Exception:
                 _log.exception('fedmsg.meta.msg2long_form failed to handle %r', message)
                 longform = u'Unparsable message details'
-        else:
-            longform = u''
 
-        try:
-            link = fedmsg.meta.msg2link(message, **config.app_conf) or u''
-        except Exception:
-            _log.exception('fedmsg.meta.msg2link failed to handle %r', message)
-            link = u'No link could be found in the message'
+            try:
+                link = fedmsg.meta.msg2link(message, **config.app_conf) or u''
+            except Exception:
+                _log.exception('fedmsg.meta.msg2link failed to handle %r', message)
+                link = u'No link could be found in the message'
 
-        timestamp = datetime.datetime.fromtimestamp(message['timestamp'])
-        content += message_template.format(
-            number=message_number, summary=summary, ts=timestamp.strftime('%c'),
-            link=link, details=longform, separator=separator)
+            timestamp = datetime.datetime.fromtimestamp(message['timestamp'])
+            summary.append(summary_template.format(number=message_number, short_title=shortform))
+            formatted_messages.append(message_template.format(
+                ts=timestamp.strftime('%c'), short_title=shortform,
+                link=link,
+                details=longform))
 
-    if len(content) > 20000000:
+        digest_content = u'\n'.join(summary) + separator + separator.join(formatted_messages)
+    else:
+        message_template = u'({ts}) {short_title}\n- {link}'
+        for message_number, message in enumerate(messages, start=1):
+            try:
+                shortform = fedmsg.meta.msg2subtitle(message, **config.app_conf) or u''
+            except Exception:
+                _log.exception('fedmsg.meta.msg2subtitle failed to handle %r', message)
+                shortform = u'Unparsable message subtitle'
+
+            try:
+                link = fedmsg.meta.msg2link(message, **config.app_conf) or u''
+            except Exception:
+                _log.exception('fedmsg.meta.msg2link failed to handle %r', message)
+                link = u'No link could be found in the message'
+
+            timestamp = datetime.datetime.fromtimestamp(message['timestamp'])
+            formatted_messages.append(message_template.format(
+                ts=timestamp.strftime('%c'), short_title=shortform,
+                link=link))
+
+        digest_content = separator.join(formatted_messages)
+
+    if len(digest_content) > 20000000:
         # This email is enormous, too large to be sent.
-        content = (u'This message digest was too large to be sent!\n'
-                   u'The following messages were batched:\n\n')
+        digest_content = (u'This message digest was too large to be sent!\n'
+                          u'The following messages were batched:\n\n')
         for msg in messages:
-            content += msg['msg_id'] + '\n'
+            digest_content += msg['msg_id'] + '\n'
 
-        if len(content) > 20000000:
+        if len(digest_content) > 20000000:
             # Even the briefest summary is too big
-            content = (u'The message digest was so large, not even a summary could be sent.\n'
-                       u'Consider adjusting your FMN settings.\n')
+            digest_content = (u'The message digest was so large, '
+                              u'not even a summary could be sent.\n'
+                              u'Consider adjusting your FMN settings.\n')
 
-    email_message.set_payload(content.encode('utf-8'), 'utf-8')
+    email_message.set_payload(digest_content.encode('utf-8'), 'utf-8')
     return email_message.as_string()
 
 

fmn/tests/test_formatters.py

@@ -617,7 +617,7 @@ def setUp(self):
                 "username": "vagrant",
             },
         ]
-        self.recipient = {
+        self.verbose_recipient = {
             "email address": "jeremy@jcline.org",
             "filter_id": 11,
             "filter_name": "test",
@@ -628,16 +628,27 @@ def setUp(self):
             "user": "jcline.id.fedoraproject.org",
             "verbose": True,
         }
+        self.not_verbose_recipient = {
+            "email address": "jeremy@jcline.org",
+            "filter_id": 11,
+            "filter_name": "test",
+            "filter_oneshot": False,
+            "markup_messages": False,
+            "shorten_links": False,
+            "triggered_by_links": False,
+            "user": "jcline.id.fedoraproject.org",
+            "verbose": False,
+        }
 
     def test_single_message(self):
         """Assert if the batch is of length one, it's the same as a plain email."""
         self.assertEqual(
-            formatters.email(self.messages[0], self.recipient),
-            formatters.email_batch([self.messages[0]], self.recipient),
+            formatters.email(self.messages[0], self.verbose_recipient),
+            formatters.email_batch([self.messages[0]], self.verbose_recipient),
         )
 
-    def test_basic_batch(self):
-        """Assert a well-formed email is returned from a basic message."""
+    def test_basic_batch_verbose(self):
+        """Assert a well-formed verbose email is returned from a basic message."""
         expected = (
             'Precedence: Bulk\n'
             'Auto-Submitted: auto-generated\n'
@@ -653,16 +664,45 @@ def test_basic_batch(self):
             'Content-Type: text/plain; charset="utf-8"\n'
             'Content-Transfer-Encoding: base64\n\n'
             'RGlnZXN0IFN1bW1hcnk6CjEuCWpjbGluZSB1cGRhdGVkIHRoZSBydWxlcyBvbiBhIGZtbiBlbWFp\n'
-            'bCBmaWx0ZXIgKEZyaSBPY3QgIDYgMTc6MjU6MzAgMjAxNykKCS0gaHR0cHM6Ly9hcHBzLmZlZG9y\n'
-            'YXByb2plY3Qub3JnL25vdGlmaWNhdGlvbnMvCmpjbGluZSB1cGRhdGVkIHRoZSBydWxlcyBvbiBh\n'
-            'IGZtbiBlbWFpbCBmaWx0ZXIKCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t\n'
-            'LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KCjIuCWJvd2xvZmVnZ3Mg\n'
-            'dXBkYXRlZCB0aGUgcnVsZXMgb24gYSBmbW4gZW1haWwgZmlsdGVyIChGcmkgT2N0ICA2IDE3OjI1\n'
-            'OjMwIDIwMTcpCgktIGh0dHBzOi8vYXBwcy5mZWRvcmFwcm9qZWN0Lm9yZy9ub3RpZmljYXRpb25z\n'
-            'Lwpib3dsb2ZlZ2dzIHVwZGF0ZWQgdGhlIHJ1bGVzIG9uIGEgZm1uIGVtYWlsIGZpbHRlcgoKLS0t\n'
-            'LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t\n'
-            'LS0tLS0tLS0tLS0tLS0tLS0tLQoK\n'
+            'bCBmaWx0ZXIKMi4JYm93bG9mZWdncyB1cGRhdGVkIHRoZSBydWxlcyBvbiBhIGZtbiBlbWFpbCBm\n'
+            'aWx0ZXIKCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t\n'
+            'LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KCihGcmkgT2N0ICA2IDE3OjI1OjMwIDIwMTcp\n'
+            'IGpjbGluZSB1cGRhdGVkIHRoZSBydWxlcyBvbiBhIGZtbiBlbWFpbCBmaWx0ZXIKLSBodHRwczov\n'
+            'L2FwcHMuZmVkb3JhcHJvamVjdC5vcmcvbm90aWZpY2F0aW9ucy8KCmpjbGluZSB1cGRhdGVkIHRo\n'
+            'ZSBydWxlcyBvbiBhIGZtbiBlbWFpbCBmaWx0ZXIKCi0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t\n'
+            'LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KCihG\n'
+            'cmkgT2N0ICA2IDE3OjI1OjMwIDIwMTcpIGJvd2xvZmVnZ3MgdXBkYXRlZCB0aGUgcnVsZXMgb24g\n'
+            'YSBmbW4gZW1haWwgZmlsdGVyCi0gaHR0cHM6Ly9hcHBzLmZlZG9yYXByb2plY3Qub3JnL25vdGlm\n'
+            'aWNhdGlvbnMvCgpib3dsb2ZlZ2dzIHVwZGF0ZWQgdGhlIHJ1bGVzIG9uIGEgZm1uIGVtYWlsIGZp\n'
+            'bHRlcg==\n'
+        )
+
+        actual = formatters.email_batch(self.messages, self.verbose_recipient)
+        self.assertEqual(expected, actual)
+
+    def test_basic_batch_not_verbose(self):
+        """Assert a well-formed not verbose email is returned from a basic message."""
+        expected = (
+            'Precedence: Bulk\n'
+            'Auto-Submitted: auto-generated\n'
+            'From: notifications@fedoraproject.org\n'
+            'To: jeremy@jcline.org\n'
+            'X-Fedmsg-Topic: org.fedoraproject.dev.fmn.filter.update\n'
+            'X-Fedmsg-Category: fmn\n'
+            'X-Fedmsg-Username: jcline\n'
+            'X-Fedmsg-Username: bowlofeggs\n'
+            'X-Fedmsg-Num-Packages: 0\n'
+            'Subject: Fedora Notifications Digest (2 updates)\n'
+            'MIME-Version: 1.0\n'
+            'Content-Type: text/plain; charset="utf-8"\n'
+            'Content-Transfer-Encoding: base64\n\n'
+            'KEZyaSBPY3QgIDYgMTc6MjU6MzAgMjAxNykgamNsaW5lIHVwZGF0ZWQgdGhlIHJ1bGVzIG9uIGEg\n'
+            'Zm1uIGVtYWlsIGZpbHRlcgotIGh0dHBzOi8vYXBwcy5mZWRvcmFwcm9qZWN0Lm9yZy9ub3RpZmlj\n'
+            'YXRpb25zLwoKLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t\n'
+            'LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQoKKEZyaSBPY3QgIDYgMTc6MjU6MzAgMjAx\n'
+            'NykgYm93bG9mZWdncyB1cGRhdGVkIHRoZSBydWxlcyBvbiBhIGZtbiBlbWFpbCBmaWx0ZXIKLSBo\n'
+            'dHRwczovL2FwcHMuZmVkb3JhcHJvamVjdC5vcmcvbm90aWZpY2F0aW9ucy8=\n'
         )
 
-        actual = formatters.email_batch(self.messages, self.recipient)
+        actual = formatters.email_batch(self.messages, self.not_verbose_recipient)
         self.assertEqual(expected, actual)