Merge lp://staging/~allenap/launchpad/convert-blueprints-bug-434056 into lp://staging/launchpad
- convert-blueprints-bug-434056
- Merge into devel
Proposed by
Gavin Panella
Status: | Merged |
---|---|
Approved by: | Brad Crittenden |
Approved revision: | no longer in the source branch. |
Merged at revision: | not available |
Proposed branch: | lp://staging/~allenap/launchpad/convert-blueprints-bug-434056 |
Merge into: | lp://staging/launchpad |
Diff against target: | None lines |
To merge this branch: | bzr merge lp://staging/~allenap/launchpad/convert-blueprints-bug-434056 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | release-critical | Approve | |
Graham Binns (community) | Approve | ||
Review via email: mp+12210@code.staging.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote : | # |
Revision history for this message
Graham Binns (gmb) : | # |
review:
Approve
Revision history for this message
Brad Crittenden (bac) wrote : | # |
Very nice work Gavin. Thank you for picking up this branch in a pinch.
review:
Approve
(release-critical)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/lp/blueprints/browser/specification.py' |
2 | --- lib/lp/blueprints/browser/specification.py 2009-09-19 08:58:42 +0000 |
3 | +++ lib/lp/blueprints/browser/specification.py 2009-09-22 09:27:26 +0000 |
4 | @@ -46,6 +46,7 @@ |
5 | from zope.app.form.browser.itemswidgets import DropdownWidget |
6 | from zope.formlib import form |
7 | from zope.formlib.form import Fields |
8 | +from zope.interface import Interface |
9 | from zope.schema import Choice |
10 | from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm |
11 | |
12 | @@ -53,8 +54,6 @@ |
13 | from canonical.config import config |
14 | from canonical.launchpad import _ |
15 | |
16 | -from canonical.lazr.utils import smartquote |
17 | - |
18 | from lp.code.interfaces.branchnamespace import IBranchNamespaceSet |
19 | from lp.registry.interfaces.distribution import IDistribution |
20 | from lp.registry.interfaces.product import IProduct |
21 | @@ -613,27 +612,33 @@ |
22 | specification.acceptBy(user) |
23 | |
24 | |
25 | -class SpecificationGoalDecideView(LaunchpadView): |
26 | +class SpecificationGoalDecideView(LaunchpadFormView): |
27 | """View used to allow the drivers of a series to accept |
28 | or decline the spec as a goal for that series. Typically they would use |
29 | the multi-select goalset view on their series, but it's also |
30 | useful for them to have this one-at-a-time view on the spec itself. |
31 | """ |
32 | |
33 | - def initialize(self): |
34 | - accept = self.request.form.get('accept') |
35 | - decline = self.request.form.get('decline') |
36 | - cancel = self.request.form.get('cancel') |
37 | - decided = False |
38 | - if accept is not None: |
39 | - self.context.acceptBy(self.user) |
40 | - decided = True |
41 | - elif decline is not None: |
42 | - self.context.declineBy(self.user) |
43 | - decided = True |
44 | - if decided or cancel is not None: |
45 | - self.request.response.redirect( |
46 | - canonical_url(self.context)) |
47 | + schema = Interface |
48 | + field_names = [] |
49 | + |
50 | + @property |
51 | + def label(self): |
52 | + return _("Accept as %s series goal?") % self.context.goal.name |
53 | + |
54 | + @action(_('Accept'), name='accept') |
55 | + def accept_action(self, action, data): |
56 | + self.context.acceptBy(self.user) |
57 | + |
58 | + @action(_('Decline'), name='decline') |
59 | + def decline_action(self, action, data): |
60 | + self.context.declineBy(self.user) |
61 | + |
62 | + @property |
63 | + def next_url(self): |
64 | + return canonical_url(self.context) |
65 | + |
66 | + cancel_url = next_url |
67 | |
68 | |
69 | class SpecificationRetargetingView(LaunchpadFormView): |
70 | @@ -1067,7 +1072,7 @@ |
71 | """Return a SpecGraph object rooted on the spec that is self.context. |
72 | """ |
73 | graph = SpecGraph() |
74 | - root = graph.newNode(self.context, root=True) |
75 | + graph.newNode(self.context, root=True) |
76 | graph.addDependencyNodes(self.context) |
77 | graph.addBlockedNodes(self.context) |
78 | return graph |
79 | |
80 | === modified file 'lib/lp/blueprints/browser/specificationfeedback.py' |
81 | --- lib/lp/blueprints/browser/specificationfeedback.py 2009-07-17 00:26:05 +0000 |
82 | +++ lib/lp/blueprints/browser/specificationfeedback.py 2009-09-21 21:18:17 +0000 |
83 | @@ -9,11 +9,13 @@ |
84 | from zope.app.form.browser.add import AddView |
85 | |
86 | from zope.component import getUtility |
87 | +from zope.interface import Interface |
88 | |
89 | from canonical.launchpad import _ |
90 | -from canonical.launchpad.webapp.interfaces import ILaunchBag |
91 | +from canonical.launchpad.helpers import english_list |
92 | from lp.registry.interfaces.person import IPersonSet |
93 | -from canonical.launchpad.webapp import canonical_url |
94 | +from canonical.launchpad.webapp import ( |
95 | + LaunchpadFormView, action, canonical_url) |
96 | |
97 | |
98 | __all__ = [ |
99 | @@ -56,79 +58,44 @@ |
100 | return canonical_url(self.context) |
101 | |
102 | |
103 | -class SpecificationFeedbackClearingView: |
104 | - |
105 | - def __init__(self, context, request): |
106 | - """A custom little view class to process the results of this unusual |
107 | - page. It is unusual because we want to display multiple objects with |
108 | - checkboxes, then process the selected items, which is not the usual |
109 | - add/edit metaphor.""" |
110 | - self.context = context |
111 | - self.request = request |
112 | - self.process_status = None |
113 | - |
114 | - @property |
115 | - def feedbackrequests(self): |
116 | - """Return the feedback requests on this spec which have been made of |
117 | - this user.""" |
118 | +class SpecificationFeedbackClearingView(LaunchpadFormView): |
119 | + |
120 | + schema = Interface |
121 | + field_names = [] |
122 | + |
123 | + @property |
124 | + def label(self): |
125 | + return _('Give feedback on this blueprint') |
126 | + |
127 | + @property |
128 | + def requests(self): |
129 | + """Return the feedback requests made of this user.""" |
130 | if self.user is None: |
131 | return None |
132 | return self.context.getFeedbackRequests(self.user) |
133 | |
134 | - @property |
135 | - def user(self): |
136 | - """Return the Launchpad person who is logged in.""" |
137 | - return getUtility(ILaunchBag).user |
138 | - |
139 | - def process_form(self): |
140 | - """Largely copied from webapp/generalform.py, without the |
141 | - schema processing bits because we are not rendering the form in the |
142 | - usual way. Instead, we are creating our own form in the page |
143 | - template and interpreting it here.""" |
144 | - |
145 | - if self.process_status is not None: |
146 | - # We've been called before. Just return the status we previously |
147 | - # computed. |
148 | - return self.process_status |
149 | - |
150 | - if 'cancel' in self.request: |
151 | - self.process_status = 'Cancelled' |
152 | - self.request.response.redirect(canonical_url(self.context)) |
153 | - return self.process_status |
154 | - |
155 | - if "FORM_SUBMIT" not in self.request: |
156 | - self.process_status = '' |
157 | - return self.process_status |
158 | - |
159 | - if self.request.method == 'POST': |
160 | - if 'feedbackrequest' not in self.request: |
161 | - self.process_status = ('Please select feedback queue items ' |
162 | - 'to clear.') |
163 | - return self.process_status |
164 | - |
165 | - clearedreqs = self.request['feedbackrequest'] |
166 | - if isinstance(clearedreqs, unicode): |
167 | - # only a single item was selected, but we want to deal with a |
168 | - # list for the general case, so convert it to a list |
169 | - clearedreqs = [clearedreqs,] |
170 | - |
171 | - queue_length = self.context.getFeedbackRequests(self.user).count() |
172 | - number_cleared = 0 |
173 | - msg = 'Cleared requests from: ' |
174 | - for clearedreq in clearedreqs: |
175 | - requester = getUtility(IPersonSet).getByName(clearedreq) |
176 | - if requester is not None: |
177 | - self.context.unqueue(self.user, requester) |
178 | - if number_cleared > 0: |
179 | - msg += ', ' |
180 | - msg += requester.displayname |
181 | - number_cleared += 1 |
182 | - |
183 | - self.process_status = msg |
184 | - |
185 | - if number_cleared == queue_length: |
186 | - # they are all done, so redirect back to the spec |
187 | - self.request.response.redirect(canonical_url(self.context)) |
188 | - |
189 | - return self.process_status |
190 | - |
191 | + @action(_('Save changes'), name='save') |
192 | + def save_action(self, action, data): |
193 | + names = self.request.form_ng.getAll('name') |
194 | + if len(names) == 0: |
195 | + self.request.response.addNotification( |
196 | + 'Please select feedback queue items to clear.') |
197 | + else: |
198 | + cleared_from = [] |
199 | + for name in names: |
200 | + requester = getUtility(IPersonSet).getByName(name) |
201 | + if requester is not None: |
202 | + self.context.unqueue(self.user, requester) |
203 | + cleared_from.append(requester.displayname) |
204 | + self.request.response.addNotification( |
205 | + 'Cleared requests from: %s' % english_list(cleared_from)) |
206 | + |
207 | + @property |
208 | + def next_url(self): |
209 | + if self.context.getFeedbackRequests(self.user).count() == 0: |
210 | + # No more queue items to process; return to the spec. |
211 | + return canonical_url(self.context) |
212 | + |
213 | + @property |
214 | + def cancel_url(self): |
215 | + return canonical_url(self.context) |
216 | |
217 | === modified file 'lib/lp/blueprints/stories/blueprints/05-reviews.txt' |
218 | --- lib/lp/blueprints/stories/blueprints/05-reviews.txt 2009-08-13 19:03:36 +0000 |
219 | +++ lib/lp/blueprints/stories/blueprints/05-reviews.txt 2009-09-22 08:20:33 +0000 |
220 | @@ -220,7 +220,7 @@ |
221 | ... Content-Length: 43 |
222 | ... Content-Type: application/x-www-form-urlencoded |
223 | ... |
224 | - ... feedbackrequest=carlos&FORM_SUBMIT=Continue""") |
225 | + ... name=carlos&field.actions.save=Save%20changes""") |
226 | HTTP/1.1 200 Ok |
227 | ...Cleared requests from: Carlos Perelló Marín... |
228 | |
229 | @@ -233,7 +233,7 @@ |
230 | ... Content-Length: 66 |
231 | ... Content-Type: application/x-www-form-urlencoded |
232 | ... |
233 | - ... feedbackrequest=name12&feedbackrequest=name16&FORM_SUBMIT=Continue""") |
234 | + ... name=name12&name=name16&field.actions.save=Save%20changes""") |
235 | HTTP/1.1 303 See Other |
236 | ... |
237 | Location: http://.../firefox/+spec/canvas |
238 | |
239 | === modified file 'lib/lp/blueprints/templates/specification-givefeedback.pt' |
240 | --- lib/lp/blueprints/templates/specification-givefeedback.pt 2009-07-17 17:59:07 +0000 |
241 | +++ lib/lp/blueprints/templates/specification-givefeedback.pt 2009-09-22 09:19:45 +0000 |
242 | @@ -1,77 +1,42 @@ |
243 | -<html |
244 | +<specification-give-feedback |
245 | xmlns="http://www.w3.org/1999/xhtml" |
246 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
247 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
248 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
249 | - xml:lang="en" |
250 | - lang="en" |
251 | - dir="ltr" |
252 | - metal:use-macro="context/@@main_template/master" |
253 | - i18n:domain="launchpad" |
254 | -> |
255 | - |
256 | -<body> |
257 | - |
258 | -<metal:leftportlets fill-slot="portlets_one"> |
259 | - <div tal:replace="structure context/@@+portlet-feedbackqueue" /> |
260 | -</metal:leftportlets> |
261 | - |
262 | -<div metal:fill-slot="main"> |
263 | - |
264 | - <h1>Give feedback on this blueprint</h1> |
265 | - |
266 | - <p class="documentDescription"> |
267 | - Select the requests which you wish to remove from your queue. |
268 | - </p> |
269 | - |
270 | - <p class="error message" |
271 | - tal:define="status view/process_form" |
272 | - tal:condition="status" |
273 | - tal:content="status" /> |
274 | - |
275 | - <form tal:attributes="action request/URL" method="POST"> |
276 | - |
277 | - <table> |
278 | - <tbody> |
279 | - <tr tal:repeat="fbreq view/feedbackrequests"> |
280 | - <td> |
281 | - <input type="checkbox" name="feedbackrequest" |
282 | - tal:attributes=" |
283 | - value fbreq/requester/name; |
284 | - id string:req_${fbreq/requester/name}" /> |
285 | - </td> |
286 | - <td> |
287 | - <label tal:content="fbreq/requester/displayname" |
288 | - tal:attributes=" |
289 | - for string:req_${fbreq/requester/name}">Foo |
290 | - Bar</label> |
291 | - <div tal:condition="fbreq/queuemsg" |
292 | - tal:content="fbreq/queuemsg" |
293 | - class="lesser" /> |
294 | - </td> |
295 | - </tr> |
296 | - </tbody> |
297 | - </table> |
298 | - |
299 | - <div class="discreet"> |
300 | - Select the items that you have addressed in this blueprint, and |
301 | - click "Continue" to clear those feedback requests from your queue. |
302 | - </div> |
303 | - |
304 | - |
305 | - <div class="actions"> |
306 | - <input type="submit" |
307 | - name="cancel" |
308 | - value="Cancel" /> |
309 | - <input tal:condition="view/feedbackrequests" |
310 | - type="submit" |
311 | - name="FORM_SUBMIT" |
312 | - value="Continue" /> |
313 | - </div> |
314 | - |
315 | - </form> |
316 | - |
317 | -</div> |
318 | - |
319 | -</body> |
320 | -</html> |
321 | + metal:use-macro="view/macro:page/main_only" |
322 | + i18n:domain="launchpad"> |
323 | + |
324 | + <div metal:fill-slot="main"> |
325 | + <div class="top-portlet"> |
326 | + <h2>Select the requests which you wish to remove from your queue</h2> |
327 | + <div metal:use-macro="context/@@launchpad_form/form"> |
328 | + <div metal:fill-slot="extra_info"> |
329 | + <p> |
330 | + Select the items that you have addressed in this |
331 | + blueprint, and click "Save changes" to clear those |
332 | + feedback requests from your queue. |
333 | + </p> |
334 | + </div> |
335 | + <div metal:fill-slot="widgets"> |
336 | + <table class="form"> |
337 | + <tbody> |
338 | + <tr tal:repeat="request view/requests"> |
339 | + <td colspan="2"> |
340 | + <input type="checkbox" name="name" |
341 | + tal:attributes="value request/requester/name; |
342 | + id string:req_${request/requester/name}" /> |
343 | + <label tal:content="request/requester/displayname" |
344 | + tal:attributes="for string:req_${request/requester/name}" /> |
345 | + <div tal:condition="request/queuemsg" |
346 | + tal:content="request/queuemsg" |
347 | + class="lesser" /> |
348 | + </td> |
349 | + </tr> |
350 | + </tbody> |
351 | + </table> |
352 | + </div> |
353 | + </div> |
354 | + </div> |
355 | + </div> |
356 | + |
357 | +</specification-give-feedback> |
358 | |
359 | === modified file 'lib/lp/blueprints/templates/specification-goaldecide.pt' |
360 | --- lib/lp/blueprints/templates/specification-goaldecide.pt 2009-07-17 17:59:07 +0000 |
361 | +++ lib/lp/blueprints/templates/specification-goaldecide.pt 2009-09-21 20:24:36 +0000 |
362 | @@ -1,79 +1,36 @@ |
363 | -<html |
364 | +<specification-goal-decide |
365 | xmlns="http://www.w3.org/1999/xhtml" |
366 | xmlns:tal="http://xml.zope.org/namespaces/tal" |
367 | xmlns:metal="http://xml.zope.org/namespaces/metal" |
368 | xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
369 | - xml:lang="en" |
370 | - lang="en" |
371 | - dir="ltr" |
372 | - metal:use-macro="context/@@main_template/master" |
373 | - i18n:domain="launchpad" |
374 | -> |
375 | - |
376 | -<body> |
377 | - |
378 | -<div metal:fill-slot="main"> |
379 | - |
380 | - <div> |
381 | - <img alt="" src="/@@/blueprint" /> |
382 | - <a tal:attributes="href context/fmt:url" tal:content="context/title" /> |
383 | + metal:use-macro="view/macro:page/main_only" |
384 | + i18n:domain="launchpad"> |
385 | + |
386 | + <div metal:fill-slot="main"> |
387 | + <div class="top-portlet"> |
388 | + <h2>Blueprint summary</h2> |
389 | + <div metal:use-macro="context/@@launchpad_form/form"> |
390 | + <div metal:fill-slot="extra_info"> |
391 | + <p tal:content="context/summary"> |
392 | + Spec summary here |
393 | + </p> |
394 | + <p> |
395 | + This goal was proposed by |
396 | + <a tal:replace="structure context/goal_proposer/fmt:link">Foo Bar</a> |
397 | + <span tal:attributes="title context/date_goal_proposed/fmt:datetime" |
398 | + tal:content="context/date_goal_proposed/fmt:displaydate" />. |
399 | + <tal:already_decided condition="context/goal_decider"> |
400 | + It was previously marked |
401 | + "<span tal:replace="context/goalstatus/title">Approved</span>" |
402 | + as a goal by |
403 | + <a tal:replace="structure context/goal_decider/fmt:link">Foo Bar</a> |
404 | + <span tal:attributes="title context/date_goal_decided/fmt:datetime" |
405 | + tal:content="context/date_goal_decided/fmt:displaydate" />. |
406 | + </tal:already_decided> |
407 | + </p> |
408 | + </div> |
409 | </div> |
410 | - |
411 | - <h1>Accept as <span tal:replace="context/goal/name">1.0 |
412 | - </span> series goal?</h1> |
413 | - |
414 | - <form method="POST" tal:attributes="action request/URL"> |
415 | - |
416 | - <div> |
417 | - |
418 | - <h2>Blueprint summary</h2> |
419 | - |
420 | - <p tal:content="context/summary"> |
421 | - Spec summary here |
422 | - </p> |
423 | - |
424 | - </div> |
425 | - |
426 | - <p> |
427 | - This goal was proposed by |
428 | - <a tal:replace="structure context/goal_proposer/fmt:link">Foo Bar</a> |
429 | - <span |
430 | - tal:attributes="title context/date_goal_proposed/fmt:datetime" |
431 | - tal:content="context/date_goal_proposed/fmt:displaydate"> |
432 | - 2006-12-13 |
433 | - </span>. |
434 | - <tal:already_decided condition="context/goal_decider"> |
435 | - It was previously marked "<span tal:replace="context/goalstatus/title"> |
436 | - Approved</span>" as a goal by |
437 | - <a tal:replace="structure context/goal_decider/fmt:link">Foo Bar</a> |
438 | - <span |
439 | - tal:attributes="title context/date_goal_decided/fmt:datetime" |
440 | - tal:content="context/date_goal_decided/fmt:displaydate"> |
441 | - 2006-12-13 |
442 | - </span>. |
443 | - </tal:already_decided> |
444 | - </p> |
445 | - |
446 | - <div class="actions"> |
447 | - <input |
448 | - type="submit" |
449 | - name="accept" |
450 | - value="Accept" |
451 | - /> |
452 | - <input |
453 | - type="submit" |
454 | - name="decline" |
455 | - value="Decline" |
456 | - /> |
457 | - <input |
458 | - type="submit" |
459 | - name="cancel" |
460 | - value="Cancel" |
461 | - /> |
462 | - </div> |
463 | - |
464 | - </form> |
465 | - |
466 | -</div> |
467 | -</body> |
468 | -</html> |
469 | + </div> |
470 | + </div> |
471 | + |
472 | +</specification-goal-decide> |
473 | |
474 | === removed file 'lib/lp/blueprints/templates/specification-superseding.pt' |
475 | --- lib/lp/blueprints/templates/specification-superseding.pt 2009-07-17 17:59:07 +0000 |
476 | +++ lib/lp/blueprints/templates/specification-superseding.pt 1970-01-01 00:00:00 +0000 |
477 | @@ -1,24 +0,0 @@ |
478 | -<html |
479 | - xmlns="http://www.w3.org/1999/xhtml" |
480 | - xmlns:tal="http://xml.zope.org/namespaces/tal" |
481 | - xmlns:metal="http://xml.zope.org/namespaces/metal" |
482 | - xmlns:i18n="http://xml.zope.org/namespaces/i18n" |
483 | - xml:lang="en" |
484 | - lang="en" |
485 | - metal:use-macro="context/@@main_template/master" |
486 | - i18n:domain="launchpad" |
487 | -> |
488 | - <body> |
489 | - |
490 | - <div metal:fill-slot="main"> |
491 | - |
492 | - <div tal:content="structure context/fmt:link" /> |
493 | - <div metal:use-macro="context/@@launchpad_form/form"> |
494 | - |
495 | - </div> |
496 | - |
497 | - </div> |
498 | - |
499 | - </body> |
500 | - |
501 | -</html> |
This branch converts two blueprints templates to 3.0 conventions:
specification -goaldecide. pt -givefeedback. pt
specification
And deletes one other, unused, template:
specification -superseding. pt
I've also converted their corresponding views to LaunchpadFormView
subclasses. This has reduced the line count, and makes it easier to
conform to the form buttons convention:
[ Save changes ] [ Other action ] or Cancel
Lint free.
bin/test -vvt blueprints
File by file:
lib/lp/ blueprints/ browser/ specification. py
Converted one view to inherit from LaunchpadFormView. The view's
form only has buttons (and now a cancel link instead of a button),
so schema and field_names are set to accept no parameters.
I can't find any existing tests for this view, and have not (yet)
added any in the name of expeditiousness.
lib/lp/ blueprints/ browser/ specificationfe edback. py
Converted one view to inherit from LaunchpadFormView. This form does
have values passed in, but there is no obvious schema to use. I'm
not a wizard at forms, so I just extracted the data from form_ng
instead of trying to craft a schema for it. The code is still a lot
clearer than what preceded it.
lib/lp/ blueprints/ stories/ blueprints/ 05-reviews. txt
I adjusted this (horrible) test to accomodate the changed parameter
names used in the form.
lib/lp/ blueprints/ templates/ specification- givefeedback. pt
Used launchpad-form for the buttons. I've inserted some hand-coded
input fields into the widgets slot.
lib/lp/ blueprints/ templates/ specification- goaldecide. pt
Again, used launchpad-form for the buttons.
lib/lp/ blueprints/ templates/ specification- superseding. pt
I couldn't find any reference to this template, and the tests pass
without it, so removed.