Merge lp:~danilo/launchpad/bug-772763-remove-unmute-dialog-part1 into lp:launchpad/db-devel

[Данило Шеган]

[r=adeuring][bug=772763][incr] Ground-work for making "Unmute bug mail" unmute directly (and restoring any previous subscription) without popping up a dialog.

= Bug 772763: remove unmute dialog, part1 =

As part of solving 772763, we remove the pop-up unmute dialog (to directly unmute instead). To prepare for that, we first do some server-side cleanups and make unmute() method return the previously masked subscription (if any).

== Proposed fix ==

This branch does a few things:

 - Make "static" (i.e. non-JS) page IBugTask:+mute not redirect to +subscribe to offer unmute but instead allow direct unmuting
 - Ensures Mute/Unmute link is shown when bug is muted
 - Takes mutes into consideration for all subscribers-calculation (direct, duplicate, also-notified) (changes in model/bug.py)
 - Make unmute() method return a previous subscription (if any)

== Pre-implementation notes ==

This is mostly Gary's branch. I am only shepherding it and providing a few tests of my own.

== Implementation details ==

Not cleaning up the lint for the model/bug.py since it'd taint the diff.

== Tests ==

bin/test -cvvt TestBugSubscriptionInfo -t TestBugSubscriptionMethods -t BugMuteSelfViewTestCase -t TestBugPortletSubscribers

== Demo and Q/A ==

Check that IBugTask:+mute page behaves as expected (iow, allows unmuting as well). Rest of the QA will be possible only when the follow-up branch lands.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py
  lib/lp/bugs/browser/tests/test_bug_views.py
  lib/lp/bugs/browser/bug.py
  lib/lp/bugs/model/bug.py
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
  lib/lp/bugs/browser/bugsubscription.py
  lib/lp/bugs/tests/test_bug.py
  lib/lp/bugs/interfaces/bug.py

./lib/lp/bugs/model/bug.py
     575: E225 missing whitespace around operator
     747: E225 missing whitespace around operator
     751: E225 missing whitespace around operator
     766: E225 missing whitespace around operator
    1461: E225 missing whitespace around operator
    1674: E261 at least two spaces before inline comment
    1676: E261 at least two spaces before inline comment
    1687: E261 at least two spaces before inline comment
    1689: E261 at least two spaces before inline comment
    1707: E225 missing whitespace around operator
    2262: E225 missing whitespace around operator
    2277: E225 missing whitespace around operator
    2287: E261 at least two spaces before inline comment
    2325: E225 missing whitespace around operator
    2587: E225 missing whitespace around operator
    2628: E225 missing whitespace around operator

[Данило Шеган (danilo)]

review: Abstain (db)

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-05-20 16:15:35 +0000
+++ database/schema/security.cfg	2011-05-23 09:01:25 +0000
@@ -546,6 +546,7 @@
 public.bugcve                           = SELECT, INSERT
 public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT, UPDATE
+public.bugmute                          = SELECT
 public.bugnomination                    = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationfilter            = SELECT, INSERT
@@ -1238,6 +1239,7 @@
 public.bugcve                           = SELECT, INSERT
 public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT
+public.bugmute                          = SELECT
 public.bugnomination                    = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationfilter            = SELECT, INSERT
@@ -1341,6 +1343,7 @@
 public.bugcve                           = SELECT, INSERT
 public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT
+public.bugmute                          = SELECT
 public.bugnomination                    = SELECT
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationfilter            = SELECT, INSERT
@@ -1638,6 +1641,7 @@
 public.bugcve                           = SELECT, INSERT
 public.bugjob                           = SELECT, INSERT
 public.bugmessage                       = SELECT, INSERT
