Merge lp:~edwin-grubbs/launchpad/bug-667900-dsp-page-upstream-link-form into lp:launchpad

[Edwin Grubbs]

[r=jcsackett,sinzui][ui=sinzui][bug=667900] Add form to DistributionSourcePackage +index page to set upstream project link more easily.

Summary
-------

Display form on DistributionSourcePackage page if the latest
sourcepackage does not have a link to an upstream project.

An upstream link isn't really needed for sourcepackages that are not
current or in development, so don't display the "Set upstream link" for
those older sourcepackages on this page.

Tests
-----

./bin/test -vv -t distributionsourcepackage-views.txt

Demo and Q/A
------------

* Open http://launchpad.dev/ubuntu/+source/alsa-utils
    * If you remove the upstream link from the latest sourcepackage, a
      form should be displayed.

[j.c.sackett (jcsackett)]

> Edwin--
>
> This is a really nicely done branch below. I have some comments, but they're mostly for my own edification, not issues with the code.
>
> === modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
> --- lib/lp/registry/browser/distributionsourcepackage.py 2010-11-06 06:52:31 +0000
> +++ lib/lp/registry/browser/distributionsourcepackage.py 2010-11-17 00:32:27 +0000
> @@ -31,7 +31,6 @@
> Interface,
> )
>
> -from canonical.launchpad.webapp.interfaces import IBreadcrumb
> from canonical.launchpad.helpers import shortlist
> from canonical.launchpad.webapp import (
> action,
> @@ -43,6 +42,7 @@
> StandardLaunchpadFacets,
> )
> from canonical.launchpad.webapp.breadcrumb import Breadcrumb
> +from canonical.launchpad.webapp.interfaces import IBreadcrumb
> from canonical.launchpad.webapp.menu import (
> ApplicationMenu,
> enabled_with_permission,
> @@ -50,14 +50,14 @@
> NavigationMenu,
> )
> from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
> -from lp.app.interfaces.launchpad import IServiceUsage
> -from lp.app.browser.tales import CustomizableFormatter
> from canonical.lazr.utils import smartquote
> from lp.answers.browser.questiontarget import (
> QuestionTargetFacetMixin,
> QuestionTargetTraversalMixin,
> )
> from lp.answers.interfaces.questionenums import QuestionStatus
> +from lp.app.browser.tales import CustomizableFormatter
> +from lp.app.interfaces.launchpad import IServiceUsage
> from lp.bugs.browser.bugtask import BugTargetTraversalMixin
> from lp.bugs.interfaces.bug import IBugSet
> from lp.registry.browser.pillar import PillarBugsMenu
> @@ -68,6 +68,7 @@
> IDistributionSourcePackage,
> )
> from lp.registry.interfaces.pocket import pocketsuffix
> +from lp.registry.interfaces.series import SeriesStatus
> from lp.services.propertycache import cachedproperty
> from lp.soyuz.browser.sourcepackagerelease import (
> extract_bug_numbers,

Thanks for cleaning up those imports.

> @@ -409,7 +410,7 @@
> packages_dict[package.distroseries] = package
> return packages_dict
>
> - @property
> + @cachedproperty
> def active_series(self):
> """Return active distroseries where this package is published.
>
> I have to confess to a lack of understanding on cachedproperty--I assume this doesn't change often enough for caching to present a problem? I can see where not caching could produce performance issues, particularly with the increased calls to this method in your other changes. This isn't a problem with the code, just me asking for my own edification.
>
>
> @@ -438,6 +439,13 @@
> return pocket_dict
>
> @property
> + def latest_sourcepackage(self):
> + if len(self.active_series) == 0:
> + return None
> + return self.active_series[0].getSourcePackage(
> + self.context.sourcepackagename)
> +
> + @property
> def version_table(self):
> """Rows of data for the template to render in the packaging table."""
> rows = []
> @@ -446,6 +454,16 @@
> # The first row for each series is the "title" row.
> packaging = packages_by_series[distroseries]....

review: Approve (code*)

[Curtis Hovey (sinzui)]

One of the opportunities we have at the moment is giving users a consistent experience. We will have to reopen a bug if we drop the logo and summary. Why are they missing from the SP page? We should be showing the icon and summary in the portlet. We may not need the logo since we show the icon.

What to do think Edwin? logo or icon?

review: Needs Information (code + ui)

[Curtis Hovey (sinzui)]

Thank you for adding the project summary to the portlet. I like the consistency we have on these pages.

review: Approve (code + ui)

=== modified file 'lib/lp/registry/browser/distributionsourcepackage.py'
--- lib/lp/registry/browser/distributionsourcepackage.py	2010-11-06 06:52:31 +0000
+++ lib/lp/registry/browser/distributionsourcepackage.py	2010-11-19 15:06:35 +0000
@@ -31,7 +31,6 @@
     Interface,
     )
 
-from canonical.launchpad.webapp.interfaces import IBreadcrumb
 from canonical.launchpad.helpers import shortlist
 from canonical.launchpad.webapp import (
     action,
@@ -43,6 +42,7 @@
     StandardLaunchpadFacets,
     )
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
+from canonical.launchpad.webapp.interfaces import IBreadcrumb
 from canonical.launchpad.webapp.menu import (
     ApplicationMenu,
     enabled_with_permission,
@@ -50,14 +50,14 @@
     NavigationMenu,
     )
 from canonical.launchpad.webapp.sorting import sorted_dotted_numbers
-from lp.app.interfaces.launchpad import IServiceUsage
-from lp.app.browser.tales import CustomizableFormatter
 from canonical.lazr.utils import smartquote
 from lp.answers.browser.questiontarget import (
     QuestionTargetFacetMixin,
     QuestionTargetTraversalMixin,
     )
 from lp.answers.interfaces.questionenums import QuestionStatus
+from lp.app.browser.tales import CustomizableFormatter
+from lp.app.interfaces.launchpad import IServiceUsage
 from lp.bugs.browser.bugtask import BugTargetTraversalMixin
 from lp.bugs.interfaces.bug import IBugSet
 from lp.registry.browser.pillar import PillarBugsMenu
@@ -68,6 +68,7 @@
     IDistributionSourcePackage,
     )
 from lp.registry.interfaces.pocket import pocketsuffix
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.propertycache import cachedproperty
 from lp.soyuz.browser.sourcepackagerelease import (
     extract_bug_numbers,
@@ -409,7 +410,7 @@
             packages_dict[package.distroseries] = package
         return packages_dict
 
-    @property
+    @cachedproperty
     def active_series(self):
         """Return active distroseries where this package is published.
 
@@ -438,6 +439,13 @@
         return pocket_dict
 
     @property
+    def latest_sourcepackage(self):
+        if len(self.active_series) == 0:
+            return None
+        return self.active_series[0].getSourcePackage(
+            self.context.sourcepackagename)
+
+    @property
     def version_table(self):
         """Rows of data for the template to render in the packaging table."""
         rows = []
@@ -446,6 +454,16 @@
             # The first row for each series is the "title" row.
             packaging = packages_by_series[distroseries].direct_packaging
             package = packages_by_series[distroseries]
+            # Don't show the "Set upstream link" action for older series
+            # without packaging info, so the user won't feel required to
+            # fill it in.
+            show_set_upstream_link = (
+                packaging is None
+                and distroseries.status in (
+                    SeriesStatus.CURRENT,
+                    SeriesStatus.DEVELOPMENT,
+                    )
+                )
             title_row = {
                 'blank_row': False,
                 'title_row': True,
@@ -453,6 +471,7 @@
                 'distroseries': distroseries,
                 'series_package': package,
                 'packaging': packaging,
+                'show_set_upstream_link': show_set_upstream_link,
                 }
             rows.append(title_row)
 

=== modified file 'lib/lp/registry/browser/tests/distributionsourcepackage-views.txt'
--- lib/lp/registry/browser/tests/distributionsourcepackage-views.txt	2010-08-24 15:29:01 +0000
+++ lib/lp/registry/browser/tests/distributionsourcepackage-views.txt	2010-11-19 15:06:35 +0000
@@ -59,13 +59,47 @@
     ...     distroseries=latest_series)
     >>> transaction.commit()
     >>> ubuntu_eel = ubuntutest.getSourcePackage('eel')
