Merge lp:~cjwatson/launchpad/mail-footer-pref into lp:launchpad

[Colin Watson]

Include filtering information in e-mail footers if PersonSettings.expanded_notification_footers is set.

Include filtering information in e-mail footers if PersonSettings.expanded_notification_footers is set.

Following https://code.launchpad.net/~cjwatson/launchpad/db-mail-footer-pref/+merge/264622, this models the new column, backfills it with False in a garbo job, and adds the relevant code to BaseMailer.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2015-07-08 15:13:48 +0000
+++ database/schema/security.cfg	2015-07-14 14:44:02 +0000
@@ -2279,6 +2279,7 @@
 public.openidconsumerassociation        = SELECT, DELETE
 public.openidconsumernonce              = SELECT, DELETE
 public.person                           = SELECT, DELETE
+public.personsettings                   = SELECT, UPDATE
 public.product                          = SELECT, UPDATE
 public.pofiletranslator                 = SELECT, INSERT, UPDATE, DELETE
 public.potranslation                    = SELECT, DELETE

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2015-07-09 20:06:17 +0000
+++ lib/lp/registry/browser/person.py	2015-07-14 14:44:02 +0000
@@ -2702,7 +2702,8 @@
 
     field_names = ['displayname', 'name', 'mugshot', 'description',
                    'hide_email_addresses', 'verbose_bugnotifications',
-                   'selfgenerated_bugnotifications']
+                   'selfgenerated_bugnotifications',
+                   'expanded_notification_footers']
     custom_widget('mugshot', ImageChangeWidget, ImageChangeWidget.EDIT_STYLE)
 
     label = 'Change your personal details'

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2015-05-11 13:17:41 +0000
+++ lib/lp/registry/interfaces/person.py	2015-07-14 14:44:02 +0000
@@ -552,6 +552,14 @@
         title=_("Send me bug notifications for changes I make"),
         required=False, default=False)
 
+    expanded_notification_footers = Bool(
+        title=_("Include filtering information in e-mail footers"),
+        description=_(
+            "Some e-mail clients do not allow filtering on arbitrary message "
+            "headers.  If you use one of these, you can set this option to "
+            "add more information to the end of message bodies."),
+        required=False, default=False)
+
 
 class IPersonPublic(IPrivacy):
     """Public attributes for a Person.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2015-07-09 20:06:17 +0000
+++ lib/lp/registry/model/person.py	2015-07-14 14:44:02 +0000
@@ -20,6 +20,7 @@
     'person_sort_key',
     'PersonLanguage',
     'PersonSet',
+    'PersonSettings',
     'SSHKey',
     'SSHKeySet',
     'TeamInvitationEvent',
@@ -105,10 +106,7 @@
 
 from lp import _
 from lp.answers.model.questionsperson import QuestionsPersonMixin
-from lp.app.enums import (
-    InformationType,
-    PRIVATE_INFORMATION_TYPES,
-    )
+from lp.app.enums import PRIVATE_INFORMATION_TYPES
 from lp.app.interfaces.launchpad import (
     IHasIcon,
     IHasLogo,
@@ -423,6 +421,8 @@
 
     selfgenerated_bugnotifications = BoolCol(notNull=True, default=False)
 
+    expanded_notification_footers = BoolCol(notNull=False, default=False)
+
 
 def readonly_settings(message, interface):
     """Make an object that disallows writes to values on the interface.

=== modified file 'lib/lp/registry/stories/person/xx-person-edit.txt'
--- lib/lp/registry/stories/person/xx-person-edit.txt	2012-01-15 13:32:27 +0000
+++ lib/lp/registry/stories/person/xx-person-edit.txt	2015-07-14 14:44:02 +0000
@@ -78,12 +78,19 @@
     False
     >>> self_generated_control.click()
 
+He will enable expanded mail notification footers.
+
+    >>> expanded_footer_control = browser.getControl(
+    ...     "Include filtering information in e-mail footers")
+    >>> expanded_footer_control.selected
+    False
+    >>> expanded_footer_control.click()
+
     >>> browser.getControl('Save').click()
     >>> browser.url
     'http://launchpad.dev/~rayjay'
 
