Merge lp:~wgrant/launchpad/bug-1100164 into lp:launchpad

[William Grant]

Permit proprietary products to have releases, forbid them from adding release files.

Proprietary products are currently banned from having releases due to a misunderstanding in the late stages of implementation. This branch rescinds that ban and instates a new one on release *files* -- the real leak.

The addReleaseFile and +adddownloadfile reject attempts to add a file, and the listing/link are hidden from Milestone/ProductRelease:+index. I also removed the Downloads section from Product:+index for non-public projects.

And I secured the Zope permissions for ProductRelease.

[Steve Kowalik (stevenk)]

review: Approve (code)

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2012-12-10 20:07:39 +0000
+++ lib/lp/registry/browser/configure.zcml	2013-01-25 06:18:24 +0000
@@ -2012,7 +2012,7 @@
         provides="lp.services.webapp.interfaces.IBreadcrumb"
         for="lp.registry.interfaces.productrelease.IProductRelease"
         factory="lp.registry.browser.milestone.MilestoneBreadcrumb"
-        permission="zope.Public"/>
+        permission="launchpad.View"/>
     <browser:url
         for="lp.registry.interfaces.productrelease.IProductRelease"
         path_expression="version"
@@ -2027,21 +2027,21 @@
         name="+index"/>
     <browser:pages
         for="lp.registry.interfaces.productrelease.IProductRelease"
-        permission="zope.Public">
+        permission="launchpad.View">
         <browser:page
             name="+portlet-downloads"
             template="../templates/productrelease-portlet-downloads.pt"/>
     </browser:pages>
     <browser:page
         for="lp.registry.interfaces.productrelease.IProductRelease"
-        permission="zope.Public"
+        permission="launchpad.View"
         name="+index"
         class="lp.registry.browser.milestone.MilestoneView"
         template="../templates/milestone-index.pt"/>
     <browser:page
         for="lp.registry.interfaces.productrelease.IProductRelease"
         class="lp.registry.browser.productrelease.ProductReleaseRdfView"
-        permission="zope.Public"
+        permission="launchpad.View"
         name="+rdf"
         attribute="__call__"/>
     <browser:page

=== modified file 'lib/lp/registry/browser/productrelease.py'
--- lib/lp/registry/browser/productrelease.py	2012-12-10 20:47:40 +0000
+++ lib/lp/registry/browser/productrelease.py	2013-01-25 06:18:24 +0000
@@ -39,7 +39,6 @@
     LaunchpadEditFormView,
     LaunchpadFormView,
     )
-from lp.app.enums import InformationType
 from lp.app.widgets.date import DateTimeWidget
 from lp.registry.browser import (
     BaseRdfView,
@@ -136,11 +135,6 @@
         notify(ObjectCreatedEvent(newrelease))
 
     @property
-    def releases_allowed(self):
-        return (self.context.product.information_type !=
-                InformationType.PROPRIETARY)
-
-    @property
     def label(self):
         """The form label."""
         return smartquote('Create a new release for %s' %
@@ -166,11 +160,6 @@
         ]
 
     def initialize(self):
-        if (self.context.product.information_type ==
-            InformationType.EMBARGOED):
-            self.request.response.addWarningNotification(
-                _("Any releases added for %s will be PUBLIC." %
-                  self.context.product.displayname))
         if self.context.product_release is not None:
             self.request.response.addErrorNotification(
                 _("A project release already exists for this milestone."))
@@ -202,14 +191,6 @@
         'changelog',
         ]
 
-    def initialize(self):
-        if (self.context.product.information_type ==
-            InformationType.EMBARGOED):
-            self.request.response.addWarningNotification(
-                _("Any releases added for %s will be PUBLIC." %
-                  self.context.displayname))
-        super(ProductReleaseFromSeriesAddView, self).initialize()
-
     def setUpFields(self):
         super(ProductReleaseFromSeriesAddView, self).setUpFields()
         self._prependKeepMilestoneActiveField()
@@ -294,6 +275,8 @@
 
     def validate(self, data):
         """See `LaunchpadFormView`."""
+        if not self.context.can_have_release_files:
+            self.addError('Only public projects can have download files.')
         file_name = None
         filecontent = self.request.form.get(self.widgets['filecontent'].name)
         if filecontent:

=== modified file 'lib/lp/registry/browser/tests/test_milestone.py'
--- lib/lp/registry/browser/tests/test_milestone.py	2012-11-15 22:22:38 +0000
+++ lib/lp/registry/browser/tests/test_milestone.py	2013-01-25 06:18:24 +0000
@@ -117,6 +117,39 @@
         with person_logged_in(None):
             browser = self.getViewBrowser(milestone, '+index')
 
