Merge lp:~bac/launchpad/bug-490518-link-suggestion into lp:launchpad

[Brad Crittenden]

Allow users to set upstream associations via a new portlet on the sourcepackage page.

[Brad Crittenden (bac)]

= Summary =

The source package page needs a portlet to display upstream information if known -or-
allow the user to set it if not known.

== Proposed fix ==

A portlet is created that creates a widget of likely Launchpad projects the user can
pick from to set the upstream for the source package. A link to go to the existing
package editing form is also provided.

== Pre-implementation notes ==

The work is based on the design proposed by Curtis at
https://dev.launchpad.net/Registry/UbuntuLinkUpstream. He and I had discussions
before and during the implementation.

== Implementation details ==

Pretty straightforward: create the portlet and DTRT.

== Tests ==

bin/test -vvt sourcepackage-views.txt

== Demo and Q/A ==

In launchpad.dev, remove the association for evolution to hoary and then view:
https://launchpad.dev/ubuntu/hoary/+source/evolution

= Launchpad lint =

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

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/registry/templates/sourcepackage-index.pt
  lib/lp/registry/browser/tests/sourcepackage-views.txt
  lib/lp/registry/templates/sourcepackage-portlet-associations.pt

== Pylint notices ==

lib/lp/registry/browser/sourcepackage.py
    28: [F0401] Unable to import 'lazr.restful.interface' (No module named restful)

[Curtis Hovey (sinzui)]

..

=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py 2009-12-18 13:25:19 +0000
+++ lib/lp/registry/browser/sourcepackage.py 2010-01-25 23:25:27 +0000

...

> @@ -294,3 +301,49 @@
> """A View to show Answers help."""
>
> page_title = 'Help and support options'
> +
> +
> +class SourcePackageAssociationPortletView(LaunchpadEditFormView):

LaunchpadEditFormView is used to update the context object, which is not
the case here; it is creating Packaging object though an indirect method.
Why was this base class used instead of LaunchpadFormView? It is because
it can be used to update the Packaging object after it is set? If so,
the view will have to be registered on IPackaging, which does not exist in
this scenario.

> + """A view for linking to an upstream package."""
> +
> + schema = Interface
> + custom_widget(
> + 'upstream', LaunchpadRadioWidget, orientation='vertical')
> + product_suggestions = None
> +
> + def setUpFields(self):
> + """See `LaunchpadFormView`."""
> + super(SourcePackageAssociationPortletView, self).setUpFields()
> + # Find registered products that are similarly named to the source
> + # package.
> + product_vocab = getVocabularyRegistry().get(None, 'Product')
> + matches = product_vocab.searchForTerms(self.context.name)
> + # Based upon the matching products, create a new vocabulary with
> + # term descriptions that include a link to the product.
> + self.product_suggestions = []
> + vocab_terms = []
> + for item in matches:
> + product = item.value
> + self.product_suggestions.append(product)
> + item_url = canonical_url(product)
> + description = """<a href="%s">%s</a>""" % (
> + item_url, product.displayname)

I think this needs escaping. product.displayname may contain [?><].

> + vocab_terms.append(SimpleTerm(product, product.name, description))
> + upstream_vocabulary = SimpleVocabulary(vocab_terms)
> +
> + self.form_fields = Fields(
> + Choice(__name__='upstream',
> + title=_('Upstream project'),

I have been pondering cases where we need to be clear that we are showing
*registered* launchpad projects. Many of the confusing-ui issues is that users
do not understand that someone must register the project for it to be used.

Does the use of *registered* provide a clue that someone may need to register
the upstream project to complete the task?

> + default=None,
> + vocabulary=upstream_vocabulary,
> + #description=_("Select the upstream project"),

I suppose this is not needed.

> + required=True))
> +
> + @action('Link to Upstream Project', name='link')
> + def link(self, action, data):
> + upstream = data.get('upstream')
> + self.context.setPackaging(upstream.development_focus, self.user)
> + self.request.response.addInfoNotification(
> + 'The project %s was linked to this source package.' %
> + upstream.displayname)
> + ...

