Merge lp:~sinzui/launchpad/distro-modification-pages into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my branch to update distro modification pages to 3.0. This effort
started out as a simple set of mechanical changes. When I realised that
two of the forms did not have views, I decided to extend the
LaunchpadEditFormView to to provide all the features needed for UI 3.0.
I did not update LaunchpadEditFormView because I am unsure of the
consequences.

    lp:~sinzui/launchpad/distro-modification-pages
    Diff size: 568
    Launchpad bug: https://bugs.launchpad.net/bugs/414043
    Test command: ./bin/test -vvt "distribution-views"
    Pre-implementation: no one
    Target release: 2.2.8

= Update distro modification pages to 3.0 =

Update the distribution add and edit pages to the new UI.

== Rules ==

    * Some are simple and can be deleted; use generic-edit.pt
    * Update labels and page_title
    * Add cancel_url as needed

Addendum

    * Some pages are using ZCML editform that must change to a view.

== QA ==

Get admin powers on staging
    * Visit https://staging.launchpad.net/distros/+add
      * Verify the label and page_title are the same.
      * Verify there is a cancel link back to the /distros page
      * Create a distro
      * Verify that you are on the new distro page.
    * Choose Change details
      * Verify the label and page_title are the same.
      * Verify there is a cancel link back to the distro page
    * Choose the edit link next to members
      * Verify the label and page_title are the same.
      * Verify there is a cancel link back to the distro page
    * Choose the edit link next to mirror admins
      * Verify the label and page_title are the same.
      * Verify there is a cancel link back to the distro page

== Lint ==

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/pagetitles.py
  lib/lp/registry/browser/__init__.py
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/distribution.py
  lib/lp/registry/browser/tests/distribution-views.txt
  lib/lp/registry/interfaces/distribution.py

== Test ==

    * lib/lp/registry/browser/tests/distribution-views.txt
      * Added tests for add, edit, edit members, and edit mirror admins.

== Implementation ==

    * lib/canonical/launchpad/pagetitles.py
      * Removed old page titles.
    * lib/lp/registry/browser/__init__.py
      * Added the RegistryEditFormView to make the conversion easier and
        all edit pages consistent.
    * lib/lp/registry/browser/configure.zcml
      * Converted editform to page directives.
      * Switched forms to use generic-edit.pt
    * lib/lp/registry/browser/distribution.py
      * Updated the label on the views to use the distro name.
      * Added page_title and cancel urls (often via RegistryEditFormView)
    * lib/lp/registry/interfaces/distribution.py
      * Updated a field description so that the page template did not need
        to explain the purpose of the field.
    * Deleted the 4 templates

[Henning Eggers (henninge)]

Very nice branch, thank you. Extending LPEView for your purposes is a very efficient approach; I wouldn't have dared either to makt those assumptions for all views.

Just one issue: There is copy&paste error on line 407 where you mention mirror_admin instead of members.

review: Approve

=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py	2009-08-13 00:39:47 +0000
+++ lib/canonical/launchpad/pagetitles.py	2009-08-15 13:37:48 +0000
@@ -459,8 +459,6 @@
 
 distributionmirror_review = ContextTitle('Review mirror %s')
 
-distribution_add = 'Register a new distribution'
-
 distribution_allpackages = ContextTitle('All packages in %s')
 
 distribution_archive_list = ContextTitle('%s Copy Archives')
@@ -469,21 +467,13 @@
 
 distribution_upstream_bug_report = ContextTitle('Upstream Bug Report for %s')
 
-distribution_change_mirror_admin = 'Change mirror administrator'
-
 distribution_cvereport = ContextTitle('CVE reports for %s')
 
-distribution_edit = 'Change distribution details'
-# We don't mention its name here, because that might be what you're changing.
-
 distribution_language_pack_admin = ContextTitle(
     'Change the language pack administrator for %s')
 
 distribution_members = ContextTitle('%s distribution members')
 
-distribution_memberteam = ContextTitle(
-    smartquote("Change %s's distribution team"))
-
 distribution_mirrors = ContextTitle("Mirrors of %s")
 
 distribution_newmirror = ContextTitle("Register a new mirror for %s")

