Merge lp:~gmb/launchpad/team-subscription-opt-out-apis into lp:launchpad/db-devel

[Graham Binns]

[r=bac][ui=none][bug=751173] APIs have been added to IBugSubscriptionFilter to allow for muting and unmuting of subscriptions. These have also been exposed in the devel API.

This branch adds APIs around the BugSubscriptionFilterMute model. The APIs added are:

 - BugSubscriptionFilter.isMuteAllowed(person):
   Returns True if `person` can add a mute for the current filter.
 - BugSubscriptionFilter.mute(person):
   Add a mute for `person` and return it. If called for a user who is already
   muted, return the existing mute. Raise an error if isMuteAllowed() would
   return False for `person`.
 - BugSubscriptionFilter.unmute(person):
   Remove any mutes for `person` on the current Filter.

I've added tests to cover these APIs.

[Brad Crittenden (bac)]

Graham this branch looks great and is quite thorough. I like your idempotent tests.

review: Approve (code)

=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml	2011-04-04 12:34:00 +0000
+++ lib/lp/bugs/configure.zcml	2011-04-04 12:34:01 +0000
@@ -636,9 +636,11 @@
         class=".model.bugsubscriptionfilter.BugSubscriptionFilter">
       <allow
           interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes"/>
+      <allow
+          interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsPublic"/>
       <require
           permission="launchpad.Edit"
-          interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethods"
+          interface=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterMethodsProtected"
           set_schema=".interfaces.bugsubscriptionfilter.IBugSubscriptionFilterAttributes" />
     </class>
 

=== modified file 'lib/lp/bugs/interfaces/bugsubscriptionfilter.py'
--- lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2011-04-04 12:34:00 +0000
+++ lib/lp/bugs/interfaces/bugsubscriptionfilter.py	2011-04-04 12:34:01 +0000
@@ -11,9 +11,15 @@
 
 
 from lazr.restful.declarations import (
+    call_with,
     export_as_webservice_entry,
     export_destructor_operation,
+    export_read_operation,
+    export_write_operation,
     exported,
+    operation_for_version,
+    operation_parameters,
+    REQUEST_USER,
     )
 from lazr.restful.fields import Reference
 from zope.interface import Interface
@@ -35,6 +41,7 @@
 from lp.bugs.interfaces.structuralsubscription import (
     IStructuralSubscription,
     )
+from lp.registry.interfaces.person import IPerson
 from lp.services.fields import (
     PersonChoice,
     SearchTag,
@@ -99,8 +106,36 @@
             value_type=SearchTag()))
 
 
-class IBugSubscriptionFilterMethods(Interface):
-    """Methods of `IBugSubscriptionFilter`."""
+class IBugSubscriptionFilterMethodsPublic(Interface):
+    """Methods on `IBugSubscriptionFilter` that can be called by anyone."""
+
+    @call_with(person=REQUEST_USER)
+    @operation_parameters(
+        person=Reference(IPerson, title=_('Person'), required=True))
+    @export_read_operation()
+    @operation_for_version('devel')
+    def isMuteAllowed(person):
+        """Return True if this filter can be muted for `person`."""
+
+    @call_with(person=REQUEST_USER)
+    @operation_parameters(
+        person=Reference(IPerson, title=_('Person'), required=True))
+    @export_write_operation()
+    @operation_for_version('devel')
+    def mute(person):
+        """Add a mute for `person` to this filter."""
+
+    @call_with(person=REQUEST_USER)
+    @operation_parameters(
+        person=Reference(IPerson, title=_('Person'), required=True))
+    @export_write_operation()
+    @operation_for_version('devel')
+    def unmute(person):
+        """Remove any mute for `person` to this filter."""
+
+
+class IBugSubscriptionFilterMethodsProtected(Interface):
+    """Methods of `IBugSubscriptionFilter` that require launchpad.Edit."""
 
     @export_destructor_operation()
     def delete():
