launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28366
[Merge] ~cjwatson/launchpad:archive-admin-artifactory-fields into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:archive-admin-artifactory-fields into launchpad:master.
Commit message:
Allow editing Artifactory-related Archive columns
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/420133
`Archive.publishing_method` and `Archive.repository_format` control publishing to Artifactory, so we need a way to manage them. For now, restrict editing these to people with `launchpad.Admin` permissions on the archive (since archives must have names corresponding to Artifactory repository names and be in a distribution with appropriate configuration), and only allow editing these if no packages have yet been published to the archive (similar to privacy switching).
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:archive-admin-artifactory-fields into launchpad:master.
diff --git a/lib/lp/soyuz/browser/archive.py b/lib/lp/soyuz/browser/archive.py
index 90c5b6e..484ffbd 100644
--- a/lib/lp/soyuz/browser/archive.py
+++ b/lib/lp/soyuz/browser/archive.py
@@ -2150,9 +2150,13 @@ class ArchiveAdminView(BaseArchiveEditView, EnableProcessorsMixin):
'authorized_size',
'relative_build_score',
'external_dependencies',
+ 'publishing_method',
+ 'repository_format',
]
custom_widget_external_dependencies = CustomWidgetFactory(
TextAreaWidget, height=3)
+ custom_widget_publishing_method = LaunchpadDropdownWidget
+ custom_widget_repository_format = LaunchpadDropdownWidget
page_title = 'Administer'
@property
@@ -2184,6 +2188,24 @@ class ArchiveAdminView(BaseArchiveEditView, EnableProcessorsMixin):
error_text = "\n".join(errors)
self.setFieldError('external_dependencies', error_text)
+ if data.get('publishing_method') != self.context.publishing_method:
+ # The publishing method is being switched.
+ if (not self.context.getPublishedSources().is_empty() or
+ not self.context.getAllPublishedBinaries().is_empty()):
+ self.setFieldError(
+ 'publishing_method',
+ 'This archive already has published packages. It is '
+ 'not possible to switch the publishing method.')
+
+ if data.get('repository_format') != self.context.repository_format:
+ # The repository format is being switched.
+ if (not self.context.getPublishedSources().is_empty() or
+ not self.context.getAllPublishedBinaries().is_empty()):
+ self.setFieldError(
+ 'repository_format',
+ 'This archive already has published packages. It is '
+ 'not possible to switch the repository format.')
+
@property
def owner_is_private_team(self):
"""Is the owner a private team?
diff --git a/lib/lp/soyuz/browser/tests/test_archive_admin_view.py b/lib/lp/soyuz/browser/tests/test_archive_admin_view.py
index 7a2abdf..21f624c 100644
--- a/lib/lp/soyuz/browser/tests/test_archive_admin_view.py
+++ b/lib/lp/soyuz/browser/tests/test_archive_admin_view.py
@@ -1,17 +1,27 @@
# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from fixtures import FakeLogger
+from zope.security.interfaces import Unauthorized
+from zope.testbrowser.browser import LinkNotFoundError
+
+from lp.services.webapp import canonical_url
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.soyuz.browser.archive import ArchiveAdminView
+from lp.soyuz.enums import (
+ ArchivePublishingMethod,
+ ArchiveRepositoryFormat,
+ )
from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
login,
+ login_person,
TestCaseWithFactory,
)
from lp.testing.layers import LaunchpadFunctionalLayer
-class TestArchivePrivacySwitchingView(TestCaseWithFactory):
+class TestArchiveAdminView(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
@@ -23,18 +33,17 @@ class TestArchivePrivacySwitchingView(TestCaseWithFactory):
# object.
login('admin@xxxxxxxxxxxxx')
- def initialize_admin_view(self, private=True):
+ def initialize_admin_view(self, archive, fields):
"""Initialize the admin view to set the privacy.."""
method = 'POST'
form = {
'field.enabled': 'on',
'field.actions.save': 'Save',
+ 'field.private': 'on' if archive.private else 'off',
+ 'field.publishing_method': archive.publishing_method.title,
+ 'field.repository_format': archive.repository_format.title,
}
-
- if private is True:
- form['field.private'] = 'on'
- else:
- form['field.private'] = 'off'
+ form.update(fields)
view = ArchiveAdminView(self.ppa, LaunchpadTestRequest(
method=method, form=form))
@@ -47,10 +56,22 @@ class TestArchivePrivacySwitchingView(TestCaseWithFactory):
publisher.prepareBreezyAutotest()
publisher.getPubSource(archive=ppa)
+ def test_unauthorized(self):
+ # A non-admin user cannot administer an archive.
+ self.useFixture(FakeLogger())
+ login_person(self.ppa.owner)
+ ppa_url = canonical_url(self.ppa)
+ browser = self.getUserBrowser(ppa_url, user=self.ppa.owner)
+ self.assertRaises(
+ LinkNotFoundError, browser.getLink, "Administer archive")
+ self.assertRaises(
+ Unauthorized, self.getUserBrowser, ppa_url + "/+admin",
+ user=self.ppa.owner)
+
def test_set_private_without_packages(self):
# If a ppa does not have packages published, it is possible to
# update the private attribute.
- view = self.initialize_admin_view(private=True)
+ view = self.initialize_admin_view(self.ppa, {"field.private": "on"})
self.assertEqual(0, len(view.errors))
self.assertTrue(view.context.private)
@@ -58,14 +79,14 @@ class TestArchivePrivacySwitchingView(TestCaseWithFactory):
# If a ppa does not have packages published, it is possible to
# update the private attribute.
self.ppa.private = True
- view = self.initialize_admin_view(private=False)
+ view = self.initialize_admin_view(self.ppa, {"field.private": "off"})
self.assertEqual(0, len(view.errors))
self.assertFalse(view.context.private)
def test_set_private_with_packages(self):
# A PPA that does have packages cannot be privatised.
self.publish_to_ppa(self.ppa)
- view = self.initialize_admin_view(private=True)
+ view = self.initialize_admin_view(self.ppa, {"field.private": "on"})
self.assertEqual(1, len(view.errors))
self.assertEqual(
'This archive already has published sources. '
@@ -78,9 +99,55 @@ class TestArchivePrivacySwitchingView(TestCaseWithFactory):
self.ppa.private = True
self.publish_to_ppa(self.ppa)
- view = self.initialize_admin_view(private=False)
+ view = self.initialize_admin_view(self.ppa, {"field.private": "off"})
self.assertEqual(1, len(view.errors))
self.assertEqual(
'This archive already has published sources. '
'It is not possible to switch the privacy.',
view.errors[0])
+
+ def test_set_publishing_method_without_packages(self):
+ # If a PPA does not have packages published, it is possible to
+ # update the publishing_method attribute.
+ self.assertEqual(
+ ArchivePublishingMethod.LOCAL, self.ppa.publishing_method)
+ view = self.initialize_admin_view(
+ self.ppa, {"field.publishing_method": "ARTIFACTORY"})
+ self.assertEqual(0, len(view.errors))
+ self.assertEqual(
+ ArchivePublishingMethod.ARTIFACTORY, self.ppa.publishing_method)
+
+ def test_set_publishing_method_with_packages(self):
+ # If a PPA has packages published, it is impossible to update the
+ # publishing_method attribute.
+ self.publish_to_ppa(self.ppa)
+ view = self.initialize_admin_view(
+ self.ppa, {"field.publishing_method": "ARTIFACTORY"})
+ self.assertEqual(1, len(view.errors))
+ self.assertEqual(
+ "This archive already has published packages. "
+ "It is not possible to switch the publishing method.",
+ view.errors[0])
+
+ def test_set_repository_format_without_packages(self):
+ # If a PPA does not have packages published, it is possible to
+ # update the repository_format attribute.
+ self.assertEqual(
+ ArchiveRepositoryFormat.DEBIAN, self.ppa.repository_format)
+ view = self.initialize_admin_view(
+ self.ppa, {"field.repository_format": "PYTHON"})
+ self.assertEqual(0, len(view.errors))
+ self.assertEqual(
+ ArchiveRepositoryFormat.PYTHON, self.ppa.repository_format)
+
+ def test_set_repository_format_with_packages(self):
+ # If a PPA has packages published, it is impossible to update the
+ # repository_format attribute.
+ self.publish_to_ppa(self.ppa)
+ view = self.initialize_admin_view(
+ self.ppa, {"field.repository_format": "PYTHON"})
+ self.assertEqual(1, len(view.errors))
+ self.assertEqual(
+ "This archive already has published packages. "
+ "It is not possible to switch the repository format.",
+ view.errors[0])
diff --git a/lib/lp/soyuz/configure.zcml b/lib/lp/soyuz/configure.zcml
index 5aec316..29cabca 100644
--- a/lib/lp/soyuz/configure.zcml
+++ b/lib/lp/soyuz/configure.zcml
@@ -367,9 +367,13 @@
interface="lp.soyuz.interfaces.archive.IArchiveAdmin"
set_attributes="authorized_size
enabled_restricted_processors
- external_dependencies name
+ external_dependencies
+ name
permit_obsolete_series_uploads
- private require_virtualized"/>
+ private
+ publishing_method
+ repository_format
+ require_virtualized"/>
<require
permission="launchpad.Moderate"
set_schema="lp.soyuz.interfaces.archive.IArchiveRestricted"/>
diff --git a/lib/lp/soyuz/interfaces/archive.py b/lib/lp/soyuz/interfaces/archive.py
index c7f3523..da4b948 100644
--- a/lib/lp/soyuz/interfaces/archive.py
+++ b/lib/lp/soyuz/interfaces/archive.py
@@ -810,11 +810,11 @@ class IArchiveView(IHasBuildRecords):
publishing_method = Choice(
title=_("Publishing method"), vocabulary=ArchivePublishingMethod,
- required=True, readonly=True)
+ required=True, readonly=False)
repository_format = Choice(
title=_("Repository format"), vocabulary=ArchiveRepositoryFormat,
- required=True, readonly=True)
+ required=True, readonly=False)
@call_with(check_permissions=True, user=REQUEST_USER)
@operation_parameters(