review: Needs Fixing (code)

[Brad Crittenden (bac)]

Thanks for the review Curtis -- all good suggestions and all changes made except for the plural issue. I'll work with you today to extend and incorporate your macro.

[Brad Crittenden (bac)]

Curtis, based on your macro I created:

<metal:plural-msg define-macro="plural-message">
  <tal:comment condition="nothing">
    Expected variables to be defined in a containing tag or global:
    count - value to check to determine plural form
    singluar - string to use when count == 1
    plural - string to use when count > 1. If no plural is given it defaults
    to the singular value + 's'.
  </tal:comment>
  <tal:singular
    condition="python: count == 1"
    replace="singular" />
  <tal:plural
     define="l_default string:s;
             l_plural plural | string:$singular$l_default;"
    condition="python: count != 1"
    replace="l_plural" />
</metal:plural-msg>

And invoked it like:

          <tal:message
             define="count view/product_suggestions_count;
                     singular string:Is the following project the upstream for this source package?;
                     plural string:Is one of these projects the upstream for this source package?"
             >
            <b>
            <metal:message use-macro="context/@@+base-layout-macros/plural-message"/>
            </b>
          </tal:message>

That's all good but it is longer to use than the old way and required me to create a new property on the view class (product_suggestions_count). So, I'm left a little underwhelmed.

What do you think?

[Curtis Hovey (sinzui)]

Your changes look good. I am going to merge your plural marcro into my branch and use it because your implementation is more universal.

review: Approve (code)

[Martin Albisetti (beuno)]

review: Approve (ui)

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2010-01-22 15:47:53 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2010-01-30 12:11:18 +0000
@@ -391,4 +391,22 @@
   </div>
 </metal:site-message>
 
+<metal:plural-msg define-macro="plural-message">
+  <tal:comment condition="nothing">
+    Expected variables to be defined in a containing tag or global:
+    count - value to check to determine plural form
+    singluar - string to use when count == 1
+    plural - string to use when count > 1.  If no plural is given it defaults
+    to the singular value + 's'.
+  </tal:comment>
+  <tal:singular
+    condition="python: count == 1"
+    replace="singular" />
+  <tal:plural
+     define="l_default string:s;
+             l_plural plural | string:$singular$l_default;"
+    condition="python: count != 1"
+    replace="l_plural" />
+</metal:plural-msg>
+
 </macros>

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2010-01-21 23:46:24 +0000
+++ lib/lp/registry/browser/configure.zcml	2010-01-30 12:11:18 +0000
@@ -1937,6 +1937,13 @@
     <browser:page
         for="lp.registry.interfaces.sourcepackage.ISourcePackage"
         permission="zope.Public"
+        name="+portlet-associations"
+        facet="overview"
+        class="lp.registry.browser.sourcepackage.SourcePackageAssociationPortletView"
+        template="../templates/sourcepackage-portlet-associations.pt"/>
+    <browser:page
+        for="lp.registry.interfaces.sourcepackage.ISourcePackage"
+        permission="zope.Public"
         class="lp.registry.browser.sourcepackage.SourcePackageHelpView"
         name="+gethelp"
         facet="answers"

=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py	2009-12-18 13:25:19 +0000
+++ lib/lp/registry/browser/sourcepackage.py	2010-01-30 12:11:18 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 __all__ = [
+    'SourcePackageAssociationPortletView',
     'SourcePackageBreadcrumb',
     'SourcePackageChangeUpstreamView',
     'SourcePackageFacets',
@@ -16,12 +17,20 @@
     ]
 
 from apt_pkg import ParseSrcDepends
+from cgi import escape
 from zope.component import getUtility, getMultiAdapter
 from zope.app.form.interfaces import IInputWidget
-from zope.formlib.form import FormFields
+from zope.formlib.form import Fields, FormFields
+from zope.interface import Interface
+from zope.schema import Choice
+from zope.schema.vocabulary import (
+    getVocabularyRegistry, SimpleVocabulary, SimpleTerm)
 
 from lazr.restful.interface import copy_field
 
