Merge lp:~sinzui/launchpad/package-link-validation-3 into lp:launchpad

[Curtis Hovey]

[Curtis Hovey (sinzui)]

This is my fourth branch to ensure valid upstream package links. There
are many oopses relating to the creation and efforts to fix invalid packages.
The root cause is a bad DB constraint and two views that do not do the
required sanity checks: +addpackage and +ubuntupkg. The views are fixed,
but we have not provided users with a way to delete packaging links
to unbuilt packages. User need to do this to fix their mistakes. We want
to do this so that we can delete the duplicate packages.

This branch refactors portlets and forms so that my next branch can add
delete links to the product +packaing report.

    lp:~sinzui/launchpad/package-link-validation-3
    Diff size: 565
    Launchpad bug: https://bugs.launchpad.net/bugs/276409
    Test command: ./bin/test -vv -t 'lp.(reg|soyuz).*(productseries|packaging)'
    Pre-implementation: flacoste, beuno
    Target release: 3.1.10

== Fixing upstream packaging links ==

Bug 276409 ["Delete Link" is too easy on distribution source package page]
    On the page for a distribution source package with upstream links, such as
    <https://launchpad.net/ubuntu/+source/hwdb-client>, the button for
    deleting an upstream link is quite large and prominent, much more so than
    the button for correcting the link.

    A small improvement would be to change the "Delete Link" button to an
    icon-only button, but that would still leave deleting much easier than
    updating.

    There is little value in making the delete action an editing action
    because the user does not have enough information to select another
    series and sourcepackage. The user will almost always delete. Fixing the
    icon is all that is needed.

== Rules ==

Bug 276409 ["Delete Link" is too easy on distribution source package page]
    * Add a link from the productseries packaing portlet to the product
      +packages view.
    * Extract the delete packaging link rules to a separate view so that
      they can be reused
    * Change the Delete link button to a remove icon.

== QA ==

On staging
    * Visit a bzr series
    * Verify the packaging portlet has a link to All packages
    * Visit the bzr DSP
    * Verify eack packaging link has a the remove and edit icons int this
      order: (-) (/)

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/distributionsourcepackage.py
  lib/lp/registry/browser/packaging.py
  lib/lp/registry/browser/tests/test_packaging.py
  lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt
  lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt
  lib/lp/registry/templates/distributionsourcepackage-index.pt
  lib/lp/registry/templates/productseries-index.pt
  lib/lp/registry/templates/productseries-portlet-packages.pt
  lib/lp/registry/templates/sourcepackage-index.pt
  lib/lp/registry/templates/team-portlet-membership.pt

== Test ==

    * lib/lp/registry/browser/tests/test_packaging.py
      * Updated the test to use the submit's name since images do not have
        labels.
    * lib/lp/registry/stories/packaging/xx-distributionsourc...

[Curtis Hovey (sinzui)]

You can see the new remove icon at
http://people.canonical.com/~curtis/dsp-packaging.png

[Abel Deuring (adeuring)]

review: Approve (code)

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2009-10-22 15:22:42 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2009-10-24 20:01:11 +0000
@@ -383,6 +383,9 @@
 form table tbody th {
     font-weight: bold;
     }
+input[type='image'] {
+    vertical-align: middle;
+    }
 em {
     font-style: italic;
     }

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2009-10-21 01:55:17 +0000
+++ lib/lp/registry/browser/configure.zcml	2009-10-24 20:01:11 +0000
@@ -2001,9 +2001,6 @@
         facet="overview"
         permission="zope.Public">
         <browser:page
-            name="+portlet-upstream"
-            template="../templates/sourcepackage-portlet-upstream.pt"/>
-        <browser:page
             name="+portlet-releases"
             template="../templates/sourcepackage-portlet-releases.pt"/>
     </browser:pages>

=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py	2009-10-23 22:09:49 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py	2009-10-24 20:01:11 +0000
@@ -20,20 +20,16 @@
 import pytz
 
 from zope.component import getUtility
-from zope.formlib import form
 from zope.interface import implements, Interface
-from zope.schema import Choice
-from zope.schema.vocabulary import SimpleTerm, SimpleVocabulary
 
 from canonical.cachedproperty import cachedproperty
-from canonical.launchpad import _
 from canonical.launchpad.interfaces import IBugSet
 from lp.answers.interfaces.questionenums import QuestionStatus
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.interfaces.distributionsourcepackagerelease import (
     IDistributionSourcePackageRelease)
 from lp.soyuz.interfaces.packagediff import IPackageDiffSet
-from lp.registry.interfaces.packaging import IPackagingUtil
+from lp.registry.browser.packaging import PackagingDeleteView
 from lp.registry.interfaces.pocket import pocketsuffix
 from lp.registry.interfaces.product import IDistributionSourcePackage
 from lp.bugs.browser.bugtask import BugTargetTraversalMixin