-We now check to ensure that the verbose bug notifications option and
-self-generated bug notification option were changed:
+We now check to ensure that the various notifications options were changed:
 
     >>> browser.open('http://launchpad.dev/~rayjay/+edit')
     >>> browser.getControl("Include bug descriptions when sending me "
@@ -92,3 +99,6 @@
     >>> browser.getControl(
     ...     "Send me bug notifications for changes I make").selected
     True
+    >>> browser.getControl(
+    ...     "Include filtering information in e-mail footers").selected
+    True

=== modified file 'lib/lp/registry/tests/test_person.py'
--- lib/lp/registry/tests/test_person.py	2015-06-26 09:59:30 +0000
+++ lib/lp/registry/tests/test_person.py	2015-07-14 14:44:02 +0000
@@ -515,6 +515,11 @@
         user = self.factory.makePerson()
         self.assertFalse(user.selfgenerated_bugnotifications)
 
+    def test_expanded_notification_footers_false_by_default(self):
+        # Default for new accounts is to disable expanded notification footers.
+        user = self.factory.makePerson()
+        self.assertFalse(user.expanded_notification_footers)
+
     def test_canAccess__anonymous(self):
         # Anonymous users cannot call Person.canAccess()
         person = self.factory.makePerson()

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2015-07-13 00:52:36 +0000
+++ lib/lp/scripts/garbo.py	2015-07-14 14:44:02 +0000
@@ -70,7 +70,10 @@
     )
 from lp.hardwaredb.model.hwdb import HWSubmission
 from lp.registry.model.commercialsubscription import CommercialSubscription
-from lp.registry.model.person import Person
+from lp.registry.model.person import (
+    Person,
+    PersonSettings,
+    )
 from lp.registry.model.product import Product
 from lp.registry.model.teammembership import TeamMembership
 from lp.services.config import config
@@ -1411,6 +1414,29 @@
         """
 
 
+class PersonSettingsENFPopulator(BulkPruner):
+    """Populates PersonSettings.expanded_notification_footers."""
+
+    target_table_class = PersonSettings
+    ids_to_prune_query = """
+        SELECT person
+        FROM PersonSettings
+        WHERE expanded_notification_footers IS NULL
+        """
+
+    def __call__(self, chunk_size):
+        """See `ITunableLoop`."""
+        result = self.store.execute("""
+            UPDATE PersonSettings
+            SET expanded_notification_footers = FALSE
+            WHERE person IN (
+                SELECT * FROM
+                cursor_fetch('%s', %d) AS f(person integer))
+            """ % (self.cursor_name, chunk_size))
+        self._num_removed = result.rowcount
+        transaction.commit()
+
+
 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
     """Abstract base class to run a collection of TunableLoops."""
     script_name = None  # Script name for locking and database user. Override.
@@ -1694,6 +1720,7 @@
         LoginTokenPruner,
         ObsoleteBugAttachmentPruner,
         OldTimeLimitedTokenDeleter,
+        PersonSettingsENFPopulator,
         RevisionAuthorEmailLinker,
         ScrubPOFileTranslator,
         SuggestiveTemplatesCacheUpdater,

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2015-07-13 00:55:56 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2015-07-14 14:44:02 +0000
@@ -16,6 +16,7 @@
 import time
 
 from pytz import UTC
+from storm.exceptions import NoneError
 from storm.expr import (
     In,
     Like,
@@ -70,6 +71,7 @@
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.model.commercialsubscription import CommercialSubscription
+from lp.registry.model.person import PersonSettings
 from lp.registry.model.teammembership import TeamMembership
 from lp.scripts.garbo import (
     AntiqueSessionPruner,
@@ -1369,6 +1371,49 @@
         self._test_LiveFSFilePruner(
             'application/octet-stream', 0, expected_count=1)
 
+    def test_PersonSettingsENFPopulator(self):
+        switch_dbuser('testadmin')
+        store = IMasterStore(PersonSettings)
+        people_enf_none = []
+        people_enf_false = []
+        people_enf_true = []
+        for _ in range(2):
+            person = self.factory.makePerson()
+            try:
+                person.expanded_notification_footers = None
+            except NoneError:
+                # Now enforced by DB NOT NULL constraint; backfilling is no
+                # longer necessary.
+                return
+            people_enf_none.append(person)
+            person = self.factory.makePerson()
+            person.expanded_notification_footers = False
+            people_enf_false.append(person)
+            person = self.factory.makePerson()
+            person.expanded_notification_footers = True
+            people_enf_true.append(person)
+        settings_count = store.find(PersonSettings).count()
+        self.runDaily()
+        switch_dbuser('testadmin')
+
+        # No rows have been deleted.
+        self.assertEqual(settings_count, store.find(PersonSettings).count())
+
+        def _assert_enf_by_person(person, expected):
+            record = store.find(
+                PersonSettings, PersonSettings.person == person.id).one()
+            self.assertEqual(expected, record.expanded_notification_footers)
+
+        # Rows with expanded_notification_footers=None have been backfilled.
+        for person in people_enf_none:
+            _assert_enf_by_person(person, False)
+
+        # Other rows have been left alone.
+        for person in people_enf_false:
+            _assert_enf_by_person(person, False)
+        for person in people_enf_true:
+            _assert_enf_by_person(person, True)
+
 
 class TestGarboTasks(TestCaseWithFactory):
     layer = LaunchpadZopelessLayer

=== modified file 'lib/lp/services/mail/basemailer.py'
--- lib/lp/services/mail/basemailer.py	2012-06-29 08:40:05 +0000
+++ lib/lp/services/mail/basemailer.py	2015-07-14 14:44:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Base class for sending out emails."""
@@ -7,6 +7,7 @@
 
 __all__ = ['BaseMailer', 'RecipientReason']
 