+from canonical.cachedproperty import cachedproperty
+from canonical.widgets import LaunchpadRadioWidget
+
 from canonical.launchpad import helpers
 from lp.bugs.browser.bugtask import BugTargetTraversalMixin
 from canonical.launchpad.browser.packagerelationship import (
@@ -35,8 +44,9 @@
 from lp.translations.interfaces.potemplate import IPOTemplateSet
 from canonical.launchpad import _
 from canonical.launchpad.webapp import (
-    action, ApplicationMenu, GetitemNavigation, LaunchpadEditFormView, Link,
-    redirection, StandardLaunchpadFacets, stepto)
+    action, ApplicationMenu, custom_widget, GetitemNavigation,
+    LaunchpadEditFormView, LaunchpadFormView, Link, redirection,
+    StandardLaunchpadFacets, stepto)
 from canonical.launchpad.webapp import canonical_url
 from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
@@ -294,3 +304,48 @@
     """A View to show Answers help."""
 
     page_title = 'Help and support options'
+
+
+class SourcePackageAssociationPortletView(LaunchpadFormView):
+    """A view for linking to an upstream package."""
+
+    schema = Interface
+    custom_widget(
+        'upstream', LaunchpadRadioWidget, orientation='vertical')
+    product_suggestions = None
+
+    def setUpFields(self):
+        """See `LaunchpadFormView`."""
+        super(SourcePackageAssociationPortletView, self).setUpFields()
+        # Find registered products that are similarly named to the source
+        # package.
+        product_vocab = getVocabularyRegistry().get(None, 'Product')
+        matches = product_vocab.searchForTerms(self.context.name)
+        # Based upon the matching products, create a new vocabulary with
+        # term descriptions that include a link to the product.
+        self.product_suggestions = []
+        vocab_terms = []
+        for item in matches:
+            product = item.value
+            self.product_suggestions.append(product)
+            item_url = canonical_url(product)
+            description = """<a href="%s">%s</a>""" % (
+                item_url, escape(product.displayname))
+            vocab_terms.append(SimpleTerm(product, product.name, description))
+        upstream_vocabulary = SimpleVocabulary(vocab_terms)
+
+        self.form_fields = Fields(
+            Choice(__name__='upstream',
+                   title=_('Registered upstream project'),
+                   default=None,
+                   vocabulary=upstream_vocabulary,
+                   required=True))
+
+    @action('Link to Upstream Project', name='link')
+    def link(self, action, data):
+        upstream = data.get('upstream')
+        self.context.setPackaging(upstream.development_focus, self.user)
+        self.request.response.addInfoNotification(
+            'The project %s was linked to this source package.' %
+            upstream.displayname)
+        self.next_url = self.request.getURL()

=== modified file 'lib/lp/registry/browser/tests/sourcepackage-views.txt'
--- lib/lp/registry/browser/tests/sourcepackage-views.txt	2009-10-27 04:04:55 +0000
+++ lib/lp/registry/browser/tests/sourcepackage-views.txt	2010-01-30 12:11:18 +0000
@@ -1,6 +1,8 @@
 SourcePackage views
 ===================
 