-    >>> view = create_initialized_view(ubuntu_eel, name='+index')
+    >>> view = create_initialized_view(
+    ...     ubuntu_eel, name='+index', principal=factory.makePerson())
     >>> for series in view.active_series:
     ...     print series.name, series.version
     latest 10.04
     breezy-autotest 6.6.6
     earliest 1.1
 
+The view has a latest_sourcepackage attribute whose series is the same
+as the first series from view.active_series.
+
+    >>> latest_series = view.latest_sourcepackage.distroseries
+    >>> print latest_series.name, latest_series.version
+    latest 10.04
+
+The view has a version_table attribute for populating a table. The "Set
+upstream link" action should only be displayed for series with the status
+CURRENT or DEVELOPMENT.
+
+    >>> view.active_series[1].status = SeriesStatus.CURRENT
+    >>> view.active_series[2].status = SeriesStatus.SUPPORTED
+    >>> for row in view.version_table:
+    ...     if row.get('distroseries') is not None:
+    ...         set_upstream_link = ''
+    ...         if row['show_set_upstream_link'] is True:
+    ...             set_upstream_link = '  set-upstream-link'
+    ...         ds = row['distroseries']
+    ...         print "%-16s %-12s %s" % (
+    ...             ds.name, ds.status.name, set_upstream_link)
+    latest           DEVELOPMENT    set-upstream-link
+    breezy-autotest  CURRENT        set-upstream-link
+    earliest         SUPPORTED
+
+If the latest sourcepackage does not have a link to an upstream project,
+this page will display a form to add one.
+
+    >>> from canonical.launchpad.testing.pages import find_tag_by_id
+    >>> upstream_portlet = find_tag_by_id(view.render(), 'upstream')
+    >>> print upstream_portlet.find(id='field.actions.link')['value']
+    Link to Upstream Project
+
 
 Related PPA versions
 --------------------

