Merge lp:~sinzui/launchpad/project-notify-4 into lp:launchpad

[Curtis Hovey]

Add generic ProductNotificationJob to send email to product maintainers.

Launchpad bug: https://bugs.launchpad.net/bugs/956246
Pre-implementation: abentley, jcsackett

This branch adds a generic ProductNotificationJob track automated and manual
jobs that are scheduled for a product. The immediate use is for an automated
system that send out commercial subscription expiration emails at 4 weeks
and 1 week before expiration. After expiration an automated process will
update or deactivate the project and send an email.

A successful design will allow Lp support staff to create jobs during
project review such as to send an email about licensing issues, or
deactivate a problem project that also sends an email.

Subsequent branches will subclass ProductNotificationJob to send automated
emails to product maintainers

--------------------------------------------------------------------

RULES

    * Create a generic notification job the sends email to product
      maintainers.

QA

    None as this branch only contains the classes needed by other
    branches.

LINT

    lib/lp/registry/interfaces/productjob.py
    lib/lp/registry/model/productjob.py
    lib/lp/registry/tests/test_productjob.py

TEST

    ./bin/test -vvc -t ProductNotificationJob lp.registry.tests.test_productjob

IMPLEMENTATION

While I tried to write an actual job that did something, I discovered that
There was a lot of work needed to ensure only the correct people are
notified and provided the correct basic information. Subsequent branches
will only need to extend the run() method nd augment the message_data to
create specific notification jobs.
    lib/lp/registry/interfaces/productjob.py
    lib/lp/registry/model/productjob.py
    lib/lp/registry/tests/test_productjob.py

[Brad Crittenden (bac)]

Hi Curtis,

This system is going to be wonderful! I wish we'd done it a long time ago.

Here are some comments on the branch, mostly hygiene.

* s/then/that in """A job then sends a notification about a product."""

* s/geBodyAndHeaders/getBodyAndHeaders

* s/messages_data/message_data

* s/Sent an email/Send an email

* For IProductNotificationJobSource.create you don't document the 'subject' param.

* s/Subclasses that are updating products can want to make changes to the product before or after upcalling this classes' run() method./
    Subclasses that are updating products may make changes to the product before or after calling this class' run() method./

Is that what you intended to say?

* I think assertTrue() is more readable than assertIs(True,...). Do you prefer assertIs?

* I find the test of 'commercial' in the template name to be fragile. Couldn't you make it more explicit by adding a param to the create() method?

* As usual, the tests are logical, readable, and seem to be thorough -- great!

review: Approve

=== modified file 'lib/lp/registry/interfaces/productjob.py'
--- lib/lp/registry/interfaces/productjob.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/interfaces/productjob.py	2012-03-24 12:44:19 +0000
@@ -7,6 +7,8 @@
 __all__ = [
     'IProductJob',
     'IProductJobSource',
+    'IProductNotificationJob',
+    'IProductNotificationJobSource',
     ]
 
 from zope.interface import Attribute
