launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/bugsupervisor-subscription-issues into lp:launchpad.

Requested reviews:
  Curtis Hovey (sinzui)
Related bugs:
  Bug #988510 in Launchpad itself: "Bug supervisor should not be subscribed on ubuntu bugs when transition from security to userdata"
  https://bugs.launchpad.net/launchpad/+bug/988510

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/bugsupervisor-subscription-issues/+merge/103559

Summary
=======
Recent changes in the subscription behavior to resolve a regression broke the
ubuntu bugs workflow. On ubuntu, at one junction of bug working, security bugs
are marked as not security related while still maintaining privacy. This
subscribes the bug supervisor (who is supposed to be attached to USERDATA
bugs).

For Ubuntu, we need to special case this transition and not subscribe the bug
supervisor.

Preimp
======
Spoke with Curtis Hovey.

Implementation
==============
A guard is put in place to check if the information type we are transition
from is security related, and if the pillar in question is ubuntu. If both of
those are true, the bug supervisor is *not* subscribed, nor is the maintainer.

Tests
=====
bin/test -vvcm lp.bugs.model.tests.test_bug

QA
==
* Create a bug marked as security related and private on ubuntu.
* Have the bug unmarked as security--leave private checked.
* Confirm the bug supervisor, ubuntu-bugs, is not subscribed.

LoC
===
This is part of the disclosure feature, which is already resourced.

Lint
====
Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/model/bug.py
  lib/lp/bugs/model/tests/test_bug.py

-- 
https://code.launchpad.net/~jcsackett/launchpad/bugsupervisor-subscription-issues/+merge/103559
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-04-24 04:23:52 +0000
+++ lib/lp/bugs/model/bug.py	2012-04-25 20:14:28 +0000
@@ -1751,6 +1751,16 @@
                 information_type in PRIVATE_INFORMATION_TYPES)
         self.updateHeat()
 
+        # When we start adding people, we need to be able to check if we're
+        # dealing with the ubuntu special case, where security related bugs
+        # get switched to userdata bugs, but the bug supervisor *does not* get
+        # subscribed.
+        def _is_ubuntu_special_case(pillar):
+            ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+            is_ubuntu = (pillar == ubuntu)
+            is_security = (self.information_type in SECURITY_INFORMATION_TYPES)
+            return (is_ubuntu and is_security)
+
         # There are several people we need to ensure are subscribed.
         # If the information type is userdata, we need to check for bug
         # supervisors who aren't subscribed and should be. If there is no
@@ -1758,10 +1768,11 @@
         pillars = self.affected_pillars
         if information_type == InformationType.USERDATA:
             for pillar in pillars:
-                if pillar.bug_supervisor is not None:
-                    missing_subscribers.add(pillar.bug_supervisor)
-                else:
-                    missing_subscribers.add(pillar.owner)
+                if not _is_ubuntu_special_case(pillar):
+                    if pillar.bug_supervisor is not None:
+                        missing_subscribers.add(pillar.bug_supervisor)
+                    else:
+                        missing_subscribers.add(pillar.owner)
 
         # If the information type is security related, we need to ensure
         # the security contacts are subscribed. If there is no security

=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2012-04-24 04:31:36 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2012-04-25 20:14:28 +0000
@@ -16,6 +16,7 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.adapters.bugchange import BugTitleChange
 from lp.bugs.enums import (
     BugNotificationLevel,
@@ -961,6 +962,34 @@
         self.private_project = False
 
 
+class TestBugPrivateAndSecurityRelatedUpdatesSpecialCase(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_transition_special_cased_for_ubuntu(self):
+        # When a bug on ubuntu is transitioned to USERDATA from
+        # EMBARGOEDSECURITY, the bug supervisor is not subscribed, and the
+        # bug's subscribers do not change.
+        # This is to protect ubuntu's workflow, which differs from the
+        # Launchpad norm.
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        admin = getUtility(ILaunchpadCelebrities).admin
+        ubuntu = removeSecurityProxy(ubuntu)
+        ubuntu.setBugSupervisor(
+            self.factory.makePerson(name='supervisor'), admin)
+        bug = self.factory.makeBug(
+            information_type=InformationType.EMBARGOEDSECURITY,
+            distribution=ubuntu)
+        bug = removeSecurityProxy(bug)
+        initial_subscribers = bug.getDirectSubscribers()
+        self.assertTrue(ubuntu.bug_supervisor not in initial_subscribers)
+        bug.transitionToInformationType(
+            InformationType.USERDATA, who=bug.owner)
+        subscribers = bug.getDirectSubscribers()
+        self.assertContentEqual(initial_subscribers, subscribers)
+        ubuntu.setBugSupervisor(None, ubuntu.owner)
+
+
 class TestBugActivityMethods(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer