Merge ~pappacena/launchpad:ui-create-ociproject-for-projects into launchpad:master

[Thiago F. Pappacena]

UI to allow creating and editing OCI projects based on projects.

[Tom Wardill (twom)]

One minor comment, one general comment, but LGTM

review: Approve

[Ioana Lasc (ilasc)]

In Unit tests I'd prob go for soupmatchers , but other than that LGTM!

review: Approve

8290700... by Thiago F. Pappacena on 2020-06-17

Displaying pillar type on error message

[Thiago F. Pappacena (pappacena)]

Pushed the requested change. Merging this MP now.

c78a3c2... by Thiago F. Pappacena on 2020-06-17

Merge branch 'master' into ui-create-ociproject-for-projects

diff --git a/lib/lp/code/browser/tests/test_product.py b/lib/lp/code/browser/tests/test_product.py
index 360745e..d33a85e 100644
--- a/lib/lp/code/browser/tests/test_product.py
+++ b/lib/lp/code/browser/tests/test_product.py
@@ -426,3 +426,17 @@ class TestCanConfigureBranches(TestCaseWithFactory):
         login_person(product.owner)
         view = create_view(product, '+branches')
         self.assertTrue(view.can_configure_branches())
+
+
+class TestProductOverviewOCIProject(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_displays_create_oci_project_link(self):
+        product = self.factory.makeProduct()
+
+        browser = self.getUserBrowser(
+            canonical_url(product), user=product.owner)
+        text = extract_text(find_tag_by_id(browser.contents, 'global-actions'))
+
+        self.assertIn("Create an OCI Project", text)
diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 5e1489b..7e97bf9 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -2170,6 +2170,13 @@
         template="../templates/ociproject-new.pt"
         />
     <browser:page
+        name="+new-oci-project"
+        for="lp.registry.interfaces.product.IProduct"
+        class="lp.registry.browser.ociproject.OCIProjectAddView"
+        permission="launchpad.AnyPerson"
+        template="../templates/ociproject-new.pt"
+        />
+    <browser:page
         name="+search-oci-project"
         for="lp.registry.interfaces.distribution.IDistribution"
         class="lp.registry.browser.distribution.DistributionOCIProjectSearchView"
diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
index df7ac85..99c5b43 100644
--- a/lib/lp/registry/browser/ociproject.py
+++ b/lib/lp/registry/browser/ociproject.py
@@ -28,6 +28,7 @@ from lp.app.browser.tales import CustomizableFormatter
 from lp.app.errors import NotFoundError
 from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
 from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
+from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.ociproject import (
     IOCIProject,
     IOCIProjectSet,
@@ -38,6 +39,7 @@ from lp.registry.interfaces.ociprojectname import (
     IOCIProjectName,
     IOCIProjectNameSet,
     )
+from lp.registry.interfaces.product import IProduct
 from lp.services.features import getFeatureFlag
 from lp.services.webapp import (
     canonical_url,
@@ -53,6 +55,15 @@ from lp.services.webapp.breadcrumb import Breadcrumb
 from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
 
 
+def getPillarFieldName(pillar):
+    if IDistribution.providedBy(pillar):
+        return 'distribution'
+    elif IProduct.providedBy(pillar):
+        return 'project'
+    raise NotImplementedError("This view only supports distribution or "
+                              "project as pillars for OCIProject.")
+
+
 class OCIProjectAddView(LaunchpadFormView):
 
     schema = IOCIProjectName
@@ -85,10 +96,10 @@ class OCIProjectAddView(LaunchpadFormView):
         oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
             self.context, oci_project_name.name)
         if oci_project:
-            self.setFieldError(
-                    'name',
-                    'There is already an OCI project in %s with this name.' % (
-                        self.context.display_name))
+            pillar_type = getPillarFieldName(self.context)
+            msg = ('There is already an OCI project in %s %s with this name.'
+                   % (pillar_type, self.context.display_name))
+            self.setFieldError('name', msg)
 
 
 class OCIProjectFormatterAPI(CustomizableFormatter):
@@ -170,11 +181,20 @@ class OCIProjectEditView(LaunchpadEditFormView):
 
     schema = IOCIProject
     field_names = [
-        'distribution',
         'name',
         'official_recipe',
         ]
 
+    def setUpFields(self):
+        pillar_key = getPillarFieldName(self.context.pillar)
+        self.field_names = [pillar_key] + self.field_names
+
+        super(OCIProjectEditView, self).setUpFields()
+
+        # Set the correct pillar field as mandatory
+        pillar_field = self.form_fields.get(pillar_key).field
+        pillar_field.required = True
+
     def extendFields(self):
         official_recipe = self.context.getOfficialRecipe()
         self.form_fields += form.Fields(
@@ -191,16 +211,17 @@ class OCIProjectEditView(LaunchpadEditFormView):
 
     def validate(self, data):
         super(OCIProjectEditView, self).validate(data)
-        distribution = data.get('distribution')
+        pillar_type_field = getPillarFieldName(self.context.pillar)
+        pillar = data.get(pillar_type_field)
         name = data.get('name')
-        if distribution and name:
+        if pillar and name:
             oci_project = getUtility(IOCIProjectSet).getByPillarAndName(
-                distribution, name)
+                pillar, name)
             if oci_project is not None and oci_project != self.context:
                 self.setFieldError(
                     'name',
-                    'There is already an OCI project in %s with this name.' % (
-                        distribution.display_name))
+                    'There is already an OCI project in %s %s with this name.'
+                    % (pillar_type_field, pillar.display_name))
 
     @action('Update OCI project', name='update')
     def update_action(self, action, data):
diff --git a/lib/lp/registry/browser/product.py b/lib/lp/registry/browser/product.py
index 5c3ebce..f974d39 100644
--- a/lib/lp/registry/browser/product.py
+++ b/lib/lp/registry/browser/product.py
@@ -275,6 +275,10 @@ class ProductNavigation(
     def traverse_commercialsubscription(self, name):
         return self.context.commercial_subscription
 
+    @stepthrough('+oci')
+    def traverse_oci(self, name):
+        return self.context.getOCIProject(name)
+
     @stepthrough('+series')
     def traverse_series(self, name):
         series = self.context.getSeries(name)
@@ -431,6 +435,7 @@ class ProductNavigationMenu(NavigationMenu):
     facet = 'overview'
     links = [
         'details',
+        'new_oci_project',
         'announcements',
         'downloads',
         ]
@@ -504,6 +509,11 @@ class ProductEditLinksMixin(StructuralSubscriptionMenuMixin):
     def sharing(self):
         return Link('+sharing', 'Sharing', icon='edit')
 
+    @enabled_with_permission('launchpad.Driver')
+    def new_oci_project(self):
+        text = 'Create an OCI Project'
+        return Link('+new-oci-project', text, icon='add')
+
 
 class IProductEditMenu(Interface):
     """A marker interface for the 'Change details' navigation menu."""
@@ -522,7 +532,8 @@ class ProductActionNavigationMenu(NavigationMenu, ProductEditLinksMixin):
 
     @cachedproperty
     def links(self):
-        links = ['edit', 'review_license', 'administer', 'sharing']
+        links = ['edit', 'review_license', 'administer', 'sharing',
+                 'new_oci_project']
         add_subscribe_link(links)
         return links
 
diff --git a/lib/lp/registry/browser/tests/test_ociproject.py b/lib/lp/registry/browser/tests/test_ociproject.py
index 5187cd6..648f4d4 100644
--- a/lib/lp/registry/browser/tests/test_ociproject.py
+++ b/lib/lp/registry/browser/tests/test_ociproject.py
@@ -79,7 +79,7 @@ class TestOCIProjectView(BrowserTestCase):
 
     layer = DatabaseFunctionalLayer
 
-    def test_index(self):
+    def test_index_distribution_pillar(self):
         distribution = self.factory.makeDistribution(displayname="My Distro")
         oci_project = self.factory.makeOCIProject(
             pillar=distribution, ociprojectname="oci-name")
@@ -91,6 +91,18 @@ class TestOCIProjectView(BrowserTestCase):
             Name: oci-name
             """, self.getMainText(oci_project))
 
+    def test_index_project_pillar(self):
+        product = self.factory.makeProduct(displayname="My Project")
+        oci_project = self.factory.makeOCIProject(
+            pillar=product, ociprojectname="oci-name")
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            OCI project oci-name for My Project
+            .*
+            OCI project information
+            Project: My Project
+            Name: oci-name
+            """, self.getMainText(oci_project))
+
 
 class TestOCIProjectEditView(BrowserTestCase):
 
@@ -123,11 +135,39 @@ class TestOCIProjectEditView(BrowserTestCase):
         self.assertThat(
             "Distribution:\n%s\nEdit OCI project" % (
                 new_distribution.display_name),
-            MatchesTagText(content, "distribution"))
+            MatchesTagText(content, "pillar"))
         self.assertThat(
             "Name:\nnew-name\nEdit OCI project",
             MatchesTagText(content, "name"))
 
