Merge lp:~rharding/launchpad/store_edit_product_info_type into lp:launchpad

[Richard Harding]

Implement updating based on changing the information type on Product edit.

= Summary =

This is the second branch in allowing edits to the Product. It updates the
setter for the information type to update policies as required.

== Pre Implementation ==

Talked with Aaron about updating the policies correctly.

== Implementation Notes ==

The first update is to change the validation to make sure we allow the
_setLicense to create a commercial subscription if we've not had one
previously. This allows for a Public project to go to a Private information
type as an edit.

The second update makes sure that after we change the information type that
any policies are updated as required.

== Q/A ==

Create a new project, as a public information type, and then after you create
it go to the "Edit details" and change it to a Private type. It should adjust
accordingly and show as Private.

Create a new project, as a private information type, and then change it to
Public and it should.

== Tests ==

test_product.py

== Lint ==

Cleaned up a couple of lint errors from this and previous branch.

[Aaron Bentley (abentley)]

review: Approve

=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/browser/tests/test_product.py	2012-10-10 12:32:23 +0000
@@ -432,6 +432,74 @@
             cache.objects['team_membership_policy_data'])
 
 
+class TestProductEditView(TestCaseWithFactory):
+    """Tests for the ProductEditView"""
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestProductEditView, self).setUp()
+
+    def _make_product_edit_form(self, proprietary=False):
+        """Return form data for product edit.
+
+        :param product: Factory product to base the form data base of.
+        """
+        if proprietary:
+            licenses = License.OTHER_PROPRIETARY.title
+            license_info = 'Commercial Subscription'
+            information_type = 'PROPRIETARY'
+        else:
+            licenses = ['MIT']
+            license_info = ''
+            information_type = 'PUBLIC'
+
+        return {
+            'field.actions.change': 'Change',
+            'field.name': self.product.name,
+            'field.displayname': self.product.displayname,
+            'field.title': self.product.title,
+            'field.summary': self.product.summary,
+            'field.information_type': information_type,
+            'field.licenses': licenses,
+            'field.license_info': license_info,
+        }
+
+    def test_change_information_type_proprietary(self):
+        product = self.factory.makeProduct(name='fnord')
+        with FeatureFixture({u'disclosure.private_projects.enabled': u'on'}):
+            login_person(product.owner)
+            form = self._make_product_edit_form(proprietary=True)
+            view = create_initialized_view(product, '+edit', form=form)
+            self.assertEqual(0, len(view.errors))
+
+            product_set = getUtility(IProductSet)
+            updated_product = product_set.getByName('fnord')
+            self.assertEqual(
+                InformationType.PROPRIETARY, updated_product.information_type)
+            # A complimentary commercial subscription is auto generated for
+            # the product when the information type is changed.
+            self.assertIsNotNone(updated_product.commercial_subscription)
+
+    def test_change_information_type_public(self):
+        owner = self.factory.makePerson(name='pting')
+        product = self.factory.makeProduct(
+            name='fnord',
+            information_type=InformationType.PROPRIETARY,
+            owner=owner,
+        )
+        with FeatureFixture({u'disclosure.private_projects.enabled': u'on'}):
+            login_person(owner)
+            form = self._make_product_edit_form()
+            view = create_initialized_view(product, '+edit', form=form)
+            self.assertEqual(0, len(view.errors))
+
+            product_set = getUtility(IProductSet)
+            updated_product = product_set.getByName('fnord')
+            self.assertEqual(
+                InformationType.PUBLIC, updated_product.information_type)
+
+
 class ProductSetReviewLicensesViewTestCase(TestCaseWithFactory):
     """Tests the ProductSetReviewLicensesView."""
 

