Merge lp://staging/~allenap/launchpad/convert-blueprints-bug-434056 into lp://staging/launchpad

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
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
To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

This branch converts two blueprints templates to 3.0 conventions:

  specification-goaldecide.pt
  specification-givefeedback.pt

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/specificationfeedback.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.

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>