+    def test_edit_oci_project_change_project_pillar(self):
+        with admin_logged_in():
+            owner = self.factory.makePerson()
+            project = self.factory.makeProduct(owner=owner)
+            new_project = self.factory.makeProduct(owner=owner)
+            oci_project = self.factory.makeOCIProject(pillar=project)
+            new_project_name = new_project.name
+
+        browser = self.getViewBrowser(
+            oci_project, user=oci_project.pillar.owner)
+        browser.getLink("Edit OCI project").click()
+        browser.getControl(name="field.project").value = [new_project_name]
+        browser.getControl(name="field.name").value = "new-name"
+        browser.getControl("Update OCI project").click()
+
+        content = find_main_content(browser.contents)
+        with person_logged_in(owner):
+            self.assertEqual(
+                "OCI project new-name for %s" % new_project.display_name,
+                extract_text(content.h1))
+            self.assertThat(
+                "Project:\n%s\nEdit OCI project" % (
+                    new_project.display_name),
+                MatchesTagText(content, "pillar"))
+            self.assertThat(
+                "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])
@@ -153,7 +193,7 @@ class TestOCIProjectEditView(BrowserTestCase):
         self.assertThat(
             "Distribution:\n%s\nEdit OCI project" % (
                 new_distribution.display_name),
-            MatchesTagText(content, "distribution"))
+            MatchesTagText(content, "pillar"))
         self.assertThat(
             "Name:\nnew-name\nEdit OCI project",
             MatchesTagText(content, "name"))