+Edit packaging view
+-------------------
 
     >>> product = factory.makeProduct(name='bonkers', displayname='Bonkers')
     >>> productseries = factory.makeProductSeries(
@@ -96,3 +98,65 @@
 
     >>> print view.request.response.notifications
     []
+
+Upstream associations portlet
+-----------------------------
+
+The upstreams associations portlet either displays the upstream
+information if it is already set or gives the user the opportunity to
+suggest the association.  The suggestion is based on a
+ProductVocabulary query using the source package name.
+
+Since the bonkers source project was associated previously with the
+bonkers project, the portlet will display that information.
+
+    >>> view = create_initialized_view(package, name='+portlet-associations')
+    >>> for product in view.product_suggestions:
+    ...     print product.name
+    bonkers
+
+    >>> from canonical.launchpad.testing.pages import (
+    ...     extract_text, find_tag_by_id)
+    >>> content = extract_text(find_tag_by_id(view.render(), 'upstreams'))
+    >>> print content
+    Project:...Bonkers...
+    Series:...crazy...
+
+A new source project that is not linked to an upstream will result in
+the portlet showing the suggested project.
+
+    >>> product = factory.makeProduct(name='lernid', displayname='Lernid')
+    >>> sourcepackagename = factory.makeSourcePackageName(name='lernid')
+    >>> package = factory.makeSourcePackage(
+    ...     sourcepackagename=sourcepackagename, distroseries=distroseries)
+
+    >>> view = create_initialized_view(package, name='+portlet-associations')
+    >>> for product in view.product_suggestions:
+    ...     print product.name
+    lernid
+
+    >>> content = extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
+    >>> print content
+    Launchpad doesn&#8217;t know which project and series this
+    package belongs to. ...
+    Is the following project the upstream for this source package?
+    Registered upstream project:
+    Lernid...
+
+If there are multiple potential matches they are all shown.
+
+    >>> product = factory.makeProduct(name='lernid-dev', displayname='Lernid Dev')
+    >>> view = create_initialized_view(package, name='+portlet-associations')
+    >>> for product in view.product_suggestions:
+    ...     print product.name
+    lernid
+    lernid-dev
+
+    >>> content = extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
+    >>> print content
+    Launchpad doesn&#8217;t know which project and series this
+    package belongs to. ...
+    Is one of these projects the upstream for this source package?
+    Registered upstream project:
+    Lernid...
+    Lernid Dev...

=== modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt'
--- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2009-12-08 05:02:32 +0000
+++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2010-01-30 12:11:18 +0000
@@ -15,13 +15,14 @@
     >>> user_browser.getLink('pmount').click()
     >>> print extract_text(find_tag_by_id(
     ...     user_browser.contents, 'no-upstreams'))
-    Launchpad doesn&#8217;t know which project and series this package
-    belongs to. ... Can you set the upstream project in Launchpad?
+    There are no projects registered in Launchpad that are a potential
+    match for this source package. Can you help us find one?
+    Set upstream link
 
 No Privileges Person knows that the pmount package comes from the thunderbird
 project. He sets the upstream packaging link and sees that it is set.
 
-    >>> user_browser.getLink('set the upstream project in Launchpad').click()
+    >>> user_browser.getLink('Set upstream link').click()
     >>> user_browser.getControl(
     ...     name="field.productseries").value = 'thunderbird/trunk'
     >>> user_browser.getControl("Change").click()

=== modified file 'lib/lp/registry/templates/sourcepackage-index.pt'
--- lib/lp/registry/templates/sourcepackage-index.pt	2009-12-03 16:54:29 +0000
+++ lib/lp/registry/templates/sourcepackage-index.pt	2010-01-30 12:11:18 +0000
@@ -55,61 +55,7 @@
         </div>
 
         <div class="yui-u">
-          <div class="portlet"
-            tal:define="series context/productseries">
-            <h2>Upstream associations</h2>
-
-            <tal:has_series condition="series">
-              <div id="upstreams" class="two-column-list">
-                <dl
-                  tal:define="project series/product/project"
-                  tal:condition="project">
-                  <dt>Project Group:</dt>
-                  <dd>
-                    <a tal:replace="structure project/fmt:link" />
-                  </dd>
-                </dl>
-
-                <dl>
-                  <dt>Project:</dt>
-                  <dd>
-                    <a tal:replace="structure series/product/fmt:link" />
-                  </dd>
-                </dl>
-
-                <dl>
-                  <dt>Series:</dt>
-                  <dd>
-                    <a
-                      tal:content="series/name"
-                      tal:attributes="href series/fmt:url">Series</a>
-                    <a tal:replace="structure context/menu:overview/edit_packaging/fmt:icon" />
-                  </dd>
-                </dl>
-              </div>
-
-              <ul class="horizontal">
-                <li>
-                  <a
-                    tal:attributes="href series/product/menu:overview/packages/fmt:url"
-                    >Show upstream links</a>
-                </li>
-              </ul>
-            </tal:has_series>
-
-            <tal:has_no_series condition="not: series">
-               <p id="no-upstreams">
-                 Launchpad doesn&#8217;t know which project and series this
-                 package belongs to. Links from distribution packages to
-                 upstream project let distribution and upstream
-                 maintainers share bugs, patches, and translations
-                 efficiently. Can you 
-                 <a tal:attributes="
-                   href context/menu:overview/edit_packaging/url">set
-                   the upstream project in Launchpad</a>?
-               </p>
-            </tal:has_no_series>
-          </div>
+          <div class="portlet" tal:content="structure context/@@+portlet-associations" />
         </div>
       </div>
 

=== added file 'lib/lp/registry/templates/sourcepackage-portlet-associations.pt'
--- lib/lp/registry/templates/sourcepackage-portlet-associations.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/sourcepackage-portlet-associations.pt	2010-01-30 12:11:18 +0000
@@ -0,0 +1,97 @@
+<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 id="portlet-associations">
+    <h2>Upstream associations</h2>
+
+    <div class="portletBody portletContent"
+         tal:define="series context/productseries">
+      <tal:has_series condition="series">
+        <div id="upstreams" class="two-column-list">
+          <dl
+             tal:define="project series/product/project"
+             tal:condition="project">
+            <dt>Project Group:</dt>
+            <dd>
+              <a tal:replace="structure project/fmt:link" />
+            </dd>
+          </dl>
+
+          <dl>
+            <dt>Project:</dt>
+            <dd>
+              <a tal:replace="structure series/product/fmt:link" />
+            </dd>
+          </dl>
+
+          <dl>
+            <dt>Series:</dt>
+            <dd>
+              <a
+                 tal:content="series/name"
+                 tal:attributes="href series/fmt:url">Series</a>
+              <a tal:replace="structure context/menu:overview/edit_packaging/fmt:icon" />
+            </dd>
+          </dl>
+        </div>
+
+        <ul class="horizontal">
+          <li>
+            <a
+               tal:attributes="href series/product/menu:overview/packages/fmt:url"
+               >Show upstream links</a>
+          </li>
+        </ul>
+      </tal:has_series>
+
+      <tal:has_no_series condition="not: series">
+        <div id="no-upstreams" tal:condition="view/product_suggestions">
+          <p>
+            Launchpad doesn&#8217;t know which project and series this
+            package belongs to. Links from distribution packages to
+            upstream project let distribution and upstream
+            maintainers share bugs, patches, and translations
+            efficiently.
+          </p>
+
+          <tal:message
+             define="count python:len(view.product_suggestions);
+                     singular string:Is the following project the upstream for this source package?;
+                     plural string:Is one of these projects the upstream for this source package?"
+             >
+            <b>
+            <metal:message use-macro="context/@@+base-layout-macros/plural-message"/>
+            </b>
+          </tal:message>
+
+          <div tal:condition="python: len(view.product_suggestions) > 1">
+            <b>Is one of these projects the upstream for this source
+            package?</b>
+          </div>
+          <div tal:condition="python: len(view.product_suggestions) == 1">
+            <b>Is the following project the upstream for this source package?</b>
+          </div>
+          <div metal:use-macro="context/@@launchpad_form/form">
+            <div class="actions" metal:fill-slot="buttons">
+              <input tal:replace="structure view/link/render"/>
+              &nbsp;or&nbsp;
+              <a tal:replace="structure
+                              context/menu:overview/set_upstream/fmt:link" />
+            </div>
+          </div>
+        </div>
+        <div id="no-upstreams" tal:condition="not: view/product_suggestions">
+          <p>
+          There are no projects registered in Launchpad that are a potential
+          match for this source package.  Can you help us find one?
+          </p>
+          <a tal:replace="structure
+                          context/menu:overview/set_upstream/fmt:link" />
+        </div>
+      </tal:has_no_series>
+    </div>
+  </div>
+</tal:root>