+from collections import OrderedDict
 import logging
 from smtplib import SMTPException
 
@@ -77,6 +78,9 @@
         headers = self._getHeaders(email)
         subject = self._getSubject(email, recipient)
         body = self._getBody(email, recipient)
+        expanded_footer = self._getExpandedFooter(headers, recipient)
+        if expanded_footer:
+            body = append_footer(body, expanded_footer)
         ctrl = self._mail_controller_class(
             self.from_address, to_addresses, subject, body, headers,
             envelope_to=[email])
@@ -100,7 +104,8 @@
     def _getHeaders(self, email):
         """Return the mail headers to use."""
         reason, rationale = self._recipients.getReason(email)
-        headers = {'X-Launchpad-Message-Rationale': reason.mail_header}
+        headers = OrderedDict()
+        headers['X-Launchpad-Message-Rationale'] = reason.mail_header
         if self.notification_type is not None:
             headers['X-Launchpad-Notification-Type'] = self.notification_type
         reply_to = self._getReplyToAddress()
@@ -146,6 +151,16 @@
         """Provide a footer to attach to the body, or None."""
         return None
 
+    def _getExpandedFooter(self, headers, recipient):
+        """Provide an expanded footer for recipients who have requested it."""
+        if not recipient.expanded_notification_footers:
+            return None
+        lines = []
+        for key, value in headers.items():
+            if key.startswith('X-Launchpad-'):
+                lines.append('%s: %s\n' % (key[2:], value))
+        return ''.join(lines)
+
     def sendAll(self):
         """Send notifications to all recipients."""
         # We never want SMTP errors to propagate from this function.

=== modified file 'lib/lp/services/mail/tests/test_basemailer.py'
--- lib/lp/services/mail/tests/test_basemailer.py	2012-01-01 02:58:52 +0000
+++ lib/lp/services/mail/tests/test_basemailer.py	2015-07-14 14:44:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 """Tests for the BaseMailer class."""
 
@@ -7,6 +7,8 @@
 
 from smtplib import SMTPException
 
+from testtools.matchers import EndsWith
+
 from lp.services.mail.basemailer import BaseMailer
 from lp.services.mail.sendmail import MailController
 from lp.testing import TestCaseWithFactory
@@ -132,6 +134,33 @@
         self.assertEqual('text/plain', attachment['Content-Type'])
         self.assertEqual('inline', attachment['Content-Disposition'])
 
+    def test_generateEmail_append_no_expanded_footer(self):
+        # Recipients without expanded_notification_footers do not receive an
+        # expanded footer on messages.
+        fake_to = self.factory.makePerson(email='[e-mail]')
+        recipients = {fake_to: FakeSubscription()}
+        mailer = BaseMailerSubclass(
+            'subject', None, recipients, '[e-mail]',
+            notification_type='test')
+        ctrl = mailer.generateEmail('[e-mail]', fake_to)
+        self.assertNotIn('Launchpad-Message-Rationale', ctrl.body)
+
+    def test_generateEmail_append_expanded_footer(self):
+        # Recipients with expanded_notification_footers receive an expanded
+        # footer on messages.
+        fake_to = self.factory.makePerson(email='[e-mail]')
+        fake_to.expanded_notification_footers = True
+        recipients = {fake_to: FakeSubscription()}
+        mailer = BaseMailerSubclass(
+            'subject', None, recipients, '[e-mail]',
+            notification_type='test')
+        ctrl = mailer.generateEmail('[e-mail]', fake_to)
+        self.assertThat(
+            ctrl.body, EndsWith(
+                '\n-- \n'
+                'Launchpad-Message-Rationale: pete\n'
+                'Launchpad-Notification-Type: test\n'))
+
     def test_sendall_single_failure_doesnt_kill_all(self):
         # A failure to send to a particular email address doesn't stop sending
         # to others.