Merge lp:~sinzui/launchpad/unlinkable-packages-0 into lp:launchpad

[Curtis Hovey]

This is my branch to improve the quality of package listings and linking.

    lp:~sinzui/launchpad/unlinkable-packages-0
    Diff size: 269
    Launchpad bug: https://bugs.launchpad.net/bugs/577492
                   https://bugs.launchpad.net/bugs/577497
                   https://bugs.launchpad.net/bugs/580124
    Test command: ./bin/test -vv \
        -t lp/.*/sourcepackage-views -t xx-sourcepackages-packagings
        -t TestDistroSeriesSet -t TestDistroSeriesPackaging
    Pre-implementation: no one. I started fixing issues as I was triaging.
    Target release: 10.05

Improve the quality of package listings and linking
---------------------------------------------------

Bug 577492 [+needs-packaging lists duplicates]
    I viewed the first 200 items listed in
    https://launchpad.net/ubuntu/maverick/+needs-packaging and +packaging and
    I saw several duplicates. The duplication is not obvious since Ubuntu has
    so many packages.

Bug 577497 [+needs-packaging lists packages that cannot be linked]
    Many metapackages and language packs cannot be linked to upstream
    projects.

Bug 580124 [sourcepackage upstream connections portlet is inconsistent]
    The Upstream connection portlet look different that the project Ubuntu
    packaging portlet. The form is ambiguous because there is a link and and
    button to link to an upstream project. The project version of the form
    solved this by moving the option to choose a different package into the
    form options. Also, the SP form will shown too many matches, which breaks
    the layout. The form will also steal focus, and with a lot of matches, the
    page will scroll.

Rules
-----

Bug 577492 [+needs-packaging lists duplicates]
    * +needs-packaging duplicate is actually random ordering for packages
      with the same score. Add a second sort on spn.name to ensure reloading
      a listing does not get a different listing.
    * This is Ubuntu fix. Another branch is needed to fix the Debian problem
      where many packages can be in the published state.

Bug 577497 [+needs-packaging lists packages that cannot be linked]
    * Lp does not store information regarding metapackage. Some meta packages
      can have code (because of rules). The most visible problem is language
      packs and they are all in the 'translations' section. Exclude that
      section

Bug 580124 [sourcepackage upstream connections portlet is inconsistent]
    * Steal from the product Ubuntu packaging form:
      * Limit the number of items to 10
      * Include an option to choose an alternate upstream project
      * Remove the link that looks redundant
      * Set initial_focus_widget to None

UI
--

    * http://people.canonical.com/~curtis/sp-before.png
      A long list with similarly named button and link
    * http://people.canonical.com/~curtis/sp-after.png
      A shorter list (okay, sample data sucks, the tests show that the list
      cannot be more than 10). A single button to set the upstream project
      and the user can choose an option to chose another unlisted project.

QA
--

Bug 577492 [+needs-packaging lists duplicates]
    * Visit https://edge.launchpad.net/ubuntu/maverick/+needs-packaging?start=100
    * Reload and verify the listing is the same.

Bug 577497 [+needs-packaging lists packages that cannot be linked]
    * Visit https://edge.launchpad.net/ubuntu/maverick/+needs-packaging
      and page though 4 pages.
    * Verify that Language packs are not included.

Bug 580124 [sourcepackage upstream connections portlet is inconsistent]
    * Visit https://edge.launchpad.net/ubuntu/maverick/+source/mpc
    * Verify the page does not scroll down to the show the form.
    * Verify 10 items are listed
    * Verify the last item is "Chose another upstream project"
    * Submit the last items
    * Verify you are seeing the page to enter an upstream project.

Lint
----

Linting changed files:
  lib/lp/registry/browser/sourcepackage.py
  lib/lp/registry/browser/tests/sourcepackage-views.txt
  lib/lp/registry/model/distroseries.py
  lib/lp/registry/templates/sourcepackage-portlet-associations.pt
  lib/lp/registry/tests/test_distroseries.py

Test
----

    * lib/lp/registry/browser/tests/sourcepackage-views.txt
      Updated the test to verify that the matches are limited to 9
      and that an option to choose another project is appended.

    * lib/lp/registry/tests/test_distroseries.py
      Added a test to verify that packages in the translations section are
      excluded. Updated a test to show that items of equal score are sorted
      by name.

Implementation
--------------

    * lib/lp/registry/browser/sourcepackage.py
      Updated the upstream packages form to limit the matches to
      9 and append an option to choose another project. Set the
      initial_focus_widget to None.

    * lib/lp/registry/model/distroseries.py
      Added sourcepakagename.name to the sort rule and added a constraint
      to exclude translations section (language packs.)

    * lib/lp/registry/templates/sourcepackage-portlet-associations.pt
      Moved the paragraph about no matches into the form (just like) the
      project version of this portlet. I removed the ambiguous link.

