launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/launchpad:oci-admin-roles-need-edit into launchpad:master.

Commit message:
Allow edit permissions to oci_project_admin

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/384191

The oci_project_admin team/role on a Distribution should be allowed to edit OCI Projects and OCI Project Series.
Add that to the list of allowed permissions.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:oci-admin-roles-need-edit into launchpad:master.

diff --git a/lib/lp/registry/browser/tests/test_ociproject.py b/lib/lp/registry/browser/tests/test_ociproject.py
index 5a22137..5187cd6 100644
--- a/lib/lp/registry/browser/tests/test_ociproject.py
+++ b/lib/lp/registry/browser/tests/test_ociproject.py
@@ -128,6 +128,36 @@ class TestOCIProjectEditView(BrowserTestCase):
             "Name:\nnew-name\nEdit OCI project",
             MatchesTagText(content, "name"))
 
+    def test_edit_oci_project_ad_oci_project_admin(self):
+        admin_person = self.factory.makePerson()
+        admin_team = self.factory.makeTeam(members=[admin_person])
+        original_distribution = self.factory.makeDistribution(
+            oci_project_admin=admin_team)
+        oci_project = self.factory.makeOCIProject(
+            pillar=original_distribution)
+        new_distribution = self.factory.makeDistribution(
+            oci_project_admin=admin_team)
+
+        browser = self.getViewBrowser(
+            oci_project, user=admin_person)
+        browser.getLink("Edit OCI project").click()
+        browser.getControl(name="field.distribution").value = [
+            new_distribution.name]
+        browser.getControl(name="field.name").value = "new-name"
+        browser.getControl("Update OCI project").click()
+
+        content = find_main_content(browser.contents)
+        self.assertEqual(
+            "OCI project new-name for %s" % new_distribution.display_name,
+            extract_text(content.h1))
+        self.assertThat(
+            "Distribution:\n%s\nEdit OCI project" % (
+                new_distribution.display_name),
+            MatchesTagText(content, "distribution"))
+        self.assertThat(
+            "Name:\nnew-name\nEdit OCI project",
+            MatchesTagText(content, "name"))
+
     def test_edit_oci_project_sets_date_last_modified(self):
         # Editing an OCI project sets the date_last_modified property.
         date_created = datetime(2000, 1, 1, tzinfo=pytz.UTC)
diff --git a/lib/lp/registry/tests/test_ociproject.py b/lib/lp/registry/tests/test_ociproject.py
index fdddb4c..6ac98bb 100644
--- a/lib/lp/registry/tests/test_ociproject.py
+++ b/lib/lp/registry/tests/test_ociproject.py
@@ -58,6 +58,20 @@ class TestOCIProject(TestCaseWithFactory):
                 registrant)
             self.assertProvides(series, IOCIProjectSeries)
 
+    def test_newSeries_as_oci_project_admin(self):
+        admin_person = self.factory.makePerson()
+        admin_team = self.factory.makeTeam(members=[admin_person])
+        distribution = self.factory.makeDistribution(
+            oci_project_admin=admin_team)
+        oci_project = self.factory.makeOCIProject(pillar=distribution)
+        registrant = self.factory.makePerson()
+        with person_logged_in(admin_person):
+            series = oci_project.newSeries(
+                'test-series',
+                'test-summary',
+                registrant)
+            self.assertProvides(series, IOCIProjectSeries)
+
     def test_newSeries_bad_permissions(self):
         distribution = self.factory.makeDistribution()
         registrant = self.factory.makePerson()
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 0eec9f3..e8d7c36 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -3462,11 +3462,9 @@ class EditOCIProject(AuthorizationBase):
 
     def checkAuthenticated(self, user):
         """Maintainers, drivers, and admins can drive projects."""
-        # XXX twom 2019-10-29 This ideally shouldn't be driver, but a
-        # new role name that cascades upwards from the OCIProject
-        # to the pillar
         return (user.in_admin or
-                user.isDriver(self.obj.pillar))
+                user.isDriver(self.obj.pillar) or
+                user.inTeam(self.obj.pillar.oci_project_admin))
 
 
 class EditOCIProjectSeries(AuthorizationBase):
@@ -3476,7 +3474,8 @@ class EditOCIProjectSeries(AuthorizationBase):
     def checkAuthenticated(self, user):
         """Maintainers, drivers, and admins can drive projects."""
         return (user.in_admin or
-                user.isDriver(self.obj.oci_project.pillar))
+                user.isDriver(self.obj.oci_project.pillar) or
+                user.inTeam(self.obj.oci_project.pillar.oci_project_admin))
 
 
 class ViewOCIRecipeBuildRequest(DelegatedAuthorization):