launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugs-use-information_type into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #933766 in Launchpad itself: "Update bug to use information_visibility_policy"
  https://bugs.launchpad.net/launchpad/+bug/933766

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugs-use-information_type/+merge/98349

Rename IBug.{private,security_related} to IBug._{private,security_related} as the next step in switching to information_type. This branch also adds IBug.{private,security_related} back as a property that depends on information_type if it is set.

I cleaned up a small amount of lint.
-- 
https://code.launchpad.net/~stevenk/launchpad/bugs-use-information_type/+merge/98349
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugs-use-information_type into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2012-03-20 06:59:19 +0000
@@ -40,10 +40,6 @@
 from lp.testing.views import create_initialized_view
 
 
-ON = 'on'
-OFF = None
-
-
 class BugsubscriptionPrivacyTests(TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
@@ -52,7 +48,7 @@
         super(BugsubscriptionPrivacyTests, self).setUp()
         self.user = self.factory.makePerson()
         self.bug = self.factory.makeBug(owner=self.user)
-        removeSecurityProxy(self.bug).private = True
+        removeSecurityProxy(self.bug).setPrivate(True, self.user)
 
     def _assert_subscription_fails(self, team):
         with person_logged_in(self.user):

=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2012-03-13 22:52:33 +0000
+++ lib/lp/bugs/interfaces/bug.py	2012-03-20 06:59:19 +0000
@@ -22,6 +22,8 @@
     'IFileBugData',
     'IFrontPageBugAddForm',
     'IProjectGroupBugAddForm',
+    'PRIVATE_BUG_TYPES',
+    'SECURITY_BUG_TYPES',
     ]
 
 from lazr.enum import DBEnumeratedType
@@ -97,6 +99,15 @@
 from lp.services.messages.interfaces.message import IMessage
 
 
+PRIVATE_BUG_TYPES = (
+    InformationType.EMBARGOEDSECURITY, InformationType.USERDATA,
+    InformationType.PROPRIETARY)
+
+
+SECURITY_BUG_TYPES = (
+    InformationType.UNEMBARGOEDSECURITY, InformationType.EMBARGOEDSECURITY)
+
+
 class CreateBugParams:
     """The parameters used to create a bug."""
 

=== modified file 'lib/lp/bugs/mail/tests/test_handler.py'
--- lib/lp/bugs/mail/tests/test_handler.py	2012-01-20 15:42:44 +0000
+++ lib/lp/bugs/mail/tests/test_handler.py	2012-03-20 06:59:19 +0000
@@ -287,7 +287,7 @@
         notification = self.getLatestBugNotification()
         bug = notification.bug
         self.assertEqual('unsecure code', bug.title)
-        self.assertEqual(True, bug.security_related)
+        self.assertTrue(bug.security_related)
         self.assertEqual(['ajax'], bug.tags)
         self.assertEqual(1, len(bug.bugtasks))
         self.assertEqual(project, bug.bugtasks[0].target)
@@ -310,7 +310,7 @@
         notification = self.getLatestBugNotification()
         bug = notification.bug
         self.assertEqual('security issue', bug.title)
-        self.assertEqual(True, bug.security_related)
+        self.assertTrue(bug.security_related)
         self.assertEqual(1, len(bug.bugtasks))
         self.assertEqual(project, bug.bugtasks[0].target)
         recipients = set()

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-03-14 12:39:42 +0000
+++ lib/lp/bugs/model/bug.py	2012-03-20 06:59:19 +0000
@@ -111,6 +111,8 @@
     IBugMute,
     IBugSet,
     IFileBugData,
+    PRIVATE_BUG_TYPES,
+    SECURITY_BUG_TYPES,
     )
 from lp.bugs.interfaces.bugactivity import IBugActivitySet
 from lp.bugs.interfaces.bugattachment import (
@@ -347,12 +349,13 @@
         dbName='duplicateof', foreignKey='Bug', default=None)
     datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
     date_last_updated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
-    private = BoolCol(notNull=True, default=False)
+    _private = BoolCol(dbName='private', notNull=True, default=False)
     date_made_private = UtcDateTimeCol(notNull=False, default=None)
     who_made_private = ForeignKey(
         dbName='who_made_private', foreignKey='Person',
         storm_validator=validate_public_person, default=None)
