launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02547
[Merge] lp:~gary/launchpad/bug713392 into lp:launchpad
Gary Poster has proposed merging lp:~gary/launchpad/bug713392 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#713392 Event filters on structural subscriptions are not reported properly on +structural-subscriptions
https://bugs.launchpad.net/bugs/713392
For more details, see:
https://code.launchpad.net/~gary/launchpad/bug713392/+merge/48939
This branch fixes bug 713392 by making the +structural-subscriptions UI (e.g., https://bugs.launchpad.dev/~no-priv/+structural-subscriptions) correctly show the effect of an event filter setting.
While I find the +structural-subscriptions page difficult or even impossible to find via Launchpad's navigation, and we hope to replace it with another UI, it is available now to everyone, so I thought there was value in having it be correct.
I added code and tests for the situation when a structural subscription's bug notification level is NOTHING. This is not expected to happen, ever, so that was probably overkill.
Lint is happy.
To run tests, run ./bin/test -vvtTestBugSubscriptionFilterView
To QA, log in as a user that has malone.advanced-subscriptions.enabled true (on production, that means you are a member of the malone alpha team; on dev, on https://launchpad.dev/+feature-rules you can set "malone.advanced-subscriptions.enabled default 100 on" as foo.bar@xxxxxxxxxxxxx:test). Then make sure that user is structurally subscribed to some project. Then go to the +structural-subscriptions page for the logged in user (replace "no-user" with the proper id in "https://bugs.launchpad.dev/~no-priv/+structural-subscriptions";) and add a filter with a limited event subscription (see the bottom of the add page, and choose "a bug is fixed or re-opened" or "Any change is made to a bug other than a new comment being added."). Then click add, and verify that what you did is reflected in the +structural-subscriptions page.
--
https://code.launchpad.net/~gary/launchpad/bug713392/+merge/48939
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug713392 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/bugsubscriptionfilter.py 2011-02-03 10:35:36 +0000
+++ lib/lp/bugs/browser/bugsubscriptionfilter.py 2011-02-08 16:33:56 +0000
@@ -29,6 +29,19 @@
from lp.services.propertycache import cachedproperty
+def bug_notification_level_description_mapping(displayname):
+ return {
+ BugNotificationLevel.LIFECYCLE: (
+ "%s is fixed or re-opened." % displayname).capitalize(),
+ BugNotificationLevel.METADATA: (
+ "Any change is made to %s, other than a new "
+ "comment being added." % displayname),
+ BugNotificationLevel.COMMENTS: (
+ "A change is made or a new comment is added to %s."
+ % displayname),
+ }
+
+
class BugSubscriptionFilterView(LaunchpadView):
"""View for `IBugSubscriptionFilter`.
@@ -44,7 +57,7 @@
@property
def description(self):
- """Return the bug filter description.
+ """Return the bug subscription filter description.
Leading and trailing whitespace is trimmed. If the description is not
set the empty string is returned.
@@ -53,9 +66,31 @@
return u"" if description is None else description.strip()
@property
+ def filters_everything(self):
+ """Return a boolean as to whether the filter masks everything.
+
+ Right now the only thing we check is the bug_notification_level. We
+ could check more later--in particular, if no importances are checked,
+ or no statuses.
+ """
+ return (self.context.bug_notification_level ==
+ BugNotificationLevel.NOTHING)
+
+ @property
def conditions(self):
- """Descriptions of the bug filter's conditions."""
+ """Descriptions of the bug subscription filter's conditions."""
conditions = []
+ bug_notification_level = self.context.bug_notification_level
+ if bug_notification_level < BugNotificationLevel.COMMENTS:
+ if bug_notification_level == BugNotificationLevel.NOTHING:
+ return conditions # The template should use
+ # filters_everything to determine whether the conditions
+ # should be rendered.
+ else:
+ mapping = bug_notification_level_description_mapping(
+ 'the bug')
+ conditions.append(
+ mapping[bug_notification_level].lower()[:-1])
statuses = self.context.statuses
if len(statuses) > 0:
conditions.append(
@@ -103,17 +138,8 @@
@cachedproperty
def _bug_notification_level_descriptions(self):
- displayname = self.target.displayname
- return {
- BugNotificationLevel.LIFECYCLE: (
- "A bug in %s is fixed or re-opened." % displayname),
- BugNotificationLevel.METADATA: (
- "Any change is made to a bug in %s, other than a new "
- "comment being added." % displayname),
- BugNotificationLevel.COMMENTS: (
- "A change is made or a new comment is added to a bug in %s."
- % displayname),
- }
+ return bug_notification_level_description_mapping(
+ 'a bug in %s' % self.target.displayname)
def setUpFields(self):
"""Set up fields for form.
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py'
--- lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-02-04 01:15:13 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscriptionfilter.py 2011-02-08 16:33:56 +0000
@@ -250,6 +250,45 @@
# If nothing is set the conditions list is empty.
self.assertEqual([], self.view.conditions)
+ def test_conditions_with_no_events_subscribed(self):
+ with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.NOTHING)
+ self.assertEqual([], self.view.conditions)
+
+ def test_filters_everything_with_no_events_subscribed(self):
+ with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.NOTHING)
+ self.failUnless(self.view.filters_everything)
+
+ def test_not_filters_everything_normally(self):
+ self.failIf(self.view.filters_everything)
+
+ def test_conditions_for_COMMENTS_events(self):
+ # If we are subscribed to comments, that is all-inclusive: no
+ # conditions are returned.
+ self.assertEqual(BugNotificationLevel.COMMENTS,
+ self.subscription_filter.bug_notification_level)
+ self.assertEqual([], self.view.conditions)
+
+ def test_conditions_for_METADATA_events(self):
+ with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.METADATA)
+ self.assertEqual(
+ [u'any change is made to the bug, other than a new comment being '
+ 'added'],
+ self.view.conditions)
+
+ def test_conditions_for_LIFECYCLE_events(self):
+ with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.LIFECYCLE)
+ self.assertEqual(
+ [u'the bug is fixed or re-opened'],
+ self.view.conditions)
+
def test_conditions_for_statuses(self):
# If no statuses have been specified nothing is returned.
self.assertEqual([], self.view.conditions)
@@ -317,6 +356,8 @@
# If conditions are set but no description, the rendered description
# is very simple, and the conditions are described.
with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.METADATA)
self.subscription_filter.statuses = [
BugTaskStatus.NEW,
BugTaskStatus.CONFIRMED,
@@ -341,6 +382,14 @@
u"\u201cThe Wait\u201d allows all mail through.",
u"There are no filter conditions!")
+ def test_render_with_no_events_allowed(self):
+ with person_logged_in(self.owner):
+ self.subscription_filter.bug_notification_level = (
+ BugNotificationLevel.NOTHING)
+ self.assertRender(
+ u"This filter allows no mail through.",
+ u"")
+
def test_render_with_description_and_conditions(self):
# If a description is set it appears in the content of the dt tag,
# surrounded by "curly" quotes.
@@ -463,7 +512,8 @@
for level in displayed_levels:
person = self.factory.makePerson()
with person_logged_in(person):
- subscription = self.target.addBugSubscription(person, person)
+ subscription = self.target.addBugSubscription(
+ person, person)
form = {
"field.description": "New description",
"field.statuses": ["NEW", "INCOMPLETE"],
=== modified file 'lib/lp/bugs/templates/bug-subscription-filter.pt'
--- lib/lp/bugs/templates/bug-subscription-filter.pt 2011-01-14 11:01:00 +0000
+++ lib/lp/bugs/templates/bug-subscription-filter.pt 2011-02-08 16:33:56 +0000
@@ -4,7 +4,8 @@
xmlns:metal="http://xml.zope.org/namespaces/metal";
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
xml:lang="en" lang="en" dir="ltr"
- tal:define="conditions view/conditions"
+ tal:define="conditions view/conditions;
+ filters_everything view/filters_everything;"
tal:omit-tag="">
<dt tal:define="description view/description">
<span tal:condition="description">
@@ -17,7 +18,12 @@
allows mail through when:
</span>
<span tal:condition="not:conditions">
- allows all mail through.
+ <tal:condition condition="filters_everything">
+ allows no mail through.
+ </tal:condition>
+ <tal:condition condition="not:filters_everything">
+ allows all mail through.
+ </tal:condition>
</span>
</dt>
<dd tal:condition="conditions">
@@ -32,7 +38,9 @@
</tal:editable>
</dd>
<dd tal:condition="not:conditions">
- There are no filter conditions!
+ <tal:condition condition="not:filters_everything">
+ There are no filter conditions!
+ </tal:condition>
<tal:editable condition="context/required:launchpad.Edit">
<br /><a tal:attributes="href context/fmt:url/+edit">(edit)</a>
</tal:editable>