=== modified file 'lib/lp/registry/browser/__init__.py'
--- lib/lp/registry/browser/__init__.py	2009-08-06 19:11:58 +0000
+++ lib/lp/registry/browser/__init__.py	2009-08-15 13:48:33 +0000
@@ -8,6 +8,7 @@
 __all__ = [
     'get_status_count',
     'MilestoneOverlayMixin',
+    'RegistryEditFormView',
     'RegistryDeleteViewMixin',
     'StatusCount',
     ]
@@ -16,9 +17,12 @@
 from operator import attrgetter
 
 from zope.component import getUtility
+
 from lp.bugs.interfaces.bugtask import BugTaskSearchParams, IBugTaskSet
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
-from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.launchpadform import (
+    action, LaunchpadEditFormView)
+from canonical.launchpad.webapp.publisher import canonical_url
 
 
 class StatusCount:
@@ -120,3 +124,22 @@
             for release_file in release.files:
                 release_file.destroySelf()
             release.destroySelf()
+
+
+class RegistryEditFormView(LaunchpadEditFormView):
+    """"A base class that provides consistent edit form behaviour."""
+    @property
+    def page_title(self):
+        """The page title."""
+        return self.label
+
+    @property
+    def cancel_url(self):
+        """See `LaunchpadFormView`."""
+        return canonical_url(self.context)
+
+    next_url = cancel_url
+
+    @action("Change", name='change')
+    def change_action(self, action, data):
+        self.updateContextFromData(data)

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-08-14 00:52:28 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-08-15 13:37:48 +0000
@@ -1964,23 +1964,21 @@
         class="lp.registry.browser.distribution.DistributionEditView"
         name="+edit"
         facet="overview"
-        template="../templates/distribution-edit.pt"/>
-    <browser:editform
+        template="../../app/templates/generic-edit.pt"/>
+    <browser:page
         name="+selectmemberteam"
         for="lp.registry.interfaces.distribution.IDistribution"
-        schema="lp.registry.interfaces.distribution.IDistribution"
+        class="lp.registry.browser.distribution.DistributionChangeMembersView"
         facet="overview"
-        fields="members"
         permission="launchpad.Edit"
-        template="../templates/distribution-memberteam.pt"/>
-    <browser:editform
+        template="../../app/templates/generic-edit.pt"/>
+    <browser:page
         name="+selectmirroradmins"
         for="lp.registry.interfaces.distribution.IDistribution"
-        schema="lp.registry.interfaces.distribution.IDistribution"
+        class="lp.registry.browser.distribution.DistributionChangeMirrorAdminView"
         facet="overview"
-        fields="mirror_admin"
         permission="launchpad.Edit"
-        template="../templates/distribution-change-mirror-admin.pt"/>
+        template="../../app/templates/generic-edit.pt"/>
     <browser:addform
         name="+linkbounty"
         for="lp.registry.interfaces.distribution.IDistribution"
@@ -2037,7 +2035,7 @@
         class="lp.registry.browser.distribution.DistributionAddView"
         facet="overview"
         permission="launchpad.Admin"
-        template="../templates/distribution-add.pt"/>
+        template="../../app/templates/generic-edit.pt"/>
     <browser:menus
         classes="
             DistributionFacets

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2009-08-05 01:49:41 +0000
+++ lib/lp/registry/browser/distribution.py	2009-08-15 13:50:42 +0000
@@ -12,6 +12,8 @@
     'DistributionArchiveMirrorsView',
     'DistributionArchivesView',
     'DistributionBreadcrumbBuilder',
+    'DistributionChangeMembersView',
+    'DistributionChangeMirrorAdminView',
     'DistributionCountryArchiveMirrorsView',
     'DistributionDisabledMirrorsView',
     'DistributionEditView',