@@ -180,8 +220,8 @@ class TestOCIProjectEditView(BrowserTestCase):
             oci_project, user=oci_project.pillar.owner)
         self.submitEditForm(browser, "two")
         self.assertEqual(
-            "There is already an OCI project in %s with this name." % (
-                pillar_display_name),
+            "There is already an OCI project in distribution %s with this "
+            "name." % (pillar_display_name),
             extract_text(find_tags_by_class(browser.contents, "message")[1]))
 
     def test_edit_oci_project_invalid_name(self):
@@ -280,11 +320,33 @@ class TestOCIProjectAddView(BrowserTestCase):
         self.assertThat(
             "Distribution:\n%s\nEdit OCI project" % (
                 new_distribution.display_name),
-            MatchesTagText(content, "distribution"))
+            MatchesTagText(content, "pillar"))
         self.assertThat(
              "Name:\nnew-name\nEdit OCI project",
              MatchesTagText(content, "name"))
 
+    def test_create_oci_project_for_project(self):
+        oci_project = self.factory.makeOCIProject()
+        user = oci_project.pillar.owner
+        project = self.factory.makeProduct(owner=user)
+        browser = self.getViewBrowser(
+            project, user=user, view_name='+new-oci-project')
+        browser.getControl(name="field.name").value = "new-name"
+        browser.getControl("Create OCI Project").click()
+
+        content = find_main_content(browser.contents)
+        with person_logged_in(user):
+            self.assertEqual(
+                "OCI project new-name for %s" % project.display_name,
+                extract_text(content.h1))
+            self.assertThat(
+                "Project:\n%s\nEdit OCI project" % (
+                    project.display_name),
+                MatchesTagText(content, "pillar"))
+            self.assertThat(
+                 "Name:\nnew-name\nEdit OCI project",
+                 MatchesTagText(content, "name"))
+
     def test_create_oci_project_already_exists(self):
         person = self.factory.makePerson()
         distribution = self.factory.makeDistribution(oci_project_admin=person)
@@ -297,9 +359,11 @@ class TestOCIProjectAddView(BrowserTestCase):
         browser.getControl(name="field.name").value = "new-name"
         browser.getControl("Create OCI Project").click()
 
