launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03603
[Merge] lp:~jtv/launchpad/resolve-conflict-with-allenap into lp:launchpad/db-devel
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/resolve-conflict-with-allenap into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/launchpad/resolve-conflict-with-allenap/+merge/60875
Resolving a devel/db-devel conflict between Gavin & me. This seems a more cost-effective solution than the usual fight to the death.
I switched two parameters in an interface method around. Don't over-think that; I was just making them consistent with the implementation.
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/browser/tests/test_archive_packages.py
lib/lp/soyuz/scripts/packagecopier.py
lib/lp/registry/doc/commercialsubscription.txt
lib/lp/registry/doc/product.txt
lib/lp/registry/browser/product.py
lib/lp/registry/browser/tests/product-views.txt
lib/lp/registry/configure.zcml
lib/lp/registry/interfaces/product.py
lib/lp/soyuz/interfaces/packagecopyjob.py
lib/lp/registry/doc/launchpadlib/project-registry.txt.disabled
lib/lp/registry/templates/products-review-licenses.pt
lib/lp/registry/templates/product-review-license.pt
lib/lp/registry/stories/webservice/xx-project-registry.txt
lib/lp/registry/templates/product-listing-for-review.pt
lib/lp/registry/stories/product/xx-productset.txt
lib/lp/app/stories/form/xx-form-layout.txt
lib/lp/registry/browser/tests/test_product.py
lib/lp/soyuz/browser/tests/test_package_copying_mixin.py
lib/lp/registry/model/product.py
lib/canonical/launchpad/database/launchpadstatistic.py
lib/lp/registry/stories/product/xx-product-index.txt
lib/lp/soyuz/browser/archive.py
./lib/lp/registry/doc/launchpadlib/project-registry.txt.disabled
94: Line exceeds 78 characters.
221: Line exceeds 78 characters.
./lib/lp/registry/templates/product-listing-for-review.pt
30: Line has trailing whitespace.
--
https://code.launchpad.net/~jtv/launchpad/resolve-conflict-with-allenap/+merge/60875
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/resolve-conflict-with-allenap into lp:launchpad/db-devel.
=== modified file 'lib/canonical/launchpad/database/launchpadstatistic.py'
--- lib/canonical/launchpad/database/launchpadstatistic.py 2011-04-26 16:39:33 +0000
+++ lib/canonical/launchpad/database/launchpadstatistic.py 2011-05-13 08:56:09 +0000
@@ -160,7 +160,7 @@
distinct=True, clauseTables=['Question']).count())
self.update(
'reviewed_products',
- Product.selectBy(license_reviewed=True, active=True).count())
+ Product.selectBy(project_reviewed=True, active=True).count())
def _updateRosettaStatistics(self, ztm):
# XXX j.c.sackett 2010-11-19 bug=677532 It's less than ideal that
=== modified file 'lib/lp/app/stories/form/xx-form-layout.txt'
--- lib/lp/app/stories/form/xx-form-layout.txt 2011-02-02 15:37:35 +0000
+++ lib/lp/app/stories/form/xx-form-layout.txt 2011-05-13 08:56:09 +0000
@@ -105,8 +105,8 @@
<tr>
<td colspan="2">
<div>
- <input ... name="field.license_reviewed" type="checkbox" ... />
- <label for="field.license_reviewed">Project reviewed</label>...
+ <input ... name="field.project_reviewed" type="checkbox" ... />
+ <label for="field.project_reviewed">Project reviewed</label>...
</td>
</tr>
...
=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py 2011-03-29 22:34:04 +0000
+++ lib/lp/registry/browser/product.py 2011-05-13 08:56:09 +0000
@@ -129,7 +129,10 @@
ReturnToReferrerMixin,
safe_action,
)
-from lp.app.browser.lazrjs import TextLineEditorWidget
+from lp.app.browser.lazrjs import (
+ BooleanChoiceWidget,
+ TextLineEditorWidget,
+ )
from lp.app.browser.tales import MenuAPI
from lp.app.enums import ServiceUsage
from lp.app.errors import NotFoundError
@@ -1133,6 +1136,49 @@
License.OTHER_OPEN_SOURCE in self.context.licenses
or License.OTHER_PROPRIETARY in self.context.licenses)
+ @cachedproperty
+ def is_proprietary(self):
+ """Is the project proprietary."""
+ return License.OTHER_PROPRIETARY in self.context.licenses
+
+ @property
+ def active_widget(self):
+ return BooleanChoiceWidget(
+ self.context, IProduct['active'],
+ content_box_id='%s-edit-active' % self.context.name,
+ edit_view='+review-license',
+ tag='span',
+ false_text='Deactivted',
+ true_text='Active',
+ header='Is this project active and usable by the community?')
+
+ @property
+ def project_reviewed_widget(self):
+ return BooleanChoiceWidget(
+ self.context, IProduct['project_reviewed'],
+ content_box_id='%s-edit-project-reviewed' % self.context.name,
+ edit_view='+review-license',
+ tag='span',
+ false_text='Unreviewed',
+ true_text='Reviewed',
+ header='Have you reviewed the project?')
+
+ @property
+ def license_approved_widget(self):
+ licenses = list(self.context.licenses)
+ if License.OTHER_PROPRIETARY in licenses:
+ return 'Commercial subscription required'
+ elif [License.DONT_KNOW] == licenses or [] == licenses:
+ return 'License required'
+ return BooleanChoiceWidget(
+ self.context, IProduct['license_approved'],
+ content_box_id='%s-edit-license-approved' % self.context.name,
+ edit_view='+review-license',
+ tag='span',
+ false_text='Unapproved',
+ true_text='Approved',
+ header='Does the license qualifiy the project for free hosting?')
+
class ProductPackagesView(LaunchpadView):
"""View for displaying product packaging"""
@@ -1619,7 +1665,7 @@
"""A view to review a project and change project privileges."""
label = "Review project"
field_names = [
- "license_reviewed",
+ "project_reviewed",
"license_approved",
"active",
"private_bugs",
@@ -1804,11 +1850,12 @@
schema = IProductReviewSearch
label = 'Review projects'
+ page_title = label
full_row_field_names = [
'search_text',
'active',
- 'license_reviewed',
+ 'project_reviewed',
'license_approved',
'license_info_is_empty',
'licenses',
@@ -1826,7 +1873,7 @@
orientation='vertical')
custom_widget('active', LaunchpadRadioWidget,
_messageNoValue="(do not filter)")
- custom_widget('license_reviewed', LaunchpadRadioWidget,
+ custom_widget('project_reviewed', LaunchpadRadioWidget,
_messageNoValue="(do not filter)")
custom_widget('license_approved', LaunchpadRadioWidget,
_messageNoValue="(do not filter)")
@@ -1858,21 +1905,25 @@
"""Return all widgets that span all columns."""
return (self.widgets[name] for name in self.full_row_field_names)
+ @property
+ def initial_values(self):
+ """See `ILaunchpadFormView`."""
+ search_params = {}
+ for name in self.schema:
+ search_params[name] = self.schema[name].default
+ return search_params
+
def forReviewBatched(self):
"""Return a `BatchNavigator` to review the matching projects."""
# Calling _validate populates the data dictionary as a side-effect
# of validation.
data = {}
self._validate(None, data)
- # Get default values from the schema since the form defaults
- # aren't available until the search button is pressed.
- search_params = {}
- for name in self.schema:
- search_params[name] = self.schema[name].default
+ search_params = self.initial_values
# Override the defaults with the form values if available.
search_params.update(data)
return BatchNavigator(self.context.forReview(**search_params),
- self.request, size=100)
+ self.request, size=50)
class ProductAddViewBase(ProductLicenseMixin, LaunchpadFormView):
=== modified file 'lib/lp/registry/browser/tests/product-views.txt'
--- lib/lp/registry/browser/tests/product-views.txt 2011-03-07 18:01:11 +0000
+++ lib/lp/registry/browser/tests/product-views.txt 2011-05-13 08:56:09 +0000
@@ -128,7 +128,7 @@
>>> view = create_initialized_view(firefox, name='+review-license')
Traceback (most recent call last):
...
- Unauthorized: (<Product..., 'license_reviewed', 'launchpad.Moderate')
+ Unauthorized: (<Product..., 'project_reviewed', 'launchpad.Moderate')
Mark is in the registry admins team and is allowed to access the page.
@@ -153,7 +153,7 @@
judge the licenses.
>>> view.field_names
- ['license_reviewed', 'license_approved', 'active', 'private_bugs',
+ ['project_reviewed', 'license_approved', 'active', 'private_bugs',
'reviewer_whiteboard']
The reviewer cannot deactivate a project if it is linked
@@ -291,14 +291,14 @@
>>> form = {
... 'field.active': 'on',
... 'field.reviewer_whiteboard': 'This is not a free license',
- ... 'field.license_reviewed': 'on',
+ ... 'field.project_reviewed': 'on',
... 'field.actions.change': 'Change',
... }
>>> view = create_initialized_view(
... firefox, name='+review-license', form=form)
>>> view.errors
[]
- >>> firefox.license_reviewed
+ >>> firefox.project_reviewed
True
>>> firefox.license_approved
False
@@ -326,7 +326,7 @@
... firefox, name='+review-license', form=form)
>>> view.errors
[]
- >>> firefox.license_reviewed
+ >>> firefox.project_reviewed
True
>>> firefox.license_approved
True
@@ -354,7 +354,7 @@
... firefox, name='+review-license', form=form)
>>> view.errors
[]
- >>> firefox.license_reviewed
+ >>> firefox.project_reviewed
True
>>> firefox.license_approved
True
=== modified file 'lib/lp/registry/browser/tests/test_product.py'
--- lib/lp/registry/browser/tests/test_product.py 2011-03-23 15:42:28 +0000
+++ lib/lp/registry/browser/tests/test_product.py 2011-05-13 08:56:09 +0000
@@ -16,6 +16,7 @@
from canonical.launchpad.testing.pages import (
find_tag_by_id,
)
+from canonical.launchpad.webapp.publisher import canonical_url
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.app.enums import ServiceUsage
from lp.registry.browser.product import (
@@ -26,6 +27,7 @@
IProductSet,
)
from lp.testing import (
+ login_celebrity,
login_person,
person_logged_in,
TestCaseWithFactory,
@@ -183,7 +185,7 @@
def setUp(self):
super(TestProductView, self).setUp()
- self.product = self.factory.makeProduct()
+ self.product = self.factory.makeProduct(name='fnord')
def test_show_programming_languages_without_languages(self):
# show_programming_languages is false when there are no programming
@@ -222,3 +224,158 @@
with person_logged_in(self.product.owner):
self.product.licenses = [License.OTHER_PROPRIETARY]
self.assertTrue(view.show_license_info)
+
+ def test_is_proprietary_with_proprietary_license(self):
+ # is_proprietary is true when the project has a proprietary license.
+ with person_logged_in(self.product.owner):
+ self.product.licenses = [License.OTHER_PROPRIETARY]
+ view = create_initialized_view(self.product, '+index')
+ self.assertTrue(view.is_proprietary)
+
+ def test_is_proprietary_without_proprietary_license(self):
+ # is_proprietary is false when the project has a proprietary license.
+ with person_logged_in(self.product.owner):
+ self.product.licenses = [License.GNU_GPL_V2]
+ view = create_initialized_view(self.product, '+index')
+ self.assertFalse(view.is_proprietary)
+
+ def test_active_widget(self):
+ # The active widget is is unique to the product.
+ view = create_initialized_view(self.product, '+index')
+ widget = view.active_widget
+ self.assertEqual('fnord-edit-active', widget.content_box_id)
+ self.assertEqual(
+ canonical_url(self.product, view_name='+review-license'),
+ widget.edit_url)
+
+ def test_project_reviewed_widget(self):
+ # The license reviewed widget is is unique to the product.
+ login_celebrity('registry_experts')
+ view = create_initialized_view(self.product, '+index')
+ widget = view.project_reviewed_widget
+ self.assertEqual('fnord-edit-project-reviewed', widget.content_box_id)
+ self.assertEqual(
+ canonical_url(self.product, view_name='+review-license'),
+ widget.edit_url)
+
+ def test_license_approved_widget_any_license(self):
+ # The license approved widget is is unique to the product.
+ login_celebrity('registry_experts')
+ view = create_initialized_view(self.product, '+index')
+ widget = view.license_approved_widget
+ self.assertEqual('fnord-edit-license-approved', widget.content_box_id)
+ self.assertEqual(
+ canonical_url(self.product, view_name='+review-license'),
+ widget.edit_url)
+
+ def test_license_approved_widget_prorietary_license(self):
+ # Proprietary projects cannot be approved.
+ with person_logged_in(self.product.owner):
+ self.product.licenses = [License.OTHER_PROPRIETARY]
+ login_celebrity('registry_experts')
+ view = create_initialized_view(self.product, '+index')
+ text = view.license_approved_widget
+ self.assertEqual('Commercial subscription required', text)
+
+ def test_license_approved_widget_no_license(self):
+ # Projects without a license cannot be approved.
+ with person_logged_in(self.product.owner):
+ self.product.licenses = [License.DONT_KNOW]
+ login_celebrity('registry_experts')
+ view = create_initialized_view(self.product, '+index')
+ text = view.license_approved_widget
+ self.assertEqual('License required', text)
+
+
+class ProductSetReviewLicensesViewTestCase(TestCaseWithFactory):
+ """Tests the ProductSetReviewLicensesView."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(ProductSetReviewLicensesViewTestCase, self).setUp()
+ self.product_set = getUtility(IProductSet)
+ self.user = login_celebrity('registry_experts')
+
+ def test_initial_values(self):
+ # The initial values show active, unreviewed, unapproved projects.
+ view = create_initialized_view(self.product_set, '+review-licenses')
+ self.assertContentEqual(
+ {'active': True,
+ 'project_reviewed': False,
+ 'license_approved': False,
+ 'search_text': None,
+ 'licenses': set(),
+ 'has_zero_licenses': None,
+ 'license_info_is_empty': None,
+ 'created_after': None,
+ 'created_before': None,
+ 'subscription_expires_after': None,
+ 'subscription_expires_before': None,
+ 'subscription_modified_after': None,
+ 'subscription_modified_before': None,
+ }.items(),
+ view.initial_values.items())
+
+ def test_forReviewBatched(self):
+ # The projects are batched.
+ view = create_initialized_view(self.product_set, '+review-licenses')
+ batch = view.forReviewBatched()
+ self.assertEqual(50, batch.default_size)
+
+ def test_project_common_data(self):
+ # The each project contains information to complete a review.
+ self.factory.makeProduct(name='fnord')
+ view = create_initialized_view(
+ self.product_set, '+review-licenses', principal=self.user)
+ content = find_tag_by_id(view.render(), 'project-fnord')
+ self.assertTrue(content.find(id='fnord-maintainer') is not None)
+ self.assertTrue(content.find(id='fnord-registrant') is not None)
+ self.assertTrue(content.find(id='fnord-desciption') is not None)
+ self.assertTrue(content.find(id='fnord-packages') is not None)
+ self.assertTrue(content.find(id='fnord-releases') is not None)
+ self.assertTrue(content.find(id='fnord-usage') is not None)
+ self.assertTrue(content.find(id='fnord-licenses') is not None)
+ self.assertTrue(content.find(id='fnord-whiteboard') is not None)
+ self.assertFalse(content.find(
+ id='fnord-commercial-subscription') is not None)
+ self.assertFalse(content.find(id='fnord-license-info') is not None)
+
+ def test_project_license_info_data(self):
+ # The projects with the OTHER_* licenese will show license_info data.
+ product = self.factory.makeProduct(name='fnord')
+ with person_logged_in(product.owner):
+ product.licenses = [License.OTHER_OPEN_SOURCE]
+ view = create_initialized_view(
+ self.product_set, '+review-licenses', principal=self.user)
+ content = find_tag_by_id(view.render(), 'project-fnord')
+ self.assertTrue(content.find(id='fnord-license-info') is not None)
+
+ def test_project_commercial_subscription_data(self):
+ # The projects with the OTHER_Proprietary license show commercial
+ # subscription information.
+ product = self.factory.makeProduct(name='fnord')
+ with person_logged_in(product.owner):
+ product.licenses = [License.OTHER_PROPRIETARY]
+ view = create_initialized_view(
+ self.product_set, '+review-licenses', principal=self.user)
+ content = find_tag_by_id(view.render(), 'project-fnord')
+ self.assertTrue(content.find(
+ id='fnord-commercial-subscription') is not None)
+
+ def test_project_widgets(self):
+ # The active, project_reviewed, and license_approved lazrjs widgets
+ # are used.
+ self.factory.makeProduct(name='fnord')
+ view = create_initialized_view(
+ self.product_set, '+review-licenses', principal=self.user)
+ content = find_tag_by_id(view.render(), 'fnord-statuses')
+ self.assertTrue(
+ 'Y.lp.app.choice.addBinaryChoice' in str(
+ content.find(id='fnord-edit-active').parent))
+ self.assertTrue(
+ 'Y.lp.app.choice.addBinaryChoice' in str(
+ content.find(id='fnord-edit-project-reviewed').parent))
+ self.assertTrue(
+ 'Y.lp.app.choice.addBinaryChoice' in str(
+ content.find(id='fnord-edit-license-approved').parent))
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2011-05-05 18:15:23 +0000
+++ lib/lp/registry/configure.zcml 2011-05-13 08:56:09 +0000
@@ -1237,7 +1237,7 @@
attributes="
reviewer_whiteboard
is_permitted
- license_reviewed
+ project_reviewed
license_approved"
set_schema="lp.registry.interfaces.product.IProductModerateRestricted"
set_attributes="active private_bugs "/>
=== modified file 'lib/lp/registry/doc/commercialsubscription.txt'
--- lib/lp/registry/doc/commercialsubscription.txt 2010-10-17 15:44:08 +0000
+++ lib/lp/registry/doc/commercialsubscription.txt 2011-05-13 08:56:09 +0000
@@ -7,7 +7,8 @@
>>> from zope.component import getUtility
>>> from canonical.launchpad.webapp.testing import verifyObject
- >>> from lp.registry.interfaces.commercialsubscription import ICommercialSubscription
+ >>> from lp.registry.interfaces.commercialsubscription import (
+ ... ICommercialSubscription)
>>> from lp.registry.interfaces.product import IProductSet
>>> from canonical.launchpad.ftests import login, ANONYMOUS
>>> login('no-priv@xxxxxxxxxxxxx')
@@ -268,22 +269,22 @@
included as one of the licenses.
>>> bzr.license_info = 'bar'
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.license_approved = True
>>> bzr.license_info
u'bar'
- >>> bzr.license_reviewed
+ >>> bzr.project_reviewed
True
>>> bzr.license_approved
True
Setting license_approved implies that the license has been reviewed,
-so license_reviewed is set automatically.
+so project_reviewed is set automatically.
- >>> bzr.license_reviewed = False
+ >>> bzr.project_reviewed = False
>>> bzr.license_approved = True
- >>> bzr.license_reviewed
+ >>> bzr.project_reviewed
True
Set the bzr license to Other/Open Source and Other/Proprietary. It
@@ -291,37 +292,38 @@
subscription.
>>> bzr.licenses = [License.OTHER_OPEN_SOURCE, License.OTHER_PROPRIETARY]
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.license_approved = True
Traceback (most recent call last):
...
- AssertionError: Projects with 'Other/Proprietary' licenses may not be...
+ ValueError: Projects without a license or have 'Other/Proprietary'
+ may not be approved.
A project with an Other/Open Source license or additional license info that
is reviewed, but not approved requires a commercial subscription.
>>> bzr.licenses = [License.OTHER_OPEN_SOURCE]
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.qualifies_for_free_hosting
False
If the licenses or license_info attributes are changed, the
-license_reviewed and license_approved attributes are reset to false.
+project_reviewed and license_approved attributes are reset to false.
>>> bzr.license_approved = True
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.license_info = 'foo'
>>> bzr.license_approved
False
- >>> bzr.license_reviewed
+ >>> bzr.project_reviewed
False
>>> bzr.license_approved = True
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.licenses = [License.BSD]
>>> bzr.license_approved
False
- >>> bzr.license_reviewed
+ >>> bzr.project_reviewed
False
When the products license is OTHER_OPEN_SOURCE or the license_info
@@ -331,7 +333,7 @@
approval, but were not approved.
However, qualifies_for_free_hosting remains true until
-it has been reviewed (license_reviewed is set to true). The
+it has been reviewed (project_reviewed is set to true). The
OTHER_PROPRIETARY license does not need to be reviewed as do the
OTHER_OPEN_SOURCE license or an unknown license in license_info.
@@ -341,7 +343,7 @@
>>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
(True, False)
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
(False, True)
@@ -350,7 +352,7 @@
>>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
(True, False)
- >>> bzr.license_reviewed = True
+ >>> bzr.project_reviewed = True
>>> bzr.qualifies_for_free_hosting, bzr.commercial_subscription_is_due
(False, True)
@@ -391,7 +393,7 @@
You can search for whether the product is marked reviewed or not.
- >>> for product in product_set.forReview(license_reviewed=True):
+ >>> for product in product_set.forReview(project_reviewed=True):
... print product.name
python-gnome2-dev
unassigned
@@ -451,7 +453,7 @@
not approved
>>> for product in product_set.forReview(
- ... license_reviewed=True, license_approved=False):
+ ... project_reviewed=True, license_approved=False):
... print product.name
python-gnome2-dev
unassigned
=== modified file 'lib/lp/registry/doc/launchpadlib/project-registry.txt.disabled'
--- lib/lp/registry/doc/launchpadlib/project-registry.txt.disabled 2010-04-23 20:41:06 +0000
+++ lib/lp/registry/doc/launchpadlib/project-registry.txt.disabled 2011-05-13 08:56:09 +0000
@@ -173,7 +173,7 @@
is_permitted: ...redacted
license_approved: ...redacted
license_info: None
- license_reviewed: ...redacted
+ project_reviewed: ...redacted
licenses: []
logo: <lazr.restfulclient.resource.HostedFile object...>
name: firefox
@@ -204,7 +204,7 @@
>>> print lp_mark.me.name
mark
>>> firefox = lp_mark.projects['firefox']
- >>> print firefox.license_reviewed
+ >>> print firefox.project_reviewed
False
In Launchpad project names may not have uppercase letters in their
=== modified file 'lib/lp/registry/doc/product.txt'
--- lib/lp/registry/doc/product.txt 2010-11-04 22:44:46 +0000
+++ lib/lp/registry/doc/product.txt 2011-05-13 08:56:09 +0000
@@ -371,7 +371,7 @@
A product's licenses is now being tracked. Previously registered products
may not have a license set, so an empty tuple is valid. If the licenses
-or license_info attributes are changed, the license_reviewed is reset
+or license_info attributes are changed, the project_reviewed is reset
to false.
>>> login('admin@xxxxxxxxxxxxx')
@@ -379,15 +379,15 @@
True
>>> product.licenses
(<...License.GNU_GPL_V2...>,)
- >>> product.license_reviewed = True
+ >>> product.project_reviewed = True
>>> product.licenses = (License.GNU_GPL_V2, License.GNU_LGPL_V2_1)
>>> product.licenses
(<...License.GNU_GPL_V2...>, <...License.GNU_LGPL_V2_1...>)
- >>> product.license_reviewed
+ >>> product.project_reviewed
False
- >>> product.license_reviewed = True
+ >>> product.project_reviewed = True
>>> product.license_info = 'foo'
- >>> product.license_reviewed
+ >>> product.project_reviewed
False
This series is set as the development focus for the product:
=== modified file 'lib/lp/registry/interfaces/product.py'
--- lib/lp/registry/interfaces/product.py 2011-04-12 23:25:45 +0000
+++ lib/lp/registry/interfaces/product.py 2011-05-13 08:56:09 +0000
@@ -389,7 +389,7 @@
"hosting or the project has an up-to-date "
"subscription.")))
- license_reviewed = exported(
+ project_reviewed = exported(
Bool(
title=_('Project reviewed'),
description=_("Whether or not this project has been reviewed. "
@@ -398,7 +398,7 @@
license_approved = exported(
Bool(
- title=_("Project approved"),
+ title=_("License approved"),
description=_(
"The project is legitimate and its license appears valid. "
"Not applicable to 'Other/Proprietary'.")))
@@ -845,7 +845,7 @@
def get_all_active(eager_load=True):
"""Get all active products.
-
+
:param eager_load: If False do not load related objects such as the
owner.
:return: An iterable of IProduct.
@@ -891,7 +891,7 @@
'project', 'homepageurl', 'screenshotsurl',
'downloadurl', 'freshmeatproject', 'wikiurl',
'sourceforgeproject', 'programminglang',
- 'license_reviewed', 'licenses', 'license_info',
+ 'project_reviewed', 'licenses', 'license_info',
'registrant'])
@export_operation_as('new_project')
def createProduct(owner, name, displayname, title, summary,
@@ -899,7 +899,7 @@
screenshotsurl=None, wikiurl=None,
downloadurl=None, freshmeatproject=None,
sourceforgeproject=None, programminglang=None,
- license_reviewed=False, mugshot=None, logo=None,
+ project_reviewed=False, mugshot=None, logo=None,
icon=None, licenses=None, license_info=None,
registrant=None):
"""Create and return a brand new Product.
@@ -910,8 +910,8 @@
@operation_parameters(
search_text=TextLine(title=_("Search text")),
active=Bool(title=_("Is the project active")),
- license_reviewed=Bool(title=_("Is the project license reviewed")),
- licenses = Set(title=_('Licenses'),
+ project_reviewed=Bool(title=_("Is the project license reviewed")),
+ licenses=Set(title=_('Licenses'),
value_type=Choice(vocabulary=License)),
license_info_is_empty=Bool(title=_("License info is empty")),
has_zero_licenses=Bool(title=_("Has zero licenses")),
@@ -930,7 +930,7 @@
@export_operation_as('licensing_search')
def forReview(search_text=None,
active=None,
- license_reviewed=None,
+ project_reviewed=None,
licenses=None,
license_info_is_empty=None,
has_zero_licenses=None,
@@ -1052,7 +1052,7 @@
title=_('Active'), values=[True, False],
required=False, default=True)
- license_reviewed = Choice(
+ project_reviewed = Choice(
title=_('Project Reviewed'), values=[True, False],
required=False, default=False)
=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py 2011-04-26 16:25:00 +0000
+++ lib/lp/registry/model/product.py 2011-05-13 08:56:09 +0000
@@ -188,7 +188,7 @@
from lp.translations.model.translationpolicy import TranslationPolicyMixin
-def get_license_status(license_approved, license_reviewed, licenses):
+def get_license_status(license_approved, project_reviewed, licenses):
"""Decide the license status for an `IProduct`.
:return: A LicenseStatus enum value.
@@ -206,7 +206,7 @@
# Notice the difference between the License and LicenseStatus.
return LicenseStatus.PROPRIETARY
elif License.OTHER_OPEN_SOURCE in licenses:
- if license_reviewed:
+ if project_reviewed:
# The OTHER_OPEN_SOURCE license was not manually approved
# by setting license_approved to true.
return LicenseStatus.PROPRIETARY
@@ -254,7 +254,7 @@
"""
naked_product = removeSecurityProxy(self.product)
return get_license_status(
- naked_product.license_approved, naked_product.license_reviewed,
+ naked_product.license_approved, naked_product.project_reviewed,
self.licenses)
@classmethod
@@ -447,7 +447,7 @@
enable_bug_expiration = BoolCol(dbName='enable_bug_expiration',
notNull=True, default=False)
- license_reviewed = BoolCol(dbName='reviewed', notNull=True, default=False)
+ project_reviewed = BoolCol(dbName='reviewed', notNull=True, default=False)
reviewer_whiteboard = StringCol(notNull=False, default=None)
private_bugs = BoolCol(
dbName='private_bugs', notNull=True, default=False)
@@ -478,7 +478,7 @@
def _validate_license_info(self, attr, value):
if not self._SO_creating and value != self.license_info:
- # Clear the license_reviewed and license_approved flags
+ # Clear the project_reviewed and license_approved flags
# if the license changes.
self._resetLicenseReview()
return value
@@ -489,14 +489,16 @@
def _validate_license_approved(self, attr, value):
"""Ensure license approved is only applied to the correct licenses."""
if not self._SO_creating:
- licenses = self.licenses
+ licenses = list(self.licenses)
if value:
- assert License.OTHER_PROPRIETARY not in licenses, (
- "Projects with 'Other/Proprietary' licenses may not be "
- "marked as license_approved.")
+ if (License.OTHER_PROPRIETARY in licenses
+ or [License.DONT_KNOW] == licenses):
+ raise ValueError(
+ "Projects without a license or have "
+ "'Other/Proprietary' may not be approved.")
# Approving a license implies it has been reviewed. Force
- # `license_reviewed` to be True.
- self.license_reviewed = True
+ # `project_reviewed` to be True.
+ self.project_reviewed = True
return value
license_approved = BoolCol(dbName='license_approved',
@@ -577,7 +579,7 @@
# Proprietary licenses need a subscription without
# waiting for a review.
return False
- elif (self.license_reviewed and
+ elif (self.project_reviewed and
(License.OTHER_OPEN_SOURCE in self.licenses or
self.license_info not in ('', None))):
# We only know that an unknown open source license
@@ -635,11 +637,11 @@
:return: A LicenseStatus enum value.
"""
return get_license_status(
- self.license_approved, self.license_reviewed, self.licenses)
+ self.license_approved, self.project_reviewed, self.licenses)
def _resetLicenseReview(self):
"""When the license is modified, it must be reviewed again."""
- self.license_reviewed = False
+ self.project_reviewed = False
self.license_approved = False
def _get_answers_usage(self):
@@ -714,7 +716,7 @@
def _getLicenses(self):
return self._cached_licenses
- def _setLicenses(self, licenses, reset_license_reviewed=True):
+ def _setLicenses(self, licenses, reset_project_reviewed=True):
"""Set the licenses from a tuple of license enums.
The licenses parameter must not be an empty tuple.
@@ -723,12 +725,12 @@
old_licenses = set(self.licenses)
if licenses == old_licenses:
return
- # Clear the license_reviewed and license_approved flags
+ # Clear the project_reviewed and license_approved flags
# if the license changes.
- # ProductSet.createProduct() passes in reset_license_reviewed=False
+ # ProductSet.createProduct() passes in reset_project_reviewed=False
# to avoid changing the value when a Launchpad Admin sets
- # license_reviewed & licenses at the same time.
- if reset_license_reviewed:
+ # project_reviewed & licenses at the same time.
+ if reset_project_reviewed:
self._resetLicenseReview()
# $product/+edit doesn't require a license if a license hasn't
# already been set, but updateContextFromData() updates all the
@@ -1428,7 +1430,7 @@
screenshotsurl=None, wikiurl=None,
downloadurl=None, freshmeatproject=None,
sourceforgeproject=None, programminglang=None,
- license_reviewed=False, mugshot=None, logo=None,
+ project_reviewed=False, mugshot=None, logo=None,
icon=None, licenses=None, license_info=None,
registrant=None):
"""See `IProductSet`."""
@@ -1444,11 +1446,11 @@
downloadurl=downloadurl, freshmeatproject=freshmeatproject,
sourceforgeproject=sourceforgeproject,
programminglang=programminglang,
- license_reviewed=license_reviewed,
+ project_reviewed=project_reviewed,
icon=icon, logo=logo, mugshot=mugshot, license_info=license_info)
if len(licenses) > 0:
- product._setLicenses(licenses, reset_license_reviewed=False)
+ product._setLicenses(licenses, reset_project_reviewed=False)
# Create a default trunk series and set it as the development focus
trunk = product.newSeries(
@@ -1461,7 +1463,7 @@
return product
def forReview(self, search_text=None, active=None,
- license_reviewed=None, license_approved=None, licenses=None,
+ project_reviewed=None, license_approved=None, licenses=None,
license_info_is_empty=None,
has_zero_licenses=None,
created_after=None, created_before=None,
@@ -1473,8 +1475,8 @@
conditions = []
- if license_reviewed is not None:
- conditions.append(Product.license_reviewed == license_reviewed)
+ if project_reviewed is not None:
+ conditions.append(Product.project_reviewed == project_reviewed)
if license_approved is not None:
conditions.append(Product.license_approved == license_approved)
=== modified file 'lib/lp/registry/stories/product/xx-product-index.txt'
--- lib/lp/registry/stories/product/xx-product-index.txt 2011-03-23 16:28:51 +0000
+++ lib/lp/registry/stories/product/xx-product-index.txt 2011-05-13 08:56:09 +0000
@@ -180,7 +180,7 @@
being shown as proprietary.
>>> admin_browser.open('http://launchpad.dev/thunderbird/+review-license')
- >>> admin_browser.getControl(name='field.license_reviewed').value = True
+ >>> admin_browser.getControl(name='field.project_reviewed').value = True
>>> admin_browser.getControl(name='field.license_approved').value = False
>>> admin_browser.getControl('Change').click()
=== modified file 'lib/lp/registry/stories/product/xx-productset.txt'
--- lib/lp/registry/stories/product/xx-productset.txt 2010-09-28 02:39:25 +0000
+++ lib/lp/registry/stories/product/xx-productset.txt 2011-05-13 08:56:09 +0000
@@ -38,79 +38,6 @@
Review projects...
-Searching from +review_licenses
-===============================
-
-The search parameters that filter on a boolean value can
-be left unspecified to get results with either value.
-
- >>> for name in ('active', 'license_reviewed', 'has_zero_licenses',
- ... 'license_info_is_empty'):
- ... control = registry_browser.getControl(name='field.%s' % name)
- ... for option in control.displayOptions:
- ... print option.decode('utf-8').encode('ascii', 'replace'),
- ... print
- ?(do not filter) ?True ?False
- ?(do not filter) ?True ?False
- ?(do not filter) ?True ?False
- ?(do not filter) ?Empty ?Not Empty
-
-The default form values will search for active projects that have not
-been reviewed.
-
- >>> registry_browser.getControl(name='field.active').value
- ['True']
- >>> registry_browser.getControl(name='field.license_reviewed').value
- ['False']
- >>> registry_browser.getControl(name='field.licenses').value
- []
-
-If there are no products that match the search conditions, a warning
-message will be displayed.
-
- >>> registry_browser.getControl(name='field.active').value = ['False']
- >>> registry_browser.getControl(name='search').click()
- >>> find_tags_by_class(registry_browser.contents, 'warning message')
- [<div...No items found with these search options...
-
-If enough projects are found, the results are paginated. A link
-is provided for each product pointint to its +review-license page.
-
- >>> registry_browser.getControl(name='field.active').value = ['True']
- >>> registry_browser.getControl(name='field.licenses').value = []
- >>> registry_browser.getControl(
- ... name='field.license_info_is_empty').value = ['Empty']
- >>> registry_browser.getControl(name='field.created_before').value = (
- ... '2007-01-01')
- >>> registry_browser.getControl(name='search').click()
- >>> pagination = find_tags_by_class(registry_browser.contents,
- ... 'batch-navigation-index')
- >>> text_in_elements = pagination[0].findAll(text=True)
- >>> print ' '.join(text.strip() for text in text_in_elements)
- 1 ... 14 of 14 results
- >>> table = find_tag_by_id(registry_browser.contents, 'product-listing')
- >>> for tr in table.findAll('tr'):
- ... td_list = tr.findAll('td')
- ... print td_list[5].a
- <a href="/tomcat/+review-license">Review</a> ...
-
-Now, we can click on the Review link, which will take us to the
-$project/+review-license page. Then clicking on the Change button
-will take us back to the page we came from.
-
- >>> projects_link = registry_browser.url
- >>> review_link = registry_browser.getLink(
- ... url='/tomcat/+review-license')
- >>> review_link.text
- 'Review'
- >>> review_link.click()
- >>> print registry_browser.url
- http://launchpad.dev/tomcat/+review-license
- >>> registry_browser.getControl(name='field.actions.change').click()
- >>> print registry_browser.url == projects_link
- True
-
-
View all projects
=================
=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt 2011-05-05 05:46:41 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt 2011-05-13 08:56:09 +0000
@@ -171,7 +171,6 @@
is_permitted: False
license_approved: False
license_info: None
- license_reviewed: False
licenses: []
logo_link: u'http://.../firefox/logo'
name: u'firefox'
@@ -179,6 +178,7 @@
owner_link: u'http://.../~name12'
programming_language: None
project_group_link: u'http://.../mozilla'
+ project_reviewed: False
qualifies_for_free_hosting: False
recipes_collection_link: u'http://.../firefox/recipes'
registrant_link: u'http://.../~name12'
@@ -236,7 +236,6 @@
is_permitted:...redacted...
license_approved:...redacted...
license_info: None
- license_reviewed:...redacted...
licenses: []
logo_link: u'http://.../firefox/logo'
name: u'firefox'
@@ -244,6 +243,7 @@
owner_link: u'http://.../~name12'
programming_language: None
project_group_link: u'http://.../mozilla'
+ project_reviewed:...redacted...
qualifies_for_free_hosting: False
recipes_collection_link: u'http://.../firefox/recipes'
registrant_link: u'http://.../~name12'
@@ -623,7 +623,7 @@
>>> unreviewed = webservice.named_get(
... "/projects", "licensing_search",
- ... license_reviewed=False).jsonBody()
+ ... project_reviewed=False).jsonBody()
>>> entries = sorted(
... unreviewed['entries'], key=itemgetter('display_name'))
@@ -636,7 +636,7 @@
>>> reviewed = webservice.named_get(
... "/projects", "licensing_search",
- ... license_reviewed=True).jsonBody()
+ ... project_reviewed=True).jsonBody()
>>> entries = sorted(
... reviewed['entries'], key=itemgetter('display_name'))
@@ -717,7 +717,7 @@
... download_url=None, freshmeat_project=None,
... sourceforge_project=None, programming_lang=None,
... licenses=(), license_info=None,
- ... license_reviewed=False,
+ ... project_reviewed=False,
... registrant=None):
... return webservice.named_post(
... "/projects", "new_project",
@@ -729,7 +729,7 @@
... sourceforge_project=sourceforge_project,
... programming_lang=programming_lang,
... licenses=licenses, license_info=license_info,
- ... license_reviewed=license_reviewed,
+ ... project_reviewed=project_reviewed,
... registrant=registrant)
Verify a project does not exist and then create it.
@@ -764,7 +764,7 @@
>>> print sorted(new_project['licenses'])
[u'GNU GPL v2', u'Zope Public License']
- >>> print new_project['license_reviewed']
+ >>> print new_project['project_reviewed']
False
>>> print new_project['homepage_url']
=== modified file 'lib/lp/registry/templates/product-listing-for-review.pt'
--- lib/lp/registry/templates/product-listing-for-review.pt 2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/product-listing-for-review.pt 2011-05-13 08:56:09 +0000
@@ -4,45 +4,107 @@
xmlns:i18n="http://xml.zope.org/namespaces/i18n";
omit-tag="">
- <tr>
- <td style="white-space: nowrap"
- tal:content="structure context/fmt:link" />
- <tal:XXX condition="nothing">
- # XXX: Edwin Grubbs 2008-06-17 bug=240759
- # The @@/yes and @@/no img tags could be handled by a tales formatter.
- </tal:XXX>
- <td style="text-align: center">
- <img src="/@@/yes" tal:condition="context/active"/>
- <img src="/@@/no" tal:condition="not: context/active"/>
- </td>
- <td style="text-align: center">
- <img src="/@@/yes" tal:condition="context/license_reviewed"/>
- <img src="/@@/no" tal:condition="not: context/license_reviewed"/>
- </td>
- <td style="text-align: center">
- <img src="/@@/yes" tal:condition="context/license_approved"/>
- <img src="/@@/no" tal:condition="not: context/license_approved"/>
- </td>
- <td style="white-space: nowrap">
- <a tal:replace="structure context/owner/fmt:link">Foo Bar</a>
- <span
- tal:attributes="title context/datecreated/fmt:datetime"
- tal:content="context/datecreated/fmt:displaydate">2005-10-12</span>
- </td>
- <td style="white-space: nowrap">
- <span tal:condition="not: context/licenses">None<br/></span>
- <tal:loop repeat="license context/licenses">
- <tal:title tal:replace="license/title"/><br/>
- </tal:loop>
- (<a tal:attributes="href string:${context/fmt:url}/+review-license"
- >Review</a>)
- </td>
- <td tal:content="structure context/license_info/fmt:shorten/80/fmt:text-to-html">
- License info goes here
- </td>
- <td tal:condition="context/commercial_subscription"
- tal:content="context/commercial_subscription/date_expires/fmt:date" />
- <td tal:condition="not: context/commercial_subscription" />
- <td tal:content="context/reviewer_whiteboard/fmt:shorten/80" />
+ <tr tal:attributes="id string:project-${context/name}">
+ <td>
+ <p>
+ <a tal:replace="structure context/fmt:link" />
+ (<span tal:replace="context/name" />)
+ registered
+ <span
+ tal:attributes="title context/datecreated/fmt:datetime"
+ tal:content="context/datecreated/fmt:displaydate">2005-10-12</span>
+ </p>
+
+ <dl class="horizontal">
+ <dt>Maintained by</dt>
+ <dd tal:attributes="id string:${context/name}-maintainer">
+ <a tal:replace="structure context/owner/fmt:link" />
+ </dd>
+ <dt>Registered by</dt>
+ <dd tal:attributes="id string:${context/name}-registrant">
+ <a tal:replace="structure context/registrant/fmt:link" />
+ </dd>
+ </dl>
+
+ <p
+ title="Beware of these words: test, prueba, personal, sandbox, repo, package, archive"
+ tal:content="context/summary" />
+
+ <div
+ title="Beware of these words: test, prueba, personal, sandbox, repo, package, archive"
+ tal:condition="context/description"
+ tal:attributes="id string:${context/name}-desciption"
+ tal:content="structure context/description/fmt:text-to-html" />
+
+ <dl class="horizontal"
+ title="Projects with packages and releases cannot be deactivated.">
+ <dt>Has packages</dt>
+ <dd
+ tal:attributes="id string:${context/name}-packages"
+ tal:content="structure context/ubuntu_packages/image:boolean" />
+ <dt>Has releases</dt>
+ <dd
+ tal:attributes="id string:${context/name}-releases"
+ tal:content="structure view/latest_release_with_download_files/image:boolean" />
+ </dl>
+
+ <dl class="horizontal"
+ title="Projects over 6 months old should be using an application."
+ tal:attributes="id string:${context/name}-usage">
+ <dt>Code usage</dt>
+ <dd tal:content="context/codehosting_usage/title" />
+ <dt>Bug usage</dt>
+ <dd tal:content="context/bug_tracking_usage/title" />
+ <dt>Translations usage</dt>
+ <dd tal:content="context/translations_usage/title" />
+ </dl>
+ </td>
+
+ <td>
+ <dl tal:attributes="id string:${context/name}-statuses">
+ <dt>Project enabled</dt>
+ <dd tal:content="structure view/active_widget" />
+ <dt>Project reviewed</dt>
+ <dd tal:content="structure view/project_reviewed_widget" />
+ <dt>License approved</dt>
+ <dd title="Approving a license also marks the project reviewed."
+ tal:content="structure view/license_approved_widget" />
+ </dl>
+
+ <dl class="horizontal"
+ tal:attributes="id string:${context/name}-licenses">
+ <dt>Licenses</dt>
+ <dd>
+ <ul class="horizontal">
+ <li tal:condition="not: context/licenses">None</li>
+ <li tal:repeat="license context/licenses">
+ <tal:title tal:replace="license/title"/>
+ </li>
+ </ul>
+ </dd>
+ </dl>
+
+ <dl class="horizontal"
+ tal:condition="view/is_proprietary"
+ tal:attributes="id string:${context/name}-commercial-subscription">
+ <dt>Commercial subscription expiration</dt>
+ <dd
+ tal:condition="context/commercial_subscription"
+ tal:content="context/commercial_subscription/date_expires/fmt:date" />
+ <dd
+ tal:condition="not: context/commercial_subscription">None</dd>
+ </dl>
+
+ <dl class="horizontal"
+ tal:condition="view/show_license_info"
+ tal:attributes="id string:${context/name}-license-info">
+ <dt>Licenses info</dt>
+ <dd tal:content="structure context/license_info/fmt:text-to-html" />
+ </dl>
+
+ <div
+ tal:attributes="id string:${context/name}-whiteboard"
+ tal:content="context/reviewer_whiteboard/fmt:text-to-html" />
+ </td>
</tr>
</tal:root>
=== modified file 'lib/lp/registry/templates/product-review-license.pt'
--- lib/lp/registry/templates/product-review-license.pt 2009-10-08 15:54:09 +0000
+++ lib/lp/registry/templates/product-review-license.pt 2011-05-13 08:56:09 +0000
@@ -13,50 +13,31 @@
<div class="top-portlet">
<div metal:use-macro="context/@@launchpad_form/form">
<div metal:fill-slot="extra_info" class="application-summary" >
- <h2>Granting approval</h2>
-
- <ul class="bulleted">
- <li>Does the project use any Launchpad services, such as code or
- bugs? Mark the <strong>project approved</strong>
- and <strong>reviewed</strong> now.
- </li>
- <li>Are the project's licenses simple and obviously open source
- and/or free software? Does the project smell legitimate? Mark
- the <strong>project approved</strong>
- and <strong>reviewed</strong> now.
- </li>
- <li>Does the project appear legitimate but incomplete? That's fine,
- mark the <strong>project approved</strong>
- and <strong>reviewed</strong> now. Consider also sending a
- request to the
- owner asking them to update the project with more details, so as
- to help recruit contributors.
- </li>
- </ul>
-
- <h2>Withholding approval</h2>
- <p>
- There are a few reasons you may want to withhold approval. You
- should always contact the project owner for clarification if you do
- not grant approval.
- </p>
- <ul class="bulleted">
- <li>The project has no license.</li>
+ <p>
+ If you have read the project's essential information, mark it
+ Approved, or if there are issues, just Reviewed.
+ </p>
+
+ <p>
+ You may need to without approval for these common reasons:
+ </p>
+
+ <ul class="bulleted">
+ <li>The project license is only "I Don't Know".</li>
<li>The project has only <strong>Other/Open source</strong> selected
- as a license and the license info field does not contain a URL
- to a license, or contains the whole license.
+ as a license and the license info field does not contain a link
+ to the license, or contains the whole license.
</li>
<li>The <strong>Other/Open source</strong> license appears to
discriminate against who can use the project, or how the
project's work can be used.
</li>
- <li>The project details are too vague to know if it is legitimate.
- </li>
+ <li>The project is <strong>Other/Proprietary</strong></li>
</ul>
<p>
<strong>If the project looks like a test, a prank, or spam,
- deactivate it.</strong>
+ deactivate it.</strong>.
</p>
<input type="hidden" name="next_url"
=== modified file 'lib/lp/registry/templates/products-review-licenses.pt'
--- lib/lp/registry/templates/products-review-licenses.pt 2010-10-10 21:54:16 +0000
+++ lib/lp/registry/templates/products-review-licenses.pt 2011-05-13 08:56:09 +0000
@@ -11,6 +11,22 @@
<metal:block fill-slot="head_epilogue">
<metal:yui-dependencies
use-macro="context/@@launchpad_widget_macros/yui2calendar-dependencies" />
+ <style type="text/css">
+ dl.horizontal dt {
+ display: inline;
+ margin-right: .5em;
+ }
+ dl.horizontal dd {
+ display: inline;
+ margin-right: 2em;
+ }
+ dl.horizontal dd ul.horizontal {
+ display: inline;
+ }
+ dl + p {
+ margin-top: 1em;
+ }
+ </style>
</metal:block>
<div metal:fill-slot="main"
@@ -23,15 +39,8 @@
<tal:navigation replace="structure batch/@@+navigation-links-upper" />
<table id="product-listing" class="listing">
<thead>
- <th>Project</th>
- <th>Active</th>
- <th>Reviewed</th>
- <th>License<br />approved</th>
- <th>Registered by</th>
- <th>Licenses</th>
- <th style="width: 40%">License info</th>
- <th>Commercial<br />Subscription<br />expiration</th>
- <th style="width: 40%">Reviewer<br />whiteboard</th>
+ <th style="max-width: 45em">Project information</th>
+ <th>Status and Licenses</th>
</thead>
<tbody>
<div tal:repeat="product batch/currentBatch"
=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py 2011-05-05 13:01:50 +0000
+++ lib/lp/soyuz/browser/archive.py 2011-05-13 08:56:09 +0000
@@ -108,6 +108,7 @@
from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
from lp.services.browser_helpers import get_user_agent_distroseries
from lp.services.database.bulk import load
+from lp.services.features import getFeatureFlag
from lp.services.propertycache import cachedproperty
from lp.services.worlddata.interfaces.country import ICountrySet
from lp.soyuz.adapters.archivedependencies import (
@@ -144,6 +145,7 @@
from lp.soyuz.interfaces.binarypackagebuild import BuildSetStatus
from lp.soyuz.interfaces.binarypackagename import IBinaryPackageNameSet
from lp.soyuz.interfaces.component import IComponentSet
+from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
from lp.soyuz.interfaces.packagecopyrequest import IPackageCopyRequestSet
from lp.soyuz.interfaces.packageset import IPackagesetSet
from lp.soyuz.interfaces.processor import IProcessorFamilySet
@@ -158,7 +160,16 @@
BinaryPackagePublishingHistory,
SourcePackagePublishingHistory,
)
-from lp.soyuz.scripts.packagecopier import do_copy
+from lp.soyuz.scripts.packagecopier import (
+ check_copy_permissions,
+ do_copy,
+ )
+
+# Feature flag: up to how many package sync requests (inclusive) are to be
+# processed synchronously within the web request?
+# Set to -1 to disable synchronous syncs.
+FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS = (
+ 'soyuz.derived_series.max_synchronous_syncs')
class ArchiveBadges(HasBadgeBase):
@@ -1214,15 +1225,163 @@
_messageNoValue = _("vocabulary-copy-to-same-series", "The same series")
+def preload_binary_package_names(copies):
+ """Preload `BinaryPackageName`s to speed up display-name construction."""
+ bpn_ids = [
+ copy.binarypackagerelease.binarypackagenameID for copy in copies
+ if isinstance(copy, BinaryPackagePublishingHistory)]
+ load(BinaryPackageName, bpn_ids)
+
+
+def compose_synchronous_copy_feedback(copies, dest_archive, dest_url=None,
+ dest_display_name=None):
+ """Compose human-readable feedback after a synchronous copy."""
+ if dest_url is None:
+ dest_url = escape(
+ canonical_url(dest_archive) + '/+packages', quote=True)
+
+ if dest_display_name is None:
+ dest_display_name = escape(dest_archive.displayname)
+
+ if len(copies) == 0:
+ return structured(
+ '<p>All packages already copied to <a href="%s">%s</a>.</p>'
+ % (dest_url, dest_display_name))
+ else:
+ messages = []
+ messages.append(
+ '<p>Packages copied to <a href="%s">%s</a>:</p>'
+ % (dest_url, dest_display_name))
+ messages.append('<ul>')
+ messages.append("\n".join([
+ '<li>%s</li>' % escape(copy) for copy in copies]))
+ messages.append('</ul>')
+ return structured("\n".join(messages))
+
+
+def copy_synchronously(source_pubs, dest_archive, dest_series, dest_pocket,
+ include_binaries, dest_url=None,
+ dest_display_name=None, person=None,
+ check_permissions=True):
+ """Copy packages right now.
+
+ :return: A `structured` with human-readable feedback about the
+ operation.
+ :raises CannotCopy: If `check_permissions` is True and the copy is
+ not permitted.
+ """
+ copies = do_copy(
+ source_pubs, dest_archive, dest_series, dest_pocket, include_binaries,
+ allow_delayed_copies=True, person=person,
+ check_permissions=check_permissions)
+
+ preload_binary_package_names(copies)
+
+ return compose_synchronous_copy_feedback(
+ [copy.displayname for copy in copies], dest_archive, dest_url,
+ dest_display_name)
+
+
+def partition_pubs_by_archive(source_pubs):
+ """Group `source_pubs` by archive.
+
+ :param source_pubs: A sequence of `SourcePackagePublishingHistory`.
+ :return: A dict mapping `Archive`s to the list of entries from
+ `source_pubs` that are in that archive.
+ """
+ by_source_archive = {}
+ for spph in source_pubs:
+ by_source_archive.setdefault(spph.archive, []).append(spph)
+ return by_source_archive
+
+
+def name_pubs_with_versions(source_pubs):
+ """Annotate each entry from `source_pubs` with its version.
+
+ :param source_pubs: A sequence of `SourcePackagePublishingHistory`.
+ :return: A list of tuples (name, version), one for each respective
+ entry in `source_pubs`.
+ """
+ sprs = [spph.sourcepackagerelease for spph in source_pubs]
+ return [(spr.sourcepackagename.name, spr.version) for spr in sprs]
+
+
+def copy_asynchronously(source_pubs, dest_archive, dest_series, dest_pocket,
+ include_binaries, dest_url=None,
+ dest_display_name=None, person=None,
+ check_permissions=True):
+ """Schedule jobs to copy packages later.
+
+ :return: A `structured` with human-readable feedback about the
+ operation.
+ :raises CannotCopy: If `check_permissions` is True and the copy is
+ not permitted.
+ """
+ if check_permissions:
+ spns = [
+ spph.sourcepackagerelease.sourcepackagename
+ for spph in source_pubs]
+ check_copy_permissions(
+ person, dest_archive, dest_series, dest_pocket, [spns])
+
+ job_source = getUtility(IPlainPackageCopyJobSource)
+ archive_pubs = partition_pubs_by_archive(source_pubs)
+ for source_archive, spphs in archive_pubs.iteritems():
+ job_source.create(
+ name_pubs_with_versions(spphs), source_archive, dest_archive,
+ dest_series, dest_pocket, include_binaries=include_binaries)
+ return structured("""
+ <p>Requested sync of %s packages.</p>
+ <p>Please allow some time for these to be processed.</p>
+ """, len(source_pubs))
+
+
+def render_cannotcopy_as_html(cannotcopy_exception):
+ """Render `CannotCopy` exception as HTML for display in the page."""
+ error_lines = str(cannotcopy_exception).splitlines()
+
+ if len(error_lines) == 1:
+ intro = "The following source cannot be copied:"
+ else:
+ intro = "The following sources cannot be copied:"
+
+ # Produce structured HTML. Include <li>%s</li> placeholders for
+ # each error line, but have "structured" interpolate the actual
+ # package names. It will escape them as needed.
+ html_text = """
+ <p>%s</p>
+ <ul>
+ %s
+ </ul>
+ """ % (intro, "<li>%s</li>" * len(error_lines))
+ return structured(html_text, *error_lines)
+
+
class PackageCopyingMixin:
"""A mixin class that adds helpers for package copying."""
+ def canCopySynchronously(self, source_pubs):
+ """Can we afford to copy `source_pubs` synchronously?"""
+ # Fixed estimate: up to 100 packages can be copied in acceptable
+ # time. Anything more than that and we go async.
+ limit = getFeatureFlag(FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS)
+ try:
+ limit = int(limit)
+ except:
+ limit = 100
+
+ return len(source_pubs) <= limit
+
def do_copy(self, sources_field_name, source_pubs, dest_archive,
dest_series, dest_pocket, include_binaries,
dest_url=None, dest_display_name=None, person=None,
check_permissions=True):
"""Copy packages and add appropriate feedback to the browser page.
+ This may either copy synchronously, if there are few enough
+ requests to process right now; or asynchronously in which case
+ it will schedule jobs that will be processed by a script.
+
:param sources_field_name: The name of the form field to set errors
on when the copy fails
:param source_pubs: A list of SourcePackagePublishingHistory to copy
@@ -1244,61 +1403,24 @@
:return: True if the copying worked, False otherwise.
"""
try:
- copies = do_copy(
- source_pubs, dest_archive, dest_series,
- dest_pocket, include_binaries, allow_delayed_copies=True,
- person=person, check_permissions=check_permissions)
+ if self.canCopySynchronously(source_pubs):
+ notification = copy_synchronously(
+ source_pubs, dest_archive, dest_series, dest_pocket,
+ include_binaries, dest_url=dest_url,
+ dest_display_name=dest_display_name, person=person,
+ check_permissions=check_permissions)
+ else:
+ notification = copy_asynchronously(
+ source_pubs, dest_archive, dest_series, dest_pocket,
+ include_binaries, dest_url=dest_url,
+ dest_display_name=dest_display_name, person=person,
+ check_permissions=check_permissions)
except CannotCopy, error:
- messages = []
- error_lines = str(error).splitlines()
- if len(error_lines) == 1:
- messages.append(
- "<p>The following source cannot be copied:</p>")
- else:
- messages.append(
- "<p>The following sources cannot be copied:</p>")
- messages.append('<ul>')
- messages.append(
- "\n".join('<li>%s</li>' % escape(line)
- for line in error_lines))
- messages.append('</ul>')
-
self.setFieldError(
- sources_field_name, structured('\n'.join(messages)))
+ sources_field_name, render_cannotcopy_as_html(error))
return False
- # Preload BPNs to save queries when calculating display names.
- load(BinaryPackageName, (
- copy.binarypackagerelease.binarypackagenameID for copy in copies
- if isinstance(copy, BinaryPackagePublishingHistory)))
-
- # Present a page notification describing the action.
- messages = []
- if dest_url is None:
- dest_url = escape(
- canonical_url(dest_archive) + '/+packages',
- quote=True)
- if dest_display_name is None:
- dest_display_name = escape(dest_archive.displayname)
- if len(copies) == 0:
- messages.append(
- '<p>All packages already copied to '
- '<a href="%s">%s</a>.</p>' % (
- dest_url,
- dest_display_name))
- else:
- messages.append(
- '<p>Packages copied to <a href="%s">%s</a>:</p>' % (
- dest_url,
- dest_display_name))
- messages.append('<ul>')
- messages.append(
- "\n".join(['<li>%s</li>' % escape(copy.displayname)
- for copy in copies]))
- messages.append('</ul>')
-
- notification = "\n".join(messages)
- self.request.response.addNotification(structured(notification))
+ self.request.response.addNotification(notification)
return True
=== modified file 'lib/lp/soyuz/browser/tests/test_archive_packages.py'
--- lib/lp/soyuz/browser/tests/test_archive_packages.py 2011-03-29 00:11:57 +0000
+++ lib/lp/soyuz/browser/tests/test_archive_packages.py 2011-05-13 08:56:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=F0401
@@ -16,7 +16,6 @@
from testtools.matchers import (
Equals,
LessThan,
- MatchesAny,
)
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
=== added file 'lib/lp/soyuz/browser/tests/test_package_copying_mixin.py'
--- lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/browser/tests/test_package_copying_mixin.py 2011-05-13 08:56:09 +0000
@@ -0,0 +1,311 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `PackageCopyingMixin`."""
+
+__metaclass__ = type
+
+from zope.component import getUtility
+
+from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.features.testing import FeatureFixture
+from lp.services.propertycache import cachedproperty
+from lp.soyuz.browser.archive import (
+ compose_synchronous_copy_feedback,
+ copy_asynchronously,
+ copy_synchronously,
+ FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS,
+ name_pubs_with_versions,
+ PackageCopyingMixin,
+ partition_pubs_by_archive,
+ render_cannotcopy_as_html,
+ )
+from lp.soyuz.interfaces.archive import CannotCopy
+from lp.soyuz.interfaces.packagecopyjob import IPlainPackageCopyJobSource
+from lp.soyuz.enums import SourcePackageFormat
+from lp.soyuz.interfaces.sourcepackageformat import (
+ ISourcePackageFormatSelectionSet,
+ )
+from lp.testing import (
+ TestCase,
+ TestCaseWithFactory,
+ )
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.views import create_initialized_view
+
+
+def find_spph_copy(archive, spph):
+ """Find copy of `spph`'s package as copied into `archive`"""
+ spr = spph.sourcepackagerelease
+ return archive.getPublishedSources(
+ name=spr.sourcepackagename.name, version=spr.version).one()
+
+
+class FakeDistribution:
+ def __init__(self):
+ self.archive = FakeArchive()
+
+
+class FakeDistroSeries:
+ def __init__(self):
+ self.distribution = FakeDistribution()
+
+
+class FakeSPN:
+ name = "spn-name"
+
+
+class FakeSPR:
+ def __init__(self):
+ self.sourcepackagename = FakeSPN()
+ self.version = "1.0"
+
+
+class FakeArchive:
+ def __init__(self, displayname="archive-name"):
+ self.displayname = displayname
+
+
+class FakeSPPH:
+ def __init__(self, archive=None):
+ if archive is None:
+ archive = FakeArchive()
+ self.sourcepackagerelease = FakeSPR()
+ self.displayname = "spph-displayname"
+ self.archive = archive
+
+
+class TestPackageCopyingMixinLight(TestCase):
+ """Test lightweight functions and methods.
+
+ This test does not run in a layer and does not access the database.
+ """
+
+ unique_number = 1
+
+ def getPocket(self):
+ """Return an arbitrary `PackagePublishingPocket`."""
+ return PackagePublishingPocket.SECURITY
+
+ def getUniqueString(self):
+ """Return an arbitrary string."""
+ self.unique_number += 1
+ return "string_%d_" % self.unique_number
+
+ def test_canCopySynchronously_allows_small_synchronous_copies(self):
+ # Small numbers of packages can be copied synchronously.
+ packages = [self.getUniqueString() for counter in range(3)]
+ self.assertTrue(PackageCopyingMixin().canCopySynchronously(packages))
+
+ def test_canCopySynchronously_disallows_large_synchronous_copies(self):
+ # Large numbers of packages must be copied asynchronously.
+ packages = [self.getUniqueString() for counter in range(300)]
+ self.assertFalse(PackageCopyingMixin().canCopySynchronously(packages))
+
+ def test_partition_pubs_by_archive_maps_archives_to_pubs(self):
+ # partition_pubs_by_archive returns a dict mapping each archive
+ # to a list of SPPHs on that archive.
+ spph = FakeSPPH()
+ self.assertEqual(
+ {spph.archive: [spph]}, partition_pubs_by_archive([spph]))
+
+ def test_partition_pubs_by_archive_splits_by_archive(self):
+ # partition_pubs_by_archive keeps SPPHs for different archives
+ # separate.
+ spphs = [FakeSPPH() for counter in xrange(2)]
+ mapping = partition_pubs_by_archive(spphs)
+ self.assertEqual(
+ dict((spph.archive, [spph]) for spph in spphs), mapping)
+
+ def test_partition_pubs_by_archive_clusters_by_archive(self):
+ # partition_pubs_by_archive bundles SPPHs for the same archive
+ # into a single dict entry.
+ archive = FakeArchive()
+ spphs = [FakeSPPH(archive=archive) for counter in xrange(2)]
+ mapping = partition_pubs_by_archive(spphs)
+ self.assertEqual([archive], mapping.keys())
+ self.assertContentEqual(spphs, mapping[archive])
+
+ def test_name_pubs_with_versions_lists_packages_and_versions(self):
+ # name_pubs_with_versions returns a list of tuples of source
+ # package name and source package version, one per SPPH.
+ spph = FakeSPPH()
+ spr = spph.sourcepackagerelease
+ self.assertEqual(
+ [(spr.sourcepackagename.name, spr.version)],
+ name_pubs_with_versions([spph]))
+
+ def test_render_cannotcopy_as_html_lists_errors(self):
+ # render_cannotcopy_as_html includes a CannotCopy error message
+ # into its HTML notice.
+ message = self.getUniqueString()
+ html_text = render_cannotcopy_as_html(CannotCopy(message)).escapedtext
+ self.assertIn(message, html_text)
+
+ def test_render_cannotcopy_as_html_escapes_error(self):
+ # render_cannotcopy_as_html escapes error messages.
+ message = "x<>y"
+ html_text = render_cannotcopy_as_html(CannotCopy(message)).escapedtext
+ self.assertNotIn(message, html_text)
+ self.assertIn("x<>y", html_text)
+
+ def test_compose_synchronous_copy_feedback_escapes_archive_name(self):
+ # compose_synchronous_copy_feedback escapes archive displaynames.
+ archive = FakeArchive(displayname="a&b")
+ notice = compose_synchronous_copy_feedback(
+ ["hi"], archive, dest_url="/")
+ html_text = notice.escapedtext
+ self.assertNotIn("a&b", html_text)
+ self.assertIn("a&b", html_text)
+
+ def test_compose_synchronous_copy_feedback_escapes_package_names(self):
+ # compose_synchronous_copy_feedback escapes package names.
+ archive = FakeArchive()
+ notice = compose_synchronous_copy_feedback(
+ ["x<y"], archive, dest_url="/")
+ html_text = notice.escapedtext
+ self.assertNotIn("x<y", html_text)
+ self.assertIn("x<y", html_text)
+
+ def test_copy_synchronously_checks_permissions(self):
+ # Unless told not to, copy_synchronously does a permissions
+ # check.
+ pocket = self.getPocket()
+ self.assertRaises(
+ CannotCopy,
+ copy_synchronously,
+ [FakeSPPH()], FakeArchive(), FakeDistroSeries(), pocket, False)
+
+ def test_copy_asynchronously_checks_permissions(self):
+ # Unless told not to, copy_asynchronously does a permissions
+ # check.
+ pocket = self.getPocket()
+ self.assertRaises(
+ CannotCopy,
+ copy_asynchronously,
+ [FakeSPPH()], FakeArchive(), FakeDistroSeries(), pocket, False)
+
+
+class TestPackageCopyingMixinIntegration(TestCaseWithFactory):
+ """Integration tests for `PackageCopyingMixin`."""
+
+ layer = LaunchpadFunctionalLayer
+
+ @cachedproperty
+ def person(self):
+ """Create a single person who gets blamed for everything.
+
+ Creating SPPHs, Archives etc. in the factory creates lots of
+ `Person`s, which turns out to be really slow. Tests that don't
+ care who's who can use this single person for all uninteresting
+ Person fields.
+ """
+ return self.factory.makePerson()
+
+ def makeDistribution(self):
+ """Create a `Distribution`, but quickly by reusing a single Person."""
+ return self.factory.makeDistribution(
+ owner=self.person, registrant=self.person)
+
+ def makeDistroSeries(self, parent_series=None):
+ """Create a `DistroSeries`, but quickly by reusing a single Person."""
+ return self.factory.makeDistroSeries(
+ distribution=self.makeDistribution(), parent_series=parent_series,
+ registrant=self.person)
+
+ def makeSPPH(self):
+ """Create a `SourcePackagePublishingHistory` quickly."""
+ archive = self.factory.makeArchive(
+ owner=self.person, distribution=self.makeDistribution())
+ return self.factory.makeSourcePackagePublishingHistory(
+ maintainer=self.person, creator=self.person, archive=archive)
+
+ def makeDerivedSeries(self):
+ """Create a derived `DistroSeries`, quickly."""
+ series = self.makeDistroSeries(parent_series=self.makeDistroSeries())
+ getUtility(ISourcePackageFormatSelectionSet).add(
+ series, SourcePackageFormat.FORMAT_1_0)
+ return series
+
+ def makeView(self):
+ """Create a `PackageCopyingMixin`-based view."""
+ return create_initialized_view(
+ self.makeDerivedSeries(), "+localpackagediffs")
+
+ def test_canCopySynchronously_obeys_feature_flag(self):
+ packages = [self.getUniqueString() for counter in range(3)]
+ mixin = PackageCopyingMixin()
+ with FeatureFixture({FEATURE_FLAG_MAX_SYNCHRONOUS_SYNCS: 2}):
+ can_copy_synchronously = mixin.canCopySynchronously(packages)
+ self.assertFalse(can_copy_synchronously)
+
+ def test_copy_synchronously_copies_packages(self):
+ # copy_synchronously copies packages into the destination
+ # archive.
+ spph = self.makeSPPH()
+ dest_series = self.makeDerivedSeries()
+ archive = dest_series.distribution.main_archive
+ pocket = self.factory.getAnyPocket()
+ copy_synchronously(
+ [spph], archive, dest_series, pocket, include_binaries=False,
+ check_permissions=False)
+ self.assertNotEqual(None, find_spph_copy(archive, spph))
+
+ def test_copy_asynchronously_does_not_copy_packages(self):
+ # copy_asynchronously does not copy packages into the destination
+ # archive; that happens later, asynchronously.
+ spph = self.makeSPPH()
+ dest_series = self.makeDerivedSeries()
+ archive = dest_series.distribution.main_archive
+ pocket = self.factory.getAnyPocket()
+ copy_asynchronously(
+ [spph], archive, dest_series, pocket, include_binaries=False,
+ check_permissions=False)
+ self.assertEqual(None, find_spph_copy(archive, spph))
+
+ def test_copy_synchronously_lists_packages(self):
+ # copy_synchronously returns feedback that includes the names of
+ # packages it copied.
+ spph = self.makeSPPH()
+ dest_series = self.makeDerivedSeries()
+ pocket = self.factory.getAnyPocket()
+ notice = copy_synchronously(
+ [spph], dest_series.distribution.main_archive, dest_series,
+ pocket, include_binaries=False,
+ check_permissions=False).escapedtext
+ self.assertIn(
+ spph.sourcepackagerelease.sourcepackagename.name, notice)
+
+ def test_copy_asynchronously_creates_copy_jobs(self):
+ # copy_asynchronously creates PackageCopyJobs.
+ spph = self.makeSPPH()
+ dest_series = self.makeDerivedSeries()
+ pocket = self.factory.getAnyPocket()
+ archive = dest_series.distribution.main_archive
+ copy_asynchronously(
+ [spph], archive, dest_series, pocket, include_binaries=False,
+ check_permissions=False)
+ jobs = list(getUtility(IPlainPackageCopyJobSource).getActiveJobs(
+ archive))
+ self.assertEqual(1, len(jobs))
+ spr = spph.sourcepackagerelease
+ self.assertEqual(
+ [[spr.sourcepackagename.name, spr.version]],
+ jobs[0].metadata['source_packages'])
+
+ def test_do_copy_goes_async_if_canCopySynchronously_says_so(self):
+ # The view opts for asynchronous copying if canCopySynchronously
+ # returns False. This creates PackageCopyJobs.
+ spph = self.makeSPPH()
+ pocket = self.factory.getAnyPocket()
+ view = self.makeView()
+ dest_series = view.context
+ archive = dest_series.distribution.main_archive
+ view.canCopySynchronously = FakeMethod(result=False)
+ view.do_copy(
+ 'selected_differences', [spph], archive, dest_series, pocket,
+ False, check_permissions=False)
+ jobs = list(getUtility(IPlainPackageCopyJobSource).getActiveJobs(
+ archive))
+ self.assertNotEqual([], jobs)
=== modified file 'lib/lp/soyuz/interfaces/packagecopyjob.py'
--- lib/lp/soyuz/interfaces/packagecopyjob.py 2011-05-11 16:50:34 +0000
+++ lib/lp/soyuz/interfaces/packagecopyjob.py 2011-05-13 08:56:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -82,16 +82,16 @@
class IPlainPackageCopyJobSource(IJobSource):
"""An interface for acquiring `IPackageCopyJobs`."""
- def create(cls, source_archive, source_packages,
+ def create(cls, source_packages, source_archive,
target_archive, target_distroseries, target_pocket,
include_binaries=False):
"""Create a new `IPackageCopyJob`.
- :param source_archive: The `IArchive` in which `source_packages` are
- found.
:param source_packages: An iterable of `(source_package_name,
version)` tuples, where both `source_package_name` and `version`
are strings.
+ :param source_archive: The `IArchive` in which `source_packages` are
+ found.
:param target_archive: The `IArchive` to which to copy the packages.
:param target_distroseries: The `IDistroSeries` to which to copy the
packages.
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2011-05-10 19:02:47 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2011-05-13 08:56:09 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""PackageCopier utilities."""
@@ -9,6 +9,7 @@
'PackageCopier',
'UnembargoSecurityPackage',
'CopyChecker',
+ 'check_copy_permissions',
'do_copy',
'_do_delayed_copy',
'_do_direct_copy',
@@ -51,6 +52,7 @@
)
from lp.soyuz.scripts.processaccepted import close_bugs_for_sourcepublication
+
# XXX cprov 2009-06-12: This function could be incorporated in ILFA,
# I just don't see a clear benefit in doing that right now.
def re_upload_file(libraryfile, restricted=False):
@@ -194,6 +196,41 @@
return {'status': BuildSetStatus.NEEDSBUILD}
+def check_copy_permissions(person, archive, series, pocket,
+ sourcepackagenames):
+ """Check that `person` has permission to copy a package.
+
+ :param person: User attempting the upload.
+ :param archive: Destination `Archive`.
+ :param series: Destination `DistroSeries`.
+ :param pocket: Destination `Pocket`.
+ :param sourcepackagenames: Sequence of `SourcePackageName`s for the
+ packages to be copied.
+ :raises CannotCopy: If the copy is not allowed.
+ """
+ if person is None:
+ raise CannotCopy("Cannot check copy permissions (no requester).")
+
+ # If there is a requester, check that he has upload permission into
+ # the destination (archive, component, pocket). This check is done
+ # here rather than in the security adapter because it requires more
+ # info than is available in the security adapter.
+ for spn in set(sourcepackagenames):
+ package = series.getSourcePackage(spn)
+ destination_component = package.latest_published_component
+
+ # If destination_component is not None, make sure the person
+ # has upload permission for this component. Otherwise, any
+ # upload permission on this archive will do.
+ strict_component = destination_component is not None
+ reason = archive.checkUpload(
+ person, series, spn, destination_component, pocket,
+ strict_component=strict_component)
+
+ if reason is not None:
+ raise CannotCopy(reason)
+
+
class CopyChecker:
"""Check copy candiates.
@@ -399,28 +436,10 @@
:raise CannotCopy when a copy is not allowed to be performed
containing the reason of the error.
"""
- # If there is a requester, check that he has upload permission
- # into the destination (archive, component, pocket). This check
- # is done here rather than in the security adapter because it
- # requires more info than is available in the security adapter.
if check_permissions:
- if person is None:
- raise CannotCopy(
- 'Cannot check copy permissions (no requester).')
- else:
- sourcepackagerelease = source.sourcepackagerelease
- sourcepackagename = sourcepackagerelease.sourcepackagename
- destination_component = series.getSourcePackage(
- sourcepackagename).latest_published_component
- # If destination_component is not None, make sure the person
- # has upload permission for this component. Otherwise, any
- # upload permission on this archive will do.
- strict_component = destination_component is not None
- reason = self.archive.checkUpload(
- person, series, sourcepackagename, destination_component,
- pocket, strict_component=strict_component)
- if reason is not None:
- raise CannotCopy(reason)
+ check_copy_permissions(
+ person, self.archive, series, pocket,
+ [source.sourcepackagerelease.sourcepackagename])
if series not in self.archive.distribution.series:
raise CannotCopy(
