Merge ~cjwatson/launchpad:oci-project-basic-views into launchpad:master

[Colin Watson]

Add basic index and edit views for OCIProject

8ea7924... by Colin Watson on 2019-11-28

Use OCIProjectNameSet.getOrCreateByName in the factory too

22aa91e... by Colin Watson on 2019-11-28

Add constraint to IOCIProject.name

There's already a constraint on IOCIProjectName.name, but duplicating it
here avoids an OOPS when editing an OCIProject in the web UI.

[Tom Wardill (twom)]

review: Approve

diff --git a/lib/lp/registry/browser/configure.zcml b/lib/lp/registry/browser/configure.zcml
index 018a53c..e277e94 100644
--- a/lib/lp/registry/browser/configure.zcml
+++ b/lib/lp/registry/browser/configure.zcml
@@ -602,6 +602,45 @@
         provides="zope.traversing.interfaces.IPathAdapter"
         for="lp.registry.interfaces.sourcepackage.ISourcePackage"
         />
+    <browser:defaultView
+        name="+index"
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        />
+    <browser:url
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        path_expression="string:+oci/${name}"
+        attribute_to_parent="pillar"
+        />
+    <browser:navigation
+        module="lp.registry.browser.ociproject"
+        classes="OCIProjectNavigation"
+        />
+    <browser:page
+        name="+index"
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        class="lp.services.webapp.LaunchpadView"
+        permission="launchpad.View"
+        template="../templates/ociproject-index.pt"
+        />
+    <browser:page
+        name="+edit"
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        class="lp.registry.browser.ociproject.OCIProjectEditView"
+        permission="launchpad.Edit"
+        template="../../app/templates/generic-edit.pt"
+        />
+    <browser:menus
+        module="lp.registry.browser.ociproject"
+        classes="
+            OCIProjectFacets
+            OCIProjectNavigationMenu"
+        />
+    <adapter
+        name="fmt"
+        factory="lp.registry.browser.ociproject.OCIProjectFormatterAPI"
+        provides="zope.traversing.interfaces.IPathAdapter"
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        />
     <browser:url
         for="lp.registry.interfaces.commercialsubscription.ICommercialSubscription"
         path_expression="string:+commercialsubscription/${id}"
