Launchpad itself

Merge lp://qastaging/~sinzui/launchpad/package-link-validation-4 into lp://qastaging/launchpad

  1. package-link-validation-4
  2. Merge into devel
Proposed by Curtis Hovey
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://qastaging/~sinzui/launchpad/package-link-validation-4
Merge into: lp://qastaging/launchpad
Diff against target: 379 lines
7 files modified
lib/lp/registry/browser/configure.zcml (+1/-1)
lib/lp/registry/browser/product.py (+43/-2)
lib/lp/registry/browser/productseries.py (+2/-0)
lib/lp/registry/browser/tests/packaging-views.txt (+62/-0)
lib/lp/registry/stories/product/xx-product-package-pages.txt (+34/-7)
lib/lp/registry/templates/product-packages.pt (+55/-30)
lib/lp/translations/stories/standalone/xx-product-translations.txt (+0/-8)
To merge this branch: bzr merge lp://qastaging/~sinzui/launchpad/package-link-validation-4
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+13987@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :
Download full text (3.5 KiB)

This is my fourth branch to ensure valid upstream package links. There
are many oopses relating to the creation and efforts to fix invalid packages.
The root cause is a bad DB constraint and two views that did not do the
required sanity checks: +addpackage and +ubuntupkg

This branch allows users to delete packaging links to SourcePackages.
(They can already delete links to DistributionSourcePackages).

    lp:~sinzui/launchpad/package-link-validation-3
    Diff size: 361
    Launchpad bug: https://bugs.launchpad.net/bugs/193469
    Test command: ./bin/test -vv -t 'lp.(reg|soyuz).*(productseries|packaging)'
    Pre-implementation: flacoste, beuno
    Target release: 3.1.10

== Fixing upstream packaging links ==