+public.bugmute                          = SELECT
 public.bugnomination                    = SELECT, INSERT, UPDATE
 public.bugnotification                  = SELECT, INSERT
 public.bugnotificationattachment        = SELECT
 
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-05-17 14:54:56 +0000
+++ lib/lp/bugs/browser/bug.py	2011-05-23 09:01:25 +0000
@@ -292,7 +292,7 @@
 
         return Link(
             link, text, icon='remove', summary=(
-                "Mute this bug so that you will never receive emails "
+                "Mute this bug so that you will not receive emails "
                 "about it."))
 
     def nominate(self):
@@ -538,9 +538,7 @@
         """Return True if the user should see the Mute link."""
         if features.getFeatureFlag('malone.advanced-subscriptions.enabled'):
             user_is_subscribed = (
-                # Note that we don't have to check for isMuted(), since
-                # if isMuted() is True isSubscribed() will also be
-                # True.
+                self.context.isMuted(self.user) or
                 self.context.isSubscribed(self.user) or
                 self.context.isSubscribedToDupes(self.user) or
                 self.context.personIsAlsoNotifiedSubscriber(self.user))
 
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-05-18 08:17:35 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-05-23 09:01:25 +0000
@@ -630,7 +630,10 @@
 
     @property
     def label(self):
-        return "Mute bug mail for bug %s" % self.context.bug.id
+        if self.context.bug.isMuted(self.user):
+            return "Unmute bug mail for bug %s" % self.context.bug.id
+        else:
+            return "Mute bug mail for bug %s" % self.context.bug.id
 
     page_title = label
 
@@ -641,15 +644,21 @@
     cancel_url = next_url
 
     def initialize(self):
+        self.is_muted = self.context.bug.isMuted(self.user)
         super(BugMuteSelfView, self).initialize()
-        # If the user is already muted, redirect them to the +subscribe
-        # page, since there's no point doing its work twice.
-        if self.context.bug.isMuted(self.user):
-            self.request.response.redirect(
-                canonical_url(self.context, view_name="+subscribe"))
 
-    @action('Mute bug mail', name='mute')
+    @action('Mute bug mail',
+            name='mute',
+            condition=lambda form, action: not form.is_muted)
     def mute_action(self, action, data):
         self.context.bug.mute(self.user, self.user)
         self.request.response.addInfoNotification(
             "Mail for bug #%s has been muted." % self.context.bug.id)
+
+    @action('Unmute bug mail',
+            name='unmute',
+            condition=lambda form, action: form.is_muted)
+    def unmute_action(self, action, data):
+        self.context.bug.unmute(self.user, self.user)
+        self.request.response.addInfoNotification(
+            "Mail for bug #%s has been unmuted." % self.context.bug.id)
 
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-17 14:54:56 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-05-23 09:01:25 +0000
@@ -173,7 +173,30 @@
                 self.assertTrue('mute_subscription' in html)
                 # The template uses user_should_see_mute_link to decide
                 # whether or not to display the mute link.
-                soup = BeautifulSoup(html)
                 self.assertTrue(
                     self._hasCSSClass(html, 'mute-link-container', 'hidden'),
                     'No "hidden" CSS class in mute-link-container.')
+
+    def test_mute_subscription_link_shown_if_muted(self):
+        # If a person is muted but not otherwise subscribed, they should still
+        # see the (un)mute link.
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            with FeatureFixture({self.feature_flag_1: 'on'}):
+                self.bug.mute(person, person)
+                # The user isn't subscribed already, but is muted.
+                self.assertFalse(self.bug.isSubscribed(person))
+                self.assertFalse(
+                    self.bug.personIsAlsoNotifiedSubscriber(
+                        person))
+                self.assertTrue(self.bug.isMuted(person))
+                view = create_initialized_view(
+                    self.bug, name="+portlet-subscribers")
+                self.assertTrue(view.user_should_see_mute_link,
+                                "User should see mute link.")
+                contents = view.render()
+                self.assertTrue('mute_subscription' in contents,
+                                "'mute_subscription' not in contents.")
+                self.assertFalse(
+                    self._hasCSSClass(
+                        contents, 'mute-link-container', 'hidden'))
 
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-05-17 11:44:33 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-05-23 09:01:25 +0000
@@ -8,7 +8,6 @@
 import transaction
 
 from canonical.launchpad.ftests import LaunchpadFormHarness