diff --git a/lib/lp/registry/browser/distribution.py b/lib/lp/registry/browser/distribution.py
index 453823c..a17ff8d 100644
--- a/lib/lp/registry/browser/distribution.py
+++ b/lib/lp/registry/browser/distribution.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Browser views for distributions."""
@@ -155,6 +155,10 @@ class DistributionNavigation(
     def traverse_sources(self, name):
         return self.context.getSourcePackage(name)
 
+    @stepthrough('+oci')
+    def traverse_oci(self, name):
+        return self.context.getOCIProject(name)
+
     @stepthrough('+milestone')
     def traverse_milestone(self, name):
         return self.context.getMilestone(name)
diff --git a/lib/lp/registry/browser/ociproject.py b/lib/lp/registry/browser/ociproject.py
new file mode 100644
index 0000000..4d760a4
--- /dev/null
+++ b/lib/lp/registry/browser/ociproject.py
@@ -0,0 +1,124 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Views, menus, and traversal related to `OCIProject`s."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'OCIProjectBreadcrumb',
+    'OCIProjectFacets',
+    'OCIProjectNavigation',
+    ]
+
+from zope.component import getUtility
+from zope.interface import implementer
+
+from lp.app.browser.launchpadform import (
+    action,
+    LaunchpadEditFormView,
+    )
+from lp.app.browser.tales import CustomizableFormatter
+from lp.app.interfaces.headings import IHeadingBreadcrumb
+from lp.code.browser.vcslisting import TargetDefaultVCSNavigationMixin
+from lp.registry.interfaces.ociproject import (
+    IOCIProject,
+    IOCIProjectSet,
+    )
+from lp.services.webapp import (
+    canonical_url,
+    enabled_with_permission,
+    Link,
+    Navigation,
+    NavigationMenu,
+    StandardLaunchpadFacets,
+    )
+from lp.services.webapp.breadcrumb import Breadcrumb
+from lp.services.webapp.interfaces import IMultiFacetedBreadcrumb
+
+
+class OCIProjectFormatterAPI(CustomizableFormatter):
+    """Adapt `IOCIProject` objects to a formatted string."""
+
+    _link_summary_template = '%(displayname)s'
+
+    def _link_summary_values(self):
+        displayname = self._context.display_name
+        return {'displayname': displayname}
+
+
+class OCIProjectNavigation(TargetDefaultVCSNavigationMixin, Navigation):
+
+    usedfor = IOCIProject
+
+
+@implementer(IHeadingBreadcrumb, IMultiFacetedBreadcrumb)
+class OCIProjectBreadcrumb(Breadcrumb):
+    """Builds a breadcrumb for an `IOCIProject`."""
+
+    @property
+    def text(self):
+        return '%s OCI project' % self.context.name
+
+
+class OCIProjectFacets(StandardLaunchpadFacets):
+
+    usedfor = IOCIProject
+    enable_only = [
+        'overview',
+        'branches',
+        ]
+
+
+class OCIProjectNavigationMenu(NavigationMenu):
+    """Navigation menu for OCI projects."""
+
+    usedfor = IOCIProject
+
+    facet = 'overview'
+
+    links = ('edit',)
+
+    @enabled_with_permission('launchpad.Edit')
+    def edit(self):
+        return Link('+edit', 'Edit OCI project', icon='edit')
+
+
+class OCIProjectEditView(LaunchpadEditFormView):
+    """Edit an OCI project."""
+
+    schema = IOCIProject
+    field_names = [
+        'distribution',
+        'name',
+        ]
+
+    @property
+    def label(self):
+        return 'Edit %s OCI project' % self.context.name
+
+    page_title = 'Edit'
+
+    def validate(self, data):
+        super(OCIProjectEditView, self).validate(data)
+        distribution = data.get('distribution')
+        name = data.get('name')
+        if distribution and name:
+            oci_project = getUtility(IOCIProjectSet).getByDistributionAndName(
+                distribution, 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))
+
+    @action('Update OCI project', name='update')
+    def update_action(self, action, data):
+        self.updateContextFromData(data)
+
+    @property
+    def next_url(self):
+        return canonical_url(self.context)
+
+    cancel_url = next_url
diff --git a/lib/lp/registry/browser/tests/test_ociproject.py b/lib/lp/registry/browser/tests/test_ociproject.py
new file mode 100644
index 0000000..57f1061
--- /dev/null
+++ b/lib/lp/registry/browser/tests/test_ociproject.py
@@ -0,0 +1,152 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test OCI project views."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = []
+
+from datetime import datetime
+
+import pytz
+
+from lp.services.database.constants import UTC_NOW
+from lp.services.webapp import canonical_url
+from lp.services.webapp.escaping import structured
+from lp.testing import (
+    BrowserTestCase,
+    person_logged_in,
+    test_tales,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.matchers import MatchesTagText
+from lp.testing.pages import (
+    extract_text,
+    find_main_content,
+    find_tags_by_class,
+    )
+from lp.testing.publication import test_traverse
+from lp.testing.views import create_initialized_view
+
+
+class TestOCIProjectFormatterAPI(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_link(self):
+        oci_project = self.factory.makeOCIProject()
+        markup = structured(
+            '<a href="/%s/+oci/%s">%s</a>',
+            oci_project.pillar.name, oci_project.name,
+            oci_project.display_name).escapedtext
+        self.assertEqual(
+            markup,
+            test_tales('oci_project/fmt:link', oci_project=oci_project))
+
+
+class TestOCIProjectNavigation(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_canonical_url(self):
+        distribution = self.factory.makeDistribution(name="mydistro")
+        oci_project = self.factory.makeOCIProject(
+            pillar=distribution, ociprojectname="myociproject")
+        self.assertEqual(
+            "http://launchpad.test/mydistro/+oci/myociproject",
+            canonical_url(oci_project))
+
+    def test_traversal(self):
+        oci_project = self.factory.makeOCIProject()
+        obj, _, _ = test_traverse(
+            "http://launchpad.test/%s/+oci/%s" %
+            (oci_project.pillar.name, oci_project.name))
+        self.assertEqual(oci_project, obj)
+
+
+class TestOCIProjectView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_index(self):
+        distribution = self.factory.makeDistribution(displayname="My Distro")
+        oci_project = self.factory.makeOCIProject(
+            pillar=distribution, ociprojectname="oci-name")
+        self.assertTextMatchesExpressionIgnoreWhitespace("""\
+            OCI project oci-name for My Distro
+            .*
+            OCI project information
+            Distribution: My Distro
+            Name: oci-name
+            """, self.getMainText(oci_project))
+
+
+class TestOCIProjectEditView(BrowserTestCase):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_edit_oci_project(self):
+        oci_project = self.factory.makeOCIProject()
+        new_distribution = self.factory.makeDistribution(
+            owner=oci_project.pillar.owner)
+
+        browser = self.getViewBrowser(
+            oci_project, user=oci_project.pillar.owner)
+        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)
+        oci_project = self.factory.makeOCIProject(date_created=date_created)
+        self.assertEqual(date_created, oci_project.date_last_modified)
+        with person_logged_in(oci_project.pillar.owner):
+            view = create_initialized_view(
+                oci_project, name="+edit", principal=oci_project.pillar.owner)
+            view.update_action.success({"name": "changed"})
+        self.assertSqlAttributeEqualsDate(
+            oci_project, "date_last_modified", UTC_NOW)
+
+    def test_edit_oci_project_already_exists(self):
+        oci_project = self.factory.makeOCIProject(ociprojectname="one")
+        self.factory.makeOCIProject(
+            pillar=oci_project.pillar, ociprojectname="two")
+        pillar_display_name = oci_project.pillar.display_name
+        browser = self.getViewBrowser(
+            oci_project, user=oci_project.pillar.owner)
+        browser.getLink("Edit OCI project").click()
+        browser.getControl(name="field.name").value = "two"
+        browser.getControl("Update OCI project").click()
+        self.assertEqual(
+            "There is already an OCI project in %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):
+        oci_project = self.factory.makeOCIProject()
+        browser = self.getViewBrowser(
+            oci_project, user=oci_project.pillar.owner)
+        browser.getLink("Edit OCI project").click()
+        browser.getControl(name="field.name").value = "invalid name"
+        browser.getControl("Update OCI project").click()
+        self.assertStartsWith(
+            extract_text(find_tags_by_class(browser.contents, "message")[1]),
+            "Invalid name 'invalid name'.")
diff --git a/lib/lp/registry/configure.zcml b/lib/lp/registry/configure.zcml
index c95a7b2..2f01072 100644
--- a/lib/lp/registry/configure.zcml
+++ b/lib/lp/registry/configure.zcml
@@ -746,18 +746,19 @@
             interface="lp.registry.interfaces.ociproject.IOCIProjectEdit"
             set_schema="lp.registry.interfaces.ociproject.IOCIProjectEditableAttributes" />
     </class>
-    <securedutility
-        class="lp.registry.model.ociproject.OCIProject"
-        provides="lp.registry.interfaces.ociproject.IOCIProject">
-        <allow
-            interface="lp.registry.interfaces.ociproject.IOCIProject"/>
-    </securedutility>
+    <subscriber
+        for="lp.registry.interfaces.ociproject.IOCIProject zope.lifecycleevent.interfaces.IObjectModifiedEvent"
+        handler="lp.registry.model.ociproject.oci_project_modified" />
     <securedutility
         class="lp.registry.model.ociproject.OCIProjectSet"
         provides="lp.registry.interfaces.ociproject.IOCIProjectSet">
         <allow
             interface="lp.registry.interfaces.ociproject.IOCIProjectSet"/>
     </securedutility>
+    <adapter
+        for="lp.registry.interfaces.ociproject.IOCIProject"
+        provides="lp.services.webapp.interfaces.IBreadcrumb"
+        factory="lp.registry.browser.ociproject.OCIProjectBreadcrumb"/>
 
     <!-- OCIProjectSeries -->
     <class
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index 0750767..e0b3c00 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -14,6 +14,7 @@ __all__ = [
 from lazr.restful.fields import (
     CollectionField,
     Reference,
+    ReferenceChoice,
     )
 from zope.interface import (
     Attribute,
@@ -23,9 +24,11 @@ from zope.schema import (
     Datetime,
     Int,
     Text,
+    TextLine,
     )
 
 from lp import _
+from lp.app.validators.name import name_validator
 from lp.bugs.interfaces.bugtarget import IBugTarget
 from lp.code.interfaces.hasgitrepositories import IHasGitRepositories
 from lp.registry.interfaces.distribution import IDistribution
@@ -54,7 +57,6 @@ class IOCIProjectView(IHasGitRepositories, Interface):
         # Really IOCIProjectSeries
         value_type=Reference(schema=Interface))
 
-    name = Attribute(_("Name"))
     display_name = Attribute(_("Display name for this OCI project."))
 
 
@@ -64,12 +66,17 @@ class IOCIProjectEditableAttributes(IBugTarget):
     These attributes need launchpad.View to see, and launchpad.Edit to change.
     """
 
