launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/model-product-info-type into lp:launchpad.

Commit message:
Implement Product.information_type in Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1060384 in Launchpad itself: "Product.information_type is not exposed via ORM"
  https://bugs.launchpad.net/launchpad/+bug/1060384

For more details, see:
https://code.launchpad.net/~abentley/launchpad/model-product-info-type/+merge/127558

= Summary =
Provide Product.information_type via Storm

== Proposed fix ==
Add Product.information_type.  Implement IInformationType, remove IProduct.information_type

== Pre-implementation notes ==
None

== LOC Rationale ==
Part of Private Projects

== Implementation details ==
The database column has already landed; this adds support to Storm.

The information_type move is to reduce duplication and to simplify security policy implementation.

information_type is nullable because the database permits this and existing data contains NULLs.  Once a garbo job has removed all NULLs, it can be updated to prohibit nulls.

== Tests ==
bin/test -t test_product_information_type test_product

== Demo and Q/A ==
View a product page.  If nothing's broken, it's good.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/model/launchpad.py
  lib/lp/blueprints/model/specification.py
  lib/lp/bugs/model/bug.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/interfaces/product.py
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_product.py
  lib/lp/testing/factory.py
-- 
https://code.launchpad.net/~abentley/launchpad/model-product-info-type/+merge/127558
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/model-product-info-type into lp:launchpad.

=== modified file 'lib/lp/app/model/launchpad.py'
--- lib/lp/app/model/launchpad.py	2012-02-28 04:24:19 +0000
+++ lib/lp/app/model/launchpad.py	2012-10-02 19:14:21 +0000
@@ -7,6 +7,7 @@
 
 __all__ = [
     'ExceptionPrivacy',
+    'InformationTypeMixin',
     'Privacy',
     ]
 
@@ -17,6 +18,7 @@
     Unauthorized,
     )
 
+from lp.app.enums import PRIVATE_INFORMATION_TYPES
 from lp.app.interfaces.launchpad import IPrivacy
 
 
@@ -38,3 +40,11 @@
         else:
             private = False
         super(ExceptionPrivacy, self).__init__(error, private)
+
+
+class InformationTypeMixin:
+    """"Common functionality for classes implementing IInformationType."""
+
+    @property
+    def private(self):
+        return self.information_type in PRIVATE_INFORMATION_TYPES

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-09-27 14:30:40 +0000
+++ lib/lp/blueprints/model/specification.py	2012-10-02 19:14:21 +0000
@@ -47,11 +47,11 @@
 
 from lp.app.enums import (
     InformationType,
-    PRIVATE_INFORMATION_TYPES,
     PUBLIC_INFORMATION_TYPES,
     )
 from lp.app.errors import UserCannotUnsubscribePerson
 from lp.app.interfaces.informationtype import IInformationType
+from lp.app.model.launchpad import InformationTypeMixin
 from lp.blueprints.adapters import SpecificationDelta
 from lp.blueprints.enums import (
     NewSpecificationDefinitionStatus,
@@ -157,7 +157,7 @@
     }
 
 
-class Specification(SQLBase, BugLinkTargetMixin):
+class Specification(SQLBase, BugLinkTargetMixin, InformationTypeMixin):
     """See ISpecification."""
 
     implements(ISpecification, IBugLinkTarget, IInformationType)
@@ -891,10 +891,6 @@
         reconcile_access_for_artifact(self, information_type, [self.target])
         return True
 
-    @property
-    def private(self):
-        return self.information_type in PRIVATE_INFORMATION_TYPES
-
     @cachedproperty
     def _known_viewers(self):
         """A set of known persons able to view the specifcation."""