+        expected_msg = (
+            "There is already an OCI project in distribution %s with this "
+            "name." % distribution.display_name)
         self.assertEqual(
-            "There is already an OCI project in %s with this name." % (
-                distribution.display_name),
+            expected_msg,
             extract_text(find_tags_by_class(browser.contents, "message")[1]))
 
     def test_create_oci_project_no_permission(self):
diff --git a/lib/lp/registry/interfaces/product.py b/lib/lp/registry/interfaces/product.py
index c15a8b3..b386fca 100644
--- a/lib/lp/registry/interfaces/product.py
+++ b/lib/lp/registry/interfaces/product.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces including and related to IProduct."""
@@ -798,6 +798,14 @@ class IProductView(
                                 filter_statuses.
         """
 
+    def canAdministerOCIProjects(person):
+        """Checks if the given person can manage OCI projects for this
+        Product."""
+
+    def getOCIProject(name):
+        """Return a `OCIProject` with the given name for this product, or None.
+        """
+
     def getPackage(distroseries):
         """Return a package in that distroseries for this product."""
 
diff --git a/lib/lp/registry/model/product.py b/lib/lp/registry/model/product.py
index a99d846..8cdd446 100644
--- a/lib/lp/registry/model/product.py
+++ b/lib/lp/registry/model/product.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes including and related to Product."""
@@ -142,6 +142,7 @@ from lp.registry.interfaces.accesspolicy import (
     IAccessPolicyGrantSource,
     IAccessPolicySource,
     )
+from lp.registry.interfaces.ociproject import IOCIProjectSet
 from lp.registry.interfaces.oopsreferences import IHasOOPSReferences
 from lp.registry.interfaces.person import (
     IPersonSet,
@@ -1140,6 +1141,23 @@ class Product(SQLBase, BugTargetBase, MakesAnnouncements,
         """See `IBugTarget`."""
         return self.name
 
+    def getOCIProject(self, name):
+        return getUtility(IOCIProjectSet).getByPillarAndName(
+            self, name)
+
+    def canAdministerOCIProjects(self, person):
+        if person is None:
+            return False
+        # XXX: pappacena 2020-05-25: Maybe we should have an attribute named
+        # oci_project_admin on Product too, the same way we have on
+        # Distribution.
+        if person.inTeam(self.driver):
+            return True
+        person_roles = IPersonRoles(person)
+        if person_roles.in_admin or person_roles.isOwner(self):
+            return True
+        return False
+
     def getPackage(self, distroseries):
         """See `IProduct`."""
         if isinstance(distroseries, Distribution):
diff --git a/lib/lp/registry/templates/ociproject-index.pt b/lib/lp/registry/templates/ociproject-index.pt
index 427b383..9a14cb7 100644
--- a/lib/lp/registry/templates/ociproject-index.pt
+++ b/lib/lp/registry/templates/ociproject-index.pt
@@ -28,11 +28,14 @@
   <div metal:fill-slot="main">
     <h2>OCI project information</h2>
     <div class="two-column-list">
-      <dl id="distribution" tal:define="distribution context/distribution">
-        <dt>Distribution:</dt>
+      <dl id="pillar" tal:define="pillar context/pillar">
+        <dt>
+            <span tal:condition="context/distribution">Distribution:</span>
+            <span tal:condition="context/project">Project:</span>
+        </dt>
         <dd>
-          <a tal:attributes="href distribution/fmt:url"
-             tal:content="distribution/display_name"/>
+          <a tal:attributes="href pillar/fmt:url"
+             tal:content="pillar/display_name"/>
           <a tal:replace="structure context/menu:overview/edit/fmt:icon"/>
         </dd>
       </dl>
diff --git a/lib/lp/security.py b/lib/lp/security.py
index f7d7bf8..dfc06c2 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -3464,7 +3464,7 @@ class EditOCIProject(AuthorizationBase):
         """Maintainers, drivers, and admins can drive projects."""
         return (user.in_admin or
                 user.isDriver(self.obj.pillar) or
-                user.inTeam(self.obj.pillar.oci_project_admin))
+                self.obj.pillar.canAdministerOCIProjects(user))
 
 
 class EditOCIProjectSeries(DelegatedAuthorization):