[Eleanor Berger (intellectronica)]

review: Approve (code)

=== modified file 'lib/lp/registry/browser/sourcepackage.py'
--- lib/lp/registry/browser/sourcepackage.py	2010-03-08 10:43:34 +0000
+++ lib/lp/registry/browser/sourcepackage.py	2010-05-18 17:08:30 +0000
@@ -439,6 +439,9 @@
     custom_widget(
         'upstream', LaunchpadRadioWidget, orientation='vertical')
     product_suggestions = None
+    initial_focus_widget = None
+    max_suggestions = 9
+    other_upstream = object()
 
     def setUpFields(self):
         """See `LaunchpadFormView`."""
@@ -452,13 +455,18 @@
         # term descriptions that include a link to the product.
         self.product_suggestions = []
         vocab_terms = []
-        for item in matches:
+        for item in matches[:self.max_suggestions]:
             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))
+        # Add an option to represent the user's decision to choose a
+        # different project. Note that project names cannot be uppercase.
+        description = 'Choose another upstream project'
+        vocab_terms.append(
+            SimpleTerm(self.other_upstream, 'OTHER_UPSTREAM', description))
         upstream_vocabulary = SimpleVocabulary(vocab_terms)
 
         self.form_fields = Fields(
@@ -471,6 +479,11 @@
     @action('Link to Upstream Project', name='link')
     def link(self, action, data):
         upstream = data.get('upstream')
+        if upstream is self.other_upstream:
+            # The user wants to link to an alternate upstream project.
+            self.next_url = canonical_url(
+                self.context, view_name="+edit-packaging")
+            return
         self.context.setPackaging(upstream.development_focus, self.user)
         self.request.response.addInfoNotification(
             'The project %s was linked to this source package.' %

=== modified file 'lib/lp/registry/browser/tests/sourcepackage-views.txt'
--- lib/lp/registry/browser/tests/sourcepackage-views.txt	2010-03-03 00:40:03 +0000
+++ lib/lp/registry/browser/tests/sourcepackage-views.txt	2010-05-18 17:08:30 +0000
@@ -197,17 +197,29 @@
     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')
+    Lernid
+    Choose another upstream project
+
+The form does not steal focus because it is not the primary purpose of the
+page.
+
+    >>> print view.initial_focus_widget
+    None
+
+If there are multiple potential matches, the first 9 are shown. The 10th
+item is reserved for the "Choose another upstream project" option.
+
+    >>> 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
 
+    >>> view.max_suggestions
+    9
+
     >>> content = extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
     >>> print content
     Launchpad doesn&#8217;t know which project and series this
@@ -216,6 +228,21 @@
     Registered upstream project:
     Lernid...
     Lernid Dev...
+    Choose another upstream project
+
+Choosing the "Choose another upstream project" option redirects the user
+to the +edit-packaging page where the user can search for a project.
+
+    >>> form = {
+    ...     'field.upstream': 'OTHER_UPSTREAM',
+    ...     'field.actions.link': 'Link to Upstream Project',
+    ...     }
+    >>> view = create_initialized_view(
+    ...     package, name='+portlet-associations', form=form)
+    >>> view.errors
+    []
+    >>> print view.next_url
+    http://launchpad.dev/youbuntu/busy/+source/lernid/+edit-packaging
 
 
 Upstream connections view
@@ -328,6 +355,8 @@
 
     >>> view = create_initialized_view(package, name='+portlet-associations')
     >>> print extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
+    Launchpad doesn&#8217;t know which project ...
     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
+    Registered upstream project:
+    Choose another upstream project

=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py	2010-04-22 08:47:32 +0000
+++ lib/lp/registry/model/distroseries.py	2010-05-18 17:08:30 +0000
@@ -371,7 +371,7 @@
             """)
         condition = SQL(conditions + "AND packaging.id IS NOT NULL")
         results = IStore(self).using(origin).find(find_spec, condition)
-        results = results.order_by('score DESC')
+        results = results.order_by('score DESC, SourcePackageName.name ASC')
         return [packaging
                 for (packaging, spn, series, product, score) in results]
 
@@ -433,6 +433,8 @@
                 ON spr.id = spph.sourcepackagerelease
             JOIN archive
                 ON spph.archive = Archive.id
+            JOIN section
+                ON spph.section = section.id
             JOIN DistroSeries
                 ON spph.distroseries = DistroSeries.id
             LEFT JOIN Packaging
@@ -443,6 +445,7 @@
             DistroSeries.id = %(distroseries)s
             AND spph.status IN %(active_status)s
             AND archive.purpose = %(primary)s
+            AND section.name != 'translations'
             """ % sqlvalues(
                 distroseries=self,
                 active_status=active_publishing_status,

=== modified file 'lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt'
--- lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2010-03-08 01:51:58 +0000
+++ lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt	2010-05-18 17:08:30 +0000
@@ -15,14 +15,18 @@
     >>> user_browser.getLink('pmount').click()
     >>> print extract_text(find_tag_by_id(
     ...     user_browser.contents, 'no-upstreams'))
+    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
+    Registered upstream project:
+    Choose another upstream project
 
 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 upstream link').click()
+    >>> user_browser.getControl(
+    ...     'Choose another upstream project').selected = True
+    >>> user_browser.getControl("Link to Upstream Project").click()
     >>> user_browser.getControl(name='field.product').value = 'thunderbird'
     >>> user_browser.getControl('Continue').click()
     >>> user_browser.getControl(name='field.productseries').value = ['trunk']

=== modified file 'lib/lp/registry/templates/sourcepackage-portlet-associations.pt'
--- lib/lp/registry/templates/sourcepackage-portlet-associations.pt	2010-02-26 22:59:30 +0000
+++ lib/lp/registry/templates/sourcepackage-portlet-associations.pt	2010-05-18 17:08:30 +0000
@@ -22,7 +22,7 @@
       </tal:has_series>
 
       <tal:has_no_series condition="not: series">
-        <div id="no-upstreams" tal:condition="view/product_suggestions">
+        <div id="no-upstreams">
           <p>
             Launchpad doesn&#8217;t know which project and series this
             package belongs to. Links from distribution packages to
@@ -31,11 +31,16 @@
             efficiently.
           </p>
 
+          <p tal:condition="not: view/product_suggestions">
+          There are no projects registered in Launchpad that are a potential
+          match for this source package.  Can you help us find one?
+          </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?"
-             >
+             condition="view/product_suggestions">
             <b>
             <metal:message use-macro="context/@@+base-layout-macros/plural-message"/>
             </b>
@@ -44,20 +49,9 @@
           <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>

=== modified file 'lib/lp/registry/tests/test_distroseries.py'
--- lib/lp/registry/tests/test_distroseries.py	2010-03-23 00:39:45 +0000
+++ lib/lp/registry/tests/test_distroseries.py	2010-05-18 17:08:30 +0000
@@ -219,16 +219,21 @@
         login(ANONYMOUS)
 
     def makeSeriesPackage(self, name,
-                          is_main=False, heat=None, messages=None):
+                          is_main=False, heat=None, messages=None,
+                          is_translations=False):
         # Make a published source package.
         if is_main:
             component = self.main_component
         else:
             component = self.universe_component
+        if is_translations:
+            section = 'translations'
+        else:
+            section = 'web'
         sourcepackagename = self.factory.makeSourcePackageName(name)
         self.factory.makeSourcePackagePublishingHistory(
             sourcepackagename=sourcepackagename, distroseries=self.series,
-            component=component)
+            component=component, section=section)
         source_package = self.factory.makeSourcePackage(
             sourcepackagename=sourcepackagename, distroseries=self.series)
         if heat is not None:
@@ -256,6 +261,15 @@
             u'main', u'hot-translatable', u'hot', u'translatable', u'normal']
         self.assertEqual(expected, names)
 