@@ -111,7 +146,8 @@
 
 
 class IBugSubscriptionFilter(
-    IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethods):
+    IBugSubscriptionFilterAttributes, IBugSubscriptionFilterMethodsProtected,
+    IBugSubscriptionFilterMethodsPublic):
     """A bug subscription filter."""
     export_as_webservice_entry()
 
@@ -119,6 +155,8 @@
 class IBugSubscriptionFilterMute(Interface):
     """A mute on an IBugSubscriptionFilter."""
 
+    export_as_webservice_entry()
+
     person = PersonChoice(
         title=_('Person'), required=True, vocabulary='ValidPersonOrTeam',
         readonly=True, description=_("The person subscribed."))

=== modified file 'lib/lp/bugs/model/bugsubscriptionfilter.py'
--- lib/lp/bugs/model/bugsubscriptionfilter.py	2011-04-04 12:34:00 +0000
+++ lib/lp/bugs/model/bugsubscriptionfilter.py	2011-04-04 12:34:01 +0000
@@ -49,6 +49,10 @@
 from lp.services.database.stormbase import StormBase
 
 
+class MuteNotAllowed(Exception):
+    """Raised when someone tries to mute a filter that can't be muted."""
+
+
 class BugSubscriptionFilter(StormBase):
     """A filter to specialize a *structural* subscription."""
 
@@ -249,6 +253,41 @@
             # subscription.
             self.structural_subscription.delete()
 
+    def isMuteAllowed(self, person):
+        """See `IBugSubscriptionFilter`."""
+        return (
+            self.structural_subscription.subscriber.isTeam() and
+            person.inTeam(self.structural_subscription.subscriber))
+
+    def mute(self, person):
+        """See `IBugSubscriptionFilter`."""
+        if not self.isMuteAllowed(person):
+            raise MuteNotAllowed(
+                "This subscription cannot be muted for %s" % person.name)
+
+        store = Store.of(self)
+        existing_mutes = store.find(
+            BugSubscriptionFilterMute,
+            BugSubscriptionFilterMute.filter_id == self.id,
+            BugSubscriptionFilterMute.person_id == person.id)
+        if not existing_mutes.is_empty():
+            return existing_mutes.one()
+        else:
+            mute = BugSubscriptionFilterMute()
+            mute.person = person
+            mute.filter = self.id
+            store.add(mute)
+            return mute
+
+    def unmute(self, person):
+        """See `IBugSubscriptionFilter`."""
+        store = Store.of(self)
+        existing_mutes = store.find(
+            BugSubscriptionFilterMute,
+            BugSubscriptionFilterMute.filter_id == self.id,
+            BugSubscriptionFilterMute.person_id == person.id)
+        existing_mutes.remove()
+
 
 class BugSubscriptionFilterMute(StormBase):
     """A filter to specialize a *structural* subscription."""

=== modified file 'lib/lp/bugs/tests/test_structuralsubscription.py'
--- lib/lp/bugs/tests/test_structuralsubscription.py	2011-03-21 18:23:31 +0000
+++ lib/lp/bugs/tests/test_structuralsubscription.py	2011-04-04 12:34:01 +0000
@@ -23,7 +23,11 @@
     BugTaskStatus,
     )
 from lp.bugs.mail.bugnotificationrecipients import BugNotificationRecipients
-from lp.bugs.model.bugsubscriptionfilter import BugSubscriptionFilter
+from lp.bugs.model.bugsubscriptionfilter import (
+    BugSubscriptionFilter,
+    BugSubscriptionFilterMute,
+    MuteNotAllowed,
+    )
 from lp.bugs.model.structuralsubscription import (
     get_structural_subscriptions_for_bug,
     get_structural_subscribers,
@@ -680,3 +684,108 @@
             [], list(
                 get_structural_subscribers(
                     bug, None, BugNotificationLevel.COMMENTS, None)))
