Merge lp:~jcsackett/launchpad/private-bugs-without-supervision into lp:launchpad

[j.c.sackett]

Fixes subscription problems when transitioning a bug to non-public information types. Also removes the unused reconcileSubscribers code involved in the transition code.

Summary
=======
The constantly changing setup of subscription and privacy rules led to a
regression, wherein a person marking a bug private would lose access to the
bug, preventing privacy from happening. Because we put the subscriber
reconcilation behind a feature flag and didn't activate it, the absence of a
bug supervisor prevents marking bugs private.

This branch ensures that:
* The bug reporter stays attached
* The bug assignee stays attached
* The bug supervisor is attached if it exists
* The pillar maintainer is attached if the supervisor doesn't exist

Preimp
======
Spoke with Curtis Hovey

Implementation
==============
The reconcileSubscribers, method, which is put behind a feature flag, is
removed as the implementation is now out of date and is unlikely to ever be
enabled. The flag and flag related checks are also removed.

The no f_flag fallback has been cleaned up; it now ensures the bug
supervisor/maintainer fallback works, and ensure the other expected
subscriptions.

A *number* of tests have been updated; they relied on the feature flag being
set, and weren't valid as they were. In some cases entire tests have been
removed, as they were testing for behavior that never existed without the flag,
which was never enabled anywhere.

Tests
=====
bin/test -vvct test_bug.*PrivateAndSecurityRelated

QA
==
Look at the setup in the bug; that should work.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/tests/test_bugvisibility.py
  lib/lp/bugs/model/bug.py
  lib/lp/services/features/flags.py

[Curtis Hovey (sinzui)]

This branch reproduces the situation that drove William to put it behind the feature flag to kill it. Let's not repeat this mistake again.

These rules cannot apply to Ubuntu. it's bug supervisor does not get automatic access to private bugs, this is why we spent an extra three months to create 3 sharing policies instead of one, Ubuntu is not like any other project in Lp. Ubuntu's maintainer does not get access to private or security bugs. recall the success criteria for sharing is that that maintainer (or us) can setup Ubuntu so that ubuntu-security is the only party that can see embargoed security bugs, and apport is the only party that can see user data bugs.

There is, or was, code that checked that the project has .private_bugs or .hasCurrentCommercialSubscription() which we know will be working with private bugs. I am not certain we want check these attributes. Maybe we should just check if the pillar is not Ubuntu -- I do not think any other project took advantage of Lp defects to create policies different from what is documented.

review: Needs Fixing (code)

[j.c.sackett (jcsackett)]

> This branch reproduces the situation that drove William to put it behind the
> feature flag to kill it. Let's not repeat this mistake again.
>
> These rules cannot apply to Ubuntu. it's bug supervisor does not get automatic
> access to private bugs, this is why we spent an extra three months to create 3
> sharing policies instead of one, Ubuntu is not like any other project in Lp.
> Ubuntu's maintainer does not get access to private or security bugs. recall
> the success criteria for sharing is that that maintainer (or us) can setup
> Ubuntu so that ubuntu-security is the only party that can see embargoed
> security bugs, and apport is the only party that can see user data bugs.