+    def test_downloads_listed(self):
+        # When a release exists a list of download files and a link to
+        # add more are shown.
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(owner=owner)
+        release = self.factory.makeProductRelease(product=product)
+        with person_logged_in(owner):
+            owner_browser = self.getViewBrowser(
+                release.milestone, '+index', user=owner)
+            html = owner_browser.contents
+            self.assertIn('Download files for this release', html)
+            self.assertIn('Add download file', html)
+        with person_logged_in(None):
+            owner_browser = self.getViewBrowser(release.milestone, '+index')
+            html = owner_browser.contents
+            self.assertIn('Download files for this release', html)
+            self.assertNotIn('Add download file', html)
+
+    def test_downloads_section_hidden_for_proprietary_product(self):
+        # Only public projects can have download files, so the downloads
+        # section is replaced with a message indicating this.
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner, information_type=InformationType.PROPRIETARY)
+        with person_logged_in(owner):
+            release = self.factory.makeProductRelease(product=product)
+            owner_browser = self.getViewBrowser(
+                release.milestone, '+index', user=owner)
+            html = owner_browser.contents
+            self.assertIn(
+                'Only public projects can have download files.', html)
+            self.assertNotIn('Add download file', html)
+
 
 class TestAddMilestoneViews(TestCaseWithFactory):
 

=== modified file 'lib/lp/registry/browser/tests/test_productrelease.py'
--- lib/lp/registry/browser/tests/test_productrelease.py	2012-12-11 13:40:11 +0000
+++ lib/lp/registry/browser/tests/test_productrelease.py	2013-01-25 06:18:24 +0000
@@ -9,7 +9,6 @@
 from lp.app.enums import InformationType
 from lp.services.webapp.escaping import html_escape
 from lp.testing import (
-    BrowserTestCase,
     person_logged_in,
     TestCaseWithFactory,
     )
@@ -17,60 +16,6 @@
 from lp.testing.views import create_initialized_view
 
 
-class NonPublicProductReleaseViewTestCase(BrowserTestCase):
-
-    layer = LaunchpadFunctionalLayer
-
-    def test_proprietary_add_milestone(self):
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(name='fnord',
-            owner=owner, information_type=InformationType.PROPRIETARY)
-        milestone = self.factory.makeMilestone(product=product)
-        with person_logged_in(owner):
-            browser = self.getViewBrowser(
-                milestone, view_name="+addrelease", user=owner)
-            msg = 'Fnord is PROPRIETARY. It cannot have any releases.'
-            self.assertTrue(html_escape(msg) in browser.contents)
-
-    def test_proprietary_add_series(self):
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(name='fnord',
-            owner=owner, information_type=InformationType.PROPRIETARY)
-        series = self.factory.makeProductSeries(product=product, name='bnord')
-        with person_logged_in(owner):
-            browser = self.getViewBrowser(
-                series, view_name="+addrelease", user=owner)
-            msg = ('The bnord series of Fnord is PROPRIETARY.'
-                   ' It cannot have any releases.')
-            self.assertTrue(html_escape(msg) in browser.contents)
-
-    def test_embargoed_add_milestone(self):
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(name='fnord',
-            owner=owner, information_type=InformationType.EMBARGOED)
-        milestone = self.factory.makeMilestone(product=product)
-        with person_logged_in(owner):
-            view = create_initialized_view(milestone, name="+addrelease")
-            notifications = [
-                nm.message for nm in view.request.response.notifications]
-            self.assertEqual(
-                [html_escape("Any releases added for Fnord will be PUBLIC.")],
-                notifications)
-
-    def test_embargoed_add_series(self):
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(name='fnord',
-            owner=owner, information_type=InformationType.EMBARGOED)
-        series = self.factory.makeProductSeries(product=product, name='bnord')
-        with person_logged_in(owner):
-            view = create_initialized_view(series, name="+addrelease")
-            notifications = [
-                nm.message for nm in view.request.response.notifications]
-            self.assertEqual(
-                [html_escape("Any releases added for bnord will be PUBLIC.")],
-                notifications)
-
-
 class ProductReleaseAddDownloadFileViewTestCase(TestCaseWithFactory):
 
     layer = LaunchpadFunctionalLayer
@@ -111,3 +56,15 @@
         self.assertEqual(
             [html_escape("The file '%s' is already uploaded." % file_name)],
             view.errors)
