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
1=== modified file 'lib/lp/registry/browser/distributionmirror.py'
2--- lib/lp/registry/browser/distributionmirror.py 2009-11-19 14:18:48 +0000
3+++ lib/lp/registry/browser/distributionmirror.py 2009-12-09 10:12:18 +0000
4@@ -189,7 +189,7 @@
5
6 implements(IDistributionMirrorMenuMarker)
7 schema = IDistributionMirror
8- field_names = ["displayname", "description", "http_base_url",
9+ field_names = ["displayname", "description", "whiteboard", "http_base_url",
10 "ftp_base_url", "rsync_base_url", "speed", "country",
11 "content", "official_candidate"]
12 @property
13@@ -213,6 +213,7 @@
14 owner=self.user, speed=data['speed'], country=data['country'],
15 content=data['content'], displayname=data['displayname'],
16 description=data['description'],
17+ whiteboard=data['whiteboard'],
18 http_base_url=data['http_base_url'],
19 ftp_base_url=data['ftp_base_url'],
20 rsync_base_url=data['rsync_base_url'],
21@@ -255,9 +256,9 @@
22 class DistributionMirrorEditView(LaunchpadEditFormView):
23
24 schema = IDistributionMirror
25- field_names = ["name", "displayname", "description", "http_base_url",
26- "ftp_base_url", "rsync_base_url", "speed", "country",
27- "content", "official_candidate"]
28+ field_names = ["name", "displayname", "description", "whiteboard",
29+ "http_base_url", "ftp_base_url", "rsync_base_url", "speed",
30+ "country", "content", "official_candidate"]
31 @property
32 def label(self):
33 """See `LaunchpadFormView`."""
34
35=== modified file 'lib/lp/registry/browser/tests/distributionmirror-views.txt'
36--- lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-04 20:21:15 +0000
37+++ lib/lp/registry/browser/tests/distributionmirror-views.txt 2009-12-09 10:12:18 +0000
38@@ -49,12 +49,14 @@
39 A HTTP or FTP URL is required to register a mirror.
40
41 >>> view.field_names
42- ['displayname', 'description', 'http_base_url', 'ftp_base_url',
43- 'rsync_base_url', 'speed', 'country', 'content', 'official_candidate']
44+ ['displayname', 'description', 'whiteboard', 'http_base_url',
45+ 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
46+ 'official_candidate']
47
48 >>> form = {
49 ... 'field.displayname': 'Illuminati',
50 ... 'field.description': 'description',
51+ ... 'field.whiteboard': 'whiteboard',
52 ... 'field.http_base_url': 'http://secret.me/',
53 ... 'field.ftp_base_url': '',
54 ... 'field.rsync_base_url': '',
55@@ -239,8 +241,9 @@
56 The user can edit the mirror fields.
57
58 >>> view.field_names
59- ['name', 'displayname', 'description', 'http_base_url', 'ftp_base_url',
60- 'rsync_base_url', 'speed', 'country', 'content', 'official_candidate']
61+ ['name', 'displayname', 'description', 'whiteboard', 'http_base_url',
62+ 'ftp_base_url', 'rsync_base_url', 'speed', 'country', 'content',
63+ 'official_candidate']
64
65 >>> print mirror.ftp_base_url
66 None
67
68=== modified file 'lib/lp/registry/configure.zcml'
69--- lib/lp/registry/configure.zcml 2009-12-05 18:37:28 +0000
70+++ lib/lp/registry/configure.zcml 2009-12-09 10:12:18 +0000
71@@ -1606,12 +1606,12 @@
72 <allow interface="lp.registry.interfaces.distributionmirror.IDistributionMirror" />
73 <require
74 permission="launchpad.Edit"
75- set_attributes="name displayname description http_base_url
76- ftp_base_url rsync_base_url enabled speed
77- country content official_candidate owner" />
78+ set_attributes="name displayname description whiteboard
79+ http_base_url ftp_base_url rsync_base_url enabled
80+ speed country content official_candidate owner" />
81 <require
82 permission="launchpad.Admin"
83- set_attributes="status whiteboard reviewer date_reviewed"
84+ set_attributes="status reviewer date_reviewed"
85 attributes="destroySelf" />
86 </class>
87
88
89=== modified file 'lib/lp/registry/doc/distribution-mirror.txt'
90--- lib/lp/registry/doc/distribution-mirror.txt 2009-11-20 03:09:59 +0000
91+++ lib/lp/registry/doc/distribution-mirror.txt 2009-12-09 10:12:18 +0000
92@@ -18,8 +18,10 @@
93 >>> brazil = getUtility(ICountrySet)['BR']
94 >>> content = MirrorContent.ARCHIVE
95 >>> http_base_url = 'http://foo.bar.com/pub'
96+ >>> whiteboard = "This mirror is based deep in the Amazon rainforest."
97 >>> new_mirror = ubuntu.newMirror(
98- ... owner, speed, brazil, content, http_base_url=http_base_url)
99+ ... owner, speed, brazil, content, http_base_url=http_base_url,
100+ ... whiteboard=whiteboard)
101
102 When a new mirror is created we'll generate a unique name for it, based
103 on its host name and content.
104
105=== modified file 'lib/lp/registry/interfaces/distribution.py'
106--- lib/lp/registry/interfaces/distribution.py 2009-12-05 18:37:28 +0000
107+++ lib/lp/registry/interfaces/distribution.py 2009-12-09 10:12:18 +0000
108@@ -310,9 +310,9 @@
109 """
110
111 def newMirror(owner, speed, country, content, displayname=None,
112- description=None, http_base_url=None, ftp_base_url=None,
113- rsync_base_url=None, enabled=False,
114- official_candidate=False):
115+ description=None, http_base_url=None,
116+ ftp_base_url=None, rsync_base_url=None, enabled=False,
117+ official_candidate=False, whiteboard=None):
118 """Create a new DistributionMirror for this distribution.
119
120 At least one of http_base_url or ftp_base_url must be provided in
121
122=== modified file 'lib/lp/registry/model/distribution.py'
123--- lib/lp/registry/model/distribution.py 2009-12-05 18:37:28 +0000
124+++ lib/lp/registry/model/distribution.py 2009-12-09 10:12:18 +0000
125@@ -393,9 +393,9 @@
126 return DistributionMirror.selectOneBy(distribution=self, name=name)
127
128 def newMirror(self, owner, speed, country, content, displayname=None,
129- description=None, http_base_url=None, ftp_base_url=None,
130- rsync_base_url=None, official_candidate=False,
131- enabled=False):
132+ description=None, http_base_url=None,
133+ ftp_base_url=None, rsync_base_url=None, official_candidate=False,
134+ enabled=False, whiteboard=None):
135 """See `IDistribution`."""
136 # NB this functionality is only available to distributions that have
137 # the full functionality of Launchpad enabled. This is Ubuntu and
138@@ -429,7 +429,8 @@
139 description=description, http_base_url=urls['http_base_url'],
140 ftp_base_url=urls['ftp_base_url'],
141 rsync_base_url=urls['rsync_base_url'],
142- official_candidate=official_candidate, enabled=enabled)
143+ official_candidate=official_candidate, enabled=enabled,
144+ whiteboard=whiteboard)
145
146 def createBug(self, bug_params):
147 """See canonical.launchpad.interfaces.IBugTarget."""
148
149=== modified file 'lib/lp/registry/templates/distributionmirror-index.pt'
150--- lib/lp/registry/templates/distributionmirror-index.pt 2009-12-04 12:09:11 +0000
151+++ lib/lp/registry/templates/distributionmirror-index.pt 2009-12-09 10:12:18 +0000
152@@ -27,14 +27,14 @@
153 <span tal:content="context/date_created/fmt:displaydate" />
154 </p>
155
156- <tal:is-admin condition="context/required:launchpad.Admin">
157+ <tal:is-admin condition="context/required:launchpad.Edit">
158 </tal:is-admin>
159 </div>
160 <h2>Archive information</h2>
161 <div class="yui-g">
162 <div class="yui-u first">
163 <div class="top-portlet">
164- <tal:is-admin condition="context/required:launchpad.Admin">
165+ <tal:is-admin condition="context/required:launchpad.Edit">
166 <dl id="whiteboard">
167 <dt>
168 Whiteboard