launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02995
[Merge] lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397 into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #735397 in Launchpad itself: "Update the +subscribe page to handle muted subscriptions properly"
https://bugs.launchpad.net/launchpad/+bug/735397
For more details, see:
https://code.launchpad.net/~gmb/launchpad/make-+subscribe-play-nice-bug-735397/+merge/53782
This branch makes the +subscribe form play nice with muted
subscriptions.
== lib/lp/bugs/browser/bugsubscription.py ==
== lib/lp/bugs/browser/tests/test_bugsubscription_views.py ==
== lib/lp/bugs/configure.zcml ==
== lib/lp/bugs/interfaces/bug.py ==
== lib/lp/bugs/javascript/bug_subscription.js ==
== lib/lp/bugs/javascript/bugtask_index_portlets.js ==
== lib/lp/bugs/model/bug.py ==
== lib/lp/bugs/templates/bug-subscription.pt ==
== lib/lp/bugs/tests/test_bug.py ==
--
https://code.launchpad.net/~gmb/launchpad/make-+subscribe-play-nice-bug-735397/+merge/53782
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/make-+subscribe-play-nice-bug-735397 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py 2011-03-10 12:42:35 +0000
+++ lib/lp/bugs/browser/bugsubscription.py 2011-03-17 10:49:37 +0000
@@ -222,9 +222,20 @@
@cachedproperty
def _update_subscription_term(self):
+ if self.user_is_muted:
+ label = "Unmute bug mail from this bug and subscribe me to it"
+ else:
+ label = "Update my current subscription"
return SimpleTerm(
- 'update-subscription', 'update-subscription',
- 'Update my current subscription')
+ 'update-subscription', 'update-subscription', label)
+
+ @cachedproperty
+ def _unsubscribe_current_user_term(self):
+ if self._use_advanced_features and self.user_is_muted:
+ label = "Unmute bug mail from this bug"
+ else:
+ label = 'Unsubscribe me from this bug'
+ return SimpleTerm(self.user, self.user.name, label)
@cachedproperty
def _subscription_field(self):
@@ -233,12 +244,12 @@
for person in self._subscribers_for_current_user:
if person.id == self.user.id:
if (self._use_advanced_features and
- self.user_is_subscribed_directly):
- subscription_terms.append(self._update_subscription_term)
+ (self.user_is_subscribed_directly or
+ self.user_is_muted)):
+ subscription_terms.append(
+ self._update_subscription_term)
subscription_terms.insert(
- 0, SimpleTerm(
- person, person.name,
- 'Unsubscribe me from this bug'))
+ 0, self._unsubscribe_current_user_term)
self_subscribed = True
else:
subscription_terms.append(
@@ -279,6 +290,8 @@
"""See `LaunchpadFormView`."""
super(BugSubscriptionSubscribeSelfView, self).setUpWidgets()
if self._use_advanced_features:
+ self.widgets['bug_notification_level'].widget_class = (
+ 'bug-notification-level-field')
if 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
@@ -298,14 +311,22 @@
self.widgets['bug_notification_level'].visible = False
@cachedproperty
+ def user_is_muted(self):
+ return self.context.bug.isMuted(self.user)
+
+ @cachedproperty
def user_is_subscribed_directly(self):
"""Is the user subscribed directly to this bug?"""
- return self.context.bug.isSubscribed(self.user)
+ return (
+ self.context.bug.isSubscribed(self.user) and not
+ self.user_is_muted)
@cachedproperty
def user_is_subscribed_to_dupes(self):
"""Is the user subscribed to dupes of this bug?"""
- return self.context.bug.isSubscribedToDupes(self.user)
+ return (
+ self.context.bug.isSubscribedToDupes(self.user) and not
+ self.user_is_muted)
@property
def user_is_subscribed(self):
@@ -347,8 +368,10 @@
bug_notification_level = None
if (subscription_person == self._update_subscription_term.value and
- self.user_is_subscribed):
+ (self.user_is_subscribed or self.user_is_muted)):
self._handleUpdateSubscription(level=bug_notification_level)
+ elif self.user_is_muted and subscription_person == self.user:
+ self._handleUnsubscribeCurrentUser()
elif (not self.user_is_subscribed and
(subscription_person == self.user)):
self._handleSubscribe(bug_notification_level)
=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-10 12:42:35 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py 2011-03-17 10:49:37 +0000
@@ -29,6 +29,9 @@
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')
@@ -276,6 +279,102 @@
BugNotificationLevel.COMMENTS,
default_notification_level_value)
+ def test_muted_subs_have_unmute_option(self):
+ # If a user has a muted subscription, the
+ # BugSubscriptionSubscribeSelfView's subscription field will
+ # show an "Unmute" option.
+ with person_logged_in(self.person):
+ self.bug.mute(self.person, self.person)
+
+ with feature_flags():
+ with person_logged_in(self.person):
+ subscribe_view = create_initialized_view(
+ self.bug.default_bugtask, name='+subscribe')
+ subscription_widget = (
+ subscribe_view.widgets['subscription'])
+ # The Unmute option is actually treated the same way as
+ # the unsubscribe option.
+ self.assertEqual(
+ "Unmute bug mail from this bug",
+ subscription_widget.vocabulary.getTerm(self.person).title)
+
+ def test_muted_subs_have_unmute_and_update_option(self):
+ # If a user has a muted subscription, the
+ # BugSubscriptionSubscribeSelfView's subscription field will
+ # show an option to unmute the subscription and update it to a
+ # new BugNotificationLevel.
+ with person_logged_in(self.person):
+ self.bug.mute(self.person, self.person)
+
+ with feature_flags():
+ with person_logged_in(self.person):
+ subscribe_view = create_initialized_view(
+ self.bug.default_bugtask, name='+subscribe')
+ subscription_widget = (
+ subscribe_view.widgets['subscription'])
+ update_term = subscription_widget.vocabulary.getTermByToken(
+ 'update-subscription')
+ self.assertEqual(
+ "Unmute bug mail from this bug and subscribe me to it",
+ update_term.title)
+
+ def test_unmute_unmutes(self):
+ # Using the "Unmute bug mail" option when the user has a muted
+ # subscription will remove the muted subscription.
+ with person_logged_in(self.person):
+ self.bug.mute(self.person, self.person)
+
+ with feature_flags():
+ with person_logged_in(self.person):
+ level = BugNotificationLevel.METADATA
+ form_data = {
+ 'field.subscription': self.person.name,
+ # Although this isn't used we must pass it for the
+ # sake of form validation.
+ 'field.bug_notification_level': level.title,
+ 'field.actions.continue': 'Continue',
+ }
+ subscribe_view = create_initialized_view(
+ self.bug.default_bugtask, form=form_data,
+ name='+subscribe')
+ self.assertFalse(self.bug.isMuted(self.person))
+ self.assertFalse(self.bug.isSubscribed(self.person))
+
+ def test_update_when_muted_updates(self):
+ # Using the "Unmute and subscribe me" option when the user has a
+ # muted subscription will update the existing subscription to a
+ # new BugNotificationLevel.
+ with person_logged_in(self.person):
+ muted_subscription = self.bug.mute(self.person, self.person)
+
+ with feature_flags():
+ with person_logged_in(self.person):
+ level = BugNotificationLevel.COMMENTS
+ form_data = {
+ 'field.subscription': 'update-subscription',
+ 'field.bug_notification_level': level.title,
+ 'field.actions.continue': 'Continue',
+ }
+ subscribe_view = create_initialized_view(
+ self.bug.default_bugtask, form=form_data,
+ name='+subscribe')
+ self.assertFalse(self.bug.isMuted(self.person))
+ self.assertTrue(self.bug.isSubscribed(self.person))
+ self.assertEqual(
+ level, muted_subscription.bug_notification_level)
+
+ def test_bug_notification_level_field_has_widget_class(self):
+ # 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():
+ subscribe_view = create_initialized_view(
+ self.bug.default_bugtask, name='+subscribe')
+ widget_class = (
+ subscribe_view.widgets['bug_notification_level'].widget_class)
+ self.assertEqual(
+ 'bug-notification-level-field', widget_class)
+
class BugPortletSubcribersIdsTests(TestCaseWithFactory):
=== modified file 'lib/lp/bugs/configure.zcml'
--- lib/lp/bugs/configure.zcml 2011-03-15 04:15:45 +0000
+++ lib/lp/bugs/configure.zcml 2011-03-17 10:49:37 +0000
@@ -782,6 +782,7 @@
linkMessage
markAsDuplicate
markUserAffected
+ mute
newMessage
removeWatch
setPrivate
@@ -791,6 +792,7 @@
unlinkBranch
unlinkCVE
unlinkHWSubmission
+ unmute
unsubscribe
unsubscribeFromDupes
updateHeat"
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2011-03-15 04:15:45 +0000
+++ lib/lp/bugs/interfaces/bug.py 2011-03-17 10:49:37 +0000
@@ -489,6 +489,12 @@
with a BugNotificationLevel of NOTHING.
"""
+ def mute(person, muted_by):
+ """Add a muted subscription for `person`."""
+
+ def unmute(person, unmuted_by):
+ """Remove a muted subscription for `person`."""
+
def getDirectSubscriptions():
"""A sequence of IBugSubscriptions directly linked to this bug."""
=== added file 'lib/lp/bugs/javascript/bug_subscription.js'
--- lib/lp/bugs/javascript/bug_subscription.js 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/javascript/bug_subscription.js 2011-03-17 10:49:37 +0000
@@ -0,0 +1,57 @@
+/* Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * A bugtracker form overlay that can create a bugtracker within any page.
+ *
+ * @namespace Y.lp.bugs.bugtracker_overlay
+ * @requires dom, node, io-base, lazr.anim, lazr.formoverlay
+ */
+YUI.add('lp.bugs.bug_subscription', function(Y) {
+var namespace = Y.namespace('lp.bugs.bug_subscription');
+var level_div;
+
+// XXX: gmb 2011-03-17 bug=728457
+// This fix for resizing needs to be incorporated into
+// lazr.effects. When that is done it should be removed from here.
+/*
+ * Make sure that the bug_notification_level div is hidden properly .
+ * @method clean_up_level_div
+ */
+namespace.clean_up_level_div = function() {
+ level_div.setStyles({
+ height: 0,
+ visibility: 'hidden'
+ });
+};
+
+namespace.set_up_bug_notification_level_field = function() {
+ level_div = Y.one('.bug-notification-level-field');
+ var subscription_radio_buttons = Y.all(
+ 'input[name=field.subscription]');
+
+ // Only collapse the bug_notification_level field if the buttons are
+ // available to display it again.
+ if (Y.Lang.isValue(level_div) && subscription_radio_buttons.size() > 1) {
+ var slide_in_anim = Y.lazr.effects.slide_in(level_div);
+ slide_in_anim.on('end', namespace.clean_up_level_div);
+ var slide_out_anim = Y.lazr.effects.slide_out(level_div);
+ slide_out_anim.on('end', function() {
+ level_div.setStyles({
+ visibility: 'visible'
+ });
+ });
+ Y.each(subscription_radio_buttons, function(subscription_button) {
+ subscription_button.on('click', function(e) {
+ if(e.target.get('value') == 'update-subscription') {
+ slide_out_anim.run();
+ } else {
+ slide_in_anim.run();
+ }
+ });
+ console.log(subscription_button.get('checked'));
+ });
+ }
+};
+
+}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects",
+ ]});
=== modified file 'lib/lp/bugs/javascript/bugtask_index_portlets.js'
--- lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-14 14:39:33 +0000
+++ lib/lp/bugs/javascript/bugtask_index_portlets.js 2011-03-17 10:49:37 +0000
@@ -593,6 +593,13 @@
centered: true,
visible: false
});
+ // Register a couple of handlers to clean up the overlay when it's
+ // hidden.
+ subscription_overlay.get('form_cancel_button').on(
+ 'click',
+ Y.lp.bugs.bug_subscription.clean_up_level_div);
+ subscription_overlay.on(
+ 'cancel', Y.lp.bugs.bug_subscription.clean_up_level_div);
subscription_overlay.render('#privacy-form-container');
return subscription_overlay;
}
@@ -615,6 +622,7 @@
subscription_overlay.set(
'form_submit_callback', function(form_data) {
handle_advanced_subscription_overlay(subscription, form_data);
+ Y.lp.bugs.bug_subscription.clean_up_level_div();
subscription_overlay.hide();
subscription_overlay.loadFormContentAndRender(
subscription_link_url);
@@ -629,6 +637,7 @@
subscription_overlay.renderUI();
subscription_overlay.bindUI();
subscription.disable_spinner();
+ Y.lp.bugs.bug_subscription.set_up_bug_notification_level_field();
subscription_overlay.show();
}
function on_failure(id, response, subscription_overlay) {
@@ -1366,4 +1375,4 @@
"lazr.overlay", "lazr.choiceedit", "lp.app.picker",
"lp.client",
"lp.client.plugins", "lp.bugs.subscriber",
- "lp.app.errors"]});
+ "lp.bugs.bug_subscription", "lp.app.errors"]});
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-03-15 04:15:45 +0000
+++ lib/lp/bugs/model/bug.py 2011-03-17 10:49:37 +0000
@@ -836,6 +836,27 @@
BugNotificationLevel.NOTHING)
return not subscriptions.is_empty()
+ def mute(self, person, muted_by):
+ """See `IBug`."""
+ # If there's an existing subscription, update it.
+ store = Store.of(self)
+ subscriptions = store.find(
+ BugSubscription,
+ BugSubscription.bug == self,
+ BugSubscription.person == person)
+ if subscriptions.is_empty():
+ return self.subscribe(
+ person, muted_by, level=BugNotificationLevel.NOTHING)
+ else:
+ subscription = subscriptions.one()
+ subscription.bug_notification_level = (
+ BugNotificationLevel.NOTHING)
+ return subscription
+
+ def unmute(self, person, unmuted_by):
+ """See `IBug`."""
+ self.unsubscribe(person, unmuted_by)
+
@property
def subscriptions(self):
"""The set of `BugSubscriptions` for this bug."""
=== modified file 'lib/lp/bugs/templates/bug-subscription.pt'
--- lib/lp/bugs/templates/bug-subscription.pt 2010-10-18 10:14:37 +0000
+++ lib/lp/bugs/templates/bug-subscription.pt 2011-03-17 10:49:37 +0000
@@ -10,6 +10,25 @@
i18n:domain="malone"
>
+ <metal:block fill-slot="head_epilogue">
+ <script type="text/javascript"
+ tal:condition="devmode"
+ tal:content="string:var yui_base='${yui}';"></script>
+ <script type="text/javascript"
+ tal:condition="devmode"
+ tal:define="lp_js string:${icingroot}/build"
+ tal:attributes="src string:${lp_js}/bugs/filebug-dupefinder.js"></script>
+
+ <script type="text/javascript" tal:condition="view/user_is_muted">
+ LPS.use('base', 'node', 'oop', 'event', 'lp.bugs.bug_subscription',
+ function(Y) {
+ Y.on(
+ 'domready',
+ Y.lp.bugs.bug_subscription.set_up_bug_notification_level_field);
+ });
+ </script>
+ </metal:block>
+
<body>
<div metal:fill-slot="main">
=== modified file 'lib/lp/bugs/tests/test_bug.py'
--- lib/lp/bugs/tests/test_bug.py 2011-03-04 16:17:08 +0000
+++ lib/lp/bugs/tests/test_bug.py 2011-03-17 10:49:37 +0000
@@ -44,3 +44,33 @@
# subscription.
with person_logged_in(self.person):
self.assertEqual(False, self.bug.isMuted(self.person))
+
+ def test_mute_mutes_user(self):
+ # Bug.mute() adds a muted subscription for the user passed to
+ # it.
+ with person_logged_in(self.person):
+ muted_subscription = self.bug.mute(
+ self.person, self.person)
+ self.assertEqual(
+ BugNotificationLevel.NOTHING,
+ muted_subscription.bug_notification_level)
+
+ def test_mute_mutes_user_with_existing_subscription(self):
+ # Bug.mute() will update an existing subscription so that it
+ # becomes muted.
+ with person_logged_in(self.person):
+ subscription = self.bug.subscribe(self.person, self.person)
+ muted_subscription = self.bug.mute(self.person, self.person)
+ self.assertEqual(subscription, muted_subscription)
+ self.assertEqual(
+ BugNotificationLevel.NOTHING,
+ subscription.bug_notification_level)
+
+ def test_unmute_unmutes_user(self):
+ # Bug.unmute() will remove a muted subscription for the user
+ # passed to it.
+ with person_logged_in(self.person):
+ self.bug.mute(self.person, self.person)
+ self.assertTrue(self.bug.isMuted(self.person))
+ self.bug.unmute(self.person, self.person)
+ self.assertFalse(self.bug.isMuted(self.person))
