Launchpad itself

Merge lp:~jpds/launchpad/fix_196173 into lp:launchpad

  1. fix_196173
  2. Merge into devel
Proposed by Jonathan Davies
Status: Merged
Approved by: Brad Crittenden
Approved revision: not available
Merged at revision: 10013
Proposed branch: lp:~jpds/launchpad/fix_196173
Merge into: lp:launchpad
Diff against target: 168 lines (+29/-22)
7 files modified
lib/lp/registry/browser/distributionmirror.py (+5/-4)
lib/lp/registry/browser/tests/distributionmirror-views.txt (+7/-4)
lib/lp/registry/configure.zcml (+4/-4)
lib/lp/registry/doc/distribution-mirror.txt (+3/-1)
lib/lp/registry/interfaces/distribution.py (+3/-3)
lib/lp/registry/model/distribution.py (+5/-4)
lib/lp/registry/templates/distributionmirror-index.pt (+2/-2)
To merge this branch: bzr merge lp:~jpds/launchpad/fix_196173
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+15807@code.launchpad.net

Commit message

Added mirror whiteboard to +newmirror page for private comments, and allow mirror admins to view the whiteboards for their mirrors too.

To post a comment you must log in.
Revision history for this message
Jonathan Davies (jpds) wrote :

= Summary =

When a user registers a new mirror, it would be beneficial for him to be able to leave comments for the mirror-list admins, similar to the whiteboard that we have when we review a mirror.

This branch adds the whiteboard to the +newmirror page, so that the registrars may leave private comments if they wish to.

It also actually makes the whiteboard viewable by mirror registrars on their mirror pages, as we thought it did all along.

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

Hi Jonathan,

Thanks again for another mirror branch.

As we discussed on IRC there are some problems that need fixing and I know you've already started on some.

First, the newMirror method needs to add support for a whiteboard. That needs to be added to interface/distribution.py and model/distribution.py.
When adding a new optional argument to an existing method it is usually best to tack it onto the end. That way you don't break existing call sites.

The doctests for newMirror are in doc/distribution-mirror.txt. You'll need to update those to ensure the whiteboard is accepted and set properly. Since the permissions for that field changed you'll need tests to ensure registrants and admins can set the whiteboard.

There may be more work that needs to be done. But have a go at these issues and we'll take another pass.

review: Needs Fixing (code)
Revision history for this message
Jonathan Davies (jpds) wrote :

Issues resolved and corrections based on IRC discussions done. All tests pass.

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