-    distribution = Reference(
-        IDistribution,
-        title=_("The distribution that this OCI project is associated with."))
+    distribution = ReferenceChoice(
+        title=_("The distribution that this OCI project is associated with."),
+        schema=IDistribution, vocabulary="Distribution",
+        required=True, readonly=False)
+    name = TextLine(
+        title=_("Name"), required=True, readonly=False,
+        constraint=name_validator,
+        description=_("The name of this OCI project."))
     ociprojectname = Reference(
         IOCIProjectName,
-        title=_("The name of this OCI project."),
+        title=_("The name of this OCI project, as an `IOCIProjectName`."),
         required=True,
         readonly=True)
     description = Text(title=_("The description for this OCI project."))
diff --git a/lib/lp/registry/interfaces/ociprojectname.py b/lib/lp/registry/interfaces/ociprojectname.py
index 3c34ffc..e2ed738 100644
--- a/lib/lp/registry/interfaces/ociprojectname.py
+++ b/lib/lp/registry/interfaces/ociprojectname.py
@@ -39,13 +39,16 @@ class IOCIProjectNameSet(Interface):
     """A set of `OCIProjectName`."""
 
     def __getitem__(name):
-        """Retrieve a `OCIProjectName` by name."""
+        """Retrieve an `OCIProjectName` by name."""
 
     def getByName(name):