=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py	2012-09-28 06:15:58 +0000
+++ lib/lp/bugs/model/bug.py	2012-10-02 19:14:21 +0000
@@ -93,6 +93,7 @@
     )
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.interfaces.services import IService
+from lp.app.model.launchpad import InformationTypeMixin
 from lp.app.validators import LaunchpadValidationError
 from lp.bugs.adapters.bug import convert_to_information_type
 from lp.bugs.adapters.bugchange import (
@@ -303,7 +304,7 @@
         self.user = user
 
 
-class Bug(SQLBase):
+class Bug(SQLBase, InformationTypeMixin):
     """A bug."""
 
     implements(IBug)
@@ -363,10 +364,6 @@
     latest_patch_uploaded = UtcDateTimeCol(default=None)
 
     @property
-    def private(self):
-        return self.information_type in PRIVATE_INFORMATION_TYPES
-
-    @property
     def security_related(self):
         return self.information_type in SECURITY_INFORMATION_TYPES
 

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-09-25 04:29:34 +0000
+++ lib/lp/registry/configure.zcml	2012-10-02 19:14:21 +0000
@@ -1241,6 +1241,11 @@
         <require
             permission="launchpad.Edit"
             interface="lp.registry.interfaces.product.IProductEditRestricted"/>
+        <allow
+            interface="lp.app.interfaces.informationtype.IInformationType"/>
+        <require
+            permission="launchpad.Edit"
+            set_schema="lp.app.interfaces.informationtype.IInformationType"/>
         <require
             permission="launchpad.Edit"
             set_attributes="answers_usage blueprints_usage codehosting_usage

=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py	2012-09-28 06:15:58 +0000
+++ lib/lp/registry/interfaces/product.py	2012-10-02 19:14:21 +0000
@@ -73,7 +73,6 @@
 
 from lp import _
 from lp.answers.interfaces.questiontarget import IQuestionTarget
-from lp.app.enums import InformationType
 from lp.app.errors import NameLookupFailed
 from lp.app.interfaces.headings import IRootContext
 from lp.app.interfaces.launchpad import (
@@ -450,13 +449,6 @@
                 'and security policy will apply to this project.')),
         exported_as='project_group')
 
-    information_type = exported(
-        Choice(
-            title=_('Information Type'), vocabulary=InformationType,
-            required=True, readonly=True,
-            description=_(
-                'The type of of data contained in this project.')))
-
     owner = exported(
         PersonChoice(
             title=_('Maintainer'),

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-09-28 19:59:35 +0000
+++ lib/lp/registry/model/product.py	2012-10-02 19:14:21 +0000
@@ -73,6 +73,7 @@
     ServiceUsage,
     )
 from lp.app.errors import NotFoundError
+from lp.app.interfaces.informationtype import IInformationType
 from lp.app.interfaces.launchpad import (
     IHasIcon,
     IHasLogo,
@@ -81,6 +82,7 @@
     ILaunchpadUsage,
     IServiceUsage,
     )
+from lp.app.model.launchpad import InformationTypeMixin
 from lp.blueprints.enums import (
     SpecificationDefinitionStatus,
     SpecificationFilter,
@@ -323,13 +325,15 @@
               HasAliasMixin, StructuralSubscriptionTargetMixin,
               HasMilestonesMixin, OfficialBugTagTargetMixin, HasBranchesMixin,
               HasCustomLanguageCodesMixin, HasMergeProposalsMixin,
-              HasCodeImportsMixin, TranslationPolicyMixin):
+              HasCodeImportsMixin, InformationTypeMixin,
+              TranslationPolicyMixin):
     """A Product."""
 
     implements(
         IBugSummaryDimension, IFAQTarget, IHasBugSupervisor,
         IHasCustomLanguageCodes, IHasIcon, IHasLogo, IHasMugshot,
-        IHasOOPSReferences, ILaunchpadUsage, IProduct, IServiceUsage)
+        IHasOOPSReferences, ILaunchpadUsage, IProduct, IServiceUsage,
+        IInformationType,)
 
     _table = 'Product'
 
@@ -405,7 +409,7 @@
         """
         return None
 
-    @date_next_suggest_packaging.setter
+    @date_next_suggest_packaging.setter  # pyflakes:ignore
     def date_next_suggest_packaging(self, value):
         """See `IProduct`
 
