Merge lp://staging/~bac/launchpad/bug-426190-etc into lp://staging/launchpad

Proposed by Brad Crittenden
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~bac/launchpad/bug-426190-etc
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~bac/launchpad/bug-426190-etc
Reviewer Review Type Date Requested Status
Paul Hummer (community) Approve
Review via email: mp+11551@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Bug 426190 suggests the +pendingreviewmirrors page add the type of mirror.
Bug 426445 points out an ungrammatical sentence due to the use of approximate dates.

== Proposed fix ==

Bug 426190
* Add a new column to the pending review mirrors page to show the archive type.
* Since pending archives could never have been updated I removed the freshness column.
* The test didn't show anything beyond the mirror name so it was beefed up to show
each line of output.

Bug 426445
* Fix the bug by removing the preceding 'on' and using fmt:displaydate, which
includes the preposition if necessary. Unfortunately it was rendering <strong>on
2009-09-09</strong> which looks funny. The <strong> was removed.
* Fixed another grammatical error where 'an Pending review' was used. The sentence
was re-worded to avoid the a/an problem.

== Pre-implementation notes ==

N/A

== Implementation details ==

As above.

== Tests ==

bin/test -vvm lp.registry -t mirror

== Demo and Q/A ==

Login as <email address hidden>

* Verify the table has 'Type' column, no freshness listed, and it is well-formed:
  https://launchpad.dev/ubuntu/+pendingreviewmirrors

* Verify the 'Last probe' message makes sense:
https://launchpad.dev/ubuntu/+mirror/random-releases-mirror

* Ensure the tables still look right:
https://launchpad.dev/ubuntu/+archivemirrors
https://launchpad.dev/ubuntu/+cdmirrors

* Verify grammar under 'Last probe':
https://launchpad.dev/ubuntu/+mirror/archive-mirror2

= 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/templates/distributionmirror-index.pt
  lib/lp/registry/stories/distribution/xx-distribution-mirrors.txt
  lib/lp/registry/templates/distribution-mirrors.pt
  lib/lp/registry/templates/distributionmirror-macros.pt
  lib/lp/registry/browser/distribution.py

