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)

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

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/configure.zcml
  lib/lp/registry/interfaces/product.py
  lib/lp/testing/factory.py
  lib/lp/registry/model/product.py
  lib/lp/registry/tests/test_product.py

./lib/lp/registry/model/product.py
     410: redefinition of function 'date_next_suggest_packaging' from line 402

^^ This is a blessed redefinition-- it's creating a property setter.
-- 
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/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-09-25 04:29:34 +0000
+++ lib/lp/registry/configure.zcml	2012-10-02 18:04:37 +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 18:04:37 +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 18:04:37 +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,
@@ -329,7 +330,8 @@
     implements(
         IBugSummaryDimension, IFAQTarget, IHasBugSupervisor,
         IHasCustomLanguageCodes, IHasIcon, IHasLogo, IHasMugshot,
-        IHasOOPSReferences, ILaunchpadUsage, IProduct, IServiceUsage)
+        IHasOOPSReferences, ILaunchpadUsage, IProduct, IServiceUsage,
+        IInformationType,)
 
     _table = 'Product'
 
@@ -413,15 +415,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 +1594,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 +1613,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 18:04:37 +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
@@ -390,6 +391,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 18:04:37 +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