Launchpad itself

Merge lp:~bac/launchpad/bug-499351-batching-dls into lp:launchpad

  1. bug-499351-batching-dls
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Curtis Hovey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-499351-batching-dls
Merge into: lp:launchpad
Diff against target: 617 lines (+271/-133)
12 files modified
configs/development/launchpad-lazr.conf (+1/-0)
lib/canonical/config/schema-lazr.conf (+4/-0)
lib/canonical/launchpad/icing/style.css (+5/-6)
lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt (+3/-2)
lib/canonical/launchpad/webapp/batching.py (+4/-0)
lib/lp/registry/browser/product.py (+34/-2)
lib/lp/registry/browser/tests/product-files-views.txt (+70/-0)
lib/lp/registry/browser/tests/test_views.py (+1/-0)
lib/lp/registry/stories/product/xx-product-files.txt (+2/-1)
lib/lp/registry/templates/product-files.pt (+129/-114)
lib/lp/registry/templates/productreleasefile-macros.pt (+1/-1)
lib/lp/testing/factory.py (+17/-7)
To merge this branch: bzr merge lp:~bac/launchpad/bug-499351-batching-dls
Reviewer Review Type Date Requested Status
Curtis Hovey (community) ui Approve
Martin Albisetti ui Pending
Review via email: mp+17492@code.launchpad.net

Commit message

Add batching to +download page.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

For products with a large number of series or releases the +download page becomes too
big, loads slowly, and sometimes times out. There is too much information to be useful.

== Proposed fix ==

Batch the releases shown.

== Pre-implementation notes ==

Talked with Curtis.

== Implementation details ==

Create a class with a tuple of (series, release) and batch those.

For product administrators the links to all series and releases are maintained at the
bottom of each page.

== Tests ==

bin/test -vv -t product-files-views.txt -t xx-product-files.txt

== Demo and Q/A ==

Look at a big project like https://edge.launchpad.net/bzr/+download and see that the
batching works right.

= Launchpad lint =

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

Linting changed files:
  lib/canonical/config/schema-lazr.conf
  lib/lp/registry/browser/tests/product-files-views.txt
  lib/lp/registry/templates/product-files.pt
  lib/lp/registry/templates/productreleasefile-macros.pt
  lib/lp/registry/browser/product.py
  lib/lp/testing/factory.py
  lib/lp/registry/stories/product/xx-product-files.txt
  lib/lp/registry/browser/tests/test_views.py
  configs/development/launchpad-lazr.conf

== Pylint notices ==

lib/lp/registry/browser/product.py
    54: [F0401] Unable to import 'lazr.delegates' (No module named delegates)

lib/lp/testing/factory.py
    17: [F0401] Unable to import 'email.Encoders' (No module named Encoders)
    18: [F0401] Unable to import 'email.Utils' (No module named Utils)
    19: [F0401] Unable to import 'email.Message' (No module named Message)
    20: [F0401] Unable to import 'email.MIMEText' (No module named MIMEText)
    21: [F0401] Unable to import 'email.MIMEMultipart' (No module named MIMEMultipart)

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thanks for doing this. As I noted on IRC, the casting of a list to a list on line 87 of the diff looks redundant. You can fix the code before landing this if it is redundant.

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

Requesting a UI review. Screenshots are attached to the bug.

Revision history for this message
Brad Crittenden (bac) wrote :

Preparing for the UI review I discovered I neglected to put the batch navigator links into the page template. I've done that and updated one of the factories to allow a release to be re-used.

Diff at http://pastebin.ubuntu.com/359047/

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi brad.

Your diff exposes an issue in the markup. The template is using:
    <h2 style="font-size: 1.2em;">
Which is inconsistent in browsers. We should either remove the style attribute or change the unit to percentage (116, 123.1):
    <h2 style="font-size: 116%;">

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Brad.

I am looking at the UI, (lp does not let me vote twice). There are some improvement we can make.
  1 - 4 of 12 results => 1 - 4 of 12 releases
You can doo this subclassing the batch navigator or by setting after __init__:
    batch = BatchNavigator(results)
    batch = batch.setHeadings('release', 'releases')