Revision history for this message
Paul Hummer (rockstar) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/browser/distribution.py'
2--- lib/lp/registry/browser/distribution.py 2009-09-03 15:04:13 +0000
3+++ lib/lp/registry/browser/distribution.py 2009-09-10 18:53:09 +0000
4@@ -867,8 +867,7 @@
5
6 implements(IDistributionMirrorMenuMarker)
7 show_freshness = True
8- show_archive_mirror = True
9- show_cd_mirror = True
10+ show_mirror_type = False
11 description = None
12
13 @cachedproperty
14@@ -945,7 +944,6 @@
15 heading = 'Official Archive Mirrors'
16 description = ('These mirrors provide repositories and archives of all '
17 'software for the distribution.')
18- show_archive_mirror = False
19
20 @cachedproperty
21 def mirrors(self):
22@@ -957,7 +955,6 @@
23 heading = 'Official CD Mirrors'
24 description = ('These mirrors offer ISO images which you can download '
25 'and burn to CD to make installation disks.')
26- show_cd_mirror = False
27 show_freshness = False
28
29 @cachedproperty
30@@ -1025,6 +1022,8 @@
31 class DistributionPendingReviewMirrorsView(DistributionMirrorsAdminView):
32
33 heading = 'Pending-review mirrors'
34+ show_mirror_type = True
35+ show_freshness = False
36
37 @cachedproperty
38 def mirrors(self):
39
40=== modified file 'lib/lp/registry/stories/distribution/xx-distribution-mirrors.txt'
41--- lib/lp/registry/stories/distribution/xx-distribution-mirrors.txt 2009-08-26 11:23:25 +0000
42+++ lib/lp/registry/stories/distribution/xx-distribution-mirrors.txt 2009-09-10 18:53:09 +0000
43@@ -22,8 +22,8 @@
44 ... # from different countries, so we'll just skip it.
45 ... pass
46 ... else:
47- ... mirrors.append(extract_text(tr.find('td')))
48-
49+ ... tds = tuple([extract_text(td) for td in tr.findAll('td')])
50+ ... mirrors.append(tds)
51
52 == Official mirrors ==
53
54@@ -33,23 +33,35 @@
55 We have one page which lists all archive mirrors and another one which lists
56 the release (cdimage) ones.
57
58+The archive mirrors display the "freshness", how far behind they are.
59+
60 >>> browser.open('http://launchpad.dev/ubuntu')
61 >>> browser.getLink('Archive mirrors').click()
62 >>> browser.title
63 'Mirrors of Ubuntu Linux'
64 >>> print_mirrors_by_countries(browser.contents)
65- Antarctica: [u'Archive-mirror2']
66- France: [u'Archive-404-mirror', u'Archive-mirror']
67- United Kingdom: [u'Canonical-archive']
68+ Antarctica:
69+ [(u'Archive-mirror2', u'http', u'128 Kbps', u'Six hours behind')]
70+ France:
71+ [(u'Archive-404-mirror', u'http', u'512 Kbps', u'Last update unknown'),
72+ (u'Archive-mirror', u'http', u'128 Kbps', u'Last update unknown')]
73+ United Kingdom:
74+ [(u'Canonical-archive', u'http', u'100 Mbps', u'Last update unknown')]
75+
76+Freshness doesn't make sense for CD mirrors so it is not shown for them.
77
78 >>> browser.open('http://launchpad.dev/ubuntu')
79 >>> browser.getLink('CD mirrors').click()
80 >>> browser.url
81 'http://launchpad.dev/ubuntu/+cdmirrors'
82 >>> print_mirrors_by_countries(browser.contents)
83- France: [u'Releases-mirror', u'Unreachable-mirror']
84- Germany: [u'Releases-mirror2']
85- United Kingdom: [u'Canonical-releases']
86+ France:
87+ [(u'Releases-mirror', u'http', u'2 Mbps'),
88+ (u'Unreachable-mirror', u'http', u'512 Kbps')]
89+ Germany:
90+ [(u'Releases-mirror2', u'http', u'2 Mbps')]
91+ United Kingdom:
92+ [(u'Canonical-releases', u'http', u'100 Mbps')]
93
94 === Disabled mirrors ===
95
96@@ -89,13 +101,14 @@
97 'http://launchpad.dev/ubuntu/+unofficialmirrors'
98
99 >>> print_mirrors_by_countries(browser.contents)
100- France: [u'Invalid-mirror']
101-
102+ France: [(u'Invalid-mirror', u'http', u'2 Mbps', u'Last update unknown')]
103
104 == Pending-review mirrors ==
105
106 These are the mirrors that were created but none of the mirror admins have
107-looked at yet.
108+looked at yet. Since all pending mirrors are grouped on one page the
109+type of mirror is shown. Also the freshness is not visible since
110+pending mirrors have never been probed.
111
112 >>> user_browser.open('http://launchpad.dev/ubuntu/+pendingreviewmirrors')
113 Traceback (most recent call last):
114@@ -103,7 +116,21 @@
115 Unauthorized: ...
116
117 >>> browser = setupBrowser(auth='Basic karl@canonical.com:test')
118+ >>> # Register an unreviewed archive mirror.
119+ >>> browser.open('http://launchpad.dev/ubuntu/+newmirror')
120+ >>> browser.getControl(name='field.displayname').value = 'Kabul LUG mirror'
121+ >>> browser.getControl(name='field.ftp_base_url').value = (
122+ ... 'ftp://kabullug.org/ubuntu')
123+ >>> browser.getControl(name='field.country').value = ['1'] # Afghanistan
124+ >>> browser.getControl(name='field.speed').value = ["S10G"]
125+ >>> browser.getControl(name='field.content').value = ["ARCHIVE"]
126+ >>> browser.getControl('Register Mirror').click()
127+
128 >>> browser.open('http://launchpad.dev/ubuntu/+pendingreviewmirrors')
129-
130 >>> print_mirrors_by_countries(browser.contents)
131- United Kingdom: [u'Random-releases-mirror']
132+ Afghanistan:
133+ [(u'Kabul LUG mirror', u'ftp', u'10 Gbps',
134+ u'Archive')]
135+ United Kingdom:
136+ [(u'Random-releases-mirror', u'http', u'100 Mbps',
137+ u'CD Image')]
138
139=== modified file 'lib/lp/registry/templates/distribution-mirrors.pt'
140--- lib/lp/registry/templates/distribution-mirrors.pt 2009-08-26 21:39:05 +0000
141+++ lib/lp/registry/templates/distribution-mirrors.pt 2009-09-10 18:53:09 +0000
142@@ -46,6 +46,7 @@
143
144 <div tal:condition="mirrors_by_country"
145 tal:define="show_freshness view/show_freshness;
146+ show_mirror_type view/show_mirror_type;
147 total_mirror_count view/mirror_count;
148 total_throughput view/total_throughput">
149 <metal:mirror-list
150
151=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
152--- lib/lp/registry/templates/distributionmirror-index.pt 2009-08-28 21:06:02 +0000
153+++ lib/lp/registry/templates/distributionmirror-index.pt 2009-09-10 19:10:08 +0000
154@@ -85,17 +85,17 @@
155 </p>
156
157 <p tal:condition="not: context/isOfficial">
158- This mirror is an
159- <tal:status replace="context/status/title">Official</tal:status>
160- <span tal:replace="context/distribution/title">Ubuntu</span> mirror,
161- and will not be verified.
162+ This mirror of
163+ <tal:distro tal:replace="context/distribution/title">Ubuntu</tal:distro>
164+ will not be verified because its status is
165+ '<tal:status replace="context/status/title">Official</tal:status>'.
166 </p>
167 </tal:not-probe>
168 <tal:probe condition="probe">
169- <p>This mirror was last verified on
170- <strong
171+ <p>This mirror was last verified
172+ <span
173 tal:attributes="title probe/date_created/fmt:datetime"
174- tal:content="probe/date_created/fmt:approximatedate" />.
175+ tal:content="probe/date_created/fmt:displaydate" />.
176
177 <tal:is-owner condition="context/required:launchpad.Edit">
178 You can see the
179
180=== modified file 'lib/lp/registry/templates/distributionmirror-macros.pt'
181--- lib/lp/registry/templates/distributionmirror-macros.pt 2009-08-26 21:39:05 +0000
182+++ lib/lp/registry/templates/distributionmirror-macros.pt 2009-09-10 18:53:09 +0000
183@@ -8,8 +8,9 @@
184 This macro expects the following variables defined:
185 :mirrors_by_country: A list of dictionaries containing country names
186 and the list of IDistributionMirrors to be listed
187- :show_freshness: A boolean saying whether there should be an extra column
188- with the mirror's freshness or not.
189+ :show_mirror_type: A boolean indicating whether to display the mirror type.
190+ :show_freshness: A boolean saying indicating whether to display an extra
191+ column with the mirror's freshness.
192 </tal:comment>
193
194 <table class="listing" id="mirrors_list">
195@@ -20,6 +21,9 @@
196 tal:content="country_and_mirrors/country" />
197 <th style="text-align: left"
198 tal:content="country_and_mirrors/throughput"/>
199+ <th style="text-align: left" tal:condition="show_mirror_type">
200+ Type
201+ </th>
202 <th style="text-align: left"
203 tal:define="mirror_count country_and_mirrors/number">
204 <tal:count replace="mirror_count" />
205@@ -41,6 +45,9 @@
206 tal:attributes="href mirror/rsync_base_url">rsync</a>
207 </td>
208 <td><span tal:replace="mirror/speed/title" /></td>
209+ <td tal:condition="show_mirror_type">
210+ <span tal:replace="mirror/content/title" />
211+ </td>
212 <td tal:condition="show_freshness"
213 tal:define="freshness mirror/getOverallFreshness">
214 <span tal:replace="freshness/title" />
215@@ -57,12 +64,13 @@
216
217 </tal:country_and_mirrors>
218 <tr class="highlighted">
219- <th colspan="4" style="text-align: left; font-weight: bold;">Total</th>
220+ <th colspan="5" style="text-align: left; font-weight: bold;">Total</th>
221 </tr>
222 <tr>
223 <td colspan="2" />
224 <td style="text-align: left; font-weight: bold;"
225 tal:content="total_throughput" />
226+ <td tal:condition="show_mirror_type"></td>
227 <td style="text-align: left; font-weight: bold;">
228 <tal:count replace="total_mirror_count" />
229 <span tal:condition="python:total_mirror_count == 1">mirror</span>