launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00216
lp:~gmb/launchpad/refactor-bugnotificationbuilder-bug-594211-part-2 into lp:launchpad/devel
Graham Binns has proposed merging lp:~gmb/launchpad/refactor-bugnotificationbuilder-bug-594211-part-2 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
BugNotification Refactoring: The saga continues
===============================================
Ah, what fun. This branch is the eventual culmination of several weeks'
development work. It's actually about two days' worth of work - there's
a whole lot of crap that I've had to revert to get tests to pass; I
realise now that my strategy was too ambitious in the first place, so
I've pared this branch down to the simplest it could be whilst still
adding value (shame I couldn't have hear Curtis' Epic talk two weeks
earlier).
Anyway.
This branch adds a BugNotificationRecipientReason class; this is the
next step in refactoring bug notifications to use BaseMailer. The new
class isn't hooked up to anything yet; I'll do that in a subsequent
branch since it seems to break every test *evar* (there are a lot of
tests that rely on notification reasons behaving like strings for some
reason).
I've added unittests to cover the new class.
No lint.
--
https://code.launchpad.net/~gmb/launchpad/refactor-bugnotificationbuilder-bug-594211-part-2/+merge/30649
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/refactor-bugnotificationbuilder-bug-594211-part-2 into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2010-07-22 12:17:41 +0000
+++ lib/lp/bugs/configure.zcml 2010-07-22 13:22:00 +0000
@@ -981,4 +981,12 @@
<class class="lp.bugs.model.bug.FileBugData">
<allow interface="lp.bugs.interfaces.bug.IFileBugData" />
</class>
+
+ <class class="lp.bugs.mail.bugnotificationrecipients.BugNotificationRecipientReason">
+ <allow attributes="
+ bug
+ getReason
+ mail_header
+ recipient"/>
+ </class>
</configure>
=== modified file 'lib/lp/bugs/mail/bugnotificationrecipients.py'
--- lib/lp/bugs/mail/bugnotificationrecipients.py 2010-07-09 19:41:05 +0000
+++ lib/lp/bugs/mail/bugnotificationrecipients.py 2010-07-22 13:22:00 +0000
@@ -12,10 +12,111 @@
from canonical.launchpad.interfaces import INotificationRecipientSet
+from lp.services.mail.basemailer import RecipientReason
from lp.services.mail.notificationrecipientset import (
NotificationRecipientSet)
+class BugNotificationRecipientReason(RecipientReason):
+ """A `RecipientReason` subsclass specifically for `BugNotification`s."""
+
+ def _getTemplateValues(self):
+ template_values = (
+ super(BugNotificationRecipientReason, self)._getTemplateValues())
+ if self.recipient != self.subscriber or self.subscriber.is_team:
+ template_values['entity_is'] = (
+ 'You are a member of %s, which is' %
+ self.subscriber.displayname)
+ template_values['lc_entity_is'] = (
+ 'you are a member of %s, which is' %
+ self.subscriber.displayname)
+ return template_values
+
+ @classmethod
+ def makeRationale(cls, rationale, person, duplicate_bug=None):
+ """See `RecipientReason.makeRationale`."""
+ rationale = RecipientReason.makeRationale(rationale, person)
+ if duplicate_bug is not None:
+ rationale = "%s via Bug %s" % (rationale, duplicate_bug.id)
+
+ return rationale
+
+ @classmethod
+ def _getReasonTemplate(cls, reason_string, duplicate_bug=None):
+ """Return a reason template to pass to __init__()."""
+ if duplicate_bug is not None:
+ reason_suffix = " (via bug %s)." % duplicate_bug.id
+ else:
+ reason_suffix = "."
+
+ reason_parts = {
+ 'prefix':
+ "You received this bug notification because %(lc_entity_is)s",
+ 'reason': reason_string,
+ 'suffix': reason_suffix,
+ }
+ return "%(prefix)s %(reason)s%(suffix)s" % reason_parts
+
+ @classmethod
+ def forDupeSubscriber(cls, person, duplicate_bug):
+ """Return a `BugNotificationRecipientReason` for a dupe subscriber.
+ """
+ header = cls.makeRationale(
+ 'Subscriber of Duplicate', person, duplicate_bug)
+
+ reason = cls._getReasonTemplate(
+ "a direct subscriber of a duplicate bug (via bug %s)" %
+ duplicate_bug.id)
+ return cls(person, person, header, reason)
+
+ @classmethod
+ def forDirectSubscriber(cls, person, duplicate_bug=None):
+ """Return a `BugNotificationRecipientReason` for a direct subscriber.
+ """
+ header = cls.makeRationale("Subscriber", person, duplicate_bug)
+ reason = cls._getReasonTemplate(
+ "a direct subscriber of the bug", duplicate_bug)
+ return cls(person, person, header, reason)
+
+ @classmethod
+ def forAssignee(cls, person, duplicate_bug=None):
+ """Return a `BugNotificationRecipientReason` for a bug assignee."""
+ header = cls.makeRationale("Assignee", person, duplicate_bug)
+ reason = cls._getReasonTemplate("a bug assignee", duplicate_bug)
+ return cls(person, person, header, reason)
+
+ @classmethod
+ def forBugSupervisor(cls, person, target, duplicate_bug=None):
+ """Return a `BugNotificationRecipientReason` for a bug supervisor."""
+ # All displaynames in these reasons should be changed to bugtargetname
+ # (as part of bug 113262) once bugtargetname is finalized for packages
+ # (bug 113258). Changing it before then would be excessively
+ # disruptive.
+ header = cls.makeRationale(
+ "Bug Supervisor (%s)" % target.displayname, person, duplicate_bug)
+ reason = cls._getReasonTemplate(
+ "the bug supervisor for %s" % target.displayname, duplicate_bug)
+ return cls(person, person, header, reason)
+
+ @classmethod
+ def forStructuralSubscriber(cls, person, target, duplicate_bug=None):
+ """Return a recipient reason for a structural subscriber."""
+ header = cls.makeRationale(
+ "Subscriber (%s)" % target.displayname, person, duplicate_bug)
+ reason = cls._getReasonTemplate(
+ "subscribed to %s" % target.displayname, duplicate_bug)
+ return cls(person, person, header, reason)
+
+ @classmethod
+ def forRegistrant(cls, person, target, duplicate_bug=None):
+ """Return a recipient reason for a registrant."""
+ header = cls.makeRationale(
+ "Registrant (%s)" % target.displayname, person, duplicate_bug)
+ reason = cls._getReasonTemplate(
+ "the registrant for %s" % target.displayname, duplicate_bug)
+ return cls(person, person, header, reason)
+
+
class BugNotificationRecipients(NotificationRecipientSet):
"""A set of emails and rationales notified for a bug change.
@@ -155,6 +256,3 @@
else:
text = "are the registrant for %s" % upstream.displayname
self._addReason(person, text, reason)
-
-
-
=== added file 'lib/lp/bugs/mail/tests/test_bugnotificationrecipients.py'
--- lib/lp/bugs/mail/tests/test_bugnotificationrecipients.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/mail/tests/test_bugnotificationrecipients.py 2010-07-22 13:22:00 +0000
@@ -0,0 +1,189 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the bugnotificationrecipients module."""
+
+__metaclass__ = type
+
+import unittest
+
+from canonical.testing import DatabaseFunctionalLayer
+
+from lp.bugs.mail.bugnotificationrecipients import (
+ BugNotificationRecipientReason)
+from lp.testing import TestCaseWithFactory
+
+
+class BugNotificationRecipientReasonTestCase(TestCaseWithFactory):
+ """A TestCase for the `BugNotificationRecipientReason` class."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(BugNotificationRecipientReasonTestCase, self).setUp()
+ self.person = self.factory.makePerson()
+ self.team = self.factory.makeTeam(owner=self.person)
+
+ def _assertReasonAndHeaderAreCorrect(self, recipient_reason,
+ expected_reason, expected_header):
+ self.assertEqual(expected_header, recipient_reason.mail_header)
+ self.assertEqual(expected_reason, recipient_reason.getReason())
+
+ def test_forDupeSubscriber(self):
+ duplicate_bug = self.factory.makeBug()
+ reason = BugNotificationRecipientReason.forDupeSubscriber(
+ self.person, duplicate_bug)
+
+ expected_header = (
+ 'Subscriber of Duplicate via Bug %s' % duplicate_bug.id)
+ expected_reason = (
+ 'You received this bug notification because you are a direct '
+ 'subscriber of a duplicate bug (via bug %s).' % duplicate_bug.id)
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forDupeSubscriber_for_team(self):
+ duplicate_bug = self.factory.makeBug()
+ reason = BugNotificationRecipientReason.forDupeSubscriber(
+ self.team, duplicate_bug)
+
+ expected_header = 'Subscriber of Duplicate @%s via Bug %s' % (
+ self.team.name, duplicate_bug.id)
+ expected_reason = (
+ 'You received this bug notification because you are a member of '
+ '%s, which is a direct subscriber of a duplicate bug (via bug '
+ '%s).' %
+ (self.team.displayname, duplicate_bug.id))
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forDirectSubscriber(self):
+ reason = BugNotificationRecipientReason.forDirectSubscriber(
+ self.person)
+
+ expected_header = "Subscriber"
+ expected_reason = (
+ "You received this bug notification because you are a direct "
+ "subscriber of the bug.")
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forDirectSubscriber_for_team(self):
+ reason = BugNotificationRecipientReason.forDirectSubscriber(
+ self.team)
+
+ expected_header = "Subscriber @%s" % self.team.name
+ expected_reason = (
+ "You received this bug notification because you are a member "
+ "of %s, which is a direct subscriber of the bug." %
+ self.team.displayname)
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forAssignee(self):
+ reason = BugNotificationRecipientReason.forAssignee(self.person)
+
+ expected_header = "Assignee"
+ expected_reason = (
+ "You received this bug notification because you are a bug "
+ "assignee.")
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forAssignee_for_team(self):
+ reason = BugNotificationRecipientReason.forAssignee(self.team)
+
+ expected_header = "Assignee @%s" % self.team.name
+ expected_reason = (
+ "You received this bug notification because you are a member "
+ "of %s, which is a bug assignee." % self.team.displayname)
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forBugSupervisor(self):
+ distro = self.factory.makeDistribution()
+ reason = BugNotificationRecipientReason.forBugSupervisor(
+ self.person, distro)
+
+ expected_header = "Bug Supervisor (%s)" % distro.displayname
+ expected_reason = (
+ "You received this bug notification because you are the bug "
+ "supervisor for %s." % distro.displayname)
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forBugSupervisor_for_team(self):
+ distro = self.factory.makeDistribution()
+ reason = BugNotificationRecipientReason.forBugSupervisor(
+ self.team, distro)
+
+ expected_header = "Bug Supervisor (%s) @%s" % (
+ distro.displayname, self.team.name)
+ expected_reason = (
+ "You received this bug notification because you are a member "
+ "of %s, which is the bug supervisor for %s." %
+ (self.team.displayname, distro.displayname))
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forStructuralSubscriber(self):
+ target = self.factory.makeProduct()
+ reason = BugNotificationRecipientReason.forStructuralSubscriber(
+ self.person, target)
+
+ expected_header = "Subscriber (%s)" % target.displayname
+ expected_reason = (
+ "You received this bug notification because you are subscribed "
+ "to %s." % target.displayname)
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forStructuralSubscriber_for_team(self):
+ target = self.factory.makeProduct()
+ reason = BugNotificationRecipientReason.forStructuralSubscriber(
+ self.team, target)
+
+ expected_header = "Subscriber (%s) @%s" % (
+ target.displayname, self.team.name)
+ expected_reason = (
+ "You received this bug notification because you are a member "
+ "of %s, which is subscribed to %s." %
+ (self.team.displayname, target.displayname))
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forRegistrant(self):
+ target = self.factory.makeProduct()
+ reason = BugNotificationRecipientReason.forRegistrant(
+ self.person, target)
+
+ expected_header = "Registrant (%s)" % target.displayname
+ expected_reason = (
+ "You received this bug notification because you are the "
+ "registrant for %s." % target.displayname)
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+ def test_forRegistrant_for_team(self):
+ target = self.factory.makeProduct()
+ reason = BugNotificationRecipientReason.forRegistrant(
+ self.team, target)
+
+ expected_header = "Registrant (%s) @%s" % (
+ target.displayname, self.team.name)
+ expected_reason = (
+ "You received this bug notification because you are a member "
+ "of %s, which is the registrant for %s." % (
+ self.team.displayname, target.displayname))
+
+ self._assertReasonAndHeaderAreCorrect(
+ reason, expected_reason, expected_header)
+
+
+def test_suite():
+ return unittest.TestLoader().loadTestsFromName(__name__)