-        """Return a `OCIProjectName` by its name.
+        """Return an `OCIProjectName` by its name.
 
         :raises NoSuchOCIProjectName: if the `OCIProjectName` can't be found.
         """
 
     def new(name):
         """Create a new `OCIProjectName`."""
+
+    def getOrCreateByName(name):
+        """Return an `OCIProjectName` by its name, creating it if necessary."""
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index f0975b3..7dcfdea 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -19,7 +19,9 @@ from storm.locals import (
     Reference,
     Unicode,
     )
+from zope.component import getUtility
 from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
 
 from lp.bugs.model.bugtarget import BugTargetBase
 from lp.registry.interfaces.distribution import IDistribution
@@ -27,10 +29,14 @@ from lp.registry.interfaces.ociproject import (
     IOCIProject,
     IOCIProjectSet,
     )
+from lp.registry.interfaces.ociprojectname import IOCIProjectNameSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.ociprojectname import OCIProjectName
 from lp.registry.model.ociprojectseries import OCIProjectSeries
-from lp.services.database.constants import DEFAULT
+from lp.services.database.constants import (
+    DEFAULT,
+    UTC_NOW,
+    )
 from lp.services.database.interfaces import (
     IMasterStore,
     IStore,
@@ -38,6 +44,17 @@ from lp.services.database.interfaces import (
 from lp.services.database.stormbase import StormBase
 
 
+def oci_project_modified(oci_project, event):
+    """Update the date_last_modified property when an OCIProject is modified.
+
+    This method is registered as a subscriber to `IObjectModifiedEvent`
+    events on OCI projects.
+    """
+    # This attribute is normally read-only; bypass the security proxy to
+    # avoid that.
+    removeSecurityProxy(oci_project).date_last_modified = UTC_NOW
+
+
 @implementer(IOCIProject)
 class OCIProject(BugTargetBase, StormBase):
     """See `IOCIProject` and `IOCIProjectSet`."""
@@ -70,6 +87,11 @@ class OCIProject(BugTargetBase, StormBase):
     def name(self):
         return self.ociprojectname.name
 
+    @name.setter
+    def name(self, value):
+        self.ociprojectname = getUtility(IOCIProjectNameSet).getOrCreateByName(
+            value)
+
     @property
     def pillar(self):
         """See `IBugTarget`."""
@@ -79,7 +101,7 @@ class OCIProject(BugTargetBase, StormBase):
     def display_name(self):
         """See `IOCIProject`."""
         return "OCI project %s for %s" % (
-            self.ociprojectname.name, self.pillar.name)
+            self.ociprojectname.name, self.pillar.display_name)
 
     bugtargetname = display_name
     bugtargetdisplayname = display_name
diff --git a/lib/lp/registry/model/ociprojectname.py b/lib/lp/registry/model/ociprojectname.py
index 9085916..5591bd8 100644
--- a/lib/lp/registry/model/ociprojectname.py
+++ b/lib/lp/registry/model/ociprojectname.py
@@ -70,3 +70,10 @@ class OCIProjectNameSet:
         project_name = OCIProjectName(name=name)
         store.add(project_name)
         return project_name
+
+    def getOrCreateByName(self, name):
+        """See `IOCIProjectNameSet`."""
+        try:
+            return self.getByName(name)
+        except NoSuchOCIProjectName:
+            return self.new(name)
diff --git a/lib/lp/registry/templates/ociproject-index.pt b/lib/lp/registry/templates/ociproject-index.pt
new file mode 100644
index 0000000..8a9a6a1
--- /dev/null
+++ b/lib/lp/registry/templates/ociproject-index.pt
@@ -0,0 +1,49 @@
+<html
+  xmlns="http://www.w3.org/1999/xhtml"
+  xmlns:tal="http://xml.zope.org/namespaces/tal"
+  xmlns:metal="http://xml.zope.org/namespaces/metal"
+  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+  metal:use-macro="view/macro:page/main_side"
+  i18n:domain="launchpad"
+>
+
+<body>
+  <metal:registering fill-slot="registering">
+    Created by
+      <tal:registrant replace="structure context/registrant/fmt:link"/>
+    on
+      <tal:created-on replace="structure context/date_created/fmt:date"/>
+    and last modified on
+      <tal:last-modified replace="structure context/date_last_modified/fmt:date"/>
+  </metal:registering>
+
+  <metal:side fill-slot="side">
+    <div tal:replace="structure context/@@+global-actions"/>
+  </metal:side>
+
+  <metal:heading fill-slot="heading">
+    <h1 tal:content="context/display_name"/>
+  </metal:heading>
+
+  <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>
+        <dd>
+          <a tal:attributes="href distribution/fmt:url"
+             tal:content="distribution/display_name"/>
+          <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
+        </dd>
+      </dl>
+      <dl id="name">
+        <dt>Name:</dt>
+        <dd>
+          <span tal:content="context/name"/>
+          <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
+        </dd>
+      </dl>
+    </div>
+  </div>
+</body>
+</html>
diff --git a/lib/lp/registry/tests/test_ociproject.py b/lib/lp/registry/tests/test_ociproject.py
index 174b12b..42c5d7d 100644
--- a/lib/lp/registry/tests/test_ociproject.py
+++ b/lib/lp/registry/tests/test_ociproject.py
@@ -71,7 +71,15 @@ class TestOCIProject(TestCaseWithFactory):
         oci_project_name = self.factory.makeOCIProjectName(name='test-name')
         oci_project = self.factory.makeOCIProject(
             ociprojectname=oci_project_name)
-        self.assertEqual(oci_project.name, 'test-name')
+        self.assertEqual('test-name', oci_project.name)
+
+    def test_display_name(self):
+        oci_project_name = self.factory.makeOCIProjectName(name='test-name')
+        oci_project = self.factory.makeOCIProject(
+            ociprojectname=oci_project_name)
+        self.assertEqual(
+            'OCI project test-name for %s' % oci_project.pillar.display_name,
+            oci_project.display_name)
 
 
 class TestOCIProjectSet(TestCaseWithFactory):
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 39d2725..0c72d31 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4901,7 +4901,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
     def makeOCIProjectName(self, name=None):
         if name is None:
             name = self.getUniqueString(u"oci-project-name")
-        return getUtility(IOCIProjectNameSet).new(name)
+        return getUtility(IOCIProjectNameSet).getOrCreateByName(name)
 
     def makeOCIProject(self, registrant=None, pillar=None,
                        ociprojectname=None, date_created=DEFAULT,