@@ -64,3 +66,56 @@
             to be a class name.
         :return: A `ResultSet` yielding `IProductJob`.
         """
+
+
+class IProductNotificationJob(IProductJob):
+    """A job that sends a notification about a product."""
+
+    subject = Attribute('The subject line of the notification.')
+    email_template_name = Attribute(
+        'The name of the email template to create the message body from.')
+    reviewer = Attribute('The user or agent sending the email.')
+    recipients = Attribute('An `INotificationRecipientSet`.')
+    message_data = Attribute(
+        'A dict that is interpolated with the email template.')
+    reply_to = Attribute('The optional address to set as the Reply-To.')
+
+    def getBodyAndHeaders(email_template, address, reply_to=None):
+        """Return a tuple of email message body and headers.
+
+        The body is constructed from the email template and message_data.
+        The headers are a dict that includes the X-Launchpad-Rationale.
+
+        :param email_template: A string that will be interpolated
+            with message_data.
+        :param address: The email address of the user the message is to.
+        :reply_to: An optional email address to set as the Reply-To header.
+        :return a tuple (string, dict):
+        """
+
+    def sendEmailToMaintainer(template_name, subject, from_address):
+        """Send an email to the product maintainer.
+
+        :param email_template_name: The name of the email template to
+            use as the email body.
+        :param subject: The subject line of the notification.
+        :param from_address: The email address sending the email.
+
+        """
+
+
+class IProductNotificationJobSource(IProductJobSource):
+    """An interface for creating `IProductNotificationJob`s."""
+
+    def create(product, email_template_name, subject,
+               reviewer, reply_to_commercial=False):
+        """Create a new `IProductNotificationJob`.
+
+        :param product: An IProduct.
+        :param email_template_name: The name of the email template without
+            the extension.
+        :param subject: The subject line of the notification.
+        :param reviewer: The user or agent sending the email.
+        :param reply_to_commercial: Set the reply_to property to the
+            commercial email address.
+        """

=== modified file 'lib/lp/registry/model/productjob.py'
--- lib/lp/registry/model/productjob.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/model/productjob.py	2012-03-24 12:44:19 +0000
@@ -16,18 +16,23 @@
     Reference,
     Unicode,
     )
+from zope.component import getUtility
 from zope.interface import (
     classProvides,
     implements,
     )
 
 from lp.registry.enums import ProductJobType
+from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.product import IProduct
 from lp.registry.interfaces.productjob import (
     IProductJob,
     IProductJobSource,
+    IProductNotificationJob,
+    IProductNotificationJobSource,
     )
 from lp.registry.model.product import Product
+from lp.services.config import config
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import EnumCol
 from lp.services.database.lpstorm import (
@@ -35,8 +40,20 @@
     IStore,
     )
 from lp.services.database.stormbase import StormBase
+from lp.services.propertycache import cachedproperty
 from lp.services.job.model.job import Job
 from lp.services.job.runner import BaseRunnableJob
+from lp.services.mail.helpers import (
+    get_email_template,
+    )
+from lp.services.mail.notificationrecipientset import NotificationRecipientSet
+from lp.services.mail.mailwrapper import MailWrapper
+from lp.services.mail.sendmail import (
+    format_address,
+    format_address_for_person,
+    simple_sendmail,
+    )
+from lp.services.webapp.publisher import canonical_url
 
 
 class ProductJob(StormBase):
@@ -145,3 +162,123 @@
             ('product', self.context.product.name),
             ])
         return vars
+
+
+class ProductNotificationJob(ProductJobDerived):
+    """A Job that send an email to the product maintainer."""
+
+    implements(IProductNotificationJob)
+    classProvides(IProductNotificationJobSource)
+    class_job_type = ProductJobType.REVIEWER_NOTIFICATION
+
+    @classmethod
+    def create(cls, product, email_template_name,
+               subject, reviewer, reply_to_commercial=False):
+        """See `IProductNotificationJob`."""
+        metadata = {
+            'email_template_name': email_template_name,
+            'subject': subject,
+            'reviewer_id': reviewer.id,
+            'reply_to_commercial': reply_to_commercial,
+            }
+        return super(ProductNotificationJob, cls).create(product, metadata)
+
+    @property
+    def subject(self):
+        """See `IProductNotificationJob`."""
+        return self.metadata['subject']
+
+    @property
+    def email_template_name(self):
+        """See `IProductNotificationJob`."""
+        return self.metadata['email_template_name']
+
+    @cachedproperty
+    def reviewer(self):
+        """See `IProductNotificationJob`."""
+        return getUtility(IPersonSet).get(self.metadata['reviewer_id'])
+
+    @property
+    def reply_to_commercial(self):
+        """See `IProductNotificationJob`."""
+        return self.metadata['reply_to_commercial']
+
+    @cachedproperty
+    def reply_to(self):
+        """See `IProductNotificationJob`."""
+        if self.reply_to_commercial:
+            return 'Commercial <[e-mail]>'
+        return None
+
+    @cachedproperty
+    def recipients(self):
+        """See `IProductNotificationJob`."""
+        maintainer = self.product.owner
+        if maintainer.is_team:
+            team_name = maintainer.displayname
+            role = "an admin of %s which is the maintainer" % team_name
+            users = maintainer.adminmembers
+        else:
+            role = "the maintainer"
+            users = maintainer
+        reason = (
+            "You received this notification because you are %s of %s.\n%s" %
+            (role, self.product.displayname, self.message_data['product_url']))
+        header = 'Maintainer'
+        notification_set = NotificationRecipientSet()
+        notification_set.add(users, reason, header)
+        return notification_set
+
+    @cachedproperty
+    def message_data(self):
+        """See `IProductNotificationJob`."""
+        return {
+            'product_name': self.product.name,
+            'product_displayname': self.product.displayname,
+            'product_url': canonical_url(self.product),
+            'reviewer_name': self.reviewer.name,
+            'reviewer_displayname': self.reviewer.displayname,
+            }
+
+    def getErrorRecipients(self):
+        """See `BaseRunnableJob`."""
+        return [format_address_for_person(self.reviewer)]
+
+    def getBodyAndHeaders(self, email_template, address, reply_to=None):
+        """See `IProductNotificationJob`."""
+        reason, rationale = self.recipients.getReason(address)
+        maintainer = self.recipients._emailToPerson[address]
+        message_data = dict(self.message_data)
+        message_data['user_name'] = maintainer.name
+        message_data['user_displayname'] = maintainer.displayname
+        raw_body = email_template % message_data
+        raw_body += '\n\n-- \n%s' % reason
+        body = MailWrapper().format(raw_body, force_wrap=True)
+        headers = {
+            'X-Launchpad-Project':
+                '%(product_displayname)s (%(product_name)s)' % message_data,
+            'X-Launchpad-Message-Rationale': rationale,
+            }
+        if reply_to is not None:
+            headers['Reply-To'] = reply_to
+        return body, headers
+
+    def sendEmailToMaintainer(self, template_name, subject, from_address):
+        """See `IProductNotificationJob`."""
+        email_template = get_email_template(
+            "%s.txt" % template_name, app='registry')
+        for address in self.recipients.getEmails():
+            body, headers = self.getBodyAndHeaders(
+                email_template, address, self.reply_to)
+            simple_sendmail(from_address, address, subject, body, headers)
+
+    def run(self):
+        """See `BaseRunnableJob`.
+
+         Subclasses that are updating products may make changes to the product
+         before or after calling this class' run() method.
+        """
+        from_address = format_address(
+            'Launchpad', config.canonical.noreply_from_address)
+        self.sendEmailToMaintainer(
+            self.email_template_name, self.subject, from_address)

=== modified file 'lib/lp/registry/tests/test_productjob.py'
--- lib/lp/registry/tests/test_productjob.py	2012-03-22 23:21:24 +0000
+++ lib/lp/registry/tests/test_productjob.py	2012-03-24 12:44:19 +0000
@@ -21,16 +21,25 @@
 from lp.registry.interfaces.productjob import (
     IProductJob,
     IProductJobSource,
+    IProductNotificationJobSource,
     )
+from lp.registry.interfaces.person import TeamSubscriptionPolicy
+from lp.registry.interfaces.teammembership import TeamMembershipStatus
 from lp.registry.model.productjob import (
     ProductJob,
     ProductJobDerived,
-    )
-from lp.testing import TestCaseWithFactory
+    ProductNotificationJob,
+    )
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadZopelessLayer,
     )
+from lp.testing.mail_helpers import pop_notifications
+from lp.services.webapp.publisher import canonical_url
 
 
 class ProductJobTestCase(TestCaseWithFactory):
@@ -183,3 +192,180 @@
         oops_vars = job.getOopsVars()
         self.assertIs(True, len(oops_vars) > 1)
         self.assertIn(('product', product.name), oops_vars)
+
+
+class ProductNotificationJobTestCase(TestCaseWithFactory):
+    """Test case for the ProductNotificationJob class."""
+
+    layer = DatabaseFunctionalLayer
+
+    def make_notification_data(self):
+        product = self.factory.makeProduct()
+        reviewer = self.factory.makePerson('[e-mail]', name='reviewer')
+        subject = "test subject"
+        email_template_name = 'product-license-dont-know'
+        return product, email_template_name, subject, reviewer
+
+    def make_maintainer_team(self, product):
+        team = self.factory.makeTeam(
+            owner=product.owner,
+            subscription_policy=TeamSubscriptionPolicy.MODERATED)
+        team_admin = self.factory.makePerson()
+        with person_logged_in(team.teamowner):
+            team.addMember(
+                team_admin, team.teamowner, status=TeamMembershipStatus.ADMIN)
+            product.owner = team
+        return team, team_admin
+
+    def test_create(self):
+        # Create an instance of ProductNotificationJob that stores
+        # the notification information.
+        data = self.make_notification_data()
+        product, email_template_name, subject, reviewer = data
+        self.assertIs(
+            True,
+            IProductNotificationJobSource.providedBy(ProductNotificationJob))
+        job = ProductNotificationJob.create(
+            product, email_template_name, subject, reviewer,
+            reply_to_commercial=False)
+        self.assertIsInstance(job, ProductNotificationJob)
+        self.assertEqual(product, job.product)
+        self.assertEqual(email_template_name, job.email_template_name)
+        self.assertEqual(subject, job.subject)
+        self.assertEqual(reviewer, job.reviewer)
+        self.assertEqual(False, job.reply_to_commercial)
+
+    def test_getErrorRecipients(self):
+        # The reviewer is the error recipient.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        self.assertEqual(
+            ['Reviewer <[e-mail]>'], job.getErrorRecipients())
+
+    def test_reply_to_commercial(self):
+        # Commercial emails have the [e-mail] reply-to
+        # by setting the reply_to_commercial arg to True.
+        data = list(self.make_notification_data())
+        data.append(True)
+        job = ProductNotificationJob.create(*data)
+        self.assertEqual('Commercial <[e-mail]>', job.reply_to)
+
+    def test_reply_to_non_commercial(self):
+        # Non-commercial emails do not have a reply-to.
+        data = list(self.make_notification_data())
+        data.append(False)
+        job = ProductNotificationJob.create(*data)
+        self.assertIs(None, job.reply_to)
+
+    def test_recipients_user(self):
+        # The product maintainer is the recipient.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        recipients = job.recipients
+        self.assertEqual([product.owner], recipients.getRecipients())
+        reason, header = recipients.getReason(product.owner)
+        self.assertEqual('Maintainer', header)
+        self.assertIn(canonical_url(product), reason)
+        self.assertIn(
+            'you are the maintainer of %s' % product.displayname, reason)
+
+    def test_recipients_team(self):
+        # The product maintainer team admins are the recipient.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        team, team_admin = self.make_maintainer_team(product)
+        recipients = job.recipients
+        self.assertContentEqual(
+            [team.teamowner, team_admin], recipients.getRecipients())
+        reason, header = recipients.getReason(team.teamowner)
+        self.assertEqual('Maintainer', header)
+        self.assertIn(canonical_url(product), reason)
+        self.assertIn(
+            'you are an admin of %s which is the maintainer of %s' %
+            (team.displayname, product.displayname),
+            reason)
+
+    def test_message_data(self):
+        # The message_data is a dict of interpolatable strings.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        self.assertEqual(product.name, job.message_data['product_name'])
+        self.assertEqual(
+            product.displayname, job.message_data['product_displayname'])
+        self.assertEqual(
+            canonical_url(product), job.message_data['product_url'])
+        self.assertEqual(reviewer.name, job.message_data['reviewer_name'])
+        self.assertEqual(
+            reviewer.displayname, job.message_data['reviewer_displayname'])
+
+    def test_getBodyAndHeaders_with_reply_to(self):
+        # The body and headers contain reasons and rationales.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        [address] = job.recipients.getEmails()
+        email_template = (
+            'hello %(user_name)s %(product_name)s %(reviewer_name)s')
+        reply_to = '[e-mail]'
+        body, headers = job.getBodyAndHeaders(
+            email_template, address, reply_to)
+        self.assertIn(reviewer.name, body)
+        self.assertIn(product.name, body)
+        self.assertIn(product.owner.name, body)
+        self.assertIn('\n\n--\nYou received', body)
+        expected_headers = [
+            ('X-Launchpad-Project', '%s (%s)' %
+              (product.displayname, product.name)),
+            ('X-Launchpad-Message-Rationale', 'Maintainer'),
+            ('Reply-To', reply_to),
+            ]
+        self.assertContentEqual(expected_headers, headers.items())
+
+    def test_getBodyAndHeaders_without_reply_to(self):
+        # The reply-to is an optional argument.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        [address] = job.recipients.getEmails()
+        email_template = 'hello'
+        body, headers = job.getBodyAndHeaders(email_template, address)
+        expected_headers = [
+            ('X-Launchpad-Project', '%s (%s)' %
+              (product.displayname, product.name)),
+            ('X-Launchpad-Message-Rationale', 'Maintainer'),
+            ]
+        self.assertContentEqual(expected_headers, headers.items())
+
+    def test_sendEmailToMaintainer(self):
+        # sendEmailToMaintainer() sends an email to the maintainers.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        team, team_admin = self.make_maintainer_team(product)
+        addresses = job.recipients.getEmails()
+        pop_notifications()
+        job.sendEmailToMaintainer(email_template_name, 'frog', '[e-mail]')
+        notifications = pop_notifications()
+        self.assertEqual(2, len(notifications))
+        self.assertEqual(addresses[0], notifications[0]['To'])
+        self.assertEqual(addresses[1], notifications[1]['To'])
+        self.assertEqual('[e-mail]', notifications[1]['From'])
+        self.assertEqual('frog', notifications[1]['Subject'])
+
+    def test_run(self):
+        # sendEmailToMaintainer() sends an email to the maintainers.
+        data = self.make_notification_data()
+        job = ProductNotificationJob.create(*data)
+        product, email_template_name, subject, reviewer = data
+        [address] = job.recipients.getEmails()
+        pop_notifications()
+        job.run()
+        notifications = pop_notifications()
+        self.assertEqual(1, len(notifications))
+        self.assertEqual(address, notifications[0]['To'])
+        self.assertEqual(subject, notifications[0]['Subject'])
+        self.assertIn(
+            'Launchpad <[e-mail]>', notifications[0]['From'])