=== modified file 'lib/lp/registry/templates/distributionsourcepackage-index.pt'
--- lib/lp/registry/templates/distributionsourcepackage-index.pt	2010-07-02 15:10:10 +0000
+++ lib/lp/registry/templates/distributionsourcepackage-index.pt	2010-11-19 15:06:35 +0000
@@ -93,25 +93,14 @@
 
     <div class="yui-u">
       <div class="portlet" id="upstream">
-        <tal:project define="project context/upstream_product">
-          <div tal:condition="project">
-            <h2>Upstream</h2>
-            <div style="float:left; padding:0 1em 1em 0;">
-              <a
-                tal:attributes="href project/fmt:url"
-                tal:content="structure project/image:logo" />
-            </div>
-            <div> 
-              <a 
-                tal:attributes="href project/fmt:url"
-                tal:content="project/displayname" />
-            </div>
-            <div tal:content="structure project/summary" />
-          </div>
-          <tal:noproject condition="not: project">
-              This package is not linked to an upstream product.
-          </tal:noproject>
-        </tal:project>
+        <div tal:condition="not: view/active_series">
+          <h2>Upstream</h2>
+          This package is no longer tracking upstream.
+        </div>
+        <tal:published_series
+          condition="view/active_series"
+          replace="structure view/latest_sourcepackage/@@+portlet-associations"
+          />
       </div><!--portlet -->
     </div><!--yui-u -->
 
@@ -132,7 +121,7 @@
              tal:content="row/distroseries/title"/>
           (<span tal:replace="row/distroseries/status/title/lower"/>)
           <div style="float:right; white-space: nowrap">
-            <a tal:condition="not: row/packaging"
+            <a tal:condition="row/show_set_upstream_link"
                tal:replace="structure row/series_package/menu:overview/set_upstream/fmt:link"/>
             <tal:has_packaging condition="row/packaging">
               <img tal:replace="structure row/packaging/productseries/image:icon"/>

=== modified file 'lib/lp/registry/templates/sourcepackage-upstream-connections.pt'
--- lib/lp/registry/templates/sourcepackage-upstream-connections.pt	2010-11-10 19:16:33 +0000
+++ lib/lp/registry/templates/sourcepackage-upstream-connections.pt	2010-11-19 15:06:35 +0000
@@ -25,6 +25,7 @@
       </span>
     </dd>
   </dl>
+  <p tal:content="series/product/summary"/>
 
   <style>
     #upstream-fields dd {

=== modified file 'lib/lp/soyuz/stories/distribution/xx-distribution-packages.txt'
--- lib/lp/soyuz/stories/distribution/xx-distribution-packages.txt	2010-08-24 15:29:01 +0000
+++ lib/lp/soyuz/stories/distribution/xx-distribution-packages.txt	2010-11-19 15:06:35 +0000
@@ -291,7 +291,9 @@
 is used in another section:
 
     >>> print extract_text(find_tag_by_id(user_browser.contents, 'upstream'))
-    This package is not linked to an upstream product.
+    Upstream connections
+    Launchpad doesn...t know which project and series this
+    package belongs to...
 
 As can be seen, the packaging is not linked yet.  We can do that now using the
 "Set upstream link" link.
@@ -319,9 +321,11 @@
 
     >>> user_browser.open("http://launchpad.dev/ubuntu/+source/iceweasel/")
     >>> print extract_text(find_tag_by_id(user_browser.contents, 'upstream'))
-    Upstream
-    Mozilla Firefox
-    The Mozilla Firefox web browser
+    Upstream connections
+    the Mozilla Project...
+    Mozilla Firefox...
+    trunk...
+    The Mozilla Firefox web browser...
 
     >>> user_browser.getLink('Mozilla Firefox')
     <Link text='Mozilla Firefox' url='http://launchpad.dev/firefox'>