launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15000
[Merge] lp:~wgrant/launchpad/bug-1100164 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-1100164 into lp:launchpad.
Commit message:
Permit proprietary products to have releases, forbid them from adding release files.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-1100164/+merge/144856
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.
--
https://code.launchpad.net/~wgrant/launchpad/bug-1100164/+merge/144856
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-1100164 into lp:launchpad.
=== 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('admin@xxxxxxxxxxxxx')
+ >>> 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 <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'
