Merge lp:~gmb/launchpad/prevent-bnl-nothing-oopses-bug-721400 into lp:launchpad

[Graham Binns]

[r=allenap][ui=none] [r=allenap][bug=721400] The BugTask:+subscribe page will no longer OOPS if the user already has a subscription with a bug_notification_level of NOTHING.

This branch fixes bug 721400, wherein the BugTask:+subscribe page would
OOPS if the user had an existing subscription with a
bug_notification_level of NOTHING. This value isn't one that's allowed
to be set using the UI, though it is allowed via the API, and the view's
validation code was kicking out the default value.

The fix for the bug was to ensure that we only use the user's existing
bug_notification_level as a default if it's in the set of values that we
allow to appear in the bug_notification_level widget.

I've added a regression test to cover this bug.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-02-22 17:17:35 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-03-04 11:03:47 +0000
@@ -39,7 +39,7 @@
     LaunchpadFormView,
     )
 from lp.bugs.browser.bug import BugViewMixin
-from lp.bugs.enum import BugNotificationLevel
+from lp.bugs.enum import BugNotificationLevel, HIDDEN_BUG_NOTIFICATION_LEVELS
 from lp.bugs.interfaces.bugsubscription import IBugSubscription
 from lp.bugs.interfaces.bugtask import IBugTaskSet
 from lp.services import features
@@ -118,11 +118,13 @@
             # drop the NOTHING option since it just makes the UI
             # confusing.
             for level in sorted(BugNotificationLevel.items, reverse=True)
-                if level != BugNotificationLevel.NOTHING]
+                if level not in HIDDEN_BUG_NOTIFICATION_LEVELS]
         bug_notification_vocabulary = SimpleVocabulary(
             bug_notification_level_terms)
 
-        if self.current_user_subscription is not None:
+        if (self.current_user_subscription is not None and
+            self.current_user_subscription.bug_notification_level not in
+                HIDDEN_BUG_NOTIFICATION_LEVELS):
             default_value = (
                 self.current_user_subscription.bug_notification_level)
         else:

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-02-22 17:17:35 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-03-04 11:03:47 +0000
@@ -20,6 +20,7 @@
     set_feature_flag,
     TestCaseWithFactory,
     )
+from lp.testing.views import create_initialized_view
 
 
 class BugSubscriptionAdvancedFeaturesTestCase(TestCaseWithFactory):
@@ -250,6 +251,31 @@
                 self.assertFalse(
                     harness.view.widgets['bug_notification_level'].visible)
 
+    def test_bug_721400(self):
+        # If a subscription exists with a BugNotificationLevel of
+        # NOTHING the view will still render correctly, even though
+        # NOTHING is not accepted as a valid value for the
+        # bug_notification_level field.
+        # This is a regression test for bug 721400.
+        bug = self.factory.makeBug()
+        person = self.factory.makePerson()
+        with person_logged_in(person):
+            subscription = bug.subscribe(
+                person, person, level=BugNotificationLevel.NOTHING)
+
+        with feature_flags():
+            with person_logged_in(person):
+                subscribe_view = create_initialized_view(
+                    bug.default_bugtask, name='+subscribe')
+                self.assertEqual(0, len(subscribe_view.errors))
+                bug_notification_level_widget = (
+                    subscribe_view.widgets['bug_notification_level'])
+                default_notification_level_value = (
+                    bug_notification_level_widget._getDefault())
+                self.assertEqual(
+                    BugNotificationLevel.COMMENTS,
+                    default_notification_level_value)
+
 
 class BugPortletSubcribersIdsTests(TestCaseWithFactory):
 

=== modified file 'lib/lp/bugs/enum.py'
--- lib/lp/bugs/enum.py	2011-02-01 17:52:45 +0000
+++ lib/lp/bugs/enum.py	2011-03-04 11:03:47 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 __all__ = [
     'BugNotificationLevel',
+    'HIDDEN_BUG_NOTIFICATION_LEVELS',
     ]
 
 from lazr.enum import (
@@ -47,3 +48,7 @@
         notifications about new events in the bugs's discussion, like new
         comments.
         """)
+
+
+# The set of bug notification levels that won't be displayed in the UI.
+HIDDEN_BUG_NOTIFICATION_LEVELS = [BugNotificationLevel.NOTHING]