launchpad-reviewers team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/launchpad/prevent-bnl-nothing-oopses-bug-721400 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  #721400 Bug:+subscribe can't cope with existing BugSubscriptions with a bug_notification_level of NOTHING
  https://bugs.launchpad.net/bugs/721400

For more details, see:
https://code.launchpad.net/~gmb/launchpad/prevent-bnl-nothing-oopses-bug-721400/+merge/52036

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.
-- 
https://code.launchpad.net/~gmb/launchpad/prevent-bnl-nothing-oopses-bug-721400/+merge/52036
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/prevent-bnl-nothing-oopses-bug-721400 into lp:launchpad.

=== 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-03 10:56:12 +0000
@@ -122,7 +122,9 @@
         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 in
+                bug_notification_level_terms):
             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-03 10:56:12 +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):