My eye gets lost scanning from the batch summary to the navigation. This is not a problem on other pages because the table border creates horizontal line for my eye to follow. I am not sure what to do in this case. This is also a reason I was aprehensive about using a batch navigator on with this layout. +search solved the problem by setting the background-color of the table and a lower/upper border to create a bar (see style.css: upper-batch-nav, lower-batch-nav). I think we could modify batchnavigator-navigation-links.pt to have a class on the table, but it does not distinguish between upper and lower. I see that you are calling upper and lower batches in the downloads template, but both those calls are mapped to the same template. Since they are using different views, the views could define the view/css_class that the template uses.

/me sees there is extra white space between the files table and the (+) Add download file when the page is viewed in webkit :(

review: Needs Fixing
Revision history for this message
Brad Crittenden (bac) wrote :

Comments from Martin on IRC:

bueno bac, I think sinzui is right on the mark with the problem 16:45
bac beuno: ok 16:45
beuno bac, one way of fixing this could be 16:46
beuno to isntead of presenting it as a navigation 16:46
beuno have an "See older releases >" link

Revision history for this message
Brad Crittenden (bac) wrote :

Thanks for the suggestions Curtis and the help with the css.

The diff is at http://pastebin.ubuntu.com/359769/

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thanks for adding rules for consistent batch navigator presenations.

review: Approve (ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2010-01-13 18:36:41 +0000
3+++ configs/development/launchpad-lazr.conf 2010-01-20 22:01:13 +0000
4@@ -130,6 +130,7 @@
5 branchlisting_batch_size: 6
6 mugshot_batch_size: 8
7 announcement_batch_size: 4
8+download_batch_size: 4
9 openid_preauthorization_acl:
10 localhost http://launchpad.dev/
11 max_bug_feed_cache_minutes: 30
12
13=== modified file 'lib/canonical/config/schema-lazr.conf'
14--- lib/canonical/config/schema-lazr.conf 2010-01-12 18:21:29 +0000
15+++ lib/canonical/config/schema-lazr.conf 2010-01-20 22:01:13 +0000
16@@ -935,6 +935,10 @@
17 # The default size used in a batched display of announcements.
18 announcement_batch_size: 5
19
20+# The default size used in a batched display of product download
21+# files. The releases are batched, not the individual files.
22+download_batch_size: 10
23+
24 # If restrict_to_team is set (such as on the beta
25 # website), then this indicates the hostname suffix for
26 # the non-restricted version of Launchpad. Replacing
27
28=== modified file 'lib/canonical/launchpad/icing/style.css'
29--- lib/canonical/launchpad/icing/style.css 2010-01-14 22:07:30 +0000
30+++ lib/canonical/launchpad/icing/style.css 2010-01-20 22:01:13 +0000
31@@ -1494,19 +1494,18 @@
32 max-width: 55em;
33 }
34
35-#search-results div.upper-batch-nav {
36+#search-results .upper-batch-nav, .freeform-batch .upper-batch-nav {
37 border-top: 1px solid #d2d2d2;
38 margin-top: 2.5em;
39- margin-bottom: 0.5em;
40-}
41-#search-results .search-batch {
42+ margin-bottom: 10px;
43 background: #e7effc;
44- margin-bottom: 10px;
45 }
46
47-#search-results div.lower-batch-nav {
48+#search-results .lower-batch-nav, .freeform-batch .lower-batch-nav {
49 margin-top: 1.5em;
50 border-top: 1px solid #d2d2d2;
51+ margin-bottom: 10px;
52+ background: #e7effc;
53 }
54
55 #search-results .search-result-link {
56
57=== modified file 'lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt'
58--- lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2009-12-21 11:36:04 +0000
59+++ lib/canonical/launchpad/templates/batchnavigator-navigation-links.pt 2010-01-20 22:01:13 +0000
60@@ -8,10 +8,11 @@
61 total context/batch/total;
62 size context/batch/size"
63 style="width: 100%;"
64+ tal:attributes="class view/css_class"
65 >
66 <tbody>
67 <tr>
68- <td style="white-space: nowrap"
69+ <td style="white-space: nowrap"
70 class="batch-navigation-index">
71 <strong tal:content="context/batch/startNumber">1</strong>
72 <tal:block condition="python: size > 1">
73@@ -22,7 +23,7 @@
74 <tal:total replace="total">42</tal:total>
75 <tal:heading content="context/heading">results</tal:heading>
76 </td>
77- <td style="text-align: right; white-space: nowrap"
78+ <td style="text-align: right; white-space: nowrap"
79 class="batch-navigation-links">
80 <a
81 tal:condition="first_page_url"
82
83=== modified file 'lib/canonical/launchpad/webapp/batching.py'
84--- lib/canonical/launchpad/webapp/batching.py 2009-08-18 22:26:02 +0000
85+++ lib/canonical/launchpad/webapp/batching.py 2010-01-20 22:01:13 +0000
86@@ -35,6 +35,8 @@
87 class UpperBatchNavigationView(LaunchpadView):
88 """Only render navigation links if there is a batch."""
89
90+ css_class = "upper-batch-nav"
91+
92 def render(self):
93 if self.context.currentBatch():
94 return LaunchpadView.render(self)
95@@ -44,6 +46,8 @@
96 class LowerBatchNavigationView(UpperBatchNavigationView):
97 """Only render bottom navigation links if there are multiple batches."""
98
99+ css_class = "lower-batch-nav"
100+
101 def render(self):
102 if (self.context.currentBatch() and
103 (self.context.nextBatchURL() or
104
105=== modified file 'lib/lp/registry/browser/product.py'
106--- lib/lp/registry/browser/product.py 2010-01-11 13:46:49 +0000
107+++ lib/lp/registry/browser/product.py 2010-01-20 22:01:13 +0000
108@@ -712,10 +712,11 @@
109 def processDeleteFiles(self):
110 """If the 'delete_files' button was pressed, process the deletions."""
111 del_count = None
112- self.delete_ids = [int(value) for key, value in self.form.items()
113- if key.startswith('checkbox')]
114 if 'delete_files' in self.form:
115 if self.request.method == 'POST':
116+ self.delete_ids = [
117+ int(value) for key, value in self.form.items()
118+ if key.startswith('checkbox')]
119 del(self.form['delete_files'])
120 releases = self.getReleases()
121 del_count = self.deleteFiles(releases)
122@@ -979,12 +980,25 @@
123 return results
124
125
126+class SeriesReleasePair:
127+ """Class for holding a series and release.
128+
129+ Replaces the use of a (series, release) tuple so that it can be more
130+ clearly addressed in the view class.
131+ """
132+ def __init__(self, series, release):
133+ self.series = series
134+ self.release = release
135+
136+
137 class ProductDownloadFilesView(LaunchpadView,
138 SortSeriesMixin,
139 ProductDownloadFileMixin):
140 """View class for the product's file downloads page."""
141 __used_for__ = IProduct
142
143+ batch_size = config.launchpad.download_batch_size
144+
145 @property
146 def page_title(self):
147 return "%s project files" % self.context.displayname
148@@ -1003,6 +1017,24 @@
149 return releases
150
151 @cachedproperty
152+ def series_and_releases_batch(self):
153+ """Get a batch of series and release
154+
155+ Each entry returned is a tuple of (series, release).
156+ """
157+ series_and_releases = []
158+ for series in self.sorted_series_list:
159+ for release in series.releases:
160+ if len(release.files) > 0:
161+ pair = SeriesReleasePair(series, release)
162+ if pair not in series_and_releases:
163+ series_and_releases.append(pair)
164+ batch = BatchNavigator(series_and_releases, self.request,
165+ size=self.batch_size)
166+ batch.setHeadings("release", "releases")
167+ return batch
168+
169+ @cachedproperty
170 def has_download_files(self):
171 """Across series and releases do any download files exist?"""
172 for series in self.product.series:
173
174=== added file 'lib/lp/registry/browser/tests/product-files-views.txt'
175--- lib/lp/registry/browser/tests/product-files-views.txt 1970-01-01 00:00:00 +0000
176+++ lib/lp/registry/browser/tests/product-files-views.txt 2010-01-20 22:01:13 +0000
177@@ -0,0 +1,70 @@
178+ Product Download Files Page
179+=============================
180+
181+Test for the product/+download page.
182+
183+ >>> product = factory.makeProduct(name='alfajore')
184+ >>> productseries = factory.makeSeries(product=product, name="sammy")
185+ >>> milestone = factory.makeMilestone(productseries=productseries,
186+ ... name="apple")
187+ >>> release_file = factory.makeProductReleaseFile(
188+ ... product=product, productseries=productseries, milestone=milestone)
189+ >>> view = create_initialized_view(product, '+download')
190+
191+ >>> view.batch_size
192+ 4
193+
194+ >>> batch = view.series_and_releases_batch.currentBatch()
195+ >>> print len(list(batch))
196+ 1
197+
198+ >>> def print_series_release(sr):
199+ ... print "%s from the %s series" % (sr.release.name_with_codename,
200+ ... sr.series.series.name)
201+
202+ >>> for sr in batch:
203+ ... print_series_release(sr)
204+ apple from the sammy series
205+
206+ >>> product = factory.makeProduct(name='bombilla')
207+ >>> for i in range(1,5):
208+ ... productseries = factory.makeSeries(product=product, name="s%d"%i)
209+ ... for j in range(1,4):
210+ ... milestone = factory.makeMilestone(productseries=productseries,
211+ ... name="%d.%d"%(i,j))
212+ ... release_file = factory.makeProductReleaseFile(
213+ ... product=product, productseries=productseries, milestone=milestone)
214+ >>> view = create_initialized_view(product, '+download')
215+ >>> batch = view.series_and_releases_batch.currentBatch()
216+ >>> print len(batch)
217+ 4
218+ >>> for sr in batch:
219+ ... print_series_release(sr)
220+ 4.3 from the s4 series
221+ 4.2 from the s4 series
222+ 4.1 from the s4 series
223+ 3.3 from the s3 series
224+
225+ >>> batch = batch.nextBatch()
226+ >>> for sr in batch:
227+ ... print_series_release(sr)
228+ 3.2 from the s3 series
229+ 3.1 from the s3 series
230+ 2.3 from the s2 series
231+ 2.2 from the s2 series
232+
233+For an administrator of the project, at the bottom of each batched
234+page will be links to add new files for each series and release.
235+
236+ >>> from canonical.launchpad.testing.pages import (
237+ ... extract_text, find_tag_by_id)
238+ >>> login_person(product.owner)
239+ >>> view = create_initialized_view(product, '+download',
240+ ... principal=product.owner)
241+ >>> admin_links = find_tag_by_id(view.render(), 'admin-links')
242+ >>> content = extract_text(admin_links)
243+ >>> print content
244+ Add download file to the s4 series for release: 4.3, 4.2, 4.1
245+ Add download file to the s3 series for release: 3.3, 3.2, 3.1
246+ Add download file to the s2 series for release: 2.3, 2.2, 2.1
247+ Add download file to the s1 series for release: 1.3, 1.2, 1.1
248
249=== modified file 'lib/lp/registry/browser/tests/test_views.py'
250--- lib/lp/registry/browser/tests/test_views.py 2010-01-09 16:48:38 +0000
251+++ lib/lp/registry/browser/tests/test_views.py 2010-01-20 22:01:13 +0000
252@@ -27,6 +27,7 @@
253 'user-to-user-views.txt': LaunchpadFunctionalLayer,
254 'distributionsourcepackage-views.txt': LaunchpadFunctionalLayer,
255 'product-edit-people-view.txt': LaunchpadFunctionalLayer,
256+ 'product-files-views.txt': LaunchpadFunctionalLayer,
257 }
258
259
260
261=== modified file 'lib/lp/registry/stories/product/xx-product-files.txt'
262--- lib/lp/registry/stories/product/xx-product-files.txt 2009-11-10 01:00:23 +0000
263+++ lib/lp/registry/stories/product/xx-product-files.txt 2010-01-20 22:01:13 +0000
264@@ -407,8 +407,9 @@
265 >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
266 >>> firefox_owner.url
267 'http://launchpad.dev/firefox/+download'
268+
269 >>> checkbox = firefox_owner.getControl(
270- ... name="checkbox_2_0_1")
271+ ... name="checkbox_2_1")
272 >>> checkbox.value = checkbox.options
273 >>> firefox_owner.getControl("Delete Files").click()
274 >>> get_feedback_messages(firefox_owner.contents)
275
276=== modified file 'lib/lp/registry/templates/product-files.pt'
277--- lib/lp/registry/templates/product-files.pt 2009-10-26 16:57:55 +0000
278+++ lib/lp/registry/templates/product-files.pt 2010-01-20 22:01:13 +0000
279@@ -7,9 +7,7 @@
280 i18n:domain="launchpad"
281 >
282
283-<h1 metal:fill-slot="heading">
284- Download project files
285-</h1>
286+<h1 metal:fill-slot="heading">Download project files</h1>
287
288 <body>
289
290@@ -37,117 +35,135 @@
291 </p>
292 </div>
293
294- <div
295- tal:repeat="series view/sorted_series_list"
296- tal:attributes="id series/name/fmt:css-id/series-">
297-
298- <tal:seriesfilesexist condition="series/has_release_files">
299- <div tal:condition="not: repeat/series/start" class="portlet-border" />
300- <tal:releases repeat="release series/releases">
301- <tal:releasefilesexist condition="release/files">
302- <div tal:condition="not: repeat/release/start" class="portlet-border" />
303- <div class="top-portlet">
304- <h2 style="font-size: 1.2em;">
305- <a tal:attributes="href release/fmt:url">
306- <span tal:replace="release/name_with_codename" /> release</a>
307- from the
308- <a tal:attributes="href series/fmt:url"
309- tal:content="series/name">name</a> series released
310- <span
311- tal:attributes="title release/datereleased/fmt:datetime"
312- tal:content="release/datereleased/fmt:approximatedate" />
313- </h2>
314-
315- <div tal:attributes="id release/version/fmt:css-id/release-information-"
316- tal:condition="python: release.release_notes or release.changelog">
317-
318- <fieldset class="collapsible collapsed" style="padding: 0px;">
319- <legend>Release information</legend>
320-
321- <div tal:condition="release/release_notes">
322- <strong>Release notes:</strong>
323- <div style="margin-bottom: 0px;"
324- tal:attributes="id release/version/fmt:css-id/release-notes-"
325- tal:define="notes release/release_notes/fmt:shorten/800"
326- tal:content="structure notes/fmt:text-to-html">
327- ProductRelease.release_notes
328- </div>
329- </div>
330-
331- <div tal:condition="release/changelog">
332- <strong>Changelog:</strong>
333- <div style="margin-bottom: 0px;"
334- tal:attributes="id release/version/fmt:css-id/changelog-"
335- tal:content="structure release/changelog/fmt:obfuscate-email/fmt:text-to-html">
336- ProductRelease.changelog.
337- </div>
338- </div>
339- </fieldset>
340- </div>
341- <div>
342- <table class="listing" style="margin-top: 1em;">
343- <thead>
344- <tr>
345- <th width="35%">File</th>
346- <th width="45%">Description</th>
347- <th>Downloads</th>
348- <th tal:condition="has_edit">Delete</th>
349- </tr>
350- </thead>
351- <tbody>
352- <tr tal:repeat="file release/files">
353- <tal:define-vars
354- tal:define="checkbox_index string:${repeat/series/index}_${repeat/release/index}_${repeat/file/index}">
355- <tal:release-file
356- metal:use-macro="file/@@+macros/detailed_display" />
357- </tal:define-vars>
358- </tr>
359- </tbody>
360- <tfoot tal:condition="release/total_downloads">
361- <tr>
362- <th colspan="2" style="padding-top: 1em; text-align: right;">
363- Total downloads:
364- </th>
365- <td style="border: none; text-align: center;">
366- <span tal:replace="release/total_downloads/fmt:intcomma" />
367- </td>
368- <td style="border: none;"
369- tal:condition="has_edit" />
370- </tr>
371- </tfoot>
372- </table>
373- </div>
374- <div tal:condition="not: release/total_downloads"
375- style="margin-top: 2em;" />
376-
377- <tal:linkexists
378- define="link release/menu:context/add_file"
379- condition="link/enabled">
380- <ul style="float: right; margin-top: 1em;">
381- <li>
382- <a tal:replace="structure link/fmt:link" />
383+ <div class="freeform-batch">
384+ <tal:batch condition="view/has_download_files"
385+ define="batch view/series_and_releases_batch">
386+ <tal:multipage tal:condition="batch/has_multiple_pages">
387+ <tal:navigation
388+ replace="structure batch/@@+navigation-links-upper"/>
389+ </tal:multipage>
390+ <tal:repeat_current_batch repeat="batch_item batch/currentBatch">
391+ <div tal:define="series batch_item/series;
392+ release batch_item/release"
393+ tal:attributes="id series/name/fmt:css-id/series-">
394+
395+ <tal:seriesfilesexist condition="series/has_release_files">
396+ <div tal:condition="not: repeat/batch_item/start" class="portlet-border" />
397+
398+ <tal:releasefilesexist condition="release/files">
399+
400+ <div class="top-portlet">
401+ <h2 style="font-size: 1.2em;">
402+ <a tal:attributes="href release/fmt:url">
403+ <span tal:replace="release/name_with_codename" /> release</a>
404+ from the
405+ <a tal:attributes="href series/fmt:url"
406+ tal:content="series/name">name</a> series released
407+ <span
408+ tal:attributes="title release/datereleased/fmt:datetime"
409+ tal:content="release/datereleased/fmt:approximatedate" />
410+ </h2>
411+
412+ <div tal:attributes="id release/version/fmt:css-id/release-information-"
413+ tal:condition="python: release.release_notes or release.changelog">
414+
415+ <fieldset class="collapsible collapsed" style="padding: 0px;">
416+ <legend>Release information</legend>
417+
418+ <div tal:condition="release/release_notes">
419+ <strong>Release notes:</strong>
420+ <div style="margin-bottom: 0px;"
421+ tal:attributes="id release/version/fmt:css-id/release-notes-"
422+ tal:define="notes release/release_notes/fmt:shorten/800"
423+ tal:content="structure notes/fmt:text-to-html">
424+ ProductRelease.release_notes
425+ </div>
426+ </div>
427+
428+ <div tal:condition="release/changelog">
429+ <strong>Changelog:</strong>
430+ <div style="margin-bottom: 0px;"
431+ tal:attributes="id release/version/fmt:css-id/changelog-"
432+ tal:content="structure release/changelog/fmt:obfuscate-email/fmt:text-to-html">
433+ ProductRelease.changelog.
434+ </div>
435+ </div>
436+ </fieldset>
437+ </div>
438+ <div>
439+ <table class="listing" style="margin-top: 1em;">
440+ <thead>
441+ <tr>
442+ <th width="35%">File</th>
443+ <th width="45%">Description</th>
444+ <th>Downloads</th>
445+ <th tal:condition="has_edit">Delete</th>
446+ </tr>
447+ </thead>
448+ <tbody>
449+ <tr tal:repeat="file release/files">
450+ <tal:define-vars
451+ tal:define="checkbox_index string:${repeat/batch_item/index}_${repeat/file/index}">
452+ <tal:release-file
453+ metal:use-macro="file/@@+macros/detailed_display" />
454+ </tal:define-vars>
455+ </tr>
456+ </tbody>
457+ <tfoot tal:condition="release/total_downloads">
458+ <tr>
459+ <th colspan="2" style="padding-top: 1em; text-align: right;">
460+ Total downloads:
461+ </th>
462+ <td style="border: none; text-align: center;">
463+ <span tal:replace="release/total_downloads/fmt:intcomma" />
464+ </td>
465+ <td style="border: none;"
466+ tal:condition="has_edit" />
467+ </tr>
468+ </tfoot>
469+ </table>
470+ </div>
471+ <div tal:condition="not: release/total_downloads"
472+ style="margin-bottom: 1em;" />
473+
474+ <tal:linkexists
475+ define="link release/menu:context/add_file"
476+ condition="link/enabled">
477+ <ul style="float: right; margin-top: 1em;">
478+ <li>
479+ <a tal:replace="structure link/fmt:link" />
480 </li>
481- </ul>
482- </tal:linkexists>
483- </div>
484- </tal:releasefilesexist>
485+ </ul>
486+ </tal:linkexists>
487+ </div>
488+ </tal:releasefilesexist>
489+
490+ <br />
491+ </tal:seriesfilesexist>
492+
493+ </div>
494+ </tal:repeat_current_batch>
495+ <tal:multipage condition="batch/has_multiple_pages">
496+ <tal:navigation
497+ replace="structure batch/@@+navigation-links-lower"/>
498+ </tal:multipage>
499+ </tal:batch>
500+ </div>
501+ <div id="admin-links">
502+ <tal:series repeat="series view/sorted_series_list">
503+ <tal:releases condition="series/releases">
504+ <p tal:condition="has_edit" class="add-files">
505+ Add download file to the
506+ <a tal:attributes="href series/fmt:url"
507+ tal:content="series/name">name</a> series for release:
508+ <tal:release repeat="release series/releases">
509+ <a tal:attributes="href string:${series/name}/${release/version}/+adddownloadfile"
510+ tal:content="release/version"
511+ >version</a><tal:comma condition="not:repeat/release/end">,</tal:comma>
512+ </tal:release>
513+ </p>
514 </tal:releases>
515- <br />
516- </tal:seriesfilesexist>
517-
518- <tal:releases condition="series/releases">
519- <p tal:condition="has_edit" class="add-files">
520- Add download file to the
521- <a tal:attributes="href series/fmt:url"
522- tal:content="series/name">name</a> series for release:
523- <tal:release repeat="release series/releases">
524- <a tal:attributes="href string:${series/name}/${release/version}/+adddownloadfile"
525- tal:content="release/version"
526- >version</a><tal:comma condition="not:repeat/release/end">,</tal:comma>
527- </tal:release>
528- </p>
529- </tal:releases>
530-
531+ </tal:series>
532 </div>
533 <input tal:condition="python: has_edit and view.has_download_files"
534 type="submit"
535@@ -157,5 +173,4 @@
536 </form>
537 </div>
538 </body>
539-</html
540->
541+</html>
542
543=== modified file 'lib/lp/registry/templates/productreleasefile-macros.pt'
544--- lib/lp/registry/templates/productreleasefile-macros.pt 2009-07-17 17:59:07 +0000
545+++ lib/lp/registry/templates/productreleasefile-macros.pt 2010-01-20 22:01:13 +0000
546@@ -10,7 +10,7 @@
547 - 'file': The ProductReleaseFile
548 - 'has_edit': A boolean saying whether the user has edit rights on this
549 ProductReleaseFile.
550- - checkbox_index: A unique integer to be appended to the name of the
551+ - checkbox_index: A unique string to be appended to the name of the
552 checkbox included in this macro.
553 </tal:comment>
554
555
556=== modified file 'lib/lp/testing/factory.py'
557--- lib/lp/testing/factory.py 2010-01-15 04:09:45 +0000
558+++ lib/lp/testing/factory.py 2010-01-20 22:01:13 +0000
559@@ -76,7 +76,7 @@
560 from lp.translations.interfaces.translationgroup import (
561 ITranslationGroupSet)
562 from lp.translations.interfaces.translator import ITranslatorSet
563-from lp.translations.interfaces.translationsperson import ITranslationsPerson
564+from lp.translations.interfaces.translationsperson import ITranslationsPerson
565 from canonical.launchpad.ftests._sqlobject import syncUpdate
566 from lp.services.mail.signedmessage import SignedMessage
567 from lp.services.worlddata.interfaces.country import ICountrySet
568@@ -512,7 +512,8 @@
569 group = self.makeTranslationGroup()
570 if person is None:
571 person = self.makePerson()
572- ITranslationsPerson(person).translations_relicensing_agreement = license
573+ ITranslationsPerson(person).translations_relicensing_agreement = (
574+ license)
575 return getUtility(ITranslatorSet).new(group, language, person)
576
577 def makeMilestone(
578@@ -527,25 +528,34 @@
579 productseries=productseries,
580 name=name)
581
582- def makeProductRelease(self, milestone=None, product=None):
583+ def makeProductRelease(self, milestone=None, product=None,
584+ productseries=None):
585 if milestone is None:
586- milestone = self.makeMilestone(product=product)
587+ milestone = self.makeMilestone(product=product,
588+ productseries=productseries)
589 return milestone.createProductRelease(
590 milestone.product.owner, datetime.now(pytz.UTC))
591
592- def makeProductReleaseFile(self, signed=True):
593+ def makeProductReleaseFile(self, signed=True,
594+ product=None, productseries=None,
595+ milestone=None,
596+ release=None,
597+ description="test file"):
598 signature_filename = None
599 signature_content = None
600 if signed:
601 signature_filename = 'test.txt.asc'
602 signature_content = '123'
603- release = self.makeProductRelease()
604+ if release is None:
605+ release = self.makeProductRelease(product=product,
606+ productseries=productseries,
607+ milestone=milestone)
608 return release.addReleaseFile(
609 'test.txt', 'test', 'text/plain',
610 uploader=release.milestone.product.owner,
611 signature_filename=signature_filename,
612 signature_content=signature_content,
613- description="test file")
614+ description=description)
615
616 def makeProduct(self, *args, **kwargs):
617 """As makeProductNoCommit with an explicit transaction commit.