That's a little perplexing; if look at the deleted clause starting on line 19, you can see that the supervisor is already being given access to USERDATA bugs. So if that was the mistake, it was not resolved by the feature flag. I can certainly make the check for Ubuntu, but I want to be clear that it's an additional change from the current behavior (not that there's anything wrong with that).

> There is, or was, code that checked that the project has .private_bugs or
> .hasCurrentCommercialSubscription() which we know will be working with private
> bugs. I am not certain we want check these attributes. Maybe we should just
> check if the pillar is not Ubuntu -- I do not think any other project took
> advantage of Lp defects to create policies different from what is documented.

I'm going to vote for "was". Or, possibly, that check is only being done on bug creation. I think just adding a check for the pillar being Ubuntu is sufficient--we already special case that, so it doesn't feel too wrong to make that check.

I will update the code so that if the pillar is Ubuntu, the bug supervisor is not given access to USERDATA bugs.

[Curtis Hovey (sinzui)]

We discusses this on IRC. The issue is that while we think the rules for Ubuntu are wrong at this moment, Ubuntu has not reported any issue with changing bugs to private, only projects have this issue. Maybe this is not an issue because apport does not using this path through the code. We will talk with the purple squad to resolve this.

review: Needs Information (code)

[Curtis Hovey (sinzui)]

We discussed this and believe your change is correct. This is good to land.

review: Approve (code)

[j.c.sackett (jcsackett)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Here's a clean diff of just what's changed since the previously
approved MP.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPjbI4AAoJEJvCBZ3E2NUPRvkH/iKGm03AAwiU4ftu2Crgb872
sBgMsDUKeDKcrvZqZC7U14OtMLeQew6Fuha6rUZghjHW+m+kQinyclKl6zdtZZAj
7zlcok4eNYQP2UMXp19OJIQkW1RUQPLqzA8x1og7Lz+KggiaSxtD9fG/QE4msGoE
vqGy59tJMZx3FwBmlsQzowNFHcQlFYNeUYGa80r7zx0ZfXJZoBnxc6RMjP5FGQwL
wk1aq20sRzfhKZxCz9aR/fP4JEAxjpqFN5/jaVgSM8TL8yU7UhHalofDwEyfm+iA
hvjjds4/gv+sjkfO/ckMQIx9hSkNqjekeKnzfLwQs4GCKPdrJ8jCuKwoAzl923k=
=RZ+X
-----END PGP SIGNATURE-----

[Curtis Hovey (sinzui)]

Thank you very very much.

review: Approve (code)

=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt	2012-03-23 06:10:28 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt	2012-04-17 18:05:27 +0000
@@ -756,9 +756,10 @@
 
     >>> for message in trigger_and_get_email_messages(bug_three):
     ...     print message['To'], message.get_all('X-Launchpad-Bug-Private')
+    [e-mail] ['yes']
+    [e-mail] ['yes']
     [e-mail] ['yes']
 
-
 The X-Launchpad-Bug-Security-Vulnerability header
 -------------------------------------------------
 
@@ -772,6 +773,8 @@
     >>> for message in trigger_and_get_email_messages(bug_three):
     ...     print message['To'], (
     ...         message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
+    [e-mail] ['no']
+    [e-mail] ['no']
     [e-mail] ['no']
 
 The presence of the security flag on a bug is, surprise, denoted by a
@@ -786,6 +789,8 @@
     >>> for message in trigger_and_get_email_messages(bug_three):
     ...     print message['To'], (
     ...         message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
+    [e-mail] ['yes']
+    [e-mail] ['yes']
     [e-mail] ['yes']
 
 
 
=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt	2012-04-03 06:14:09 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt	2012-04-17 18:05:27 +0000
@@ -380,17 +380,7 @@
     ()
 
 When a bug is marked private, specific people like the bugtask bug supervisors
-will be automatically subscribed, and only specifically allowed existing
-direct subscribers (eg bugtask pillar maintainers) will remain subscribed.
-
-We currently use a feature flag to control who is subscribed when a bug is
-made private.
-
-    >>> from lp.services.features.testing import FeatureFixture
-    >>> feature_flag = {
-    ...     'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
-    >>> flags = FeatureFixture(feature_flag)
-    >>> flags.setUp()
+will be automatically subscribed.
 
     >>> from zope.event import notify
 
@@ -438,26 +428,20 @@
     >>> print_displayname(linux_source_bug.getDirectSubscribers())
     Foo Bar
     Mark Shuttleworth
+    Robert Collins
+    Ubuntu Team
 
     >>> print_displayname(linux_source_bug.getIndirectSubscribers())
     Martin Pitt
     No Privileges Person
-    Robert Collins
     Sample Person
     Scott James Remnant
-    Ubuntu Team
 
     >>> print_displayname(linux_source_bug.getAlsoNotifiedSubscribers())
     Martin Pitt
     No Privileges Person
-    Robert Collins
     Sample Person
     Scott James Remnant
-    Ubuntu Team
-
-Clean up the feature flag.
-
-    >>> flags.cleanUp()
 
 To find out which email addresses should receive a notification email on
 a bug, and why, IBug.getBugNotificationRecipients() assembles an
@@ -472,8 +456,8 @@
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
-     ('[e-mail]', u'Subscriber (Mozilla Firefox)'),
-     ('[e-mail]', u'Subscriber (Ubuntu) @ubuntu-team'),
+     ('[e-mail]', 'Subscriber'),
+     ('[e-mail]', u'Subscriber @ubuntu-team'),
      ('[e-mail]', 'Assignee')]
 
 If IBug.getBugNotificationRecipients() is passed a  BugNotificationLevel
@@ -486,8 +470,8 @@
     >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
-     ('[e-mail]', u'Subscriber (Mozilla Firefox)'),
-     ('[e-mail]', u'Subscriber (Ubuntu) @ubuntu-team'),
+     ('[e-mail]', 'Subscriber'),
+     ('[e-mail]', u'Subscriber @ubuntu-team'),
      ('[e-mail]', 'Assignee')]
 
 When Sample Person is unsubscribed from linux_source_bug, he is no
@@ -501,8 +485,8 @@
     >>> [(address, recipients.getReason(address)[1]) for address in addresses]
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
-     ('[e-mail]', u'Subscriber (Mozilla Firefox)'),
-     ('[e-mail]', u'Subscriber (Ubuntu) @ubuntu-team'),
+     ('[e-mail]', 'Subscriber'),
+     ('[e-mail]', u'Subscriber @ubuntu-team'),
      ('[e-mail]', 'Assignee')]
 
 ...but remains included for the level LIFECYCLE.
@@ -515,8 +499,8 @@
     [('[e-mail]', 'Subscriber'),
      ('[e-mail]', 'Subscriber'),
      ('[e-mail]', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
-     ('[e-mail]', u'Subscriber (Mozilla Firefox)'),
-     ('[e-mail]', u'Subscriber (Ubuntu) @ubuntu-team'),
+     ('[e-mail]', 'Subscriber'),
+     ('[e-mail]', u'Subscriber @ubuntu-team'),
      ('[e-mail]', 'Assignee')]
 
 To find out if someone is already directly subscribed to a bug, call
 
=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
--- lib/lp/bugs/doc/initial-bug-contacts.txt	2012-04-03 06:14:09 +0000
+++ lib/lp/bugs/doc/initial-bug-contacts.txt	2012-04-17 18:05:27 +0000
@@ -288,16 +288,6 @@
      u'Sample Person', u'Steve Alexander', u'Ubuntu Gnome Team',
      u'Ubuntu Team']
 
-
-We currently use a feature flag to control who is subscribed when a bug is
-made private.
-
-    >>> from lp.services.features.testing import FeatureFixture
-    >>> feature_flag = {
-    ...     'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
-    >>> flags = FeatureFixture(feature_flag)
-    >>> flags.setUp()
-
 If a bug is private, no changes are made to the subscriber list when a
 bug is reassigned to a different package.
 
@@ -329,13 +319,9 @@
 the unallowed subscribers are removed:
 
     >>> subscriber_names(bug_one_in_ubuntu_firefox.bug)
-    [u'Foo Bar', u'Mark Shuttleworth', u'Sample Person',
+    [u'Foo Bar', u'Sample Person',
      u'Steve Alexander', u'Ubuntu Team']
 
-Clean up the feature flag.
-
-    >>> flags.cleanUp()
-
 
 Product Bug Supervisors and Bug Tasks
 -------------------------------------
 
=== modified file 'lib/lp/bugs/doc/security-teams.txt'
--- lib/lp/bugs/doc/security-teams.txt	2012-04-04 05:46:26 +0000
+++ lib/lp/bugs/doc/security-teams.txt	2012-04-17 18:05:27 +0000
@@ -280,17 +280,7 @@
 
 
 When a bug becomes security-related, the security contacts for the pillars it
-affects are subscribed to it. This happens regardless of whether the feature
-flag is set.
-
-We currently use a feature flag to control who is subscribed when a bug is
-made security related.
-
-    >>> from lp.services.features.testing import FeatureFixture
-    >>> feature_flag = {
-    ...     'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
-    >>> security_flags = FeatureFixture(feature_flag)
-    >>> security_flags.setUp()
+affects are subscribed to it. 
 
     >>> from zope.event import notify
     >>> from lazr.lifecycle.event import ObjectModifiedEvent
@@ -317,30 +307,3 @@
     Bug Reporter
     Distribution Security Contact
     Product Security Contact
-
-Clean up the feature flag.
-
-    >>> security_flags.cleanUp()
-
-And once more without the feature flag.
-
-    >>> product = factory.makeProduct()
-    >>> product.security_contact = factory.makePerson(
-    ...     displayname='Product Security Contact')
-    >>> distribution = factory.makeDistribution()
-    >>> distribution.security_contact = factory.makePerson(
-    ...     displayname='Distribution Security Contact')
-    >>> reporter = factory.makePerson(displayname=u'Bug Reporter')
-    >>> bug = factory.makeBug(product=product, owner=reporter)
-    >>> bug.addTask(owner=reporter, target=distribution)
-    <BugTask ...>
-    >>> old_state = Snapshot(bug, providing=IBug)
-    >>> bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
-    True
-    >>> notify(ObjectModifiedEvent(bug, old_state, ['security_related']))
-    >>> for subscriber_name in sorted(
-    ...     s.displayname for s in bug.getDirectSubscribers()):
-    ...         print subscriber_name
-    Bug Reporter
-    Distribution Security Contact
-    Product Security Contact
 
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-04-17 01:11:47 +0000
+++ lib/lp/bugs/model/bug.py	2012-04-17 18:05:27 +0000
@@ -1733,10 +1733,6 @@
                 "Cannot transition the information type to proprietary.")
         if self.information_type == information_type:
             return False
-        f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
-        f_flag = bool(getFeatureFlag(f_flag_str))
-        if f_flag:
-            self.reconcileSubscribers(information_type, who)
         if (information_type == InformationType.PROPRIETARY and
             len(self.affected_pillars) > 1):
             raise BugCannotBePrivate(
@@ -1744,34 +1740,47 @@
         if information_type in PRIVATE_INFORMATION_TYPES:
             self.who_made_private = who
             self.date_made_private = UTC_NOW
+            missing_subscribers = set([who, self.owner])
         else:
             self.who_made_private = None
             self.date_made_private = None
+            missing_subscribers = set()
         # XXX: This should be a bulk update. RBC 20100827
         # bug=https://bugs.launchpad.net/storm/+bug/625071
         for attachment in self.attachments_unpopulated:
             attachment.libraryfile.restricted = (
                 information_type in PRIVATE_INFORMATION_TYPES)
         self.updateHeat()
-        if not f_flag and information_type == InformationType.USERDATA:
-            # If we didn't call reconcileSubscribers(), we may have
-            # bug supervisors who should be on this bug, but aren't.
-            supervisors = set()
-            for bugtask in self.bugtasks:
-                supervisors.add(bugtask.pillar.bug_supervisor)
-            if None in supervisors:
-                supervisors.remove(None)
-            for s in supervisors:
-                if not get_structural_subscriptions_for_bug(self, s):
-                    self.subscribe(s, who)
-        if not f_flag and information_type in SECURITY_INFORMATION_TYPES:
-            # The bug turned out to be security-related, subscribe the
-            # security contact. We do it here only if the feature flag
-            # is not set, otherwise it's done in
-            # reconcileSubscribers().
-            for pillar in self.affected_pillars:
+
+        # 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
+        # bug supervisor, we need to subscribe the maintainer.
+        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 the information type is security related, we need to ensure
+        # the security contacts are subscribed. If there is no security
+        # contact, we need to subscribe the maintainer.
+        if information_type in SECURITY_INFORMATION_TYPES:
+            for pillar in pillars:
                 if pillar.security_contact is not None:
-                    self.subscribe(pillar.security_contact, who)
+                    missing_subscribers.add(pillar.security_contact)
+                else:
+                    missing_subscribers.add(pillar.owner)
+
+        for s in missing_subscribers:
+            # Don't subscribe someone if they're already subscribed via a
+            # team.
+            already_subscribed_teams = self.getSubscribersForPerson(s)
+            if already_subscribed_teams.is_empty():
+                self.subscribe(s, who)
+
         self.information_type = information_type
         # Set the legacy attributes for now.
         self._private = information_type in PRIVATE_INFORMATION_TYPES
@@ -1838,81 +1847,6 @@
                     bug_supervisors.append(pillar.bug_supervisor)
         return bug_supervisors, security_contacts
 
-    def reconcileSubscribers(self, information_type, who):
-        """ Ensure only appropriate people are subscribed to private bugs.
-
-        When a bug is marked as either private = True or security_related =
-        True, we need to ensure that only people who are authorised to know
-        about the privileged contents of the bug remain directly subscribed
-        to it. So we:
-          1. Get the required subscribers depending on the bug status.
-          2. Get the auto removed subscribers depending on the bug status.
-             eg security contacts when a bug is updated to security related =
-             false.
-          3. Get the allowed subscribers = required subscribers
-                                            + bugtask owners
-          4. Remove any current direct subscribers who are not allowed or are
-             to be auto removed.
-          5. Add any subscribers who are required.
-        """
-        current_direct_subscribers = (
-            self.getSubscriptionInfo().direct_subscribers)
-        required_subscribers = self.getRequiredSubscribers(
-            information_type, who)
-        removed_bug_supervisors, removed_security_contacts = (
-            self.getAutoRemovedSubscribers(information_type))
-        for subscriber in removed_bug_supervisors:
-            recipients = BugNotificationRecipients()
-            recipients.addBugSupervisor(subscriber)
-            notification_text = ("This bug is no longer private so the bug "
-                "supervisor was unsubscribed. They will no longer be "
-                "notified of changes to this bug for privacy related "
-                "reasons, but may receive notifications about this bug from "
-                "other subscriptions.")
-            self.unsubscribe(
-                subscriber, who, ignore_permissions=True,
-                send_notification=True,
-                notification_text=notification_text,
-                recipients=recipients)
-        for subscriber in removed_security_contacts:
-            recipients = BugNotificationRecipients()
-            recipients.addSecurityContact(subscriber)
-            notification_text = ("This bug is no longer security related so "
-                "the security contact was unsubscribed. They will no longer "
-                "be notified of changes to this bug for security related "
-                "reasons, but may receive notifications about this bug "
-                "from other subscriptions.")
-            self.unsubscribe(
-                subscriber, who, ignore_permissions=True,
-                send_notification=True,
-                notification_text=notification_text,
-                recipients=recipients)
-
-        # If this bug is for a project that is marked as having private bugs
-        # by default, and the bug is private or security related, we will
-        # unsubscribe any unauthorised direct subscribers.
-        pillar = self.default_bugtask.pillar
-        private_project = IProduct.providedBy(pillar) and pillar.private_bugs
-        privileged_info = information_type != InformationType.PUBLIC
-        if private_project and privileged_info:
-            allowed_subscribers = set()
-            allowed_subscribers.add(self.owner)
-            for bugtask in self.bugtasks:
-                allowed_subscribers.add(bugtask.owner)
-                allowed_subscribers.add(bugtask.pillar.owner)
-                allowed_subscribers.update(set(bugtask.pillar.drivers))
-            allowed_subscribers = required_subscribers.union(
-                allowed_subscribers)
-            subscribers_to_remove = (
-                current_direct_subscribers.difference(allowed_subscribers))
-            for subscriber in subscribers_to_remove:
-                self.unsubscribe(subscriber, who, ignore_permissions=True)
-
-        subscribers_to_add = (
-            required_subscribers.difference(current_direct_subscribers))
-        for subscriber in subscribers_to_add:
-            self.subscribe(subscriber, who)
-
     def getBugTask(self, target):
         """See `IBug`."""
         for bugtask in self.bugtasks:
 
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py	2012-04-17 01:11:47 +0000
+++ lib/lp/bugs/model/tests/test_bug.py	2012-04-17 18:05:27 +0000
@@ -567,14 +567,6 @@
 
     layer = DatabaseFunctionalLayer
 
-    def setUp(self):
-        super(TestBugPrivateAndSecurityRelatedUpdatesMixin, self).setUp()
-        f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
-        feature_flag = {f_flag_str: 'on'}
-        flags = FeatureFixture(feature_flag)
-        flags.setUp()
-        self.addCleanup(flags.cleanUp)
-
     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.
@@ -589,8 +581,8 @@
         # marking the bug private will not be subscribed if they're
         # already a member of a team which is a direct subscriber.
         bug = self.factory.makeBug()
-        team = self.factory.makeTeam()
-        person = team.teamowner
+        person = self.factory.makePerson(name='teamowner')
+        team = self.factory.makeTeam(owner=person, name='team')
         with person_logged_in(person):
             bug.subscribe(team, person)
             bug.setPrivate(True, person)
@@ -632,78 +624,71 @@
         return (bug, bug_owner, naked_bugtask_a, naked_bugtask_b,
                 naked_default_bugtask)
 
-    def test_setPrivateTrueAndSecurityRelatedTrue(self):
-        # When a bug is marked as private=true and security_related=true, the
-        # direct subscribers should include:
+    def test_transition_to_EMBARGOEDSECURITY_information_type(self):
+        # When a bug is marked as EMBARGOEDSECURITY, the direct subscribers
+        # should include:
         # - the bug reporter
         # - the bugtask pillar security contacts (if set)
         # - the person changing the state
         # - and bug/pillar owners, drivers if they are already subscribed
-        # If the bug is for a private project, then other direct subscribers
-        # should be unsubscribed.
 
         (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
             self.createBugTasksAndSubscribers())
         initial_subscribers = set((
-            self.factory.makePerson(), bugtask_a.owner, bug_owner,
-            bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
+            self.factory.makePerson(name='subscriber'), bugtask_a.owner,
+            bug_owner, bugtask_a.pillar.security_contact,
+            bugtask_a.pillar.driver))
+        initial_subscribers.update(bug.getDirectSubscribers())
 
         with person_logged_in(bug_owner):
             for subscriber in initial_subscribers:
                 bug.subscribe(subscriber, bug_owner)
-            who = self.factory.makePerson()
+            who = self.factory.makePerson(name='who')
             bug.transitionToInformationType(
                 InformationType.EMBARGOEDSECURITY, who=who)
             subscribers = bug.getDirectSubscribers()
+            initial_subscribers.update(bug.getDirectSubscribers())
         expected_subscribers = set((
             bugtask_a.owner,
-            default_bugtask.pillar.bug_supervisor,
             default_bugtask.pillar.driver,
             default_bugtask.pillar.security_contact,
             bug_owner, who))
-        if not self.private_project:
-            expected_subscribers.update(initial_subscribers)
+        expected_subscribers.update(initial_subscribers)
         self.assertContentEqual(expected_subscribers, subscribers)
 
-    def test_setPrivateTrueAndSecurityRelatedFalse(self):
-        # When a bug is marked as private=true and security_related=false, the
-        # direct subscribers should include:
+    def test_transition_to_USERDATA_information_type(self):
+        # When a bug is marked as USERDATA, the direct subscribers should
+        # include:
         # - the bug reporter
         # - the bugtask pillar bug supervisors (if set)
         # - the person changing the state
         # - and bug/pillar owners, drivers if they are already subscribed
-        # If the bug is for a private project, then other direct subscribers
-        # should be unsubscribed.
 
         (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
             self.createBugTasksAndSubscribers(private_security_related=True))
         initial_subscribers = set((
-            self.factory.makePerson(), bug_owner,
+            self.factory.makePerson(name='subscriber'), bug_owner,
             bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
 
         with person_logged_in(bug_owner):
             for subscriber in initial_subscribers:
                 bug.subscribe(subscriber, bug_owner)
-            who = self.factory.makePerson()
+            who = self.factory.makePerson(name='who')
             bug.transitionToInformationType(InformationType.USERDATA, who)
             subscribers = bug.getDirectSubscribers()
         expected_subscribers = set((
             default_bugtask.pillar.bug_supervisor,
             default_bugtask.pillar.driver,
             bug_owner, who))
-        if not self.private_project:
-            expected_subscribers.update(initial_subscribers)
-            expected_subscribers.remove(bugtask_a.pillar.security_contact)
+        expected_subscribers.update(initial_subscribers)
         self.assertContentEqual(expected_subscribers, subscribers)
 
-    def test_setPrivateFalseAndSecurityRelatedTrue(self):
-        # When a bug is marked as private=false and security_related=true, the
-        # direct subscribers should include:
+    def test_transition_to_UNEMBARGOEDSECURITY_information_type(self):
+        # When a security bug is unembargoed, direct subscribers should
+        # include:
         # - the bug reporter
         # - the bugtask pillar security contacts (if set)
         # - and bug/pillar owners, drivers if they are already subscribed
-        # If the bug is for a private project, then other direct subscribers
-        # should be unsubscribed.
 
         (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
             self.createBugTasksAndSubscribers(private_security_related=True))
@@ -715,7 +700,7 @@
         with person_logged_in(bug_owner):
             for subscriber in initial_subscribers:
                 bug.subscribe(subscriber, bug_owner)
-            who = self.factory.makePerson()
+            who = self.factory.makePerson(name='who')
             bug.transitionToInformationType(
                 InformationType.UNEMBARGOEDSECURITY, who)
             subscribers = bug.getDirectSubscribers()
@@ -723,36 +708,33 @@
             default_bugtask.pillar.driver,
             default_bugtask.pillar.security_contact,
             bug_owner))
-        if not self.private_project:
-            expected_subscribers.update(initial_subscribers)
-            expected_subscribers.remove(default_bugtask.pillar.bug_supervisor)
+        expected_subscribers.update(initial_subscribers)
         self.assertContentEqual(expected_subscribers, subscribers)
 
-    def test_setPrivateFalseAndSecurityRelatedFalse(self):
-        # When a bug is marked as private=false and security_related=false,
-        # any existing subscriptions are left alone.
+    def test_transition_to_PUBLIC_information_type(self):
+        # Subscriptions aren't altered when a bug is transitioned to the
+        # PUBLIC information type.
 
         (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
             self.createBugTasksAndSubscribers(private_security_related=True))
         initial_subscribers = set((
-            self.factory.makePerson(), bug_owner,
+            self.factory.makePerson(name='subscriber'), bug_owner,
             bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
 
         with person_logged_in(bug_owner):
             for subscriber in initial_subscribers:
                 bug.subscribe(subscriber, bug_owner)
-            who = self.factory.makePerson()
-            expected_direct_subscribers = set(bug.getDirectSubscribers())
+            who = self.factory.makePerson(name='who')
+            subscribers_before_public = set(bug.getDirectSubscribers())
             bug.transitionToInformationType(InformationType.PUBLIC, who)
-        subscribers = set(bug.getDirectSubscribers())
-        expected_direct_subscribers.difference_update(
-            (default_bugtask.pillar.security_contact,
-             default_bugtask.pillar.bug_supervisor,
-             bugtask_a.pillar.security_contact))
-        self.assertContentEqual(expected_direct_subscribers, subscribers)
+        subscribers_after_public = set(bug.getDirectSubscribers())
+        self.assertContentEqual(
+            subscribers_before_public,
+            subscribers_after_public)
 
     def test_setPillarOwnerSubscribedIfNoBugSupervisor(self):
-        # The pillar owner is subscribed if the bug supervisor is not set.
+        # The pillar owner is subscribed if the bug supervisor is not set and
+        # the bug is marked as USERDATA.
 
         bug_owner = self.factory.makePerson(name='bugowner')
         bug = self.factory.makeBug(owner=bug_owner)
@@ -766,18 +748,19 @@
             subscribers)
 
     def test_setPillarOwnerSubscribedIfNoSecurityContact(self):
-        # The pillar owner is subscribed if the security contact is not set.
+        # The pillar owner is subscribed if the security contact is not set
+        # and the bug is marked as EMBARGOEDSECURITY.
 
         bug_owner = self.factory.makePerson(name='bugowner')
         bug = self.factory.makeBug(owner=bug_owner)
         with person_logged_in(bug_owner):
-            who = self.factory.makePerson()
+            who = self.factory.makePerson(name='who')
             bug.transitionToInformationType(
-                InformationType.UNEMBARGOEDSECURITY, who)
+                InformationType.EMBARGOEDSECURITY, who)
             subscribers = bug.getDirectSubscribers()
         naked_bugtask = removeSecurityProxy(bug).default_bugtask
         self.assertContentEqual(
-            set((naked_bugtask.pillar.owner, bug_owner)),
+            set((naked_bugtask.pillar.owner, bug_owner, who)),
             subscribers)
 
     def _fetch_notifications(self, bug, reason_header):
@@ -833,32 +816,6 @@
                     actual_recipients.append(recipient.person)
         self.assertContentEqual(expected_recipients, actual_recipients)
 
-    def test_bugSupervisorUnsubscribedIfBugMadePublic(self):
-        # The bug supervisors are unsubscribed if a bug is made public and an
-        # email is sent telling them they have been unsubscribed.
-
-        (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
-            self.createBugTasksAndSubscribers(private_security_related=True))
-
-        with person_logged_in(bug_owner):
-            bug.subscribe(default_bugtask.pillar.bug_supervisor, bug_owner)
-            who = self.factory.makePerson(name="who")
-            bug.transitionToInformationType(
-                InformationType.UNEMBARGOEDSECURITY, who)
-            subscribers = bug.getDirectSubscribers()
-            self.assertNotIn(
-                default_bugtask.pillar.bug_supervisor, subscribers)
-
-            expected_recipients = [
-                default_bugtask.pillar.bug_supervisor,
-                ]
-            expected_body_text = '** This bug is no longer private'
-            expected_reason_body = ('You received this bug notification '
-                    'because you are a bug supervisor.')
-            self._check_notifications(
-                bug, expected_recipients, expected_body_text,
-                expected_reason_body, False, True, 'Bug Supervisor')
-
     def test_structural_bug_supervisor_becomes_direct_on_private(self):
         # If a bug supervisor has a structural subscription to the bug, and
         # the bug is marked as private, the supervisor should get a direct
@@ -876,31 +833,6 @@
             bug.transitionToInformationType(InformationType.USERDATA, who)
         self.assertTrue(bug_supervisor in bug.getDirectSubscribers())
 
-    def test_securityContactUnsubscribedIfBugNotSecurityRelated(self):
-        # The security contacts are unsubscribed if a bug has security_related
-        # set to false and an email is sent telling them they have been
-        # unsubscribed.
-
-        (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
-            self.createBugTasksAndSubscribers(private_security_related=True))
-
-        with person_logged_in(bug_owner):
-            bug.subscribe(bugtask_a.pillar.security_contact, bug_owner)
-            who = self.factory.makePerson(name="who")
-            bug.transitionToInformationType(InformationType.USERDATA, who)
-            subscribers = bug.getDirectSubscribers()
-            self.assertFalse(bugtask_a.pillar.security_contact in subscribers)
-
-            expected_recipients = [
-                bugtask_a.pillar.security_contact,
-                ]
-            expected_body_text = '** This bug is no longer security related'
-            expected_reason_body = ('You received this bug notification '
-                    'because you are a security contact.')
-            self._check_notifications(
-                bug, expected_recipients, expected_body_text,
-                expected_reason_body, True, False, 'Security Contact')
-
 
 class TestBugPrivacy(TestCaseWithFactory):
 
 
=== modified file 'lib/lp/bugs/tests/test_bug_mirror_access_triggers.py'
--- lib/lp/bugs/tests/test_bug_mirror_access_triggers.py	2012-04-03 06:14:09 +0000
+++ lib/lp/bugs/tests/test_bug_mirror_access_triggers.py	2012-04-17 18:05:27 +0000
@@ -141,7 +141,9 @@
         bug.setPrivate(True, bug.owner)
         self.assertIsNot(
             None, getUtility(IAccessArtifactSource).find([bug]).one())
-        self.assertEqual((1, 1), self.assertMirrored(bug))
+        # There are two grants--one for the reporter, one for the product
+        # owner or supervisor (if set). There is only one policy, USERDATA.
+        self.assertEqual((2, 1), self.assertMirrored(bug))
 
     def test_security_related(self):
         # Setting the security_related flag uses EMBARGOEDSECURITY
@@ -153,7 +155,9 @@
             [InformationType.USERDATA],
             self.getPolicyTypesForArtifact(artifact))
         bug.setSecurityRelated(True, bug.owner)
-        self.assertEqual((1, 1), self.assertMirrored(bug))
+        # Both the reporter and either the product owner or the product's
+        # security contact have grants. 
+        self.assertEqual((2, 1), self.assertMirrored(bug))
         self.assertContentEqual(
             [InformationType.EMBARGOEDSECURITY],
             self.getPolicyTypesForArtifact(artifact))
 
=== modified file 'lib/lp/bugs/tests/test_bugvisibility.py'
--- lib/lp/bugs/tests/test_bugvisibility.py	2012-02-15 02:01:14 +0000
+++ lib/lp/bugs/tests/test_bugvisibility.py	2012-04-17 18:05:27 +0000
@@ -3,11 +3,17 @@
 
 """Tests for visibility of a bug."""
 
+from contextlib import contextmanager
+
+from lp.registry.interfaces.person import TeamSubscriptionPolicy
 from lp.testing import (
     celebrity_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 
 
 class TestPublicBugVisibility(TestCaseWithFactory):
 
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py	2012-04-11 05:21:33 +0000
+++ lib/lp/services/features/flags.py	2012-04-17 18:05:27 +0000
@@ -216,13 +216,6 @@
      '',
      '',
      ''),
-    ('disclosure.enhanced_private_bug_subscriptions.enabled',
-     'boolean',
-     ('Enables the auto subscribing and unsubscribing of users as a bug '
-      'transitions between public, private and security related states.'),
-     '',
-     '',
-     ''),
     ('disclosure.users_hide_own_bug_comments.enabled',
      'boolean',
      'Allows users in project roles and comment owners to hide bug comments.',