Merge lp:~stevenk/launchpad/mute-when-not-allowed into lp:launchpad

[Steve Kowalik]

MuteNotAllowed is now a bad request rather than an OOPS, and raise a specific error if team with a contact address calls BugSubscriptionFilter.mute().

Currently if you call mute on a BugSubscriptionFilter for a team that has a contact address set over the API you get a generic error message, and an OOPS for your trouble.

I have decorated MuteNotAllowed to be a bad request, and raise an error with a hint if the team has a contact address.

[Ian Booth (wallyworld)]

Thanks.

review: Approve

=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py	2012-08-02 04:46:31 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py	2012-09-19 05:12:19 +0000
@@ -12,7 +12,9 @@
     ]
 
 from itertools import chain
+import httplib
 
+from lazr.restful.declarations import error_status
 import pytz
 from storm.locals import (
     Bool,
@@ -44,6 +46,7 @@
 from lp.services.database.stormbase import StormBase
 
 
+@error_status(httplib.BAD_REQUEST)
 class MuteNotAllowed(Exception):
     """Raised when someone tries to mute a filter that can't be muted."""
 
@@ -252,6 +255,11 @@
 
     def mute(self, person):
         """See `IBugSubscriptionFilter`."""
+        subscriber = self.structural_subscription.subscriber
+        if subscriber.is_team and subscriber.preferredemail:
+            raise MuteNotAllowed(
+                "This subscription cannot be muted because team %s has a "
+                "contact address." % subscriber.name)
         if not self.isMuteAllowed(person):
             raise MuteNotAllowed(
                 "This subscription cannot be muted for %s" % person.name)

=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py	2012-08-08 11:48:29 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py	2012-09-19 05:12:19 +0000
@@ -925,12 +925,30 @@
             non_team_subscription = self.target.addBugSubscription(
                 person, person)
         filter = non_team_subscription.bug_filters.one()
+        expected = "This subscription cannot be muted for %s" % person.name
         self.assertFalse(filter.isMuteAllowed(person))
-        self.assertRaises(MuteNotAllowed, filter.mute, person)
+        self.assertRaisesWithContent(
+            MuteNotAllowed, expected, filter.mute, person)
 
     def test_mute_raises_error_for_non_team_members(self):
         # BugSubscriptionFilter.mute() will raise an error if called on
         # a subscription of which the calling person is not a member.
         non_team_person = self.factory.makePerson()
         self.assertFalse(self.filter.isMuteAllowed(non_team_person))
-        self.assertRaises(MuteNotAllowed, self.filter.mute, non_team_person)
+        expected = "This subscription cannot be muted for %s" % (
+            non_team_person.name,)
+        self.assertRaisesWithContent(
+            MuteNotAllowed, expected, self.filter.mute, non_team_person)
+        
+    def test_mute_on_team_with_contact_address(self):
+        # BugSubscriptionFilter.mute() will raise an error if called on
+        # a subscription if the team has a contact address.
+        person = self.factory.makePerson()
+        team = self.factory.makeTeam(email='[e-mail]', owner=person)
+        with person_logged_in(person):
+            ss = self.target.addBugSubscription(team, person)
+        filter = ss.bug_filters.one()
+        expected = ("This subscription cannot be muted because team %s has a "
+            "contact address." % team.name)
+        self.assertRaisesWithContent(
+            MuteNotAllowed, expected, filter.mute, person)