+    def test_getPrioritizedUnlinkedSourcePackages_no_language_packs(self):
+        # Verify that translations packages are not listed.
+        self.makeSeriesPackage('language-pack-vi', is_translations=True)
+        package_summaries = self.series.getPrioritizedUnlinkedSourcePackages()
+        names = [summary['package'].name for summary in package_summaries]
+        expected = [
+            u'main', u'hot-translatable', u'hot', u'translatable', u'normal']
+        self.assertEqual(expected, names)
+
     def test_getPrioritizedlPackagings(self):
         # Verify the ordering of packagings that need more upstream info.
         for name in ['main', 'hot-translatable', 'hot', 'translatable']:
@@ -274,7 +288,7 @@
         product_series.product.bugtraker = self.factory.makeBugTracker()
         packagings = self.series.getPrioritizedlPackagings()
         names = [packaging.sourcepackagename.name for packaging in packagings]
-        expected = [u'hot', u'linked', u'cold']
+        expected = [u'hot', u'cold', u'linked']
         self.assertEqual(expected, names)
 
     def test_getPrioritizedlPackagings_branch(self):
@@ -339,7 +353,7 @@
                 translatables, self._ref_translatables()))
 
         new_sourcepackagename = self.factory.makeSourcePackageName()
-        new_potemplate = self.factory.makePOTemplate(
+        self.factory.makePOTemplate(
             distroseries=new_distroseries,
             sourcepackagename=new_sourcepackagename)
         transaction.commit()
@@ -368,7 +382,6 @@
 
     def test_fromSuite_non_release_pocket(self):
         series = self.factory.makeDistroRelease()
-        pocket = PackagePublishingPocket.BACKPORTS
         suite = '%s-backports' % series.name
         result = getUtility(IDistroSeriesSet).fromSuite(
             series.distribution, suite)