-from canonical.launchpad.webapp import canonical_url
 from canonical.testing.layers import LaunchpadFunctionalLayer
 
 from lp.bugs.browser.bugsubscription import (
@@ -409,6 +408,44 @@
         self.bug = self.factory.makeBug()
         self.person = self.factory.makePerson()
 
+    def test_is_muted_false(self):
+        # BugMuteSelfView initialization sets the is_muted property.
+        # When the person has not muted the bug, it's false.
+        with person_logged_in(self.person):
+            self.assertFalse(self.bug.isMuted(self.person))
+            view = create_initialized_view(
+                self.bug.default_bugtask, name="+mute")
+            self.assertFalse(view.is_muted)
+
+    def test_is_muted_true(self):
+        # BugMuteSelfView initialization sets the is_muted property.
+        # When the person has muted the bug, it's true.
+        with person_logged_in(self.person):
+            self.bug.mute(self.person, self.person)
+            self.assertTrue(self.bug.isMuted(self.person))
+            view = create_initialized_view(
+                self.bug.default_bugtask, name="+mute")
+            self.assertTrue(view.is_muted)
+
+    def test_label_nonmuted(self):
+        # Label to use for the button.
+        with person_logged_in(self.person):
+            self.assertFalse(self.bug.isMuted(self.person))
+            expected_label = "Mute bug mail for bug %s" % self.bug.id
+            view = create_initialized_view(
+                self.bug.default_bugtask, name="+mute")
+            self.assertEqual(expected_label, view.label)
+
+    def test_label_muted(self):
+        # Label to use for the button.
+        with person_logged_in(self.person):
+            self.bug.mute(self.person, self.person)
+            self.assertTrue(self.bug.isMuted(self.person))
+            expected_label = "Unmute bug mail for bug %s" % self.bug.id
+            view = create_initialized_view(
+                self.bug.default_bugtask, name="+mute")
+            self.assertEqual(expected_label, view.label)
+
     def test_bug_mute_self_view_mutes_bug(self):
         # The BugMuteSelfView mutes bug mail for the current user when
         # its form is submitted.
@@ -419,17 +456,13 @@
                 form={'field.actions.mute': 'Mute bug mail'})
             self.assertTrue(self.bug.isMuted(self.person))
 
-    def test_bug_mute_self_view_redirects_muted_users(self):
-        # The BugMuteSelfView redirects muted users to the +subscribe
-        # page, where they can remove their muted subscription or change
-        # their BugNotificationLevel.
+    def test_bug_mute_self_view_unmutes_bug(self):
+        # The BugMuteSelfView unmutes bug mail for the current user when
+        # its form is submitted and the bug was already muted.
         with person_logged_in(self.person):
             self.bug.mute(self.person, self.person)
-            mute_view = create_initialized_view(
-                self.bug.default_bugtask, name="+mute")
-            response = mute_view.request.response
-            self.assertEqual(302, response.getStatus())
-            self.assertEqual(
-                canonical_url(self.bug.default_bugtask,
-                    view_name="+subscribe"),
-                response.getHeader('Location'))
+            self.assertTrue(self.bug.isMuted(self.person))
+            create_initialized_view(
+                self.bug.default_bugtask, name="+mute",
+                form={'field.actions.unmute': 'Unmute bug mail'})
+            self.assertFalse(self.bug.isMuted(self.person))
 
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2011-05-18 08:17:35 +0000
+++ lib/lp/bugs/interfaces/bug.py	2011-05-23 09:01:25 +0000
@@ -509,7 +509,9 @@
     @export_write_operation()
     @operation_for_version('devel')
     def unmute(person, unmuted_by):
-        """Remove a muted subscription for `person`."""
+        """Remove a muted subscription for `person`.
+
+        Returns previously muted direct subscription, if any."""
 
     def getDirectSubscriptions():
         """A sequence of IBugSubscriptions directly linked to this bug."""