+
+
+class TestBugSubscriptionFilterMute(TestCaseWithFactory):
+    """Tests for the BugSubscriptionFilterMute class."""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestBugSubscriptionFilterMute, self).setUp()
+        self.target = self.factory.makeProduct()
+        self.team = self.factory.makeTeam()
+        self.team_member = self.factory.makePerson()
+        with person_logged_in(self.team.teamowner):
+            self.team.addMember(self.team_member, self.team.teamowner)
+            self.team_subscription = self.target.addBugSubscription(
+                self.team, self.team.teamowner)
+            self.filter = self.team_subscription.bug_filters.one()
+
+    def test_isMuteAllowed_returns_true_for_team_subscriptions(self):
+        # BugSubscriptionFilter.isMuteAllowed() will return True for
+        # subscriptions where the owner of the subscription is a team.
+        self.assertTrue(self.filter.isMuteAllowed(self.team_member))
+
+    def test_isMuteAllowed_returns_false_for_non_team_subscriptions(self):
+        # BugSubscriptionFilter.isMuteAllowed() will return False for
+        # subscriptions where the owner of the subscription is not a team.
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            non_team_subscription = self.target.addBugSubscription(
+                person, person)
+        filter = non_team_subscription.bug_filters.one()
+        self.assertFalse(filter.isMuteAllowed(person))
+
+    def test_isMuteAllowed_returns_false_for_non_team_members(self):
+        # BugSubscriptionFilter.isMuteAllowed() will return False if the
+        # user passed to it is not a member of the subscribing team.
+        non_team_person = self.factory.makePerson()
+        self.assertFalse(self.filter.isMuteAllowed(non_team_person))
+
+    def test_mute_adds_mute(self):
+        # BugSubscriptionFilter.mute() adds a mute for the filter.
+        filter_id = self.filter.id
+        person_id = self.team_member.id
+        store = Store.of(self.filter)
+        mutes = store.find(
+            BugSubscriptionFilterMute,
+            BugSubscriptionFilterMute.filter == filter_id,
+            BugSubscriptionFilterMute.person == person_id)
+        self.assertTrue(mutes.is_empty())
+        self.filter.mute(self.team_member)
+        store.flush()
+
+    def test_unmute_removes_mute(self):
+        # BugSubscriptionFilter.unmute() removes any mute for a given
+        # person on that filter.
+        filter_id = self.filter.id
+        person_id = self.team_member.id
+        store = Store.of(self.filter)
+        self.filter.mute(self.team_member)
+        store.flush()
+        mutes = store.find(
+            BugSubscriptionFilterMute,
+            BugSubscriptionFilterMute.filter == filter_id,
+            BugSubscriptionFilterMute.person == person_id)
+        self.assertFalse(mutes.is_empty())
+        self.filter.unmute(self.team_member)
+        store.flush()
+        self.assertTrue(mutes.is_empty())
+
+    def test_mute_is_idempotent(self):
+        # Muting works even if the user is already muted.
+        store = Store.of(self.filter)
+        mute = self.filter.mute(self.team_member)
+        store.flush()
+        second_mute = self.filter.mute(self.team_member)
+        self.assertEqual(mute, second_mute)
+
+    def test_unmute_is_idempotent(self):
+        # Unmuting works even if the user is not muted
+        store = Store.of(self.filter)
+        mutes = store.find(
+            BugSubscriptionFilterMute,
+            BugSubscriptionFilterMute.filter == self.filter.id,
+            BugSubscriptionFilterMute.person == self.team_member.id)
+        self.assertTrue(mutes.is_empty())
+        self.filter.unmute(self.team_member)
+        self.assertTrue(mutes.is_empty())
+
+    def test_mute_raises_error_for_non_team_subscriptions(self):
+        # BugSubscriptionFilter.mute() will raise an error if called on
+        # a non-team subscription.
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            non_team_subscription = self.target.addBugSubscription(
+                person, person)
+        filter = non_team_subscription.bug_filters.one()
+        self.assertFalse(filter.isMuteAllowed(person))
+        self.assertRaises(MuteNotAllowed, 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)