Bug 193469 [Unlink (Ubuntu) package from series]
    I've accidently linked a series to the wrong Ubuntu release. But I cannot
    find a way to remove/unlink it.

    You can delete a bad packaging link it was built (as seen on the
    distribution source package page
    https://launchpad.net/ubuntu/+source/jedit), but not if it never was built
    (as is this case https://launchpad.net/ubuntu/hoary/+source/jedit)

== Rules ==

Bug 193469 [Unlink (Ubuntu) package from series]
    * Add a (-) Remove action to each packaging link listed in the table

ADDENDUM
    * The permission on the packaging links and views are very inconsistent.
      Product +addpackage requires edit. The link to it and +ubuntupkg are
      public. The view and two links should be launchpad.View like the
      rules for adding and deleting links from the distro side. The working
      principle is no user knows all packaging for a series, we trust any
      logged in user to make an honest effort to record what he knows.

== QA ==

On staging
    * Visit a productseries
    * Choose (+) Ubuntu packaging
    * Verify there is a link to +addpackage
    * Verify the table items are linked.
    * Submit the form with 'gedit'
    * Verify the form error message explains that the gedit is already
      packaged in Ubuntu karmic. Follow the link
    * Verify the “gedit” source package in Karmic page displays.

== Lint ==

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/productseries.py
  lib/lp/registry/browser/tests/packaging-views.txt
  lib/lp/registry/stories/product/xx-product-package-pages.txt
  lib/lp/registry/templates/product-packages.pt

== Test ==

    * lib/lp/registry/browser/tests/packaging-views.txt
      * Added test coverage for the updated +packages view.
    * lib/lp/registry/stories/product/xx-product-package-pages.txt
      * Fixed some style/lint issues
      * Added test coverage for deleting packaging links.

== Implementation ==

    * lib/lp/registry/browser/configure.zcml
      * Reconciled the +addpackage permissions with the distro permission to
        add a package.
    * lib/lp/registry/browser/product.py
      * Updated ProductPackagesView to descend from PackagingDeleteView so
        that it acquires the features need to delete a bad packaging link.
      * Added the required all_packaging
      * Added series_packages to help the template insert the f...

Read more...

Revision history for this message
Brad Crittenden (bac) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/configure.zcml'
2--- lib/lp/registry/browser/configure.zcml 2009-10-22 22:21:07 +0000
3+++ lib/lp/registry/browser/configure.zcml 2009-10-27 18:36:14 +0000
4@@ -1655,7 +1655,7 @@
5 for="lp.registry.interfaces.productseries.IProductSeries"
6 class="lp.registry.browser.packaging.PackagingAddView"
7 facet="overview"
8- permission="launchpad.Edit"
9+ permission="launchpad.View"
10 template="../templates/productseries-packaging.pt"/>
11 <browser:page
12 name="+ubuntupkg"
13
14=== modified file 'lib/lp/registry/browser/product.py'
15--- lib/lp/registry/browser/product.py 2009-10-22 09:24:53 +0000
16+++ lib/lp/registry/browser/product.py 2009-10-27 18:36:15 +0000
17@@ -82,6 +82,7 @@
18 from lp.registry.browser.distribution import UsesLaunchpadMixin
19 from lp.registry.browser.menu import (
20 IRegistryCollectionNavigationMenu, RegistryCollectionActionMenuBase)
21+from lp.registry.browser.packaging import PackagingDeleteView
22 from lp.answers.browser.faqtarget import FAQTargetNavigationMixin
23 from canonical.launchpad.browser.feeds import FeedsMixin
24 from lp.registry.browser.productseries import get_series_branch_error
25@@ -919,10 +920,50 @@
26 check_permission('launchpad.Commercial', self.context))
27
28
29-class ProductPackagesView(ProductView):
30+class ProductPackagesView(PackagingDeleteView):
31 """View for displaying product packaging"""
32
33- label = 'Packages in Launchpad'
34+ label = 'Linked packages'
35+
36+ @property
37+ def all_packaging(self):
38+ """See `PackagingDeleteView`."""
39+ for series in self.context.serieses:
40+ for packaging in series.packagings:
41+ yield packaging
42+
43+ @cachedproperty
44+ def series_packages(self):
45+ """A hierarchy of product series, packaging and field data.
46+
47+ A dict of series and packagings. Each packaging is a dict of the
48+ packaging and a hidden HTML field for forms:
49+ [{series: <hoary>,
50+ packagings: {
51+ packaging: <packaging>,
52+ field: '<input type=''hidden' ...>},
53+ }]
54+ """
55+ # This method is a superset of all_packaging. While all_packaging will
56+ # be called several times as data is mutated, series_packages should
57+ # only be called during render().
58+ packaged_series = []
59+ for series in self.context.serieses:
60+ packagings = []
61+ for packaging in series.packagings:
62+ form_id = 'delete-%s-%s-%s' % (
63+ packaging.distroseries.name,
64+ packaging.sourcepackagename.name,
65+ packaging.productseries.name,
66+ )
67+ packaging_field = dict(
68+ packaging=packaging,
69+ form_id=form_id,
70+ field=self._renderHiddenPackagingField(packaging))
71+ packagings.append(packaging_field)
72+ packaged_series.append(dict(
73+ series=series, packagings=packagings))
74+ return packaged_series
75
76
77 class ProductDistributionsView(ProductView):
78
79=== modified file 'lib/lp/registry/browser/productseries.py'
80--- lib/lp/registry/browser/productseries.py 2009-10-23 16:24:33 +0000
81+++ lib/lp/registry/browser/productseries.py 2009-10-27 18:36:15 +0000
82@@ -193,11 +193,13 @@
83 summary = 'The code branch that for this series.'
84 return Link('+linkbranch', text, summary, icon=icon)
85
86+ @enabled_with_permission('launchpad.View')
87 def ubuntupkg(self):
88 """Return a link to link this series to an ubuntu sourcepackage."""
89 text = 'Link to Ubuntu package'
90 return Link('+ubuntupkg', text, icon='add')
91
92+ @enabled_with_permission('launchpad.View')
93 def add_package(self):
94 """Return a link to link this series to a sourcepackage."""
95 text = 'Link package'
96
97=== modified file 'lib/lp/registry/browser/tests/packaging-views.txt'
98--- lib/lp/registry/browser/tests/packaging-views.txt 2009-10-23 16:21:47 +0000
99+++ lib/lp/registry/browser/tests/packaging-views.txt 2009-10-27 18:36:15 +0000
100@@ -235,3 +235,65 @@
101 ... print packaging.sourcepackagename.name
102 grumpy hot
103 hoary thunderbird
104+
105+
106+Product packages view
107+----------------------
108+
109+The +packages named view displays the packages links to the product's series.
110+
111+ >>> view = create_initialized_view(product, name='+packages')
112+ >>> print view.label
113+ Linked packages
114+
115+The view defines the all_packages property used by the PackagingDeleteView
116+to create a vocabulary.
117+
118+ >>> for package in view.all_packaging:
119+ ... print package.distroseries.name, package.productseries.name
120+ grumpy hotter
121+ hoary hotter
122+
123+The view provides the series_packages property that returns a list of
124+dicts. Each dict as a series and a list of package dicts. The package dict
125+contains the package and field for form actions.
126+
127+ >>> for series_dict in view.series_packages:
128+ ... print series_dict['series'].name
129+ ... for package_dict in series_dict['packagings']:
130+ ... print package_dict['packaging'].distroseries.name
131+ ... print package_dict['form_id']
132+ ... print package_dict['field']
133+ cold
134+ hotter
135+ grumpy
136+ delete-grumpy-hot-hotter
137+ <input type="hidden" name="field.packaging" .../>
138+ hoary
139+ delete-hoary-thunderbird-hotter
140+ <input type="hidden" name="field.packaging" .../>
141+ trunk
142+
143+The +packages named view descends from PackagingDeleteView to provide remove
144+link actions for the product's linked packages.
145+
146+ >>> from lp.registry.browser.packaging import PackagingDeleteView
147+
148+ >>> isinstance(view, PackagingDeleteView)
149+ True
150+
151+A packaging link can be deleted if the owner believes it is an error. The
152+package linked to hoary is wrong; thunderbird is the wrong sourcepackage.
153+
154+ >>> [hoary_package] = [package for package in view.all_packaging
155+ ... if package.distroseries.name == 'hoary']
156+ >>> form = {
157+ ... 'field.packaging': '%s' % hoary_package.id,
158+ ... 'field.actions.delete_packaging': 'Delete upstream link',
159+ ... }
160+ >>> view = create_initialized_view(product, name='+packages', form=form)
161+ >>> view.errors
162+ []
163+ >>> for package in view.all_packaging:
164+ ... print package.distroseries.name, package.productseries.name
165+ grumpy hotter
166
167=== modified file 'lib/lp/registry/stories/product/xx-product-package-pages.txt'
168--- lib/lp/registry/stories/product/xx-product-package-pages.txt 2009-09-28 12:59:33 +0000
169+++ lib/lp/registry/stories/product/xx-product-package-pages.txt 2009-10-27 18:36:15 +0000
170@@ -1,4 +1,8 @@
171-Launchpad can show you a product's packages by distribution, with links to each.
172+Product Packaging
173+=================
174+
175+Launchpad can show you a product's packages by distribution, with links to
176+each.
177
178 >>> anon_browser.open(
179 ... 'http://launchpad.dev/evolution/+distributions')
180@@ -16,17 +20,20 @@
181 ... find_tag_by_id(anon_browser.contents, 'maincontent').h1)
182 “evolution” source package in Warty
183
184-It can also show the packages by product series. And if you have permission (for
185-example, you're the owner of the product), Launchpad lets you register packages
186-for each series. In this case, mark@example.com is the owner of Evolution.
187+It can also show the packages by product series. And if you have permission
188+(for example, you're the owner of the product), Launchpad lets you register
189+packages for each series. In this case, mark@example.com is the owner of
190+Evolution.
191
192 >>> evo_owner = setupBrowser(auth='Basic mark@example.com:test')
193 >>> evo_owner.open('http://launchpad.dev/evolution/+packages')
194 >>> print evo_owner.title
195- Packages in Launchpad...
196- >>> print extract_text(find_tag_by_id(evo_owner.contents, 'packages'))
197+ Linked packages ...
198+ >>> print extract_text(find_tag_by_id(
199+ ... evo_owner.contents, 'packages-trunk'))
200 Distribution Series Source Package Version
201- Ubuntu 4.10 (Warty) evolution ...
202+ Ubuntu 4.10 (Warty) evolution
203+ Ubuntu 5.04 (Hoary) evolution 1.0
204
205 >>> evo_owner.getLink(url='/ubuntu/hoary/+source/evolution') is not None
206 True
207@@ -41,3 +48,23 @@
208 Traceback (most recent call last):
209 ...
210 LinkNotFoundError
211+
212+
213+Deleting packaging links
214+------------------------
215+
216+Packaging links can be deleted if they were created in error.
217+
218+ >>> form = evo_owner.getForm("delete-warty-evolution-trunk")
219+ >>> form.getControl(name="field.actions.delete_packaging").click()
220+ >>> print evo_owner.title
221+ Linked packages ...
222+
223+ >>> for message in get_feedback_messages(evo_owner.contents):
224+ ... print message
225+ Removed upstream association between Evolution trunk and Warty.
226+
227+ >>> print extract_text(find_tag_by_id(
228+ ... evo_owner.contents, 'packages-trunk'))
229+ Distribution Series Source Package Version
230+ Ubuntu 5.04 (Hoary) evolution 1.0
231
232=== modified file 'lib/lp/registry/templates/product-packages.pt'
233--- lib/lp/registry/templates/product-packages.pt 2009-10-20 16:13:57 +0000
234+++ lib/lp/registry/templates/product-packages.pt 2009-10-27 18:36:14 +0000
235@@ -10,20 +10,25 @@
236 <body>
237
238 <div metal:fill-slot="main">
239+ <div class="top-portlet">
240 <ul class="horizontal">
241 <li>
242 <a tal:replace="structure context/menu:overview/distributions/fmt:link" />
243 </li>
244 </ul>
245+ </div>
246
247- <tal:series repeat="series context/serieses">
248- <h2 style="margin-top: 2em">
249+ <tal:series_dict repeat="series_dict view/series_packages">
250+ <tal:series_packagings
251+ define="series series_dict/series;
252+ packagings series_dict/packagings">
253+ <h2>
254 <a tal:attributes="href series/name">
255- Series &#8220;<span tal:replace="series/name">main</span>&#8221;
256+ <span tal:replace="series/name">main</span> series
257 </a>
258 </h2>
259
260- <p tal:condition="not: series/packagings">
261+ <p tal:condition="not: packagings">
262 No packages linked to this series.
263 </p>
264
265@@ -31,49 +36,69 @@
266 This DIV is necessary for the table-actions:nth-child stylesheet.
267 </tal:comment>
268 <div>
269- <table id="packages" class="listing" tal:condition="series/packagings">
270+ <table class="listing"
271+ tal:condition="packagings"
272+ tal:attributes="id series/name/fmt:css-id/packages-">
273 <thead>
274 <tr>
275 <th>Distribution</th>
276 <th>Series</th>
277 <th>Source Package</th>
278 <th>Version</th>
279+ <th style="width: 1em;"
280+ tal:condition="view/can_delete_packaging">&nbsp;</th>
281 </tr>
282 </thead>
283 <tbody>
284- <tr tal:repeat="pkging series/packagings">
285- <td>
286- <a tal:replace="structure pkging/distroseries/distribution/fmt:link" />
287- </td>
288-
289- <td>
290- <a tal:attributes="href pkging/distroseries/fmt:url"><tal:version
291- replace="pkging/distroseries/version" />
292- (<tal:name replace="pkging/distroseries/displayname" />)</a>
293- </td>
294-
295- <td>
296- <a
297- tal:attributes="href string:${pkging/distroseries/fmt:url}/+source/${pkging/sourcepackagename/name}"
298- tal:content="pkging/sourcepackagename/name"
299- >Apache</a>
300- </td>
301-
302- <td><span tal:condition="pkging/sourcepackage/currentrelease"
303- tal:replace="pkging/sourcepackage/currentrelease/version"
304- >2.3.4-1</span></td>
305+ <tr tal:repeat="packaging_dict packagings">
306+ <tal:packaging_field
307+ define="packaging packaging_dict/packaging;
308+ form_id packaging_dict/form_id;
309+ field packaging_dict/field">
310+ <td>
311+ <a tal:replace="structure packaging/distroseries/distribution/fmt:link" />
312+ </td>
313+
314+ <td>
315+ <a tal:attributes="href packaging/distroseries/fmt:url"><tal:version
316+ replace="packaging/distroseries/version" />
317+ (<tal:name replace="packaging/distroseries/displayname" />)</a>
318+ </td>
319+
320+ <td>
321+ <a
322+ tal:attributes="href string:${packaging/distroseries/fmt:url}/+source/${packaging/sourcepackagename/name}"
323+ tal:content="packaging/sourcepackagename/name">Apache</a>
324+ </td>
325+
326+ <td>
327+ <tal:currentrelease
328+ replace="packaging/sourcepackage/currentrelease/version|nothing">
329+ 2.3.4-1
330+ </tal:currentrelease>
331+ </td>
332+ <td tal:condition="view/can_delete_packaging">
333+ <form style="display: inline" method="post"
334+ tal:attributes="action request/URL;
335+ id form_id">
336+ <tal:hidden replace="structure field" />
337+ <tal:action replace="structure view/renderDeletePackagingAction" />
338+ </form>
339+ </td>
340+ </tal:packaging_field>
341 </tr>
342 </tbody>
343 </table>
344
345- <ul class="table-actions">
346- <li tal:condition="context/required:launchpad.Edit" >
347+ <ul class="table-actions"
348+ tal:condition="series/menu:overview/add_package/linked">
349+ <li>
350 <a tal:replace="structure series/menu:overview/add_package/fmt:link" />
351 </li>
352 </ul>
353 </div>
354-
355- </tal:series>
356+ </tal:series_packagings>
357+ </tal:series_dict>
358
359 </div>
360 </body>
361
362=== modified file 'lib/lp/translations/stories/standalone/xx-product-translations.txt'
363--- lib/lp/translations/stories/standalone/xx-product-translations.txt 2009-10-22 09:28:56 +0000
364+++ lib/lp/translations/stories/standalone/xx-product-translations.txt 2009-10-27 18:36:15 +0000
365@@ -107,14 +107,6 @@
366 ...
367 Unauthorized:...
368
369-(In the packages case, you can look at the packages, but you can't link any.)
370-
371- >>> unprivileged.open(
372- ... 'http://translations.launchpad.dev/gnomebaker/trunk/+addpackage')
373- Traceback (most recent call last):
374- ...
375- Unauthorized:...
376-
377
378 Meanwhile, if you're not logged in at all, you might or might not be the
379 registrant, so the text reflects both possibilities.