@@ -779,9 +781,9 @@
             schema=Interface, title=_('Target'), required=False),
         nominations=List(
             title=_("Nominations to search through."),
-            value_type=Reference(schema=Interface), # IBugNomination
+            value_type=Reference(schema=Interface),  # IBugNomination
             required=False))
-    @operation_returns_collection_of(Interface) # IBugNomination
+    @operation_returns_collection_of(Interface)  # IBugNomination
     @export_read_operation()
     def getNominations(target=None, nominations=None):
         """Return a list of all IBugNominations for this bug.
 
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2011-05-18 08:17:35 +0000
+++ lib/lp/bugs/model/bug.py	2011-05-23 09:01:25 +0000
@@ -54,6 +54,7 @@
     Count,
     Desc,
     Exists,
+    In,
     Join,
     LeftJoin,
     Max,
@@ -531,7 +532,7 @@
                     parent = message_by_id.get(parent.id, parent)
             else:
                 message, bugmessage = row
-                parent = None # parent attribute is not going to be accessed.
+                parent = None  # parent attribute is not going to be accessed.
             index = bugmessage.index
             result = IndexedMessage(message, inside, index, parent)
             if include_parents:
@@ -891,6 +892,7 @@
             person = unmuted_by
         mutes = self._getMutes(person)
         store.remove(mutes.one())
+        return self.getSubscriptionForPerson(person)
 
     @property
     def subscriptions(self):
@@ -2249,7 +2251,9 @@
         return IStore(BugSubscription).find(
             BugSubscription,
             BugSubscription.bug_notification_level >= self.level,
-            BugSubscription.bug == self.bug)
+            BugSubscription.bug == self.bug,
+            Not(In(BugSubscription.person_id,
+                   Select(BugMute.person_id, BugMute.bug_id==self.bug.id))))
 
     @cachedproperty
     @freeze(BugSubscriptionSet)
@@ -2262,12 +2266,14 @@
                 BugSubscription,
                 BugSubscription.bug_notification_level >= self.level,
                 BugSubscription.bug_id == Bug.id,
-                Bug.duplicateof == self.bug)
+                Bug.duplicateof == self.bug,
+                Not(In(BugSubscription.person_id,
+                       Select(BugMute.person_id, BugMute.bug_id==Bug.id))))
 
     @cachedproperty
     @freeze(BugSubscriptionSet)
     def duplicate_only_subscriptions(self):
-        """Subscripitions to duplicates of the bug.
+        """Subscriptions to duplicates of the bug.
 
         Excludes subscriptions for people who have a direct subscription or
         are also notified for another reason.
@@ -2308,11 +2314,15 @@
         if self.bug.private:
             return BugSubscriberSet()
         else:
+            muted = IStore(BugMute).find(
+                Person,
+                BugMute.person_id==Person.id,
+                BugMute.bug==self.bug)
             return BugSubscriberSet().union(
                 self.structural_subscriptions.subscribers,
                 self.all_pillar_owners_without_bug_supervisors,
                 self.all_assignees).difference(
-                self.direct_subscriptions.subscribers)
+                self.direct_subscriptions.subscribers).difference(muted)
 
     @cachedproperty
     def indirect_subscribers(self):
 
=== modified file 'lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py'
--- lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py	2011-05-16 16:57:55 +0000
+++ lib/lp/bugs/model/tests/test_bugsubscriptioninfo.py	2011-05-23 09:01:25 +0000
@@ -132,21 +132,42 @@
         return BugSubscriptionInfo(
             self.bug, BugNotificationLevel.LIFECYCLE)
 
+    def _create_direct_subscriptions(self):
+        subscribers = (
+            self.factory.makePerson(),
+            self.factory.makePerson())
+        with person_logged_in(self.bug.owner):
+            subscriptions = tuple(
+                self.bug.subscribe(subscriber, subscriber)
+                for subscriber in subscribers)
+        return subscribers, subscriptions
+
     def test_direct(self):
         # The set of direct subscribers.
-        subscribers = (
-            self.factory.makePerson(),
-            self.factory.makePerson())
-        with person_logged_in(self.bug.owner):
-            subscriptions = tuple(
-                self.bug.subscribe(subscriber, subscriber)
-                for subscriber in subscribers)
+        subscribers, subscriptions = self._create_direct_subscriptions()
         found_subscriptions = self.getInfo().direct_subscriptions
         self.assertEqual(set(subscriptions), found_subscriptions)
         self.assertEqual(subscriptions, found_subscriptions.sorted)
         self.assertEqual(set(subscribers), found_subscriptions.subscribers)
         self.assertEqual(subscribers, found_subscriptions.subscribers.sorted)
 
+    def test_muted_direct(self):
+        # If a direct is muted, it is not listed.
+        subscribers, subscriptions = self._create_direct_subscriptions()
+        with person_logged_in(subscribers[0]):
+            self.bug.mute(subscribers[0], subscribers[0])
+        found_subscriptions = self.getInfo().direct_subscriptions
+        self.assertEqual(set([subscriptions[1]]), found_subscriptions)
+
+    def _create_duplicate_subscription(self):
+        duplicate_bug = self.factory.makeBug(product=self.target)
+        with person_logged_in(duplicate_bug.owner):
+            duplicate_bug.markAsDuplicate(self.bug)
+            duplicate_bug_subscription = (
+                duplicate_bug.getSubscriptionForPerson(
+                    duplicate_bug.owner))
+        return duplicate_bug, duplicate_bug_subscription
+
     def test_duplicate(self):
         # The set of subscribers from duplicate bugs.
         found_subscriptions = self.getInfo().duplicate_subscriptions
@@ -154,12 +175,8 @@
         self.assertEqual((), found_subscriptions.sorted)
         self.assertEqual(set(), found_subscriptions.subscribers)
         self.assertEqual((), found_subscriptions.subscribers.sorted)
-        duplicate_bug = self.factory.makeBug(product=self.target)
-        with person_logged_in(duplicate_bug.owner):
-            duplicate_bug.markAsDuplicate(self.bug)
-            duplicate_bug_subscription = (
-                duplicate_bug.getSubscriptionForPerson(
-                    duplicate_bug.owner))
+        duplicate_bug, duplicate_bug_subscription = (
+            self._create_duplicate_subscription())
         found_subscriptions = self.getInfo().duplicate_subscriptions
         self.assertEqual(
             set([duplicate_bug_subscription]),
@@ -174,6 +191,15 @@
             (duplicate_bug.owner,),
             found_subscriptions.subscribers.sorted)
 
+    def test_muted_duplicate(self):
+        # If a duplicate is muted, it is not listed.
+        duplicate_bug, duplicate_bug_subscription = (
+            self._create_duplicate_subscription())
+        with person_logged_in(duplicate_bug.owner):
+            self.bug.mute(duplicate_bug.owner, duplicate_bug.owner)
+        found_subscriptions = self.getInfo().duplicate_subscriptions
+        self.assertEqual(set(), found_subscriptions)
+
     def test_duplicate_for_private_bug(self):
         # The set of subscribers from duplicate bugs is always empty when the
         # master bug is private.
@@ -274,11 +300,7 @@
             (bugtask.pillar.owner, bugtask2.pillar.owner),
             found_owners.sorted)
 
-    def test_also_notified_subscribers(self):
-        # The set of also notified subscribers.
-        found_subscribers = self.getInfo().also_notified_subscribers
-        self.assertEqual(set(), found_subscribers)
-        self.assertEqual((), found_subscribers.sorted)
+    def _create_also_notified_subscribers(self):
         # Add an assignee, a bug supervisor and a structural subscriber.
         bugtask = self.bug.default_bugtask
         assignee = self.factory.makePerson()
@@ -291,6 +313,15 @@
         with person_logged_in(structural_subscriber):
             bugtask.target.addSubscription(
                 structural_subscriber, structural_subscriber)
+        return assignee, supervisor, structural_subscriber
+
+    def test_also_notified_subscribers(self):
+        # The set of also notified subscribers.
+        found_subscribers = self.getInfo().also_notified_subscribers
+        self.assertEqual(set(), found_subscribers)
+        self.assertEqual((), found_subscribers.sorted)
+        assignee, supervisor, structural_subscriber = (
+            self._create_also_notified_subscribers())
         # Add a direct subscription.
         direct_subscriber = self.factory.makePerson()
         with person_logged_in(self.bug.owner):
@@ -305,6 +336,30 @@
             (assignee, supervisor, structural_subscriber),
             found_subscribers.sorted)
 
+    def test_muted_also_notified_subscribers(self):
+        # If someone is muted, they are not listed in the
+        # also_notified_subscribers.
+        assignee, supervisor, structural_subscriber = (
+            self._create_also_notified_subscribers())
+        # As a control, we first show that the
+        # the assignee, supervisor and structural subscriber do show up
+        # when they are not muted.
+        found_subscribers = self.getInfo().also_notified_subscribers
+        self.assertEqual(
+            set([assignee, supervisor, structural_subscriber]),
+            found_subscribers)
+        # Now we mute all of the subscribers.
+        with person_logged_in(assignee):
+            self.bug.mute(assignee, assignee)
+        with person_logged_in(supervisor):
+            self.bug.mute(supervisor, supervisor)
+        with person_logged_in(structural_subscriber):
+            self.bug.mute(structural_subscriber, structural_subscriber)
+        # Now we don't see them.
+        found_subscribers = self.getInfo().also_notified_subscribers
+        self.assertEqual(
+            set(), found_subscribers)
+
     def test_also_notified_subscribers_for_private_bug(self):
         # The set of also notified subscribers is always empty when the master
         # bug is private.
@@ -472,7 +527,7 @@
             self.info.all_pillar_owners_without_bug_supervisors
 
     def test_also_notified_subscribers(self):
-        with self.exactly_x_queries(5):
+        with self.exactly_x_queries(6):
             self.info.also_notified_subscribers
 
     def test_also_notified_subscribers_later(self):
@@ -482,11 +537,11 @@
         self.info.all_pillar_owners_without_bug_supervisors
         self.info.direct_subscriptions.subscribers
         self.info.structural_subscriptions.subscribers
-        with self.exactly_x_queries(0):
+        with self.exactly_x_queries(1):
             self.info.also_notified_subscribers
 
     def test_indirect_subscribers(self):
-        with self.exactly_x_queries(6):
+        with self.exactly_x_queries(7):
             self.info.indirect_subscribers
 
     def test_indirect_subscribers_later(self):
 
=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py	2011-05-18 08:17:35 +0000
+++ lib/lp/bugs/tests/test_bug.py	2011-05-23 09:01:25 +0000
@@ -105,6 +105,22 @@
             self.bug.unmute(self.person, self.person)
             self.assertFalse(self.bug.isMuted(self.person))
 
+    def test_unmute_returns_direct_subscription(self):
+        # Bug.unmute() returns the previously muted direct subscription, if
+        # any.
+        with person_logged_in(self.person):
+            self.bug.mute(self.person, self.person)
+            self.assertEqual(True, self.bug.isMuted(self.person))
+            self.assertEqual(None, self.bug.unmute(self.person, self.person))
+            self.assertEqual(False, self.bug.isMuted(self.person))
+            subscription = self.bug.subscribe(
+                self.person, self.person,
+                level=BugNotificationLevel.METADATA)
+            self.bug.mute(self.person, self.person)
+            self.assertEqual(True, self.bug.isMuted(self.person))
+            self.assertEqual(
+                subscription, self.bug.unmute(self.person, self.person))
+
     def test_unmute_mutes_unmuter(self):
         # When exposed in the web API, the unmute method regards the
         # first, `person` argument as optional, and the second