launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01650
[Merge] lp:~gmb/launchpad/include-bnl-bug-651108 into lp:launchpad/devel
Graham Binns has proposed merging lp:~gmb/launchpad/include-bnl-bug-651108 into lp:launchpad/devel with lp:~gmb/launchpad/add-profiling-feature-flag as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
#651108 Update the bug +subscribe view to include the options for bug_notification_level
https://bugs.launchpad.net/bugs/651108
This branch adds a feature-flagged bug_notification_level field to BugSubscriptionSubscribeSelf View. This is the first step in allowing users to chose what level their direct subscriptions work at.
== lib/lp/bugs/browser/bugsubscription.py ==
- I've added the bug_notification_level field and feature-flagged it.
- I've updated the subscribe_action() and _handleSubscribe() methods to deal with bug_notification_levels correctly.
== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==
- I've added a test to show that when the malone.advanced-subscriptions.enabled flag is turned on, the bug_notification_level field appears in the UI.
== lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt ==
- I've added a unit test to show that when the advanced subscriptions features are turned on the bug_notification_level field is used to create a subscription at the appropriate level.
--
https://code.launchpad.net/~gmb/launchpad/include-bnl-bug-651108/+merge/39047
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/include-bnl-bug-651108 into lp:launchpad/devel.
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2010-10-19 10:29:00 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2010-10-21 14:43:22 +0000
@@ -18,8 +18,6 @@
from zope import formlib
from zope.app.form import CustomWidgetFactory
from zope.app.form.browser.itemswidgets import RadioWidget
-from zope.app.form.interfaces import IInputWidget
-from zope.app.form.utility import setUpWidget
from zope.schema import Choice
from zope.schema.vocabulary import (
SimpleTerm,
@@ -30,7 +28,6 @@
from canonical.launchpad.webapp import (
action,
canonical_url,
- custom_widget,
LaunchpadFormView,
LaunchpadView,
)
@@ -39,7 +36,6 @@
from canonical.launchpad.webapp.menu import structured
from lp.bugs.browser.bug import BugViewMixin
from lp.bugs.interfaces.bugsubscription import IBugSubscription
-from lp.registry.enum import BugNotificationLevel
from lp.services import features
from lp.services.propertycache import cachedproperty
@@ -85,10 +81,13 @@
"""A view to handle the +subscribe page for a bug."""
schema = IBugSubscription
- # XXX 2010-10-18 gmb bug=651108:
- # field_names is currently empty because we don't use any of
- # them. This will be amended in a subsequent branch.
- field_names = []
+
+ @property
+ def field_names(self):
+ if self._use_advanced_features:
+ return ['bug_notification_level']
+ else:
+ return []
@property
def next_url(self):
@@ -112,7 +111,6 @@
return next_url
cancel_url = next_url
- _redirecting_to_bug_list = False
@cachedproperty
def _subscribers_for_current_user(self):
@@ -128,6 +126,18 @@
self._subscriber_count_for_current_user = person_count
return persons_for_user.values()
+ @cachedproperty
+ def _use_advanced_features(self):
+ """Return True if advanced subscriptions features are enabled."""
+ return features.getFeatureFlag(
+ 'malone.advanced-subscriptions.enabled')
+
+ def initialize(self):
+ """See `LaunchpadFormView`."""
+ self._subscriber_count_for_current_user = 0
+ self._redirecting_to_bug_list = False
+ super(BugSubscriptionSubscribeSelfView, self).initialize()
+
def setUpFields(self):
"""See `LaunchpadFormView`."""
super(BugSubscriptionSubscribeSelfView, self).setUpFields()
@@ -162,10 +172,42 @@
__name__='subscription', title=_("Subscription options"),
vocabulary=subscription_vocabulary, required=True,
default=default_subscription_value)
- self.form_fields += formlib.form.Fields(subscription_field)
+
+ if self._use_advanced_features:
+ # We need to pop the bug_notification_level field out of
+ # form_fields so that we can put the subscription_field first in
+ # the list. formlib.form.Fields doesn't have an insert() method.
+ bug_notification_level_field = self.form_fields.select(
+ 'bug_notification_level')
+ self.form_fields = self.form_fields.omit('bug_notification_level')
+ self.form_fields += formlib.form.Fields(subscription_field)
+ self.form_fields += bug_notification_level_field
+ self.form_fields['bug_notification_level'].custom_widget = (
+ CustomWidgetFactory(RadioWidget))
+ else:
+ self.form_fields += formlib.form.Fields(subscription_field)
self.form_fields['subscription'].custom_widget = CustomWidgetFactory(
RadioWidget)
+ def setUpWidgets(self):
+ """See `LaunchpadFormView`."""
+ super(BugSubscriptionSubscribeSelfView, self).setUpWidgets()
+ if self._use_advanced_features:
+ if (not self.user_is_subscribed and
+ self._subscriber_count_for_current_user == 0):
+ # We hide the subscription widget if the user isn't
+ # subscribed, since we know who the subscriber is and we
+ # don't need to present them with a single radio button.
+ self.widgets['subscription'].visible = False
+ elif (not self.user_is_subscribed and
+ self._subscriber_count_for_current_user > 0):
+ # We show the subscription widget when the user is
+ # subscribed via a team, because they can either
+ # subscribe theirself or unsubscribe their team.
+ self.widgets['subscription'].visible = True
+ else:
+ self.widgets['bug_notification_level'].visible = False
+
@cachedproperty
def user_is_subscribed(self):
"""Is the user subscribed to this bug?"""
@@ -195,14 +237,18 @@
subscription_person = self.widgets['subscription'].getInputValue()
if (not self.user_is_subscribed and
(subscription_person == self.user)):
- self._handleSubscribe()
+ if self._use_advanced_features:
+ bug_notification_level = data['bug_notification_level']
+ else:
+ bug_notification_level = None
+ self._handleSubscribe(bug_notification_level)
else:
self._handleUnsubscribe(subscription_person)
self.request.response.redirect(self.next_url)
- def _handleSubscribe(self):
+ def _handleSubscribe(self, level=None):
"""Handle a subscribe request."""
- self.context.bug.subscribe(self.user, self.user)
+ self.context.bug.subscribe(self.user, self.user, level=level)
self.request.response.addNotification(
"You have been subscribed to this bug.")
=== added file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2010-10-21 14:43:22 +0000
@@ -0,0 +1,65 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for BugSubscription views."""
+
+__metaclass__ = type
+
+from storm.store import Store
+
+from canonical.launchpad.ftests import LaunchpadFormHarness
+from canonical.testing.layers import LaunchpadFunctionalLayer
+
+from lp.bugs.browser.bugsubscription import BugSubscriptionSubscribeSelfView
+from lp.bugs.model.bugsubscription import BugSubscription
+from lp.registry.enum import BugNotificationLevel
+from lp.services.features.testing import FeatureFixture
+from lp.testing import person_logged_in, TestCaseWithFactory
+
+
+class BugSubscriptionAdvancedFeaturesTestCase(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(BugSubscriptionAdvancedFeaturesTestCase, self).setUp()
+ self.useFixture(
+ FeatureFixture({
+ 'malone.advanced-subscriptions.enabled': 'on'}))
+
+ def _getBugSubscriptionForUserAndBug(self, user, bug):
+ """Return the BugSubscription for a given user, bug combination."""
+ store = Store.of(bug)
+ return store.find(
+ BugSubscription,
+ BugSubscription.person == user,
+ BugSubscription.bug == bug).one()
+
+ def test_subscribe_uses_bug_notification_level(self):
+ # When a user subscribes to a bug using the advanced features on
+ # the Bug +subscribe page, the bug notification level they
+ # choose is taken into account.
+ bug = self.factory.makeBug()
+ # We unsubscribe the bug's owner because if we don't there will
+ # be two COMMENTS-level subscribers.
+ with person_logged_in(bug.owner):
+ bug.unsubscribe(bug.owner, bug.owner)
+
+ for level in BugNotificationLevel.items:
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ harness = LaunchpadFormHarness(
+ bug.default_bugtask, BugSubscriptionSubscribeSelfView)
+ form_data = {
+ 'field.subscription': person.name,
+ 'field.bug_notification_level': level.name,
+ }
+ harness.submit('continue', form_data)
+
+ subscription = self._getBugSubscriptionForUserAndBug(
+ person, bug)
+ self.assertEqual(
+ level, subscription.bug_notification_level,
+ "Bug notification level of subscription should be %s, is "
+ "actually %s." % (
+ level.name, subscription.bug_notification_level.name))
=== added file 'lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-personal-subscriptions-advanced-features.txt 2010-10-21 14:43:22 +0000
@@ -0,0 +1,42 @@
+Advanced personal subscriptions
+-------------------------------
+
+There are advanced features for bug subscriptions. These require a
+feature flag to be turned on for them to be accessible.
+
+ >>> from lp.services.features.model import FeatureFlag, getFeatureStore
+ >>> feature_store = getFeatureStore()
+ >>> ignore = feature_store.add(FeatureFlag(
+ ... scope=u'default',
+ ... flag=u'malone.advanced-subscriptions.enabled',
+ ... value=u'on', priority=1))
+ >>> feature_store.flush()
+
+Now when a user visits the +subscribe page of a bug they are given the
+option to subscribe to the bug at a given BugNotificationLevel.
+
+ >>> from canonical.launchpad.webapp import canonical_url
+ >>> from lp.testing.sampledata import USER_EMAIL
+ >>> login(USER_EMAIL)
+ >>> bug = factory.makeBug()
+ >>> logout()
+ >>> user_browser.open(
+ ... canonical_url(bug.default_bugtask, view_name='+subscribe'))
+ >>> bug_notification_level_control = user_browser.getControl(
+ ... name='field.bug_notification_level')
+ >>> for control in bug_notification_level_control.controls:
+ ... print control.optionValue
+ NOTHING
+ LIFECYCLE
+ METADATA
+ COMMENTS
+
+The user can now subscribe to the bug at any of the given notification
+levels. In this case, they want to subscribe to just metadata updates:
+
+ >>> bug_notification_level_control.getControl('Details').click()
+ >>> user_browser.getControl('Continue').click()
+
+ >>> for message in find_tags_by_class(user_browser.contents, 'message'):
+ ... print extract_text(message)
+ You have been subscribed to this bug.
=== modified file 'lib/lp/services/memcache/doc/tales-cache.txt'
--- lib/lp/services/memcache/doc/tales-cache.txt 2010-10-21 14:43:20 +0000
+++ lib/lp/services/memcache/doc/tales-cache.txt 2010-10-21 14:43:22 +0000
@@ -349,23 +349,9 @@
Memcache in templates can be disabled entirely by setting the memcache flag to
'disabled'.
-<<<<<<< TREE
- >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
- >>> from lp.services.features.model import FeatureFlag, getFeatureStore
- >>> from lp.services.features import per_thread
- >>> from lp.services.features.flags import FeatureController
- >>> from lp.services.features.webapp import ScopesFromRequest
- >>> ignore = getFeatureStore().add(FeatureFlag(
- ... scope=u'default', flag=u'memcache', value=u'disabled',
- ... priority=1))
- >>> empty_request = LaunchpadTestRequest()
- >>> per_thread.features = FeatureController(
- ... ScopesFromRequest(empty_request).lookup)
-=======
>>> from lp.services.features.testing import FeatureFixture
>>> fixture = FeatureFixture({'memcache': 'disabled'})
>>> fixture.setUp()
->>>>>>> MERGE-SOURCE
And now what cached before will not cache.
