launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03849
[Merge] lp:~gmb/launchpad/bug-61531 into lp:launchpad
Graham Binns has proposed merging lp:~gmb/launchpad/bug-61531 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #61531 in Launchpad itself: "Forbidden error when trying to mark a bug as private"
https://bugs.launchpad.net/launchpad/+bug/61531
For more details, see:
https://code.launchpad.net/~gmb/launchpad/bug-61531/+merge/63672
This branch fixes bug 61531 by subscribing the user who makes a bug private to that bug (unless they're already subscribed). This approach was arrived at after discussion with Gary.
This branch handles the non-JS workflow; the JS stuff will be done in a second branch.
--
https://code.launchpad.net/~gmb/launchpad/bug-61531/+merge/63672
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gmb/launchpad/bug-61531 into lp:launchpad.
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2011-06-07 06:24:04 +0000
+++ lib/lp/bugs/browser/bug.py 2011-06-07 10:14:36 +0000
@@ -848,6 +848,10 @@
bug_before_modification = Snapshot(
bug, providing=providedBy(bug))
private = data.pop('private')
+ user_will_be_subscribed = False
+ if private:
+ user_will_be_subscribed = (
+ not bug.personIsDirectSubscriber(self.user))
security_related = data.pop('security_related')
private_changed = bug.setPrivate(
private, getUtility(ILaunchBag).user)
@@ -856,6 +860,21 @@
changed_fields = []
if private_changed:
changed_fields.append('private')
+ if user_will_be_subscribed:
+ notification_text = (
+ "Since you marked this bug as private you have "
+ "automatically been subscribed to it.")
+ if features.getFeatureFlag(
+ 'malone.advanced-subscriptions.enabled'):
+ notification_text = (
+ "%s If you don't want to receive email about "
+ "this bug you can <a href=\"%s\">mute your "
+ "subscription or unsubscribe</a>." % (
+ notification_text,
+ canonical_url(
+ self.context, view_name='+subscribe')))
+ self.request.response.addInfoNotification(
+ structured(notification_text))
if security_related_changed:
changed_fields.append('security_related')
notify(ObjectModifiedEvent(
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2011-06-07 06:24:04 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2011-06-07 10:14:36 +0000
@@ -202,6 +202,7 @@
self.assertTrue(
self._hasCSSClass(html, 'mute-link-container', 'hidden'),
'No "hidden" CSS class in mute-link-container.')
+<<<<<<< TREE
def test_mute_subscription_link_not_rendered_for_anonymous(self):
# If a person is not already subscribed to a bug in some way,
@@ -248,3 +249,72 @@
self.assertEqual(
'muted-false hidden subscribed-false dup-subscribed-false',
view.current_user_mute_class)
+=======
+
+
+class TestBugSecrecyViews(TestCaseWithFactory):
+ """Tests for the Bug secrecy views."""
+
+ layer = DatabaseFunctionalLayer
+
+ def test_notification_shown_if_marking_private_and_not_subscribed(self):
+ # If a user who is not subscribed to a bug marks that bug as
+ # private, the user will be subscribed to the bug. This allows
+ # them to un-mark the bug if they choose to, rather than being
+ # blocked from doing so.
+ person = self.factory.makePerson()
+ bug = self.factory.makeBug()
+ with person_logged_in(person):
+ view = create_initialized_view(
+ bug.default_bugtask, name='+secrecy', form={
+ 'field.private': 'on',
+ 'field.security_related': '',
+ 'field.actions.change': 'Change',
+ })
+ self.assertEqual(1, len(view.request.response.notifications))
+ message_text = (
+ "Since you marked this bug as private you have "
+ "automatically been subscribed to it.")
+ self.assertEqual(
+ message_text, view.request.response.notifications[0].message)
+
+ def test_no_notification_shown_if_marking_private_and_subscribed(self):
+ # If a user who is subscribed to a bug marks that bug as
+ # private, the user will see not notification.
+ person = self.factory.makePerson()
+ bug = self.factory.makeBug()
+ with person_logged_in(person):
+ bug.subscribe(person, person)
+ view = create_initialized_view(
+ bug.default_bugtask, name='+secrecy', form={
+ 'field.private': 'on',
+ 'field.security_related': '',
+ 'field.actions.change': 'Change',
+ })
+ self.assertEqual(0, len(view.request.response.notifications))
+
+ def test_notification_includes_link_for_advanced_subscriptions(self):
+ # If the advanced subscriptions feature flag is turned on, the
+ # notification will include a link to the edit subscription
+ # page.
+ person = self.factory.makePerson()
+ bug = self.factory.makeBug()
+ with FeatureFixture({'malone.advanced-subscriptions.enabled': 'on'}):
+ with person_logged_in(person):
+ view = create_initialized_view(
+ bug.default_bugtask, name='+secrecy', form={
+ 'field.private': 'on',
+ 'field.security_related': '',
+ 'field.actions.change': 'Change',
+ })
+ self.assertEqual(1, len(view.request.response.notifications))
+ message_text = (
+ "Since you marked this bug as private you have "
+ "automatically been subscribed to it. If you don't want "
+ "to receive email about this bug you can <a href=\""
+ "%s\">mute your subscription or unsubscribe</a>."
+ % canonical_url(
+ bug.default_bugtask, view_name='+subscribe'))
+ self.assertEqual(
+ message_text, view.request.response.notifications[0].message)
+>>>>>>> MERGE-SOURCE
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-06-07 06:24:04 +0000
+++ lib/lp/bugs/model/bug.py 2011-06-07 10:14:36 +0000
@@ -1645,6 +1645,10 @@
# the bug is private.
for person in self.getIndirectSubscribers():
self.subscribe(person, who)
+ # We also add `who` as a subscriber so that they can
+ # see the bug they've just marked private. If they're
+ # already subscribed this will be a no-op.
+ self.subscribe(who, who)
self.private = private
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2011-06-07 06:24:04 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2011-06-07 10:14:36 +0000
@@ -261,3 +261,12 @@
with person_logged_in(bug.owner):
info = bug.getSubscriptionInfo(BugNotificationLevel.METADATA)
self.assertEqual(BugNotificationLevel.METADATA, info.level)
+
+ def test_setPrivate_subscribes_person_who_makes_bug_private(self):
+ # When setPrivate(True) is called on a bug, the person who is
+ # marking the bug private is subscribed to the bug.
+ bug = self.factory.makeBug()
+ person = self.factory.makePerson()
+ with person_logged_in(person):
+ bug.setPrivate(True, person)
+ self.assertTrue(bug.personIsDirectSubscriber(person))