+
+    def test_refuses_proprietary_products(self):
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            owner=owner, information_type=InformationType.PROPRIETARY)
+        with person_logged_in(owner):
+            release = self.factory.makeProductRelease(product=product)
+            form = self.makeForm('something.tar.gz')
+            view = create_initialized_view(
+                release, '+adddownloadfile', form=form)
+        self.assertEqual(
+            ['Only public projects can have download files.'], view.errors)

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2012-12-06 16:09:31 +0000
+++ lib/lp/registry/configure.zcml	2013-01-25 06:18:24 +0000
@@ -1875,10 +1875,13 @@
         <allow
             interface="lp.registry.interfaces.productrelease.IProductReleasePublic"/>
         <require
+            permission="launchpad.View"
+            interface="lp.registry.interfaces.productrelease.IProductReleaseView"/>
+        <require
             permission="launchpad.Edit"
             interface="lp.registry.interfaces.productrelease.IProductReleaseEditRestricted"/>
         <require
-            permission="zope.Public"
+            permission="launchpad.Edit"
             set_schema="lp.registry.interfaces.productrelease.IProductRelease"/>
     </class>
     <adapter

=== modified file 'lib/lp/registry/interfaces/productrelease.py'
--- lib/lp/registry/interfaces/productrelease.py	2012-11-21 21:29:29 +0000
+++ lib/lp/registry/interfaces/productrelease.py	2013-01-25 06:18:24 +0000
@@ -274,6 +274,10 @@
 
     id = Int(title=_('ID'), required=True, readonly=True)
 