@@ -42,7 +38,7 @@
 from canonical.launchpad.browser.structuralsubscription import (
     StructuralSubscriptionTargetTraversalMixin)
 from canonical.launchpad.webapp import (
-    ApplicationMenu, LaunchpadEditFormView, LaunchpadFormView, LaunchpadView,
+    ApplicationMenu, LaunchpadEditFormView, LaunchpadView,
     Link, Navigation, StandardLaunchpadFacets, action, canonical_url,
     redirection)
 from canonical.launchpad.webapp.menu import (
@@ -57,6 +53,7 @@
 
 class DistributionSourcePackageBreadcrumb(Breadcrumb):
     """Builds a breadcrumb for an `IDistributionSourcePackage`."""
+
     @property
     def text(self):
         return smartquote('"%s" package') % (
@@ -71,6 +68,7 @@
 
 
 class DistributionSourcePackageLinksMixin:
+
     def subscribe(self):
         return Link('+subscribe', 'Subscribe to bug mail', icon='edit')
 
@@ -184,6 +182,8 @@
     """Common features to all `DistributionSourcePackage` views."""
 
     def releases(self):
+        """All releases for this `IDistributionSourcePackage`."""
+
         def not_empty(text):
             return (
                 text is not None and isinstance(text, basestring)
@@ -215,11 +215,10 @@
             result_set = self.context.getPersonsByEmail(unique_emails)
             # Ignore the persons who want their email addresses hidden.
             self._person_data = dict(
-                [(email.email,person) for (email,person) in result_set
+                [(email.email, person) for (email, person) in result_set
                  if not person.hide_email_addresses])
         else:
             self._person_data = None
-
         # Collate diffs for relevant SourcePackageReleases
         pkg_diffs = getUtility(IPackageDiffSet).getDiffsToReleases(sprs)
         spr_diffs = {}
@@ -235,7 +234,7 @@
 
 
 class DistributionSourcePackageView(DistributionSourcePackageBaseView,
-                                    LaunchpadFormView):
+                                    PackagingDeleteView):
     """View class for DistributionSourcePackage."""
     implements(IDistributionSourcePackageActionMenu)
 
@@ -243,17 +242,18 @@
     def label(self):
         return self.context.title
 
-    def setUpFields(self):
+    @property
+    def next_url(self):
         """See `LaunchpadFormView`."""
-        # No schema is set in this form, because all fields are created with
-        # custom vocabularies. So we must not call the inherited setUpField
-        # method.
-        self.form_fields = self._createPackagingField()
+        return canonical_url(self.context)
 
     @property
-    def can_delete_packaging(self):
-        """Whether the user can delete existing packaging links."""
-        return self.user is not None
+    def all_packaging(self):
+        """See `PackagingDeleteView`."""
+        for sourcepackage in self.context.get_distroseries_packages():
+            packaging = sourcepackage.direct_packaging
+            if packaging is not None:
+                yield packaging
 
     @property
     def all_published_in_active_distroseries(self):
@@ -267,11 +267,11 @@
         for pub in self.context.current_publishing_records:
             if pub.distroseries.active:
                 entry = {
-                    "suite" : (pub.distroseries.name.capitalize() +
+                    "suite": (pub.distroseries.name.capitalize() +
                                pocketsuffix[pub.pocket]),
-                    "description" : "(%s): %s/%s" % (
+                    "description": "(%s): %s/%s" % (
                         pub.sourcepackagerelease.version,
-                        pub.component.name, pub.section.name)
+                        pub.component.name, pub.section.name),
                     }
                 results.append(entry)
         return results
@@ -331,12 +331,12 @@
                 versions.append(
                     "%s (%s)" % (
                         pub.distroseries.displayname,
-                        pub.source_package_version
+                        pub.source_package_version,
                         )
                     )
             archive_versions.append({
                 'archive': archive,
-                'versions': ", ".join(versions)
+                'versions': ", ".join(versions),
                 })
 
         return archive_versions
@@ -350,68 +350,6 @@
             self.context.name,
             )
 
-    def _createPackagingField(self):
-        """Create a field to specify a Packaging association.
-
-        Create a contextual vocabulary that can specify one of the Packaging
-        associated to this DistributionSourcePackage.
-        """
-        terms = []
-        for sourcepackage in self.context.get_distroseries_packages():
-            packaging = sourcepackage.direct_packaging
-            if packaging is None:
-                continue
-            terms.append(SimpleTerm(packaging, packaging.id))
-        return form.Fields(
-            Choice(__name__='packaging', vocabulary=SimpleVocabulary(terms),
-                   required=True))
-
-    def _renderHiddenPackagingField(self, packaging):
-        """Render a hidden input that fills in the packaging field."""
-        if not self.can_delete_packaging:
-            return None
-        vocabulary = self.form_fields['packaging'].field.vocabulary
-        return '<input type="hidden" name="field.packaging" value="%s" />' % (
-            vocabulary.getTerm(packaging).token)
-
-    def renderDeletePackagingAction(self):
-        """Render a submit input for the delete_packaging_action."""
-        assert self.can_delete_packaging, 'User cannot delete Packaging.'
-        return ('<input type="submit" class="button" value="Delete Link" '
-                'style="padding: 0pt; font-size: 80%%" '
-                'name="%s"/>' % (self.delete_packaging_action.__name__,))
-
-    def handleDeletePackagingError(self, action, data, errors):
-        """Handle errors on package link deletion.
-
-        If 'packaging' is not set in the form data, we assume that means the
-        provided Packaging id was not found, which should only happen if the
-        same Packaging object was concurrently deleted. In this case, we want
-        to display a more informative error message than the default 'Invalid
-        value'.
-        """
-        if data.get('packaging') is None:
-            self.setFieldError(
-                'packaging',
-                _("This upstream association was deleted already."))
-
-    @action(_("Delete Link"), name='delete_packaging',
-            failure=handleDeletePackagingError)
-    def delete_packaging_action(self, action, data):
-        """Delete a Packaging association."""
-        packaging = data['packaging']
-        productseries = packaging.productseries
-        distroseries = packaging.distroseries
-        getUtility(IPackagingUtil).deletePackaging(
-            productseries, packaging.sourcepackagename, distroseries)
-        self.request.response.addNotification(
-            _("Removed upstream association between ${product} "
-              "${productseries} and ${distroseries}.", mapping=dict(
-              product=productseries.product.displayname,
-              productseries=productseries.displayname,
-              distroseries=distroseries.displayname)))
-        self.next_url = canonical_url(self.context)
-
     @cachedproperty
     def active_distroseries_packages(self):
         """Cached proxy call to context/get_distroseries_packages."""

=== modified file 'lib/lp/registry/browser/packaging.py'
--- lib/lp/registry/browser/packaging.py	2009-10-21 21:16:09 +0000
+++ lib/lp/registry/browser/packaging.py	2009-10-24 20:01:11 +0000
@@ -5,9 +5,13 @@
 
 __all__ = [
     'PackagingAddView',
+    'PackagingDeleteView',
     ]
 
 from zope.component import getUtility
+from zope.formlib import form
+from zope.schema import Choice
+from zope.schema.vocabulary import SimpleTerm, SimpleVocabulary
 
 from canonical.launchpad import _
 from lp.registry.interfaces.packaging import (
@@ -77,3 +81,82 @@
         getUtility(IPackagingUtil).createPackaging(
             productseries, data['sourcepackagename'], data['distroseries'],
             data['packaging'], owner=self.user)
+
+
+class PackagingDeleteView(LaunchpadFormView):
+    """A base view that provides packaging link deletion."""
+
+    @property
+    def all_packaging(self):
+        """An iterator of the context's packaging links."""
+        raise NotImplementedError
+
+    def setUpFields(self):
+        """See `LaunchpadFormView`."""
+        # No schema is set in this form, because all fields are created with
+        # custom vocabularies. So we must not call the inherited setUpField
+        # method.
+        self.form_fields = self._createPackagingField()
+
+    @property
+    def can_delete_packaging(self):
+        """Whether the user can delete existing packaging links."""
+        return self.user is not None
+
+    def _createPackagingField(self):
+        """Create a field to specify a Packaging association.
+
+        Create a contextual vocabulary that can specify one of the Packaging
+        associated to this DistributionSourcePackage.
+        """
+        terms = []
+        for packaging in self.all_packaging:
+            terms.append(SimpleTerm(packaging, packaging.id))
+        return form.Fields(
+            Choice(__name__='packaging', vocabulary=SimpleVocabulary(terms),
+                   required=True))
+
+    def _renderHiddenPackagingField(self, packaging):
+        """Render a hidden input that fills in the packaging field."""
+        if not self.can_delete_packaging:
+            return None
+        vocabulary = self.form_fields['packaging'].field.vocabulary
+        return '<input type="hidden" name="field.packaging" value="%s" />' % (
+            vocabulary.getTerm(packaging).token)
+
+    def renderDeletePackagingAction(self):
+        """Render a submit input for the delete_packaging_action."""
+        assert self.can_delete_packaging, 'User cannot delete Packaging.'
+        return ('<input type="image" value="Delete Link" '
+                'src="/@@/remove" title="Delete upsteam link" '
+                'name="%s"/>' % self.delete_packaging_action.__name__)
+
+    def handleDeletePackagingError(self, action, data, errors):
+        """Handle errors on package link deletion.
+
+        If 'packaging' is not set in the form data, we assume that means the
+        provided Packaging id was not found, which should only happen if the
+        same Packaging object was concurrently deleted. In this case, we want
+        to display a more informative error message than the default 'Invalid
+        value'.
+        """
+        if data.get('packaging') is None:
+            self.setFieldError(
+                'packaging',
+                _("This upstream association was deleted already."))
+
+    @action(_("Delete Link"), name='delete_packaging',
+            failure=handleDeletePackagingError)
+    def delete_packaging_action(self, action, data):
+        """Delete a Packaging association."""
+        packaging = data['packaging']
+        productseries = packaging.productseries
+        distroseries = packaging.distroseries
+        getUtility(IPackagingUtil).deletePackaging(
+            productseries, packaging.sourcepackagename, distroseries)
+        self.request.response.addNotification(
+            _("Removed upstream association between ${product} "
+              "${productseries} and ${distroseries}.", mapping=dict(
+              product=productseries.product.displayname,
+              productseries=productseries.displayname,
+              distroseries=distroseries.displayname)))

=== added file 'lib/lp/registry/browser/tests/packaging-views.txt'
--- lib/lp/registry/browser/tests/packaging-views.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/browser/tests/packaging-views.txt	2009-10-24 20:01:11 +0000
@@ -0,0 +1,237 @@
+Packaging views
+===============
+
+Packaging links connect a sourcepackage to a distroseries and a productseries.
+
+
+Productseries linking packages
+------------------------------
+
+Distro series sourcepackages can be linked to product series using the
++addpackage named view.
+
+    >>> from canonical.launchpad.interfaces.launchpad import (
+    ...     ILaunchpadCelebrities)
+
+    >>> ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+    >>> hoary = ubuntu.getSeries('hoary')
+    >>> sourcepackagename = factory.makeSourcePackageName('hot')
+    >>> sourcepackage = factory.makeSourcePackage(
+    ...     sourcepackagename=sourcepackagename, distroseries=hoary)
+    >>> product = factory.makeProduct(name="hot", displayname='Hot')
+    >>> productseries = factory.makeProductSeries(
+    ...     product=product, name='hotter')
+    >>> productseries.sourcepackages
+    []
+
+The view has a label and requires a distro series, source package name,
+and a packaging contents.
+
+    >>> view = create_view(productseries, '+addpackage')
+    >>> print view.label
+    Packaging of hotter in distributions
+
+    >>> print view.page_title
+    Packaging of hotter in distributions
+
+    >>> print view.field_names
+    ['distroseries', 'sourcepackagename', 'packaging']
+
+    >>> print view.cancel_url
+    http://launchpad.dev/hot/hotter
+
+    >>> form = {
+    ...     'field.distroseries': 'ubuntu/hoary',
+    ...     'field.sourcepackagename': 'hot',
+    ...     'field.packaging': 'Primary Product',
+    ...     'field.actions.continue': 'Continue',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+addpackage', form=form)
+    >>> view.errors
+    []
+    >>> for package in productseries.sourcepackages:
+    ...     print package.name
+    hot
+
+    >>> transaction.commit()
+
+It is an error to link a series to the same package and distro series twice.
+
+    >>> form = {
+    ...     'field.distroseries': 'ubuntu/hoary',
+    ...     'field.sourcepackagename': 'hot',
+    ...     'field.packaging': 'Primary Product',
+    ...     'field.actions.continue': 'Continue',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+addpackage', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    This series is already packaged in Hoary.
+
+Once a distro series sourcepackage is linked to a product series, no other
+product series can link to it.
+
+    >>> other_productseries = factory.makeProductSeries(
+    ...     product=product, name='hotest')
+    >>> form = {
+    ...     'field.distroseries': 'ubuntu/hoary',
+    ...     'field.sourcepackagename': 'hot',
+    ...     'field.packaging': 'Primary Product',
+    ...     'field.actions.continue': 'Continue',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     other_productseries, '+addpackage', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    The <a href=".../hoary/+source/hot">hot</a> package in Hoary is already
+    linked to another series.
+
+A source package name must be provided.
+
+    >>> form = {
+    ...     'field.distroseries': 'ubuntu/hoary',
+    ...     'field.sourcepackagename': '',
+    ...     'field.packaging': 'Primary Product',
+    ...     'field.actions.continue': 'Continue',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+addpackage', form=form)
+    >>> for error in view.errors:
+    ...     print error
+    ('sourcepackagename', u'Source Package Name', RequiredMissing())
+    You must choose the source package name.
+
+The +addpackage view provides the default_distroseries property. It is None
+by default, but subclasses may change it.
+
+    >>> print view.default_distroseries
+    None
+
+
+Productseries linking Ubuntu packages
+-------------------------------------
+
+The +ubuntupkg named view is a subclass of the +addpackage named view. It
+allows the user to update the current linked Ubuntu package.
+
+    >>> from lp.registry.browser.packaging import PackagingAddView
+
+    >>> view = create_initialized_view(productseries, '+ubuntupkg')
+    >>> isinstance(view, PackagingAddView)
+    True
+
+    >>> print view.label
+    Ubuntu source packaging
+
+    >>> print view.page_title
+    Ubuntu source packaging
+
+    >>> print view.field_names
+    ['sourcepackagename']
+
+    >>> print view.cancel_url
+    http://launchpad.dev/hot/hotter
+
+The view restricts the packaging to the current Ubuntu series.
+
+    >>> print view.default_distroseries.name
+    hoary
+
+The sourcepackagename is None if the package link was never set. The view's
+packaging history is empty, and the sourcepackagename widget is empty.
+
+    >>> new_productseries = factory.makeProductSeries(
+    ...     product=product, name='cold')
+    >>> view = create_initialized_view(new_productseries, '+ubuntupkg')
+
+    >>> print view.default_sourcepackagename
+    None
+
+    >>> print view.widgets.get('sourcepackagename')._getFormValue()
+    <BLANKLINE>
+
+    >>> print view.ubuntu_history
+    []
+
+Series have been packaged in Ubuntu do have the current information and
+a history.
+
+    >>> view = create_initialized_view(productseries, '+ubuntupkg')
+    >>> print view.default_sourcepackagename.name
+    hot
+
+    >>> print view.widgets.get('sourcepackagename')._getFormValue().name
+    hot
+
+    >>> for packaging in view.ubuntu_history:
+    ...     print packaging.distroseries.name
+    ...     print packaging.sourcepackagename.name
+    hoary hot
+
+The package in the current Ubuntu series can be updated.
+
+    >>> form = {
+    ...     'field.sourcepackagename': 'thunderbird',
+    ...     'field.actions.continue': 'Update',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+ubuntupkg', form=form)
+    >>> view.errors
+    []
+
+    >>> for packaging in view.ubuntu_history:
+    ...     print packaging.distroseries.name
+    ...     print packaging.sourcepackagename.name
+    hoary thunderbird
+
+It is not an error to submit the same sourcepackagename information, the
+action is ignored because there is no change
+
+    >>> form = {
+    ...     'field.sourcepackagename': 'thunderbird',
+    ...     'field.actions.continue': 'Update',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+ubuntupkg', form=form)
+    >>> view.errors
+    []
+
+    >>> for packaging in view.ubuntu_history:
+    ...     print packaging.distroseries.name
+    ...     print packaging.sourcepackagename.name
+    hoary thunderbird
+
+When the current Ubuntu series changes, the sourcepackagename is not known,
+and a new entry can be added to the packaging history.
+
+    >>> from lp.registry.interfaces.distroseries import DistroSeriesStatus
+
+    >>> login('[e-mail]')
+    >>> hoary.status = DistroSeriesStatus.CURRENT
+    >>> grumpy_series = ubuntu.getSeries('grumpy')
+    >>> grumpy_series.status = DistroSeriesStatus.FROZEN
+
+    >>> a_user = factory.makePerson(name="hedgehog")
+    >>> login_person(a_user)
+    >>> form = {
+    ...     'field.sourcepackagename': 'hot',
+    ...     'field.actions.continue': 'Update',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     productseries, '+ubuntupkg', form=form)
+    >>> view.errors
+    []
+
+    >>> print view.default_distroseries.name
+    grumpy
+
+    >>> print view.default_sourcepackagename
+    None
+
+    >>> for packaging in view.ubuntu_history:
+    ...     print packaging.distroseries.name
+    ...     print packaging.sourcepackagename.name
+    grumpy hot
+    hoary thunderbird

=== modified file 'lib/lp/registry/browser/tests/productseries-views.txt'
--- lib/lp/registry/browser/tests/productseries-views.txt	2009-10-21 17:09:39 +0000
+++ lib/lp/registry/browser/tests/productseries-views.txt	2009-10-24 20:01:11 +0000
@@ -323,232 +323,3 @@
     True
     >>> print productseries.name
     field-rabbit-20090501-193424
-
-
-Linking packages
-----------------
-
-Distro series sourcepackages can be linked to product series using the
-+addpackage named view.
-
-    >>> ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-    >>> hoary = ubuntu.getSeries('hoary')
-    >>> sourcepackagename = factory.makeSourcePackageName('hot')
-    >>> sourcepackage = factory.makeSourcePackage(
-    ...     sourcepackagename=sourcepackagename, distroseries=hoary)
-    >>> product = factory.makeProduct(name="hot", displayname='Hot')
-    >>> productseries = factory.makeProductSeries(
-    ...     product=product, name='hotter')
-    >>> productseries.sourcepackages
-    []
-
-The view has a label and requires a distro series, source package name,
-and a packaging contents.
-
-    >>> view = create_view(productseries, '+addpackage')
-    >>> print view.label
-    Packaging of hotter in distributions
-
-    >>> print view.page_title
-    Packaging of hotter in distributions
-
-    >>> print view.field_names
-    ['distroseries', 'sourcepackagename', 'packaging']
-
-    >>> print view.cancel_url
-    http://launchpad.dev/hot/hotter
-
-    >>> form = {
-    ...     'field.distroseries': 'ubuntu/hoary',
-    ...     'field.sourcepackagename': 'hot',
-    ...     'field.packaging': 'Primary Product',
-    ...     'field.actions.continue': 'Continue',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+addpackage', form=form)
-    >>> view.errors
-    []
-    >>> for package in productseries.sourcepackages:
-    ...     print package.name
-    hot
-
-    >>> transaction.commit()
-
-It is an error to link a series to the same package and distro series twice.
-
-    >>> form = {
-    ...     'field.distroseries': 'ubuntu/hoary',
-    ...     'field.sourcepackagename': 'hot',
-    ...     'field.packaging': 'Primary Product',
-    ...     'field.actions.continue': 'Continue',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+addpackage', form=form)
-    >>> for error in view.errors:
-    ...     print error
-    This series is already packaged in Hoary.
-
-Once a distro series sourcepackage is linked to a product series, no other
-product series can link to it.
-
-    >>> other_productseries = factory.makeProductSeries(
-    ...     product=product, name='hotest')
-    >>> form = {
-    ...     'field.distroseries': 'ubuntu/hoary',
-    ...     'field.sourcepackagename': 'hot',
-    ...     'field.packaging': 'Primary Product',
-    ...     'field.actions.continue': 'Continue',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     other_productseries, '+addpackage', form=form)
-    >>> for error in view.errors:
-    ...     print error
-    The <a href=".../hoary/+source/hot">hot</a> package in Hoary is already
-    linked to another series.
-
-A source package name must be provided.
-
-    >>> form = {
-    ...     'field.distroseries': 'ubuntu/hoary',
-    ...     'field.sourcepackagename': '',
-    ...     'field.packaging': 'Primary Product',
-    ...     'field.actions.continue': 'Continue',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+addpackage', form=form)
-    >>> for error in view.errors:
-    ...     print error
-    ('sourcepackagename', u'Source Package Name', RequiredMissing())
-    You must choose the source package name.
-
-The +addpackage view provides the default_distroseries property. It is None
-by default, but subclasses may change it.
-
-    >>> print view.default_distroseries
-    None
-
-
-Linking Ubuntu packages
------------------------
-
-The +ubuntupkg named view is a subclass of the +addpackage named view. It
-allows the user to update the current linked Ubuntu package.
-
-    >>> from lp.registry.browser.packaging import PackagingAddView
-
-    >>> view = create_initialized_view(productseries, '+ubuntupkg')
-    >>> isinstance(view, PackagingAddView)
-    True
-
-    >>> print view.label
-    Ubuntu source packaging
-
-    >>> print view.page_title
-    Ubuntu source packaging
-
-    >>> print view.field_names
-    ['sourcepackagename']
-
-    >>> print view.cancel_url
-    http://launchpad.dev/hot/hotter
-
-The view restricts the packaging to the current Ubuntu series.
-
-    >>> print view.default_distroseries.name
-    hoary
-
-The sourcepackagename is None if the package link was never set. The view's
-packaging history is empty, and the sourcepackagename widget is empty.
-
-    >>> new_productseries = factory.makeProductSeries(
-    ...     product=product, name='cold')
-    >>> view = create_initialized_view(new_productseries, '+ubuntupkg')
-
-    >>> print view.default_sourcepackagename
-    None
-
-    >>> print view.widgets.get('sourcepackagename')._getFormValue()
-    <BLANKLINE>
-
-    >>> print view.ubuntu_history
-    []
-
-Series have been packaged in Ubuntu do have the current information and
-a history.
-
-    >>> view = create_initialized_view(productseries, '+ubuntupkg')
-    >>> print view.default_sourcepackagename.name
-    hot
-
-    >>> print view.widgets.get('sourcepackagename')._getFormValue().name
-    hot
-
-    >>> for packaging in view.ubuntu_history:
-    ...     print packaging.distroseries.name
-    ...     print packaging.sourcepackagename.name
-    hoary hot
-
-The package in the current Ubuntu series can be updated.
-
-    >>> form = {
-    ...     'field.sourcepackagename': 'thunderbird',
-    ...     'field.actions.continue': 'Update',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+ubuntupkg', form=form)
-    >>> view.errors
-    []
-
-    >>> for packaging in view.ubuntu_history:
-    ...     print packaging.distroseries.name
-    ...     print packaging.sourcepackagename.name
-    hoary thunderbird
-
-It is not an error to submit the same sourcepackagename information, the
-action is ignored because there is no change
-
-    >>> form = {
-    ...     'field.sourcepackagename': 'thunderbird',
-    ...     'field.actions.continue': 'Update',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+ubuntupkg', form=form)
-    >>> view.errors
-    []
-
-    >>> for packaging in view.ubuntu_history:
-    ...     print packaging.distroseries.name
-    ...     print packaging.sourcepackagename.name
-    hoary thunderbird
-
-When the current Ubuntu series changes, the sourcepackagename is not known,
-and a new entry can be added to the packaging history.
-
-    >>> from lp.registry.interfaces.distroseries import DistroSeriesStatus
-
-    >>> login('[e-mail]')
-    >>> hoary.status = DistroSeriesStatus.CURRENT
-    >>> grumpy_series = ubuntu.getSeries('grumpy')
-    >>> grumpy_series.status = DistroSeriesStatus.FROZEN
-
-    >>> login_person(a_user)
-    >>> form = {
-    ...     'field.sourcepackagename': 'hot',
-    ...     'field.actions.continue': 'Update',
-    ...     }
-    >>> view = create_initialized_view(
-    ...     productseries, '+ubuntupkg', form=form)
-    >>> view.errors
-    []
-
-    >>> print view.default_distroseries.name
-    grumpy
-
-    >>> print view.default_sourcepackagename
-    None
-
-    >>> for packaging in view.ubuntu_history:
-    ...     print packaging.distroseries.name
-    ...     print packaging.sourcepackagename.name
-    grumpy hot
-    hoary thunderbird

=== modified file 'lib/lp/registry/browser/tests/test_packaging.py'
--- lib/lp/registry/browser/tests/test_packaging.py	2009-10-16 15:00:55 +0000
+++ lib/lp/registry/browser/tests/test_packaging.py	2009-10-24 20:01:11 +0000
@@ -56,7 +56,7 @@
         user_browser = self.user_browser
         user_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
         form = user_browser.getForm("delete_warty_alsa-utils_trunk")
-        form.getControl("Delete Link").click()
+        form.getControl(name="field.actions.delete_packaging").click()
         # Check that the change was committed.
         login('[e-mail]')
         self.assertFalse(packaging_util.packagingEntryExists(

=== renamed file 'lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging-concurrent-deletion.txt' => 'lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt'
--- lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging-concurrent-deletion.txt	2009-08-27 20:08:53 +0000
+++ lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging-concurrent-deletion.txt	2009-10-24 20:01:11 +0000
@@ -1,4 +1,5 @@
-=== Concurrent Deletion of Packaging ===
+Concurrent Deletion of Packaging
+================================
 
 When two browsers are used to concurrently delete the same packaging
 association, only one of them can succeed. The other one does not oops
@@ -16,7 +17,7 @@
 deletion succeeds and the usual informational message is displayed.
 
     >>> form = first_browser.getForm("delete_warty_alsa-utils_trunk")
-    >>> form.getControl("Delete Link").click()
+    >>> form.getControl(name="field.actions.delete_packaging").click()
     >>> content = first_browser.contents
     >>> for tag in find_tags_by_class(content, 'error'):
     ...     print extract_text(tag)
@@ -37,7 +38,7 @@
 -- David Allouche 2007-12-07
 
     >>> form = second_browser.getForm("delete_warty_alsa-utils_trunk")
-    >>> form.getControl("Delete Link").click()
+    >>> form.getControl(name="field.actions.delete_packaging").click()
     >>> content = second_browser.contents
     >>> for tag in find_tags_by_class(content, 'informational'):
     ...     print extract_text(tag)

=== renamed file 'lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt' => 'lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt'
--- lib/lp/registry/stories/distribution/xx-distributionsourcepackage-packaging.txt	2009-10-21 01:55:17 +0000
+++ lib/lp/registry/stories/packaging/xx-distributionsourcepackage-packaging.txt	2009-10-24 20:01:11 +0000
@@ -1,4 +1,5 @@
-== Distribution Packaging ==
+Distribution Packaging
+======================
 
 The packaging records for a source package in a given distribution are
 displayed on the page of the distribution source package.
@@ -18,15 +19,16 @@
       1.0.9a-4        release (main) ... weeks ago
 
 
-=== Delete Link Button ===
+Delete Link Button
+------------------
 
 A button is displayed to authenticated users to delete existing
 packaging links.
 
     >>> user_browser.open('http://launchpad.dev/ubuntu/+source/alsa-utils')
     >>> form = user_browser.getForm("delete_warty_alsa-utils_trunk")
-    >>> print form.getControl("Delete Link")
-    <SubmitControl name='field.actions.delete_packaging' type='submit'>
+    >>> print form.getControl(name="field.actions.delete_packaging")
+    <ImageControl name='field.actions.delete_packaging' type='image'>
 
 This button is not displayed to anonymous users.
 
@@ -39,7 +41,7 @@
 Clicking this button deletes the corresponding packaging association.
 
     >>> form = user_browser.getForm("delete_warty_alsa-utils_trunk")
-    >>> form.getControl("Delete Link").click()
+    >>> form.getControl(name="field.actions.delete_packaging").click()
     >>> content = user_browser.contents
     >>> for tag in find_tags_by_class(content, 'error'):
     ...     print extract_text(tag)

=== modified file 'lib/lp/registry/templates/distributionsourcepackage-index.pt'
--- lib/lp/registry/templates/distributionsourcepackage-index.pt	2009-09-25 17:00:20 +0000
+++ lib/lp/registry/templates/distributionsourcepackage-index.pt	2009-10-24 20:01:11 +0000
@@ -149,10 +149,6 @@
             <tal:has_packaging condition="row/packaging">
               <img tal:replace="structure row/packaging/productseries/image:icon"/>
               <a tal:replace="structure row/packaging/productseries/fmt:link"/>
-              <a tal:attributes="
-                href row/series_package/menu:overview/edit_packaging/url">
-                <img src="/@@/edit"/>
-              </a>
               <form style="display: inline"
                     method="POST"
                     tal:condition="row/hidden_packaging_field"
@@ -161,6 +157,11 @@
                 <tal:hidden replace="structure row/hidden_packaging_field" />
                 <tal:action replace="structure view/renderDeletePackagingAction" />
               </form>
+              <a tal:attributes="
+                href row/series_package/menu:overview/edit_packaging/url;
+                title string:Edit upsteam link">
+                <img src="/@@/edit"/>
+              </a>
             </tal:has_packaging>
           </div><!--float right-->
         </td>
@@ -215,7 +216,7 @@
       </tal:rows>
     </table>
     <script
-       tal:content="string:LP.client.cache['archive_context_url'] = '${archive/fmt:url}';"/>
+       tal:content="string:LP.client.cache['archive_context_url'] = '${archive/fmt:url}';"></script>
     <metal:js use-macro="archive/@@+macros/expandable-table-js"/>
 
     <p style="float:right; padding-top:1em">

=== modified file 'lib/lp/registry/templates/productseries-index.pt'
--- lib/lp/registry/templates/productseries-index.pt	2009-10-20 16:13:57 +0000
+++ lib/lp/registry/templates/productseries-index.pt	2009-10-24 20:01:11 +0000
@@ -24,8 +24,7 @@
     </tal:heading>
 
     <div metal:fill-slot="main"
-      tal:define="overview_menu context/menu:overview;
-                  sourcepackages context/sourcepackages">
+      tal:define="overview_menu context/menu:overview">
 
       <div class="top-portlet">
         <div id="description"
@@ -144,41 +143,7 @@
             tal:content="structure context/@@+portlet-latestbugs"
             tal:condition="context/@@+get-involved/official_malone" />
 
-          <div class="portlet">
-            <h2>Distribution packaging</h2>
-
-            <p id="distribution-packaging-explanation">
-              <tal:distro condition="sourcepackages">
-                This series is packaged in the following distribution series:
-              </tal:distro>
-              <tal:no-distro condition="not: sourcepackages">
-                This series is not packaged in any distribution series.
-              </tal:no-distro>
-            </p>
-
-            <ul id="distribution-packaging"
-              tal:condition="sourcepackages">
-              <li
-                tal:repeat="package sourcepackages">
-                <a class="sprite package-source"
-                  tal:attributes="href package/fmt:url"><tal:distro
-                  tal:content="package/distroseries/distribution/displayname">
-                  Ubuntu</tal:distro>
-                  <tal:series tal:replace="package/distroseries/displayname">
-                  Warty</tal:series>
-                  <tal:package tal:content="package/name">firefox</tal:package></a>
-              </li>
-            </ul>
-
-            <ul class="horizontal">
-              <li>
-                <a tal:replace="structure overview_menu/ubuntupkg/fmt:icon-link" />
-              </li>
-              <li>
-                <a tal:replace="structure overview_menu/add_package/fmt:icon-link" />
-              </li>
-            </ul>
-          </div>
+          <div tal:content="structure context/@@+portlet-packages" />
         </div>
 
         <div class="yui-u">

=== modified file 'lib/lp/registry/templates/productseries-portlet-packages.pt'
--- lib/lp/registry/templates/productseries-portlet-packages.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/productseries-portlet-packages.pt	2009-10-24 20:01:11 +0000
@@ -1,25 +1,46 @@
-<tal:root
+<div class="portlet" id="portlet-packages"
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  omit-tag="">
-
-<div class="portlet" id="portlet-packages"
-     tal:condition="context/sourcepackages">
-  <h2>Distribution packaging</h2>
-
-  <div class="portletBody portletContent ">
-    This series is packaged in the following places:
-    <ul>
-      <li tal:repeat="package context/sourcepackages">
-         <span tal:replace="package/distroseries/distribution/name"
-         />
-         <span tal:replace="package/distroseries/name"
-         />
-         <a tal:content="package/name"
-            tal:attributes="href package/fmt:url">apache</a>
-      </li>
-    </ul>
-  </div>
+  tal:define="sourcepackages context/sourcepackages">
+  <h2>
+    <span class="see-all">
+      <a tal:attributes="href context/product/menu:overview/packages/fmt:url">
+        All packages
+      </a>
+    </span>
+    Distribution packaging
+  </h2>
+
+  <p id="distribution-packaging-explanation">
+    <tal:distro condition="sourcepackages">
+      This series is packaged in the following distribution series:
+    </tal:distro>
+    <tal:no-distro condition="not: sourcepackages">
+      This series is not packaged in any distribution series.
+    </tal:no-distro>
+  </p>
+
+  <ul id="distribution-packaging"
+    tal:condition="sourcepackages">
+    <li
+      tal:repeat="package sourcepackages">
+      <a class="sprite package-source"
+        tal:attributes="href package/fmt:url"><tal:distro
+        tal:content="package/distroseries/distribution/displayname">
+        Ubuntu</tal:distro>
+        <tal:series tal:replace="package/distroseries/displayname">
+        Warty</tal:series>
+        <tal:package tal:content="package/name">firefox</tal:package></a>
+    </li>
+  </ul>
+
+  <ul class="horizontal">
+    <li>
+      <a tal:replace="structure context/menu:overview/ubuntupkg/fmt:icon-link" />
+    </li>
+    <li>
+      <a tal:replace="structure context/menu:overview/add_package/fmt:icon-link" />
+    </li>
+  </ul>
 </div>
-</tal:root>

=== modified file 'lib/lp/registry/templates/sourcepackage-index.pt'
--- lib/lp/registry/templates/sourcepackage-index.pt	2009-09-25 17:00:20 +0000
+++ lib/lp/registry/templates/sourcepackage-index.pt	2009-10-24 20:01:11 +0000
@@ -114,9 +114,7 @@
         </div>
 
         <div class="yui-u">
-          <div tal:replace="structure context/@@+portlet-upstream" />
-
-          <div tal:condition="current">
+          <div class="portlet" tal:condition="current">
             <h2>Binary packages</h2>
 
             <div id="binaries" tal:define="binaries view/binaries">

=== removed file 'lib/lp/registry/templates/sourcepackage-portlet-upstream.pt'
--- lib/lp/registry/templates/sourcepackage-portlet-upstream.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/registry/templates/sourcepackage-portlet-upstream.pt	1970-01-01 00:00:00 +0000
@@ -1,17 +0,0 @@
-<tal:root
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  omit-tag="">
-
-<div class="portlet" id="portlet-upstream">
-  <h2>Upstream release series</h2>
-  <div class="portletBody portletContent">
-
-  <a tal:condition="context/productseries"
-     tal:content="context/productseries/title"
-     tal:attributes="href context/productseries/fmt:url">apache head</a>
-     <i tal:condition="not: context/productseries">None linked.</i>
-  </div>
-</div>
-</tal:root>