-    security_related = BoolCol(notNull=True, default=False)
+    _security_related = BoolCol(
+        dbName='security_related', notNull=True, default=False)
     information_type = EnumCol(
         enum=InformationType, default=InformationType.PUBLIC)
 
@@ -389,6 +392,20 @@
     heat_last_updated = UtcDateTimeCol(default=None)
     latest_patch_uploaded = UtcDateTimeCol(default=None)
 
+    @property
+    def private(self):
+        if self.information_type:
+            return self.information_type in PRIVATE_BUG_TYPES
+        else:
+            return self._private
+
+    @property
+    def security_related(self):
+        if self.information_type:
+            return self.information_type in SECURITY_BUG_TYPES
+        else:
+            return self._security_related
+
     @cachedproperty
     def _subscriber_cache(self):
         """Caches known subscribers."""
@@ -1698,11 +1715,11 @@
         return bugtask
 
     def _setInformationType(self):
-        if self.private and self.security_related:
+        if self._private and self._security_related:
             self.information_type = InformationType.EMBARGOEDSECURITY
-        elif self.private:
+        elif self._private:
             self.information_type = InformationType.USERDATA
-        elif self.security_related:
+        elif self._security_related:
             self.information_type = InformationType.UNEMBARGOEDSECURITY
         else:
             self.information_type = InformationType.PUBLIC
@@ -1734,7 +1751,7 @@
                     raise BugCannotBePrivate(
                         "Multi-pillar bugs cannot be private.")
             private_changed = True
-            self.private = private
+            self._private = private
 
             if private:
                 self.who_made_private = who
@@ -1750,7 +1767,7 @@
 
         if self.security_related != security_related:
             security_related_changed = True
-            self.security_related = security_related
+            self._security_related = security_related
 
         if private_changed or security_related_changed:
             # Correct the heat for the bug immediately, so that we don't have
@@ -2829,9 +2846,6 @@
                 bug.subscribe(params.product.bug_supervisor, params.owner)
             else:
                 bug.subscribe(params.product.owner, params.owner)
-        else:
-            # nothing to do
-            pass
 
         # Create the task on a product if one was passed.
         if params.product:
@@ -2908,9 +2922,9 @@
 
         bug = Bug(
             title=params.title, description=params.description,
-            private=params.private, owner=params.owner,
+            _private=params.private, owner=params.owner,
             datecreated=params.datecreated,
-            security_related=params.security_related,
+            _security_related=params.security_related,
             **extra_params)
 
         if params.subscribe_owner:

=== modified file 'lib/lp/bugs/tests/test_bug_mirror_access_triggers.py'
--- lib/lp/bugs/tests/test_bug_mirror_access_triggers.py	2012-03-09 03:32:09 +0000
+++ lib/lp/bugs/tests/test_bug_mirror_access_triggers.py	2012-03-20 06:59:19 +0000
@@ -136,7 +136,7 @@
         bug = self.makeBugAndPolicies(private=True)
         self.assertIsNot(
             None, getUtility(IAccessArtifactSource).find([bug]).one())
-        bug.private = False
+        removeSecurityProxy(bug).setPrivate(False, bug.owner)
         self.assertIs(
             None, getUtility(IAccessArtifactSource).find([bug]).one())
 
@@ -144,7 +144,7 @@
         bug = self.makeBugAndPolicies(private=False)
         self.assertIs(
             None, getUtility(IAccessArtifactSource).find([bug]).one())
-        bug.private = True
+        removeSecurityProxy(bug).setPrivate(True, bug.owner)
         self.assertIsNot(
             None, getUtility(IAccessArtifactSource).find([bug]).one())
         self.assertEqual((1, 1), self.assertMirrored(bug))
@@ -158,7 +158,7 @@
         self.assertContentEqual(
             [InformationType.USERDATA],
             self.getPolicyTypesForArtifact(artifact))
-        bug.security_related = True
+        removeSecurityProxy(bug).setSecurityRelated(True, bug.owner)
         self.assertEqual((1, 1), self.assertMirrored(bug))
         self.assertContentEqual(
             [InformationType.EMBARGOEDSECURITY],