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

Proposed by Curtis Hovey
Status: Merged
Approved by: Eleanor Berger
Approved revision: no longer in the source branch.
Merged at revision: 10883
Proposed branch: lp:~sinzui/launchpad/unlinkable-packages-0
Merge into: lp:launchpad
Diff against target: 293 lines (+84/-28)
6 files modified
lib/lp/registry/browser/sourcepackage.py (+14/-1)
lib/lp/registry/browser/tests/sourcepackage-views.txt (+35/-6)
lib/lp/registry/model/distroseries.py (+4/-1)
lib/lp/registry/stories/packaging/xx-sourcepackage-packaging.txt (+6/-2)
lib/lp/registry/templates/sourcepackage-portlet-associations.pt (+7/-13)
lib/lp/registry/tests/test_distroseries.py (+18/-5)
To merge this branch: bzr merge lp:~sinzui/launchpad/unlinkable-packages-0
Reviewer Review Type Date Requested Status
Eleanor Berger (community) code Approve
Review via email: mp+25260@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Eleanor Berger (intellectronica) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
439 custom_widget(439 custom_widget(
440 'upstream', LaunchpadRadioWidget, orientation='vertical')440 'upstream', LaunchpadRadioWidget, orientation='vertical')
441 product_suggestions = None441 product_suggestions = None
442 initial_focus_widget = None
443 max_suggestions = 9
444 other_upstream = object()
442445
443 def setUpFields(self):446 def setUpFields(self):
444 """See `LaunchpadFormView`."""447 """See `LaunchpadFormView`."""
@@ -452,13 +455,18 @@
452 # term descriptions that include a link to the product.455 # term descriptions that include a link to the product.
453 self.product_suggestions = []456 self.product_suggestions = []
454 vocab_terms = []457 vocab_terms = []
455 for item in matches:458 for item in matches[:self.max_suggestions]:
456 product = item.value459 product = item.value
457 self.product_suggestions.append(product)460 self.product_suggestions.append(product)
458 item_url = canonical_url(product)461 item_url = canonical_url(product)
459 description = """<a href="%s">%s</a>""" % (462 description = """<a href="%s">%s</a>""" % (
460 item_url, escape(product.displayname))463 item_url, escape(product.displayname))
461 vocab_terms.append(SimpleTerm(product, product.name, description))464 vocab_terms.append(SimpleTerm(product, product.name, description))
465 # Add an option to represent the user's decision to choose a
466 # different project. Note that project names cannot be uppercase.
467 description = 'Choose another upstream project'
468 vocab_terms.append(
469 SimpleTerm(self.other_upstream, 'OTHER_UPSTREAM', description))
462 upstream_vocabulary = SimpleVocabulary(vocab_terms)470 upstream_vocabulary = SimpleVocabulary(vocab_terms)
463471
464 self.form_fields = Fields(472 self.form_fields = Fields(
@@ -471,6 +479,11 @@
471 @action('Link to Upstream Project', name='link')479 @action('Link to Upstream Project', name='link')
472 def link(self, action, data):480 def link(self, action, data):
473 upstream = data.get('upstream')481 upstream = data.get('upstream')
482 if upstream is self.other_upstream:
483 # The user wants to link to an alternate upstream project.
484 self.next_url = canonical_url(
485 self.context, view_name="+edit-packaging")
486 return
474 self.context.setPackaging(upstream.development_focus, self.user)487 self.context.setPackaging(upstream.development_focus, self.user)
475 self.request.response.addInfoNotification(488 self.request.response.addInfoNotification(
476 'The project %s was linked to this source package.' %489 'The project %s was linked to this source package.' %
477490
=== 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 @@
197 package belongs to. ...197 package belongs to. ...
198 Is the following project the upstream for this source package?198 Is the following project the upstream for this source package?
199 Registered upstream project:199 Registered upstream project:
200 Lernid...200 Lernid
201201 Choose another upstream project
202If there are multiple potential matches they are all shown.202
203203The form does not steal focus because it is not the primary purpose of the
204 >>> product = factory.makeProduct(name='lernid-dev', displayname='Lernid Dev')204page.
205
206 >>> print view.initial_focus_widget
207 None
208
209If there are multiple potential matches, the first 9 are shown. The 10th
210item is reserved for the "Choose another upstream project" option.
211
212 >>> product = factory.makeProduct(
213 ... name='lernid-dev', displayname='Lernid Dev')
205 >>> view = create_initialized_view(package, name='+portlet-associations')214 >>> view = create_initialized_view(package, name='+portlet-associations')
206 >>> for product in view.product_suggestions:215 >>> for product in view.product_suggestions:
207 ... print product.name216 ... print product.name
208 lernid217 lernid
209 lernid-dev218 lernid-dev
210219
220 >>> view.max_suggestions
221 9
222
211 >>> content = extract_text(find_tag_by_id(view.render(), 'no-upstreams'))223 >>> content = extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
212 >>> print content224 >>> print content
213 Launchpad doesn&#8217;t know which project and series this225 Launchpad doesn&#8217;t know which project and series this
@@ -216,6 +228,21 @@
216 Registered upstream project:228 Registered upstream project:
217 Lernid...229 Lernid...
218 Lernid Dev...230 Lernid Dev...
231 Choose another upstream project
232
233Choosing the "Choose another upstream project" option redirects the user
234to the +edit-packaging page where the user can search for a project.
235
236 >>> form = {
237 ... 'field.upstream': 'OTHER_UPSTREAM',
238 ... 'field.actions.link': 'Link to Upstream Project',
239 ... }
240 >>> view = create_initialized_view(
241 ... package, name='+portlet-associations', form=form)
242 >>> view.errors
243 []
244 >>> print view.next_url
245 http://launchpad.dev/youbuntu/busy/+source/lernid/+edit-packaging
219246
220247
221Upstream connections view248Upstream connections view
@@ -328,6 +355,8 @@
328355
329 >>> view = create_initialized_view(package, name='+portlet-associations')356 >>> view = create_initialized_view(package, name='+portlet-associations')
330 >>> print extract_text(find_tag_by_id(view.render(), 'no-upstreams'))357 >>> print extract_text(find_tag_by_id(view.render(), 'no-upstreams'))
358 Launchpad doesn&#8217;t know which project ...
331 There are no projects registered in Launchpad that are a potential359 There are no projects registered in Launchpad that are a potential
332 match for this source package. Can you help us find one?360 match for this source package. Can you help us find one?
333 Set upstream link361 Registered upstream project:
362 Choose another upstream project
334363
=== 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 @@
371 """)371 """)
372 condition = SQL(conditions + "AND packaging.id IS NOT NULL")372 condition = SQL(conditions + "AND packaging.id IS NOT NULL")
373 results = IStore(self).using(origin).find(find_spec, condition)373 results = IStore(self).using(origin).find(find_spec, condition)
374 results = results.order_by('score DESC')374 results = results.order_by('score DESC, SourcePackageName.name ASC')
375 return [packaging375 return [packaging
376 for (packaging, spn, series, product, score) in results]376 for (packaging, spn, series, product, score) in results]
377377
@@ -433,6 +433,8 @@
433 ON spr.id = spph.sourcepackagerelease433 ON spr.id = spph.sourcepackagerelease
434 JOIN archive434 JOIN archive
435 ON spph.archive = Archive.id435 ON spph.archive = Archive.id
436 JOIN section
437 ON spph.section = section.id
436 JOIN DistroSeries438 JOIN DistroSeries
437 ON spph.distroseries = DistroSeries.id439 ON spph.distroseries = DistroSeries.id
438 LEFT JOIN Packaging440 LEFT JOIN Packaging
@@ -443,6 +445,7 @@
443 DistroSeries.id = %(distroseries)s445 DistroSeries.id = %(distroseries)s
444 AND spph.status IN %(active_status)s446 AND spph.status IN %(active_status)s
445 AND archive.purpose = %(primary)s447 AND archive.purpose = %(primary)s
448 AND section.name != 'translations'
446 """ % sqlvalues(449 """ % sqlvalues(
447 distroseries=self,450 distroseries=self,
448 active_status=active_publishing_status,451 active_status=active_publishing_status,
449452
=== 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 @@
15 >>> user_browser.getLink('pmount').click()15 >>> user_browser.getLink('pmount').click()
16 >>> print extract_text(find_tag_by_id(16 >>> print extract_text(find_tag_by_id(
17 ... user_browser.contents, 'no-upstreams'))17 ... user_browser.contents, 'no-upstreams'))
18 Launchpad...
18 There are no projects registered in Launchpad that are a potential19 There are no projects registered in Launchpad that are a potential
19 match for this source package. Can you help us find one?20 match for this source package. Can you help us find one?
20 Set upstream link21 Registered upstream project:
22 Choose another upstream project
2123
22No Privileges Person knows that the pmount package comes from the thunderbird24No Privileges Person knows that the pmount package comes from the thunderbird
23project. He sets the upstream packaging link and sees that it is set.25project. He sets the upstream packaging link and sees that it is set.
2426
25 >>> user_browser.getLink('Set upstream link').click()27 >>> user_browser.getControl(
28 ... 'Choose another upstream project').selected = True
29 >>> user_browser.getControl("Link to Upstream Project").click()
26 >>> user_browser.getControl(name='field.product').value = 'thunderbird'30 >>> user_browser.getControl(name='field.product').value = 'thunderbird'
27 >>> user_browser.getControl('Continue').click()31 >>> user_browser.getControl('Continue').click()
28 >>> user_browser.getControl(name='field.productseries').value = ['trunk']32 >>> user_browser.getControl(name='field.productseries').value = ['trunk']
2933
=== 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 @@
22 </tal:has_series>22 </tal:has_series>
2323
24 <tal:has_no_series condition="not: series">24 <tal:has_no_series condition="not: series">
25 <div id="no-upstreams" tal:condition="view/product_suggestions">25 <div id="no-upstreams">
26 <p>26 <p>
27 Launchpad doesn&#8217;t know which project and series this27 Launchpad doesn&#8217;t know which project and series this
28 package belongs to. Links from distribution packages to28 package belongs to. Links from distribution packages to
@@ -31,11 +31,16 @@
31 efficiently.31 efficiently.
32 </p>32 </p>
3333
34 <p tal:condition="not: view/product_suggestions">
35 There are no projects registered in Launchpad that are a potential
36 match for this source package. Can you help us find one?
37 </p>
38
34 <tal:message39 <tal:message
35 define="count python:len(view.product_suggestions);40 define="count python:len(view.product_suggestions);
36 singular string:Is the following project the upstream for this source package?;41 singular string:Is the following project the upstream for this source package?;
37 plural string:Is one of these projects the upstream for this source package?"42 plural string:Is one of these projects the upstream for this source package?"
38 >43 condition="view/product_suggestions">
39 <b>44 <b>
40 <metal:message use-macro="context/@@+base-layout-macros/plural-message"/>45 <metal:message use-macro="context/@@+base-layout-macros/plural-message"/>
41 </b>46 </b>
@@ -44,20 +49,9 @@
44 <div metal:use-macro="context/@@launchpad_form/form">49 <div metal:use-macro="context/@@launchpad_form/form">
45 <div class="actions" metal:fill-slot="buttons">50 <div class="actions" metal:fill-slot="buttons">
46 <input tal:replace="structure view/link/render"/>51 <input tal:replace="structure view/link/render"/>
47 &nbsp;or&nbsp;
48 <a tal:replace="structure
49 context/menu:overview/set_upstream/fmt:link" />
50 </div>52 </div>
51 </div>53 </div>
52 </div>54 </div>
53 <div id="no-upstreams" tal:condition="not: view/product_suggestions">
54 <p>
55 There are no projects registered in Launchpad that are a potential
56 match for this source package. Can you help us find one?
57 </p>
58 <a tal:replace="structure
59 context/menu:overview/set_upstream/fmt:link" />
60 </div>
61 </tal:has_no_series>55 </tal:has_no_series>
62 </div>56 </div>
63 </div>57 </div>
6458
=== 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 @@
219 login(ANONYMOUS)219 login(ANONYMOUS)
220220
221 def makeSeriesPackage(self, name,221 def makeSeriesPackage(self, name,
222 is_main=False, heat=None, messages=None):222 is_main=False, heat=None, messages=None,
223 is_translations=False):
223 # Make a published source package.224 # Make a published source package.
224 if is_main:225 if is_main:
225 component = self.main_component226 component = self.main_component
226 else:227 else:
227 component = self.universe_component228 component = self.universe_component
229 if is_translations:
230 section = 'translations'
231 else:
232 section = 'web'
228 sourcepackagename = self.factory.makeSourcePackageName(name)233 sourcepackagename = self.factory.makeSourcePackageName(name)
229 self.factory.makeSourcePackagePublishingHistory(234 self.factory.makeSourcePackagePublishingHistory(
230 sourcepackagename=sourcepackagename, distroseries=self.series,235 sourcepackagename=sourcepackagename, distroseries=self.series,
231 component=component)236 component=component, section=section)
232 source_package = self.factory.makeSourcePackage(237 source_package = self.factory.makeSourcePackage(
233 sourcepackagename=sourcepackagename, distroseries=self.series)238 sourcepackagename=sourcepackagename, distroseries=self.series)
234 if heat is not None:239 if heat is not None:
@@ -256,6 +261,15 @@
256 u'main', u'hot-translatable', u'hot', u'translatable', u'normal']261 u'main', u'hot-translatable', u'hot', u'translatable', u'normal']
257 self.assertEqual(expected, names)262 self.assertEqual(expected, names)
258263
264 def test_getPrioritizedUnlinkedSourcePackages_no_language_packs(self):
265 # Verify that translations packages are not listed.
266 self.makeSeriesPackage('language-pack-vi', is_translations=True)
267 package_summaries = self.series.getPrioritizedUnlinkedSourcePackages()
268 names = [summary['package'].name for summary in package_summaries]
269 expected = [
270 u'main', u'hot-translatable', u'hot', u'translatable', u'normal']
271 self.assertEqual(expected, names)
272
259 def test_getPrioritizedlPackagings(self):273 def test_getPrioritizedlPackagings(self):
260 # Verify the ordering of packagings that need more upstream info.274 # Verify the ordering of packagings that need more upstream info.
261 for name in ['main', 'hot-translatable', 'hot', 'translatable']:275 for name in ['main', 'hot-translatable', 'hot', 'translatable']:
@@ -274,7 +288,7 @@
274 product_series.product.bugtraker = self.factory.makeBugTracker()288 product_series.product.bugtraker = self.factory.makeBugTracker()
275 packagings = self.series.getPrioritizedlPackagings()289 packagings = self.series.getPrioritizedlPackagings()
276 names = [packaging.sourcepackagename.name for packaging in packagings]290 names = [packaging.sourcepackagename.name for packaging in packagings]
277 expected = [u'hot', u'linked', u'cold']291 expected = [u'hot', u'cold', u'linked']
278 self.assertEqual(expected, names)292 self.assertEqual(expected, names)
279293
280 def test_getPrioritizedlPackagings_branch(self):294 def test_getPrioritizedlPackagings_branch(self):
@@ -339,7 +353,7 @@
339 translatables, self._ref_translatables()))353 translatables, self._ref_translatables()))
340354
341 new_sourcepackagename = self.factory.makeSourcePackageName()355 new_sourcepackagename = self.factory.makeSourcePackageName()
342 new_potemplate = self.factory.makePOTemplate(356 self.factory.makePOTemplate(
343 distroseries=new_distroseries,357 distroseries=new_distroseries,
344 sourcepackagename=new_sourcepackagename)358 sourcepackagename=new_sourcepackagename)
345 transaction.commit()359 transaction.commit()
@@ -368,7 +382,6 @@
368382
369 def test_fromSuite_non_release_pocket(self):383 def test_fromSuite_non_release_pocket(self):
370 series = self.factory.makeDistroRelease()384 series = self.factory.makeDistroRelease()
371 pocket = PackagePublishingPocket.BACKPORTS
372 suite = '%s-backports' % series.name385 suite = '%s-backports' % series.name
373 result = getUtility(IDistroSeriesSet).fromSuite(386 result = getUtility(IDistroSeriesSet).fromSuite(
374 series.distribution, suite)387 series.distribution, suite)