=== modified file 'lib/lp/registry/javascript/product_views.js'
--- lib/lp/registry/javascript/product_views.js	2012-10-10 12:32:23 +0000
+++ lib/lp/registry/javascript/product_views.js	2012-10-10 12:32:23 +0000
@@ -345,8 +345,10 @@
          */
         information_type_change: function (value) {
             toggle_license_field(value);
-            var driver_cont = Y.one('input[name="field.driver"]').ancestor('td');
-            var bug_super_cont = Y.one('input[name="field.bug_supervisor"]').ancestor('td');
+            var driver_cont =
+                Y.one('input[name="field.driver"]').ancestor('td');
+            var bug_super_cont =
+                Y.one('input[name="field.bug_supervisor"]').ancestor('td');
 
             if (info_type.get_cache_data_from_key(value, 'value',
                                                   'is_private')) {

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/model/product.py	2012-10-10 12:32:23 +0000
@@ -433,17 +433,43 @@
         # Proprietary check works only after creation, because during
         # creation, has_commercial_subscription cannot give the right value
         # and triggers an inappropriate DB flush.
-        if (not self._SO_creating and value in PROPRIETARY_INFORMATION_TYPES
-            and not self.has_current_commercial_subscription):
-            raise CommercialSubscribersOnly(
-                'A valid commercial subscription is required for private'
-                ' Projects.')
+
+        # If you're changing the license, and setting a PROPRIETARY
+        # information type, yet you don't have a subscription you get one when
+        # the license is set.
+
+        # If you have a commercial subscription, but it's not current, you
+        # cannot set the information type to a PROPRIETARY type.
+        if not self._SO_creating and value in PROPRIETARY_INFORMATION_TYPES:
+            # Create the complimentary commercial subscription for the product.
+            self._ensure_complimentary_subscription()
+
+            if not self.has_current_commercial_subscription:
+                raise CommercialSubscribersOnly(
+                    'A valid commercial subscription is required for private'
+                    ' Projects.')
+
         return value
 
-    information_type = EnumCol(
+    _information_type = EnumCol(
         enum=InformationType, default=InformationType.PUBLIC,
+        dbName="information_type",
         storm_validator=_valid_product_information_type)
 
+    def _get_information_type(self):
+        return self._information_type or InformationType.PUBLIC
+
+    def _set_information_type(self, value):
+        self._information_type = value
+        # Make sure that policies are updated to grant permission to the
+        # maintainer as required for the Product.
+        # However, only on edits. If this is a new Product it's handled
+        # already.
+        if not self._SO_creating:
+            self._ensurePolicies([value])
+
+    information_type = property(_get_information_type, _set_information_type)
+
     security_contact = None
 
     @property
@@ -952,6 +978,15 @@
         get_property_cache(self)._cached_licenses = tuple(sorted(licenses))
         if (License.OTHER_PROPRIETARY in licenses
             and self.commercial_subscription is None):
+            self._ensure_complimentary_subscription()
+
+        notify(LicensesModifiedEvent(self))
+
+    licenses = property(_getLicenses, _setLicenses)
+
+    def _ensure_complimentary_subscription(self):
+        """Create a complementary commercial subscription for the product"""
+        if not self.commercial_subscription:
             lp_janitor = getUtility(ILaunchpadCelebrities).janitor
             now = datetime.datetime.now(pytz.UTC)
             date_expires = now + datetime.timedelta(days=30)
@@ -964,9 +999,6 @@
                 registrant=lp_janitor, purchaser=lp_janitor,
                 sales_system_id=sales_system_id, whiteboard=whiteboard)
             get_property_cache(self).commercial_subscription = subscription
-        notify(LicensesModifiedEvent(self))
-
-    licenses = property(_getLicenses, _setLicenses)
 
     def _getOwner(self):
         """Get the owner."""

=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt	2012-10-08 10:07:11 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt	2012-10-10 12:32:23 +0000
@@ -168,7 +168,7 @@
     freshmeat_project: None
     homepage_url: None
     icon_link: u'http://.../firefox/icon'
-    information_type: None
+    information_type: u'Public'
     is_permitted: True
     license_approved: False
     license_info: None

=== modified file 'lib/lp/registry/tests/test_product.py'
--- lib/lp/registry/tests/test_product.py	2012-10-10 02:45:36 +0000
+++ lib/lp/registry/tests/test_product.py	2012-10-10 12:32:23 +0000
@@ -4,8 +4,10 @@
 __metaclass__ = type
 
 from cStringIO import StringIO
-import datetime
-
+from datetime import (
+    datetime,
+    timedelta,
+    )
 import pytz
 from storm.locals import Store
 from testtools.matchers import MatchesAll
@@ -500,21 +502,46 @@
                 with person_logged_in(product.owner):
                     product.information_type = info_type
 
-    def test_product_information_set_proprietary_requires_commercial(self):
-        # Cannot set Product.information_type to proprietary values if no
-        # commercial subscription.
-        product = self.factory.makeProduct()
-        self.useContext(person_logged_in(product.owner))
+    def test_set_proprietary_gets_commerical_subscription(self):
+        # Changing a Product to Proprietary will auto generate a complimentary
+        # subscription just as choosing a proprietary license at creation time.
+        owner = self.factory.makePerson(name='pting')
+        product = self.factory.makeProduct(owner=owner)
+        self.useContext(person_logged_in(owner))
+        self.assertIsNone(product.commercial_subscription)
+
+        product.information_type = InformationType.PROPRIETARY
+        self.assertEqual(InformationType.PROPRIETARY, product.information_type)
+        self.assertIsNotNone(product.commercial_subscription)
+
+    def test_set_proprietary_fails_expired_commerical_subscription(self):
+        # Cannot set information type to proprietary with an expired
+        # complimentary subscription.
+        owner = self.factory.makePerson(name='pting')
+        product = self.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY,
+            owner=owner,
+        )
+        self.useContext(person_logged_in(owner))
+
+        # The Product now has a complimentary commercial subscription.
+        new_expires_date = datetime.now(pytz.timezone('UTC')) - timedelta(1)
+        naked_subscription = removeSecurityProxy(
+            product.commercial_subscription)
+        naked_subscription.date_expires = new_expires_date
+
+        # We can make the product PUBLIC
+        product.information_type = InformationType.PUBLIC
+        self.assertEqual(InformationType.PUBLIC, product.information_type)
+
+        # However we can't change it back to a Proprietary because our
+        # commercial subscription has expired.
         for info_type in PROPRIETARY_INFORMATION_TYPES:
             with ExpectedException(
                 CommercialSubscribersOnly,
                 'A valid commercial subscription is required for private'
                 ' Projects.'):
                 product.information_type = info_type
-        product.redeemSubscriptionVoucher(
-            'hello', product.owner, product.owner, 1)
-        for info_type in PROPRIETARY_INFORMATION_TYPES:
-            product.information_type = info_type
 
     def test_product_information_init_proprietary_requires_commercial(self):
         # Cannot create a product with proprietary types without specifying
@@ -980,9 +1007,9 @@
             cs = product.commercial_subscription
             self.assertIsNotNone(cs)
             self.assertIn('complimentary-30-day', cs.sales_system_id)
-            now = datetime.datetime.now(pytz.UTC)
+            now = datetime.now(pytz.UTC)
             self.assertTrue(now >= cs.date_starts)
-            future_30_days = now + datetime.timedelta(days=30)
+            future_30_days = now + timedelta(days=30)
             self.assertTrue(future_30_days >= cs.date_expires)
             self.assertIn(
                 "Complimentary 30 day subscription. -- Launchpad",
@@ -1003,9 +1030,9 @@
             cs = product.commercial_subscription
             self.assertIsNotNone(cs)
             self.assertIn('complimentary-30-day', cs.sales_system_id)
-            now = datetime.datetime.now(pytz.UTC)
+            now = datetime.now(pytz.UTC)
             self.assertTrue(now >= cs.date_starts)
-            future_30_days = now + datetime.timedelta(days=30)
+            future_30_days = now + timedelta(days=30)
             self.assertTrue(future_30_days >= cs.date_expires)
             self.assertIn(
                 "Complimentary 30 day subscription. -- Launchpad",
@@ -1283,8 +1310,8 @@
         product = question.product
         transaction.commit()
         ws_product = self.wsObject(product, product.owner)
-        now = datetime.datetime.now(tz=pytz.utc)
-        day = datetime.timedelta(days=1)
+        now = datetime.now(tz=pytz.utc)
+        day = timedelta(days=1)
         self.failUnlessEqual(
             [oopsid],
             ws_product.findReferencedOOPS(start_date=now - day, end_date=now))
@@ -1301,8 +1328,8 @@
         product = self.factory.makeProduct()
         transaction.commit()
         ws_product = self.wsObject(product, product.owner)
-        now = datetime.datetime.now(tz=pytz.utc)
-        day = datetime.timedelta(days=1)
+        now = datetime.now(tz=pytz.utc)
+        day = timedelta(days=1)
         self.failUnlessEqual(
             [],
             ws_product.findReferencedOOPS(start_date=now - day, end_date=now))

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2012-10-08 10:07:11 +0000
+++ lib/lp/scripts/garbo.py	2012-10-10 12:32:23 +0000
@@ -928,10 +928,10 @@
     def __call__(self, chunk_size):
         """See `TunableLoop`."""
         subselect = Select(
-            Product.id, Product.information_type == None,
+            Product.id, Product._information_type == None,
             limit=chunk_size)
         result = self.store.execute(
-            Update({Product.information_type: 1},
+            Update({Product._information_type: 1},
             Product.id.is_in(subselect)))
         transaction.commit()
         self.rows_updated = result.rowcount