@@ -67,7 +69,7 @@
     IPublishedPackageSet)
 from canonical.launchpad.webapp import (
     action, ApplicationMenu, canonical_url, ContextMenu, custom_widget,
-    enabled_with_permission, GetitemNavigation, LaunchpadEditFormView,
+    enabled_with_permission, GetitemNavigation,
     LaunchpadFormView, LaunchpadView, Link, Navigation, redirection,
     StandardLaunchpadFacets, stepthrough, stepto)
 from canonical.launchpad.webapp.interfaces import (
@@ -78,6 +80,8 @@
 from canonical.launchpad.webapp.breadcrumb import BreadcrumbBuilder
 from canonical.widgets.image import ImageChangeWidget
 
+from lp.registry.browser import RegistryEditFormView
+
 
 class UsesLaunchpadMixin:
     """This mixin is used for the overview page of products and distros."""
@@ -776,12 +780,22 @@
 class DistributionAddView(LaunchpadFormView):
 
     schema = IDistribution
-    label = "Create a new distribution"
+    label = "Register a new distribution"
     field_names = ["name", "displayname", "title", "summary", "description",
                    "domainname", "members",
                    "official_malone", "official_blueprints",
                    "official_rosetta", "official_answers"]
 
+    @property
+    def page_title(self):
+        """The page title."""
+        return self.label
+
+    @property
+    def cancel_url(self):
+        """See `LaunchpadFormView`."""
+        return canonical_url(self.context)
+
     @action("Save", name='save')
     def save_action(self, action, data):
         distribution = getUtility(IDistributionSet).new(
@@ -798,10 +812,9 @@
         self.next_url = canonical_url(distribution)
 
 
-class DistributionEditView(LaunchpadEditFormView):
+class DistributionEditView(RegistryEditFormView):
 
     schema = IDistribution
-    label = "Change distribution details"
     field_names = ['displayname', 'title', 'summary', 'description',
                    'bug_reporting_guidelines', 'icon', 'logo', 'mugshot',
                    'official_malone', 'enable_bug_expiration',
@@ -812,6 +825,11 @@
     custom_widget('logo', ImageChangeWidget, ImageChangeWidget.EDIT_STYLE)
     custom_widget('mugshot', ImageChangeWidget, ImageChangeWidget.EDIT_STYLE)
 
+    @property
+    def label(self):
+        """See `LaunchpadFormView`."""
+        return 'Change %s details' % self.context.displayname
+
     def validate(self, data):
         """Constrain bug expiration to Launchpad Bugs tracker."""
         # enable_bug_expiration is disabled by JavaScript when official_malone
@@ -821,10 +839,27 @@
         if not official_malone:
             data['enable_bug_expiration'] = False
 
-    @action("Change", name='change')
-    def change_action(self, action, data):
-        self.updateContextFromData(data)
-        self.next_url = canonical_url(self.context)
+
+class DistributionChangeMirrorAdminView(RegistryEditFormView):
+    """A view to change the mirror administrator."""
+    schema = IDistribution
+    field_names = ['mirror_admin']
+
+    @property
+    def label(self):
+        """See `LaunchpadFormView`."""
+        return "Change the %s mirror administrator" % self.context.displayname
+
+
+class DistributionChangeMembersView(RegistryEditFormView):
+    """A view to change the members team."""
+    schema = IDistribution
+    field_names = ['members']
+
+    @property
+    def label(self):
+        """See `LaunchpadFormView`."""
+        return "Change the %s members team" % self.context.displayname
 
 
 class DistributionCountryArchiveMirrorsView(LaunchpadView):

=== modified file 'lib/lp/registry/browser/tests/distribution-views.txt'
--- lib/lp/registry/browser/tests/distribution-views.txt	2009-07-27 14:08:17 +0000
+++ lib/lp/registry/browser/tests/distribution-views.txt	2009-08-15 13:37:48 +0000
@@ -9,7 +9,7 @@
 country.
 
     >>> from zope.component import getMultiAdapter, getUtility
-    >>> from canonical.launchpad.interfaces import IDistributionSet
+    >>> from lp.registry.interfaces.distribution import IDistributionSet
     >>> from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
     >>> login("[e-mail]")
@@ -227,3 +227,176 @@
     >>> for pkg in search_results:
     ...     print pkg.name
     mozilla-firefox
+
+
+Distribution modification views
+===============================
+
+
+Registering a distribution
+--------------------------
+
+The +add view of the DistributionSet allows admins to register distributions.
+
+    >>> distributionset = getUtility(IDistributionSet)
+    >>> view = create_view(distributionset, '+add')
+    >>> print view.label
+    Register a new distribution
+
+    >>> print view.page_title
+    Register a new distribution
+
+The view provides a cancel link.
+
+    >>> print view.cancel_url
+    http://launchpad.dev/distros
+
+The view accepts the basic fields to register a distribution.
+
+    >>> view.field_names
+    ['name', 'displayname', 'title', 'summary', 'description', 'domainname',
+     'members', 'official_malone', 'official_blueprints', 'official_rosetta',
+     'official_answers']
+
+    >>> form = {
+    ...     'field.name': 'youbuntu',
+    ...     'field.displayname': 'YoUbuntu',
+    ...     'field.title': 'YoUbuntu OS',
+    ...     'field.summary': 'summary',
+    ...     'field.description': 'description',
+    ...     'field.domainname': 'youbuntu.me',
+    ...     'field.members': 'landscape-developers',
+    ...     'field.actions.save': 'Save',
+    ...     }
+    >>> view = create_initialized_view(distributionset, '+add', form=form)
+    >>> view.errors
+    []
+    >>> print view.next_url
+    http://launchpad.dev/youbuntu
+
+    >>> distribution = distributionset.getByName("youbuntu")
+    >>> print distribution.name
+    youbuntu
+
+
+Editing a distribution
+----------------------
+
+The +edit view allows an owner or admin to change a distribution. It provides
+a label, page_title, and cancel_url.
+
+    >>> view = create_view(distribution, '+edit')
+    >>> print view.label
+    Change YoUbuntu details
+
+    >>> print view.page_title
+    Change YoUbuntu details
+
+    >>> print view.cancel_url
+    http://launchpad.dev/youbuntu
+
+The view accepts most of the distribution fields.
+
+    >>> distribution.official_malone
+    False
+
+    >>> view.field_names
+    ['displayname', 'title', 'summary', 'description',
+     'bug_reporting_guidelines', 'icon', 'logo', 'mugshot', 'official_malone',
+     'enable_bug_expiration', 'official_blueprints', 'official_rosetta',
+     'official_answers', 'translation_focus']
+
+    >>> del form['field.name']
+    >>> del form['field.actions.save']
+    >>> form['field.bug_reporting_guidelines'] = 'guidelines'
+    >>> form['field.official_malone'] = 'on'
+    >>> form['field.actions.change'] = 'Change'
+    >>> view = create_initialized_view(distribution, '+edit', form=form)
+    >>> view.errors
+    []
+    >>> print view.next_url
+    http://launchpad.dev/youbuntu
+
+    >>> print distribution.bug_reporting_guidelines
+    guidelines
+
+    >>> distribution.official_malone
+    True
+
+
+Changing a distribution mirror administrator
+--------------------------------------------
+
+the +selectmirroradmins allows the owner or admin to change the mirror
+administrator.
+
+    >>> view = create_view(distribution, '+selectmirroradmins')
+    >>> print view.label
+    Change the YoUbuntu mirror administrator
+
+    >>> print view.page_title
+    Change the YoUbuntu mirror administrator
+
+    >>> print view.cancel_url
+    http://launchpad.dev/youbuntu
+
+The view accepts the mirror_admin field.
+
+    >>> print distribution.mirror_admin.name
+    name16
+
+    >>> view.field_names
+    ['mirror_admin']
+
+    >>> form = {
+    ...     'field.mirror_admin': 'no-priv',
+    ...     'field.actions.change': 'Change',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     distribution, '+selectmirroradmins', form=form)
+    >>> view.errors
+    []
+    >>> print view.next_url
+    http://launchpad.dev/youbuntu
+
+    >>> print distribution.mirror_admin.name
+    no-priv
+
+
+Changing a distribution members team
+------------------------------------
+
+the +selectmirroradmins allows the owner or admin to change the members team.
+
+    >>> view = create_view(distribution, '+selectmemberteam')
+    >>> print view.label
+    Change the YoUbuntu members team
+
+    >>> print view.page_title
+    Change the YoUbuntu members team
+
+    >>> print view.cancel_url
+    http://launchpad.dev/youbuntu
+
+The view accepts the mirror_admin field.
+
+    >>> print distribution.members.name
+    landscape-developers
+
+    >>> view.field_names
+    ['members']
+
+    >>> form = {
+    ...     'field.members': 'ubuntu-team',
+    ...     'field.actions.change': 'Change',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     distribution, '+selectmemberteam', form=form)
+    >>> view.errors
+    []
+    >>> print view.next_url
+    http://launchpad.dev/youbuntu
+
+    >>> print distribution.members.name
+    ubuntu-team
+

=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py	2009-08-09 18:01:59 +0000
+++ lib/lp/registry/interfaces/distribution.py	2009-08-15 13:12:34 +0000
@@ -182,8 +182,8 @@
         vocabulary='ValidPersonOrTeam')
     mirror_admin = PublicPersonChoice(
         title=_("Mirror Administrator"),
-        description=_("The person or team that has the rights to administer "
-                      "this distribution's mirrors"),
+        description=_("The person or team that has the rights to review and "
+                      "mark this distribution's mirrors as official."),
         required=True, vocabulary='ValidPersonOrTeam')
     lucilleconfig = TextLine(
         title=_("Lucille Config"),

=== removed file 'lib/lp/registry/templates/distribution-add.pt'
--- lib/lp/registry/templates/distribution-add.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/distribution-add.pt	1970-01-01 00:00:00 +0000
@@ -1,28 +0,0 @@
-<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"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-
-<body>
-
-<metal:leftportlets fill-slot="portlets_one">
-</metal:leftportlets>
-
-<metal:rightportlets fill-slot="portlets_two">
-</metal:rightportlets>
-
-<div metal:fill-slot="main">
-
-  <div metal:use-macro="context/@@launchpad_form/form" />
-
-</div>
-</body>
-</html>
-

=== removed file 'lib/lp/registry/templates/distribution-change-mirror-admin.pt'
--- lib/lp/registry/templates/distribution-change-mirror-admin.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/distribution-change-mirror-admin.pt	1970-01-01 00:00:00 +0000
@@ -1,29 +0,0 @@
-<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"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-<body>
-<tal:tag condition="view/update"/>
-<metal:heading fill-slot="pageheading">
-  <h1>Select this Distribution's Mirror Administrator</h1>
-</metal:heading>
-
-<div metal:fill-slot="main">
-
-  <div metal:use-macro="context/@@launchpad_editform/editform">
-
-    <p metal:fill-slot="extra_info">
-      The mirror administrators are the ones allowed to review and mark this
-      distribution's mirrors as official.
-    </p>
-  </div>
-</div>
-</body>
-</html>

=== removed file 'lib/lp/registry/templates/distribution-edit.pt'
--- lib/lp/registry/templates/distribution-edit.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/distribution-edit.pt	1970-01-01 00:00:00 +0000
@@ -1,23 +0,0 @@
-<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/default"
-  i18n:domain="launchpad"
->
-  <body>
-    <div metal:fill-slot="main">
-      <div metal:use-macro="context/@@launchpad_form/form">
-        <p metal:fill-slot="extra_info">
-          The branding images are used to throughout Launchpad to represent
-          <span tal:replace="context/displayname">Ubuntu Linux</span>.
-          Changing them here will affect every page related to
-          <span tal:replace="context/displayname">Ubuntu Linux</span> as well as
-          listings which include it. You must use <strong>exactly</strong> the
-          correct size of image.
-        </p>
-      </div>
-    </div>
-  </body>
-</html>

=== removed file 'lib/lp/registry/templates/distribution-memberteam.pt'
--- lib/lp/registry/templates/distribution-memberteam.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/distribution-memberteam.pt	1970-01-01 00:00:00 +0000
@@ -1,33 +0,0 @@
-<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"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-  <body>
-    <tal:tag condition="view/update"/>
-    <metal:heading fill-slot="pageheading">
-      <h1>Select distribution members team</h1>
-    </metal:heading>
-
-    <div metal:fill-slot="main">
-
-      <div metal:use-macro="context/@@launchpad_editform/editform">
-
-        <p metal:fill-slot="extra_info">
-          Select the Launchpad team that defines the members of
-          <span tal:replace="context/title">Ubuntu Linux</span>.
-        </p>
-        <tal:XXX condition="nothing">
-          # XXX 2006-02-20 mpt: That blurb is not useful, and should be replaced by
-          # an explanation of what the distribution members team is for.
-        </tal:XXX>
-      </div>
-    </div>
-  </body>
-</html>