launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03418
[Merge] lp:~bac/launchpad/nix-subscriber-list into lp:launchpad
Brad Crittenden has proposed merging lp:~bac/launchpad/nix-subscriber-list into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #768483 in Launchpad itself: "When using advanced subscriptions do not show 'Also notified'"
https://bugs.launchpad.net/launchpad/+bug/768483
For more details, see:
https://code.launchpad.net/~bac/launchpad/nix-subscriber-list/+merge/58723
= Summary =
With advanced bug subscriptions it is unclear whether a person will be
notified for any given change to a bug. Because of this new granularity
of notification, the list of subscribed people shown under 'Also
notified' is misleading. We feel the list is now of diminished
usefulness and are removing it.
== Proposed fix ==
Use a feature flag currently tied to the malone-alpha team to suppress
the generation of that portion of the portlet.
== Pre-implementation notes ==
Chat with Gary and Benji.
== Implementation details ==
Some changes were made to other parts of the test for clarity regarding
the use of feature flags.
== Tests ==
bin/test -vvm lp.bugs -t test_bugscription_views
== Demo and Q/A ==
= Launchpad lint =
I'll fix the lint.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/templates/bug-portlet-subscribers-content.pt
lib/lp/bugs/browser/bug.py
lib/lp/bugs/browser/tests/test_bugsubscription_views.py
./lib/lp/bugs/browser/tests/test_bugsubscription_views.py
190: local variable 'level' is assigned to but never used
210: local variable 'level' is assigned to but never used
235: local variable 'level' is assigned to but never used
269: local variable 'subscription' is assigned to but never used
340: local variable 'subscribe_view' is assigned to but never used
361: local variable 'subscribe_view' is assigned to but never used
450: local variable 'sub' is assigned to but never used
500: local variable 'mute_view' is assigned to but never used
--
https://code.launchpad.net/~bac/launchpad/nix-subscriber-list/+merge/58723
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bac/launchpad/nix-subscriber-list into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2011-04-05 17:55:44 +0000
+++ lib/lp/bugs/browser/bug.py 2011-04-21 17:50:52 +0000
@@ -275,7 +275,7 @@
def editsubscriptions(self):
"""Return the 'Edit subscriptions' Link."""
- text = 'Edit subscriptions'
+ text = 'Edit all of your subscriptions'
return Link(
'+subscriptions', text, icon='edit', summary=(
'View and change your subscriptions to this bug'))
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-04-15 11:02:24 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-04-21 17:50:52 +0000
@@ -15,26 +15,28 @@
BugSubscriptionSubscribeSelfView,
)
from lp.bugs.enum import BugNotificationLevel
+from lp.services.features.testing import FeatureFixture
from lp.testing import (
- feature_flags,
person_logged_in,
- set_feature_flag,
TestCaseWithFactory,
)
from lp.testing.views import create_initialized_view
+ON = 'on'
+OFF = None
+
+
class BugSubscriptionAdvancedFeaturesTestCase(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
+ feature_flag = 'malone.advanced-subscriptions.enabled'
def setUp(self):
super(BugSubscriptionAdvancedFeaturesTestCase, self).setUp()
self.bug = self.factory.makeBug()
self.person = self.factory.makePerson()
self.team = self.factory.makeTeam()
- with feature_flags():
- set_feature_flag(u'malone.advanced-subscriptions.enabled', u'on')
def test_subscribe_uses_bug_notification_level(self):
# When a user subscribes to a bug using the advanced features on
@@ -48,7 +50,7 @@
# We don't display BugNotificationLevel.NOTHING as an option.
# This is tested below.
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
displayed_levels = [
level for level in BugNotificationLevel.items
if level != BugNotificationLevel.NOTHING]
@@ -76,7 +78,7 @@
# someone is trying to subscribe.
bug = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
level = BugNotificationLevel.NOTHING
harness = LaunchpadFormHarness(
@@ -98,7 +100,7 @@
# BugSubscriptionSubscribeSelfView.
bug = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
bug.subscribe(person, person, BugNotificationLevel.COMMENTS)
# Now the person updates their subscription so they're
@@ -125,7 +127,7 @@
# BugSubscriptionSubscribeSelfView.
bug = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
bug.subscribe(person, person)
harness = LaunchpadFormHarness(
@@ -147,7 +149,7 @@
# level.
bug = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
# We subscribe using the harness rather than doing it
# directly so that we don't have to commit() between
@@ -183,7 +185,7 @@
# subscription that doesn't exist).
bug = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
level = BugNotificationLevel.METADATA
harness = LaunchpadFormHarness(
@@ -202,7 +204,7 @@
bug = self.factory.makeBug()
person = self.factory.makePerson()
team = self.factory.makeTeam(owner=person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
bug.subscribe(team, person)
level = BugNotificationLevel.METADATA
@@ -224,7 +226,7 @@
duplicate = self.factory.makeBug()
person = self.factory.makePerson()
team = self.factory.makeTeam(owner=person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
duplicate.markAsDuplicate(bug)
duplicate.subscribe(person, person)
@@ -246,7 +248,7 @@
bug = self.factory.makeBug()
duplicate = self.factory.makeBug()
person = self.factory.makePerson()
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
duplicate.markAsDuplicate(bug)
duplicate.subscribe(person, person)
@@ -267,7 +269,7 @@
subscription = bug.subscribe(
person, person, level=BugNotificationLevel.NOTHING)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(person):
subscribe_view = create_initialized_view(
bug.default_bugtask, name='+subscribe')
@@ -287,7 +289,7 @@
with person_logged_in(self.person):
self.bug.mute(self.person, self.person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(self.person):
subscribe_view = create_initialized_view(
self.bug.default_bugtask, name='+subscribe')
@@ -307,7 +309,7 @@
with person_logged_in(self.person):
self.bug.mute(self.person, self.person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(self.person):
subscribe_view = create_initialized_view(
self.bug.default_bugtask, name='+subscribe')
@@ -325,7 +327,7 @@
with person_logged_in(self.person):
self.bug.mute(self.person, self.person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(self.person):
level = BugNotificationLevel.METADATA
form_data = {
@@ -348,7 +350,7 @@
with person_logged_in(self.person):
muted_subscription = self.bug.mute(self.person, self.person)
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
with person_logged_in(self.person):
level = BugNotificationLevel.COMMENTS
form_data = {
@@ -368,7 +370,7 @@
# The bug_notification_level widget has a widget_class property
# that can be used to manipulate it with JavaScript.
with person_logged_in(self.person):
- with feature_flags():
+ with FeatureFixture({self.feature_flag: ON}):
subscribe_view = create_initialized_view(
self.bug.default_bugtask, name='+subscribe')
widget_class = (
@@ -377,6 +379,41 @@
'bug-notification-level-field', widget_class)
+class BugSubscriptionAdvancedFeaturesPortletTestCase(TestCaseWithFactory):
+
+ layer = LaunchpadFunctionalLayer
+ feature_flag = 'malone.advanced-subscriptions.enabled'
+
+ def setUp(self):
+ super(BugSubscriptionAdvancedFeaturesPortletTestCase, self).setUp()
+ self.bug = self.factory.makeBug()
+ self.person = self.factory.makePerson()
+ self.target = self.bug.default_bugtask.target
+ subscriber = self.factory.makePerson()
+ with person_logged_in(self.person):
+ self.target.addBugSubscription(subscriber, subscriber)
+
+ def get_contents(self, flag):
+ with person_logged_in(self.person):
+ with FeatureFixture({self.feature_flag: flag}):
+ # Subscribe someone to the target.
+ bug_view = create_initialized_view(
+ self.bug, name="+bug-portlet-subscribers-content")
+ return bug_view.render()
+
+ def test_also_notified_suppressed(self):
+ # If the advanced-subscription.enabled feature flag is on then the
+ # "Also notified" portion of the portlet is suppressed.
+ contents = self.get_contents(ON)
+ self.assertFalse('Also notified' in contents)
+
+ def test_also_notified_not_suppressed(self):
+ # If the advanced-subscription.enabled feature flag is off then the
+ # "Also notified" portion of the portlet is shown.
+ contents = self.get_contents(OFF)
+ self.assertTrue('Also notified' in contents)
+
+
class BugPortletSubcribersIdsTests(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
=== modified file 'lib/lp/bugs/templates/bug-portlet-subscribers-content.pt'
--- lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2011-02-03 05:14:54 +0000
+++ lib/lp/bugs/templates/bug-portlet-subscribers-content.pt 2011-04-21 17:50:52 +0000
@@ -57,15 +57,17 @@
<img src="/@@/spinner" />
</div>
</div>
- <div
- tal:condition="also_notified_subscribers"
- id="subscribers-indirect"
- class="Section"
- >
- <h2>Also notified</h2>
+ <tal:also_notified condition="not: request/features/malone.advanced-subscriptions.enabled">
<div
- tal:repeat="subscriber also_notified_subscribers"
- tal:content="structure subscriber/fmt:link"
- />
- </div>
+ tal:condition="also_notified_subscribers"
+ id="subscribers-indirect"
+ class="Section"
+ >
+ <h2>Also notified</h2>
+ <div
+ tal:repeat="subscriber also_notified_subscribers"
+ tal:content="structure subscriber/fmt:link"
+ />
+ </div>
+ </tal:also_notified>
</div>
