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

= 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)

..

=== 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)
> + ...

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.

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?

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