The changes you've made look good Jonathan. I'll be happy to land this branch for you.

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/distributionmirror.py'
--- lib/lp/registry/browser/distributionmirror.py 2009-11-19 14:18:48 +0000
+++ lib/lp/registry/browser/distributionmirror.py 2009-12-09 10:12:18 +0000
@@ -189,7 +189,7 @@
189189
190 implements(IDistributionMirrorMenuMarker)190 implements(IDistributionMirrorMenuMarker)
191 schema = IDistributionMirror191 schema = IDistributionMirror
192 field_names = ["displayname", "description", "http_base_url",192 field_names = ["displayname", "description", "whiteboard", "http_base_url",
193 "ftp_base_url", "rsync_base_url", "speed", "country",193 "ftp_base_url", "rsync_base_url", "speed", "country",
194 "content", "official_candidate"]194 "content", "official_candidate"]
195 @property195 @property
@@ -213,6 +213,7 @@
213 owner=self.user, speed=data['speed'], country=data['country'],213 owner=self.user, speed=data['speed'], country=data['country'],
214 content=data['content'], displayname=data['displayname'],214 content=data['content'], displayname=data['displayname'],
215 description=data['description'],215 description=data['description'],
216 whiteboard=data['whiteboard'],
216 http_base_url=data['http_base_url'],217 http_base_url=data['http_base_url'],
217 ftp_base_url=data['ftp_base_url'],218 ftp_base_url=data['ftp_base_url'],
218 rsync_base_url=data['rsync_base_url'],219 rsync_base_url=data['rsync_base_url'],
@@ -255,9 +256,9 @@
255class DistributionMirrorEditView(LaunchpadEditFormView):256class DistributionMirrorEditView(LaunchpadEditFormView):
256257
257 schema = IDistributionMirror258 schema = IDistributionMirror
258 field_names = ["name", "displayname", "description", "http_base_url",259 field_names = ["name", "displayname", "description", "whiteboard",
259 "ftp_base_url", "rsync_base_url", "speed", "country",260 "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
260 "content", "official_candidate"]261 "country", "content", "official_candidate"]
261 @property262 @property
262 def label(self):263 def label(self):
263 """See `LaunchpadFormView`."""264 """See `LaunchpadFormView`."""
264265
=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
--- lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-04 20:21:15 +0000
+++ lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-09 10:12:18 +0000
@@ -49,12 +49,14 @@
49A HTTP or FTP URL is required to register a mirror.49A HTTP or FTP URL is required to register a mirror.
5050
51 >>> view.field_names51 >>> view.field_names
52 ['displayname', 'description', 'http_base_url', 'ftp_base_url',52 ['displayname', 'description', 'whiteboard', 'http_base_url',
53 'rsync_base_url', 'speed', 'country', 'content', 'official_candidate']53 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
54 'official_candidate']
5455
55 >>> form = {56 >>> form = {
56 ... 'field.displayname': 'Illuminati',57 ... 'field.displayname': 'Illuminati',
57 ... 'field.description': 'description',58 ... 'field.description': 'description',
59 ... 'field.whiteboard': 'whiteboard',
58 ... 'field.http_base_url': 'http://secret.me/',60 ... 'field.http_base_url': 'http://secret.me/',
59 ... 'field.ftp_base_url': '',61 ... 'field.ftp_base_url': '',
60 ... 'field.rsync_base_url': '',62 ... 'field.rsync_base_url': '',
@@ -239,8 +241,9 @@
239The user can edit the mirror fields.241The user can edit the mirror fields.
240242
241 >>> view.field_names243 >>> view.field_names
242 ['name', 'displayname', 'description', 'http_base_url', 'ftp_base_url',244 ['name', 'displayname', 'description', 'whiteboard', 'http_base_url',
243 'rsync_base_url', 'speed', 'country', 'content', 'official_candidate']245 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
246 'official_candidate']
244247
245 >>> print mirror.ftp_base_url248 >>> print mirror.ftp_base_url
246 None249 None
247250
=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml 2009-12-05 18:37:28 +0000
+++ lib/lp/registry/configure.zcml 2009-12-09 10:12:18 +0000
@@ -1606,12 +1606,12 @@
1606 <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />1606 <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />
1607 <require1607 <require
1608 permission="launchpad.Edit"1608 permission="launchpad.Edit"
1609 set_attributes="name displayname description http_base_url1609 set_attributes="name displayname description whiteboard
1610 ftp_base_url rsync_base_url enabled speed1610 http_base_url ftp_base_url rsync_base_url enabled
1611 country content official_candidate owner" />1611 speed country content official_candidate owner" />
1612 <require1612 <require
1613 permission="launchpad.Admin"1613 permission="launchpad.Admin"
1614 set_attributes="status whiteboard reviewer date_reviewed"1614 set_attributes="status reviewer date_reviewed"
1615 attributes="destroySelf" />1615 attributes="destroySelf" />
1616 </class>1616 </class>
16171617
16181618
=== modified file 'lib/lp/registry/doc/distribution-mirror.txt'
--- lib/lp/registry/doc/distribution-mirror.txt 2009-11-20 03:09:59 +0000
+++ lib/lp/registry/doc/distribution-mirror.txt 2009-12-09 10:12:18 +0000
@@ -18,8 +18,10 @@
18 >>> brazil = getUtility(ICountrySet)['BR']18 >>> brazil = getUtility(ICountrySet)['BR']
19 >>> content = MirrorContent.ARCHIVE19 >>> content = MirrorContent.ARCHIVE
20 >>> http_base_url = 'http://foo.bar.com/pub'20 >>> http_base_url = 'http://foo.bar.com/pub'
21 >>> whiteboard = "This mirror is based deep in the Amazon rainforest."
21 >>> new_mirror = ubuntu.newMirror(22 >>> new_mirror = ubuntu.newMirror(
22 ... owner, speed, brazil, content, http_base_url=http_base_url)23 ... owner, speed, brazil, content, http_base_url=http_base_url,
24 ... whiteboard=whiteboard)
2325
24When a new mirror is created we'll generate a unique name for it, based26When a new mirror is created we'll generate a unique name for it, based
25on its host name and content.27on its host name and content.
2628
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2009-12-05 18:37:28 +0000
+++ lib/lp/registry/interfaces/distribution.py 2009-12-09 10:12:18 +0000
@@ -310,9 +310,9 @@
310 """310 """
311311
312 def newMirror(owner, speed, country, content, displayname=None,312 def newMirror(owner, speed, country, content, displayname=None,
313 description=None, http_base_url=None, ftp_base_url=None,313 description=None, http_base_url=None,
314 rsync_base_url=None, enabled=False,314 ftp_base_url=None, rsync_base_url=None, enabled=False,
315 official_candidate=False):315 official_candidate=False, whiteboard=None):
316 """Create a new DistributionMirror for this distribution.316 """Create a new DistributionMirror for this distribution.
317317
318 At least one of http_base_url or ftp_base_url must be provided in318 At least one of http_base_url or ftp_base_url must be provided in
319319
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2009-12-05 18:37:28 +0000
+++ lib/lp/registry/model/distribution.py 2009-12-09 10:12:18 +0000
@@ -393,9 +393,9 @@
393 return DistributionMirror.selectOneBy(distribution=self, name=name)393 return DistributionMirror.selectOneBy(distribution=self, name=name)
394394
395 def newMirror(self, owner, speed, country, content, displayname=None,395 def newMirror(self, owner, speed, country, content, displayname=None,
396 description=None, http_base_url=None, ftp_base_url=None,396 description=None, http_base_url=None,
397 rsync_base_url=None, official_candidate=False,397 ftp_base_url=None, rsync_base_url=None, official_candidate=False,
398 enabled=False):398 enabled=False, whiteboard=None):
399 """See `IDistribution`."""399 """See `IDistribution`."""
400 # NB this functionality is only available to distributions that have400 # NB this functionality is only available to distributions that have
401 # the full functionality of Launchpad enabled. This is Ubuntu and401 # the full functionality of Launchpad enabled. This is Ubuntu and
@@ -429,7 +429,8 @@
429 description=description, http_base_url=urls['http_base_url'],429 description=description, http_base_url=urls['http_base_url'],
430 ftp_base_url=urls['ftp_base_url'],430 ftp_base_url=urls['ftp_base_url'],
431 rsync_base_url=urls['rsync_base_url'],431 rsync_base_url=urls['rsync_base_url'],
432 official_candidate=official_candidate, enabled=enabled)432 official_candidate=official_candidate, enabled=enabled,
433 whiteboard=whiteboard)
433434
434 def createBug(self, bug_params):435 def createBug(self, bug_params):
435 """See canonical.launchpad.interfaces.IBugTarget."""436 """See canonical.launchpad.interfaces.IBugTarget."""
436437
=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
--- lib/lp/registry/templates/distributionmirror-index.pt 2009-12-04 12:09:11 +0000
+++ lib/lp/registry/templates/distributionmirror-index.pt 2009-12-09 10:12:18 +0000
@@ -27,14 +27,14 @@
27 <span tal:content="context/date_created/fmt:displaydate" />27 <span tal:content="context/date_created/fmt:displaydate" />
28 </p>28 </p>
2929
30 <tal:is-admin condition="context/required:launchpad.Admin">30 <tal:is-admin condition="context/required:launchpad.Edit">
31 </tal:is-admin>31 </tal:is-admin>
32 </div>32 </div>
33 <h2>Archive information</h2>33 <h2>Archive information</h2>
34 <div class="yui-g">34 <div class="yui-g">
35 <div class="yui-u first">35 <div class="yui-u first">
36 <div class="top-portlet">36 <div class="top-portlet">
37 <tal:is-admin condition="context/required:launchpad.Admin">37 <tal:is-admin condition="context/required:launchpad.Edit">
38 <dl id="whiteboard">38 <dl id="whiteboard">
39 <dt>39 <dt>
40 Whiteboard40 Whiteboard