@@ -413,15 +417,8 @@
         """
         pass
 
-    @property
-    def information_type(self):
-        """See `IProduct`
-
-        Place holder for a db column.
-        XXX: rharding 2012-09-10 bug=1048720: Waiting on db patch to connect
-        into place.
-        """
-        pass
+    information_type = EnumCol(
+        enum=InformationType, default=InformationType.PUBLIC)
 
     security_contact = None
 
@@ -1599,12 +1596,15 @@
                       sourceforgeproject=None, programminglang=None,
                       project_reviewed=False, mugshot=None, logo=None,
                       icon=None, licenses=None, license_info=None,
-                      registrant=None, bug_supervisor=None, driver=None):
+                      registrant=None, bug_supervisor=None, driver=None,
+                      information_type=None):
         """See `IProductSet`."""
         if registrant is None:
             registrant = owner
         if licenses is None:
             licenses = set()
+        if information_type is None:
+            information_type = InformationType.PUBLIC
         product = Product(
             owner=owner, registrant=registrant, name=name,
             displayname=displayname, title=title, project=project,
@@ -1615,7 +1615,8 @@
             programminglang=programminglang,
             project_reviewed=project_reviewed,
             icon=icon, logo=logo, mugshot=mugshot, license_info=license_info,
-            bug_supervisor=bug_supervisor, driver=driver)
+            bug_supervisor=bug_supervisor, driver=driver,
+            information_type=information_type)
 
         # Set up the sharing policies and product licence.
         bug_sharing_policy_to_use = BugSharingPolicy.PUBLIC

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-09-28 06:15:58 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-02 19:14:21 +0000
@@ -7,6 +7,7 @@
 import datetime
 
 import pytz
+from storm.locals import Store
 from testtools.matchers import MatchesAll
 import transaction
 from zope.component import getUtility
@@ -28,6 +29,7 @@
     ILaunchpadUsage,
     IServiceUsage,
     )
+from lp.app.interfaces.informationtype import IInformationType
 from lp.app.interfaces.services import IService
 from lp.bugs.interfaces.bugsummary import IBugSummaryDimension
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
@@ -111,6 +113,7 @@
             IHasLogo,
             IHasMugshot,
             IHasOOPSReferences,
+            IInformationType,
             ILaunchpadUsage,
             IServiceUsage,
             ]
@@ -390,6 +393,29 @@
         expected = [InformationType.PROPRIETARY]
         self.assertContentEqual(expected, [policy.type for policy in aps])
 
+    def test_product_information_type(self):
+        # Product is created with specified information_type
+        product = self.factory.makeProduct(
+            information_type=InformationType.EMBARGOED)
+        self.assertEqual(InformationType.EMBARGOED, product.information_type)
+        # Owner can set information_type
+        with person_logged_in(product.owner):
+            product.information_type = InformationType.PROPRIETARY
+        self.assertEqual(InformationType.PROPRIETARY, product.information_type)
+        # Database persists information_type value
+        store = Store.of(product)
+        store.flush()
+        store.reset()
+        product = store.get(Product, product.id)
+        self.assertEqual(InformationType.PROPRIETARY, product.information_type)
+
+    def test_product_information_type_default(self):
+        # Default information_type is PUBLIC
+        owner = self.factory.makePerson()
+        product = getUtility(IProductSet).createProduct(
+            owner, 'fnord', 'Fnord', 'Fnord', 'test 1', 'test 2')
+        self.assertEqual(InformationType.PUBLIC, product.information_type)
+
 
 class TestProductBugInformationTypes(TestCaseWithFactory):
 

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-10-02 07:08:13 +0000
+++ lib/lp/testing/factory.py	2012-10-02 19:14:21 +0000
@@ -964,7 +964,8 @@
         title=None, summary=None, official_malone=None,
         translations_usage=None, bug_supervisor=None, private_bugs=False,
         driver=None, icon=None, bug_sharing_policy=None,
-        branch_sharing_policy=None, specification_sharing_policy=None):
+        branch_sharing_policy=None, specification_sharing_policy=None,
+        information_type=None):
         """Create and return a new, arbitrary Product."""
         if owner is None:
             owner = self.makePerson()
@@ -993,7 +994,8 @@
                 licenses=licenses,
                 project=project,
                 registrant=registrant,
-                icon=icon)
+                icon=icon,
+                information_type=information_type)
         naked_product = removeSecurityProxy(product)
         if official_malone is not None:
             naked_product.official_malone = official_malone