+
+class IProductReleaseView(Interface):
+    """launchpad.View-restricted `IProductRelease` properties."""
+
     datereleased = exported(
         Datetime(
             title=_('Date released'), required=True,
@@ -334,6 +338,8 @@
         Text(title=u'Constructed title for a project release.', readonly=True)
         )
 
+    can_have_release_files = Attribute("Whether release files can be added.")
+
     product = exported(
         Reference(title=u'The project that made this release.',
                   schema=Interface, readonly=True),
@@ -371,7 +377,7 @@
         """Does the release have a file that matches the name?"""
 
 
-class IProductRelease(IProductReleaseEditRestricted,
+class IProductRelease(IProductReleaseEditRestricted, IProductReleaseView,
                       IProductReleasePublic):
     """A specific release (i.e. version) of a product.
 

=== modified file 'lib/lp/registry/model/milestone.py'
--- lib/lp/registry/model/milestone.py	2013-01-22 05:07:31 +0000
+++ lib/lp/registry/model/milestone.py	2013-01-25 06:18:24 +0000
@@ -36,7 +36,6 @@
 from zope.component import getUtility
 from zope.interface import implements
 
-from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
 from lp.blueprints.model.specification import Specification
 from lp.blueprints.model.specificationsearch import (
@@ -56,7 +55,6 @@
 from lp.bugs.model.structuralsubscription import (
     StructuralSubscriptionTargetMixin,
     )
-from lp.registry.errors import ProprietaryProduct
 from lp.registry.interfaces.milestone import (
     IHasMilestones,
     IMilestone,
@@ -277,10 +275,6 @@
     def createProductRelease(self, owner, datereleased,
                              changelog=None, release_notes=None):
         """See `IMilestone`."""
-        info_type = self.product.information_type
-        if info_type == InformationType.PROPRIETARY:
-            raise ProprietaryProduct(
-                "Proprietary products cannot have releases.")
         if self.product_release is not None:
             raise MultipleProductReleases()
         release = ProductRelease(

=== modified file 'lib/lp/registry/model/productrelease.py'
--- lib/lp/registry/model/productrelease.py	2012-11-21 21:29:29 +0000
+++ lib/lp/registry/model/productrelease.py	2013-01-25 06:18:24 +0000
@@ -25,8 +25,12 @@
 from zope.component import getUtility
 from zope.interface import implements
 
+from lp.app.enums import InformationType
 from lp.app.errors import NotFoundError
-from lp.registry.errors import InvalidFilename
+from lp.registry.errors import (
+    InvalidFilename,
+    ProprietaryProduct,
+    )
 from lp.registry.interfaces.person import (
     validate_person,
     validate_public_person,
@@ -104,6 +108,11 @@
         """See `IProductRelease`."""
         return self.milestone.title
 
+    @property
+    def can_have_release_files(self):
+        """See `IProductRelease`."""
+        return self.product.information_type == InformationType.PUBLIC
+
     @staticmethod
     def normalizeFilename(filename):
         # Replace slashes in the filename with less problematic dashes.
@@ -141,6 +150,9 @@
                        file_type=UpstreamFileType.CODETARBALL,
                        description=None):
         """See `IProductRelease`."""
+        if not self.can_have_release_files:
+            raise ProprietaryProduct(
+                "Only public projects can have download files.")
         if self.hasReleaseFile(filename):
             raise InvalidFilename
         # Create the alias for the file.

=== modified file 'lib/lp/registry/stories/product/xx-product-files.txt'
--- lib/lp/registry/stories/product/xx-product-files.txt	2012-12-10 13:43:47 +0000
+++ lib/lp/registry/stories/product/xx-product-files.txt	2013-01-25 06:18:24 +0000
@@ -47,6 +47,18 @@
     File                            Description   Downloads
     firefox_0.9.2.orig.tar.gz (md5)               -
 
+Only public projects can have download files, so the portlet is omitted
+otherwise.
+
+    >>> from lp.app.enums import InformationType
+    >>> login('[e-mail]')
+    >>> prop_prod = factory.makeProduct(
+    ...     name='prop-prod', information_type=InformationType.PROPRIETARY)
+    >>> logout()
+    >>> admin_browser.open('http://launchpad.dev/prop-prod')
+    >>> print find_tag_by_id(admin_browser.contents, 'downloads')
+    None
+
 
 Deletion is only for the privileged
 ===================================

=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt	2012-12-07 20:37:04 +0000
+++ lib/lp/registry/templates/product-index.pt	2013-01-25 06:18:24 +0000
@@ -236,6 +236,7 @@
       <div tal:replace="structure context/@@+get-involved" />
 
       <div id="downloads" class="top-portlet downloads"
+        tal:condition="context/information_type/enumvalue:PUBLIC"
         tal:define="release view/latest_release_with_download_files">
         <h2>Downloads</h2>
 

=== modified file 'lib/lp/registry/templates/productrelease-add-from-series.pt'
--- lib/lp/registry/templates/productrelease-add-from-series.pt	2012-12-10 20:24:40 +0000
+++ lib/lp/registry/templates/productrelease-add-from-series.pt	2013-01-25 06:18:24 +0000
@@ -64,7 +64,6 @@
 </metal:block>
 
 <div metal:fill-slot="main">
-  <tal:releases-allowed condition="view/releases_allowed">
   <div metal:use-macro="context/@@launchpad_form/form">
     <div metal:fill-slot="extra_info" tal:condition="context/releases"
          style="border-bottom: solid 1px black">
@@ -78,12 +77,6 @@
       </ul>
     </div>
   </div>
-  </tal:releases-allowed>
-  <tal:releases-forbidden condition="not: view/releases_allowed">
-    The <tal:seriesname replace="context/displayname"
-    /> series of <tal:productname replace="context/product/displayname"
-    /> is PROPRIETARY. It cannot have any releases.
-  </tal:releases-forbidden>
 </div>
 
 </body>

=== modified file 'lib/lp/registry/templates/productrelease-add.pt'
--- lib/lp/registry/templates/productrelease-add.pt	2012-12-10 20:24:40 +0000
+++ lib/lp/registry/templates/productrelease-add.pt	2013-01-25 06:18:24 +0000
@@ -14,7 +14,6 @@
   </metal:block>
 
 <div metal:fill-slot="main">
-  <tal:releases-allowed condition="view/releases_allowed">
   <div metal:use-macro="context/@@launchpad_form/form">
     <div id="other-releases"
          metal:fill-slot="extra_info"
@@ -30,12 +29,6 @@
       </ul>
     </div>
   </div>
-  </tal:releases-allowed>
-  <tal:releases-forbidden condition="not: view/releases_allowed">
-    <tal:productname replace="context/product/displayname"
-    /> is PROPRIETARY. It cannot have any releases.
-  </tal:releases-forbidden>
-
 </div>
 
 </body>

=== modified file 'lib/lp/registry/templates/productrelease-portlet-data.pt'
--- lib/lp/registry/templates/productrelease-portlet-data.pt	2012-09-10 03:45:21 +0000
+++ lib/lp/registry/templates/productrelease-portlet-data.pt	2013-01-25 06:18:24 +0000
@@ -6,8 +6,10 @@
 
   <div class="portlet"
     tal:define="has_edit view/release/required:launchpad.Edit;">
-    <form method="POST" tal:attributes="action request/URL">
-      <h2>Download files for this release</h2>
+    <h2>Download files for this release</h2>
+    <form
+      tal:condition="view/release/can_have_release_files"
+      method="POST" tal:attributes="action request/URL">
 
       <p id="how-to-verify" tal:condition="view/download_files">
         After you've downloaded a file, you can verify its authenticity
@@ -64,8 +66,12 @@
         </ul>
       </div>
     </form>
+    <tal:nofiles tal:condition="not:view/release/can_have_release_files">
+      <em>Only public projects can have download files.</em>
+    </tal:nofiles>
   </div>
 
+
   <div class="top-portlet">
     <h2>Release notes&nbsp;<a tal:replace="structure view/release/menu:context/edit/fmt:icon" /></h2>
 

=== modified file 'lib/lp/registry/tests/test_milestone.py'
--- lib/lp/registry/tests/test_milestone.py	2012-12-26 01:04:05 +0000
+++ lib/lp/registry/tests/test_milestone.py	2013-01-25 06:18:24 +0000
@@ -5,8 +5,8 @@
 
 __metaclass__ = type
 
-import datetime
 from operator import attrgetter
+import unittest
 
 from storm.exceptions import NoneError
 from zope.component import getUtility
@@ -45,18 +45,16 @@
 from lp.testing.matchers import DoesNotSnapshot
 
 
-class MilestoneTest(TestCaseWithFactory):
+class MilestoneTest(unittest.TestCase):
     """Milestone tests."""
 
     layer = LaunchpadFunctionalLayer
 
     def setUp(self):
-        super(MilestoneTest, self).setUp()
         login(ANONYMOUS)
 
     def tearDown(self):
         logout()
-        super(MilestoneTest, self).tearDown()
 
     def testMilestoneSetIterator(self):
         """Test of MilestoneSet.__iter__()."""
@@ -116,15 +114,6 @@
             all_visible_milestones_ids,
             [1, 2, 3])
 
-    def test_proprietary_product_milestones_cannot_have_releases(self):
-        owner = self.factory.makePerson()
-        product = self.factory.makeProduct(
-            owner=owner, information_type=InformationType.PROPRIETARY)
-        milestone = self.factory.makeMilestone(product=product)
-        with person_logged_in(owner):
-            self.assertRaises(ProprietaryProduct,
-                milestone.createProductRelease, owner, datetime.date.today())
-
 
 class MilestoneSecurityAdaperTestCase(TestCaseWithFactory):
     """A TestCase for the security adapter of milestones."""

=== modified file 'lib/lp/registry/tests/test_productrelease.py'
--- lib/lp/registry/tests/test_productrelease.py	2012-11-19 20:42:45 +0000
+++ lib/lp/registry/tests/test_productrelease.py	2013-01-25 06:18:24 +0000
@@ -7,7 +7,11 @@
 
 from zope.component import getUtility
 
-from lp.registry.errors import InvalidFilename
+from lp.app.enums import InformationType
+from lp.registry.errors import (
+    InvalidFilename,
+    ProprietaryProduct,
+    )
 from lp.registry.interfaces.productrelease import (
     IProductReleaseSet,
     UpstreamFileType,
@@ -70,6 +74,7 @@
 
     def test_addReleaseFile(self):
         release = self.factory.makeProductRelease()
+        self.assertTrue(release.can_have_release_files)
         maintainer = release.milestone.product.owner
         with person_logged_in(maintainer):
             release_file = release.addReleaseFile(
@@ -89,3 +94,14 @@
             self.assertRaises(
                 InvalidFilename, release.addReleaseFile,
                 library_file.filename, 'test', 'text/plain', maintainer)
+
+    def test_addReleaseFile_only_works_on_public_products(self):
+        owner = self.factory.makePerson()
+        product = self.factory.makeProduct(
+            information_type=InformationType.PROPRIETARY, owner=owner)
+        with person_logged_in(owner):
+            release = self.factory.makeProductRelease(product=product)
+            self.assertFalse(release.can_have_release_files)
+            self.assertRaises(
+                ProprietaryProduct, release.addReleaseFile,
+                'README', 'test', 'text/plain', owner)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2013-01-16 04:51:47 +0000
+++ lib/lp/security.py	2013-01-25 06:18:24 +0000
@@ -529,11 +529,6 @@
     usedfor = IDistributionMirror
 
 
-class ViewAbstractMilestone(AnonymousAuthorization):
-    """Anyone can view an IMilestone or an IProjectGroupMilestone."""
-    usedfor = IAbstractMilestone
-
-
 class EditSpecificationBranch(AuthorizationBase):
 
     usedfor = ISpecificationBranch
@@ -1740,10 +1735,14 @@
             self, user)
 
 
-class ViewProductRelease(AnonymousAuthorization):
-
+class ViewProductRelease(DelegatedAuthorization):
+    permission = 'launchpad.View'
     usedfor = IProductRelease
 
+    def __init__(self, obj):
+        super(ViewProductRelease, self).__init__(
+            obj, obj.milestone, 'launchpad.View')
+
 
 class AdminTranslationImportQueueEntry(AuthorizationBase):
     permission = 'launchpad.Admin'