launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #01894
[Merge] lp:~gmb/launchpad/add-bnl-to-structsubs-bug-672507 into lp:launchpad/devel
Graham Binns has proposed merging lp:~gmb/launchpad/add-bnl-to-structsubs-bug-672507 into lp:launchpad/devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#672507 Add bug_notification_level to the structural +subscribe view
https://bugs.launchpad.net/bugs/672507
this branch makes the bug_notification_level field available in the
structural subscription ui when the
malone.advanced-subscriptions.enabled flag is turned on.
== .bzrignore ==
- i've added the generated testrunner configs to .bzrignore, since i
have strict commits enabled and they were annoying the hell out of
me.
== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==
- i've updated the bugsubscription_views tests after noticing that a
test that should have been carried out once per iteration of a loop
was happening outside the loop (as it happens it still passes, but
now the testing is more thorough).
== lib/lp/registry/browser/structuralsubscription.py ==
- i've made structuralsubscriptionview descend from
advancedsubscriptionmixin so that the bug_notification_level field
becomes available.
- i've added a current_user_subscription property, in accordance with
the requirements of advancedsubscriptionmixin.
- i've updated the callsites for
istructuralsubscriptiontarget.addbugsubscription() to also pass the
bug_notification_level where available.
== lib/lp/registry/browser/tests/test_structuralsubscription.py ==
- i've added tests to cover the changes to structuralsubscriptionview,
covering the subscription of people and teams to a
structuralsubscriptiontarget.
== lib/lp/registry/interfaces/structuralsubscription.py ==
- i've updated the declaration of
istructuralsubscriptiontarget.addbugsubscription() to include the
bug_notifcation_level field.
== lib/lp/registry/model/structuralsubscription.py ==
- i've updated the implementation of
istructuralsubscriptiontarget.addbugsubscription() to make use of the
bug_notification_level field.
There's no lint.
--
https://code.launchpad.net/~gmb/launchpad/add-bnl-to-structsubs-bug-672507/+merge/40522
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/add-bnl-to-structsubs-bug-672507 into lp:launchpad/devel.
=== modified file '.bzrignore'
--- .bzrignore 2010-11-08 05:22:26 +0000
+++ .bzrignore 2010-11-10 11:44:51 +0000
@@ -74,3 +74,5 @@
.project
.pydevproject
librarian.log
+configs/testrunner_*
+configs/testrunner-appserver_*
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2010-11-03 13:56:02 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2010-11-10 11:44:51 +0000
@@ -54,12 +54,12 @@
}
harness.submit('continue', form_data)
- subscription = bug.getSubscriptionForPerson(person)
- 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))
+ subscription = bug.getSubscriptionForPerson(person)
+ 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))
def test_nothing_is_not_a_valid_level(self):
# BugNotificationLevel.NOTHING isn't considered valid when
@@ -164,4 +164,5 @@
BugNotificationLevel.METADATA,
default_notification_level_value,
"Default value for bug_notification_level should be "
- "METADATA, is actually %s" % default_notification_level_value)
+ "METADATA, is actually %s" %
+ default_notification_level_value)
=== modified file 'lib/lp/registry/browser/structuralsubscription.py'
--- lib/lp/registry/browser/structuralsubscription.py 2010-09-14 19:16:47 +0000
+++ lib/lp/registry/browser/structuralsubscription.py 2010-11-10 11:44:51 +0000
@@ -32,6 +32,7 @@
from canonical.launchpad.webapp.authorization import check_permission
from canonical.launchpad.webapp.menu import Link
from canonical.widgets import LabeledMultiCheckBoxWidget
+from lp.bugs.browser.bugsubscription import AdvancedSubscriptionMixin
from lp.registry.enum import BugNotificationLevel
from lp.registry.interfaces.distributionsourcepackage import (
IDistributionSourcePackage,
@@ -45,7 +46,8 @@
from lp.services.propertycache import cachedproperty
-class StructuralSubscriptionView(LaunchpadFormView):
+class StructuralSubscriptionView(LaunchpadFormView,
+ AdvancedSubscriptionMixin):
"""View class for structural subscriptions."""
schema = IStructuralSubscriptionForm
@@ -55,6 +57,21 @@
override_title_breadcrumbs = True
+ @cachedproperty
+ def _bug_notification_level_descriptions(self):
+ return {
+ BugNotificationLevel.LIFECYCLE: (
+ "A bug in %s is fixed or re-opened." %
+ self.context.displayname),
+ BugNotificationLevel.METADATA: (
+ "Any change is made to a bug in %s, other than a new "
+ "comment being added." %
+ self.context.displayname),
+ BugNotificationLevel.COMMENTS: (
+ "A change is made or a new comment is added to a bug in %s."
+ % self.context.displayname),
+ }
+
@property
def page_title(self):
return 'Subscribe to Bugs in %s' % self.context.title
@@ -75,6 +92,7 @@
remove_other = self._createRemoveOtherSubscriptionsField()
if remove_other:
self.form_fields += form.Fields(remove_other)
+ self._setUpBugNotificationLevelField()
def _createTeamSubscriptionsField(self):
"""Create field with a list of the teams the user is a member of.
@@ -172,6 +190,11 @@
"""Return True, if the current user is subscribed."""
return self.isSubscribed(self.user)
+ @cachedproperty
+ def current_user_subscription(self):
+ """Return the subscription of the current user."""
+ return self.context.getSubscription(self.user)
+
def userCanAlter(self):
if self.context.userCanAlterBugSubscription(self.user, self.user):
return True
@@ -194,7 +217,9 @@
is_subscribed = self.isSubscribed(self.user)
subscribe = data['subscribe_me']
if (not is_subscribed) and subscribe:
- target.addBugSubscription(self.user, self.user)
+ target.addBugSubscription(
+ self.user, self.user,
+ data.get('bug_notification_level', None))
self.request.response.addNotification(
'You have subscribed to "%s". You will now receive an '
'e-mail each time someone reports or changes one of '
@@ -223,7 +248,8 @@
team for team in teams if self.isSubscribed(team))
for team in form_selected_teams - subscriptions:
- target.addBugSubscription(team, self.user)
+ target.addBugSubscription(
+ team, self.user, data.get('bug_notification_level', None))
self.request.response.addNotification(
'The %s team will now receive an e-mail each time '
'someone reports or changes a public bug in "%s".' % (
=== modified file 'lib/lp/registry/browser/tests/test_structuralsubscription.py'
--- lib/lp/registry/browser/tests/test_structuralsubscription.py 2010-10-04 19:50:45 +0000
+++ lib/lp/registry/browser/tests/test_structuralsubscription.py 2010-11-10 11:44:51 +0000
@@ -9,12 +9,16 @@
from zope.publisher.interfaces import NotFound
from canonical.launchpad.ftests import (
+ LaunchpadFormHarness,
login,
logout,
)
from canonical.launchpad.webapp.publisher import canonical_url
from canonical.launchpad.webapp.servers import StepsToGo
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.registry.browser.distribution import DistributionNavigation
from lp.registry.browser.distributionsourcepackage import (
DistributionSourcePackageNavigation,
@@ -24,10 +28,19 @@
from lp.registry.browser.product import ProductNavigation
from lp.registry.browser.productseries import ProductSeriesNavigation
from lp.registry.browser.project import ProjectNavigation
-from lp.testing import TestCaseWithFactory
+from lp.registry.browser.structuralsubscription import (
+ StructuralSubscriptionView)
+from lp.registry.enum import BugNotificationLevel
+from lp.testing import (
+ feature_flags,
+ person_logged_in,
+ set_feature_flag,
+ TestCaseWithFactory,
+ )
class FakeLaunchpadRequest(FakeRequest):
+
@property
def stepstogo(self):
"""See `IBasicLaunchpadRequest`."""
@@ -94,6 +107,7 @@
class TestProductSeriesStructuralSubscriptionTraversal(
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IProductSeries."""
+
def setUpTarget(self):
self.target = self.factory.makeProduct(name='fooix').newSeries(
self.eric, '0.1', '0.1')
@@ -103,6 +117,7 @@
class TestMilestoneStructuralSubscriptionTraversal(
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IMilestone."""
+
def setUpTarget(self):
self.target = self.factory.makeProduct(name='fooix').newSeries(
self.eric, '0.1', '0.1').newMilestone('0.1.0')
@@ -112,6 +127,7 @@
class TestProjectStructuralSubscriptionTraversal(
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IProjectGroup."""
+
def setUpTarget(self):
self.target = self.factory.makeProject(name='fooix-project')
self.navigation = ProjectNavigation
@@ -120,6 +136,7 @@
class TestDistributionStructuralSubscriptionTraversal(
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IDistribution."""
+
def setUpTarget(self):
self.target = self.factory.makeDistribution(name='debuntu')
self.navigation = DistributionNavigation
@@ -128,6 +145,7 @@
class TestDistroSeriesStructuralSubscriptionTraversal(
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IDistroSeries."""
+
def setUpTarget(self):
self.target = self.factory.makeDistribution(name='debuntu').newSeries(
'5.0', '5.0', '5.0', '5.0', '5.0', '5.0', None, self.eric)
@@ -138,6 +156,7 @@
StructuralSubscriptionTraversalTestBase):
"""Test IStructuralSubscription traversal from IDistributionSourcePackage.
"""
+
def setUpTarget(self):
debuntu = self.factory.makeDistribution(name='debuntu')
fooix = self.factory.makeSourcePackageName('fooix')
@@ -145,5 +164,123 @@
self.navigation = DistributionSourcePackageNavigation
+class TestStructuralSubscriptionAdvancedFeaturesBase(TestCaseWithFactory):
+ """A base class for testing advanced structural subscription features."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def setUp(self):
+ super(TestStructuralSubscriptionAdvancedFeaturesBase, self).setUp()
+ self.setUpTarget()
+ with feature_flags():
+ set_feature_flag(u'malone.advanced-subscriptions.enabled', u'on')
+
+ def setUpTarget(self):
+ self.target = self.factory.makeProduct()
+
+ def test_subscribe_uses_bug_notification_level(self):
+ # When advanced features are turned on for subscriptions a user
+ # can specify a bug_notification_level on the +subscribe form.
+ with feature_flags():
+ # We don't display BugNotificationLevel.NOTHING as an option.
+ displayed_levels = [
+ level for level in BugNotificationLevel.items
+ if level != BugNotificationLevel.NOTHING]
+ for level in displayed_levels:
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ harness = LaunchpadFormHarness(
+ self.target, StructuralSubscriptionView)
+ form_data = {
+ 'field.subscribe_me': 'on',
+ 'field.bug_notification_level': level.name,
+ }
+ harness.submit('save', form_data)
+ self.assertFalse(harness.hasErrors())
+
+ subscription = self.target.getSubscription(person)
+ 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))
+
+ def test_subscribe_uses_bug_notification_level_for_teams(self):
+ # The bug_notification_level field is also used when subscribing
+ # a team.
+ with feature_flags():
+ displayed_levels = [
+ level for level in BugNotificationLevel.items
+ if level != BugNotificationLevel.NOTHING]
+ for level in displayed_levels:
+ person = self.factory.makePerson()
+ team = self.factory.makeTeam(owner=person)
+ with person_logged_in(person):
+ harness = LaunchpadFormHarness(
+ self.target, StructuralSubscriptionView)
+ form_data = {
+ 'field.subscribe_me': '',
+ 'field.subscriptions_team': team.name,
+ 'field.bug_notification_level': level.name,
+ }
+ harness.submit('save', form_data)
+ self.assertFalse(harness.hasErrors())
+
+ subscription = self.target.getSubscription(team)
+ 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))
+
+ def test_nothing_is_not_a_valid_level(self):
+ # BugNotificationLevel.NOTHING isn't considered valid when a
+ # user is subscribing via the web UI.
+ person = self.factory.makePerson()
+ with feature_flags():
+ with person_logged_in(person):
+ harness = LaunchpadFormHarness(
+ self.target, StructuralSubscriptionView)
+ form_data = {
+ 'field.subscribe_me': 'on',
+ 'field.bug_notification_level': (
+ BugNotificationLevel.NOTHING),
+ }
+ harness.submit('save', form_data)
+ self.assertTrue(harness.hasErrors())
+
+
+class TestProductSeriesAdvancedSubscriptionFeatures(
+ TestStructuralSubscriptionAdvancedFeaturesBase):
+ """Test advanced subscription features for ProductSeries."""
+
+ def setUpTarget(self):
+ self.target = self.factory.makeProductSeries()
+
+
+class TestDistributionAdvancedSubscriptionFeatures(
+ TestStructuralSubscriptionAdvancedFeaturesBase):
+ """Test advanced subscription features for distributions."""
+
+ def setUpTarget(self):
+ self.target = self.factory.makeDistribution()
+
+
+class TestDistroSeriesAdvancedSubscriptionFeatures(
+ TestStructuralSubscriptionAdvancedFeaturesBase):
+ """Test advanced subscription features for DistroSeries."""
+
+ def setUpTarget(self):
+ self.target = self.factory.makeDistroSeries()
+
+
+class TestMilestoneAdvancedSubscriptionFeatures(
+ TestStructuralSubscriptionAdvancedFeaturesBase):
+ """Test advanced subscription features for Milestones."""
+
+ def setUpTarget(self):
+ self.target = self.factory.makeMilestone()
+
+
def test_suite():
return unittest.TestLoader().loadTestsFromName(__name__)
=== modified file 'lib/lp/registry/interfaces/structuralsubscription.py'
--- lib/lp/registry/interfaces/structuralsubscription.py 2010-10-25 13:24:39 +0000
+++ lib/lp/registry/interfaces/structuralsubscription.py 2010-11-10 11:44:51 +0000
@@ -217,7 +217,8 @@
required=False))
@call_with(subscribed_by=REQUEST_USER)
@export_factory_operation(IStructuralSubscription, [])
- def addBugSubscription(subscriber, subscribed_by):
+ def addBugSubscription(subscriber, subscribed_by,
+ bug_notification_level=None):
"""Add a bug subscription for this structure.
This method is used to create a new `IStructuralSubscription`
@@ -227,6 +228,9 @@
:subscriber: The IPerson who will be subscribed. If omitted,
subscribed_by will be used.
:subscribed_by: The IPerson creating the subscription.
+ :bug_notification_level: The BugNotificationLevel for the
+ subscription. If omitted, BugNotificationLevel.COMMENTS will be
+ used.
:return: The new bug subscription.
"""
=== modified file 'lib/lp/registry/model/structuralsubscription.py'
--- lib/lp/registry/model/structuralsubscription.py 2010-10-25 13:24:39 +0000
+++ lib/lp/registry/model/structuralsubscription.py 2010-11-10 11:44:51 +0000
@@ -368,7 +368,8 @@
return False
return True
- def addBugSubscription(self, subscriber, subscribed_by):
+ def addBugSubscription(self, subscriber, subscribed_by,
+ bug_notification_level=None):
"""See `IStructuralSubscriptionTarget`."""
# This is a helper method for creating a structural
# subscription and immediately giving it a full
@@ -381,7 +382,9 @@
subscribed_by.name, subscriber.name))
sub = self.addSubscription(subscriber, subscribed_by)
- sub.bug_notification_level = BugNotificationLevel.COMMENTS
+ if bug_notification_level is None:
+ bug_notification_level = BugNotificationLevel.COMMENTS
+ sub.bug_notification_level = bug_notification_level
return sub
def removeBugSubscription(self, subscriber, unsubscribed_by):
