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

[Gavin Panella]

[Gavin Panella (allenap)]

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.

[Graham Binns (gmb)]

review: Approve

[Brad Crittenden (bac)]

Very nice work Gavin. Thank you for picking up this branch in a pinch.

review: Approve (release-critical)

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2009-09-19 08:58:42 +0000
+++ lib/lp/blueprints/browser/specification.py	2009-09-22 09:27:26 +0000
@@ -46,6 +46,7 @@
 from zope.app.form.browser.itemswidgets import DropdownWidget
 from zope.formlib import form
 from zope.formlib.form import Fields
+from zope.interface import Interface
 from zope.schema import Choice
 from zope.schema.vocabulary import SimpleVocabulary, SimpleTerm
 
@@ -53,8 +54,6 @@
 from canonical.config import config
 from canonical.launchpad import _
 
-from canonical.lazr.utils import smartquote
-
 from lp.code.interfaces.branchnamespace import IBranchNamespaceSet
 from lp.registry.interfaces.distribution import IDistribution
 from lp.registry.interfaces.product import IProduct
@@ -613,27 +612,33 @@
         specification.acceptBy(user)
 
 
-class SpecificationGoalDecideView(LaunchpadView):
+class SpecificationGoalDecideView(LaunchpadFormView):
     """View used to allow the drivers of a series to accept
     or decline the spec as a goal for that series. Typically they would use
     the multi-select goalset view on their series, but it's also
     useful for them to have this one-at-a-time view on the spec itself.
     """
 
-    def initialize(self):
-        accept = self.request.form.get('accept')
-        decline = self.request.form.get('decline')
-        cancel = self.request.form.get('cancel')
-        decided = False
-        if accept is not None:
-            self.context.acceptBy(self.user)
-            decided = True
-        elif decline is not None:
-            self.context.declineBy(self.user)
-            decided = True
-        if decided or cancel is not None:
-            self.request.response.redirect(
-                canonical_url(self.context))
+    schema = Interface
+    field_names = []
+
+    @property
+    def label(self):
+        return _("Accept as %s series goal?") % self.context.goal.name
+
+    @action(_('Accept'), name='accept')
+    def accept_action(self, action, data):
+        self.context.acceptBy(self.user)
+
+    @action(_('Decline'), name='decline')
+    def decline_action(self, action, data):
+        self.context.declineBy(self.user)
+
+    @property
+    def next_url(self):
+        return canonical_url(self.context)
+
+    cancel_url = next_url
 
 
 class SpecificationRetargetingView(LaunchpadFormView):
@@ -1067,7 +1072,7 @@
         """Return a SpecGraph object rooted on the spec that is self.context.
         """
         graph = SpecGraph()
-        root = graph.newNode(self.context, root=True)
+        graph.newNode(self.context, root=True)
         graph.addDependencyNodes(self.context)
         graph.addBlockedNodes(self.context)
         return graph

=== modified file 'lib/lp/blueprints/browser/specificationfeedback.py'
--- lib/lp/blueprints/browser/specificationfeedback.py	2009-07-17 00:26:05 +0000
+++ lib/lp/blueprints/browser/specificationfeedback.py	2009-09-21 21:18:17 +0000
@@ -9,11 +9,13 @@
 from zope.app.form.browser.add import AddView
 
 from zope.component import getUtility
+from zope.interface import Interface
 
 from canonical.launchpad import _
-from canonical.launchpad.webapp.interfaces import ILaunchBag
+from canonical.launchpad.helpers import english_list
 from lp.registry.interfaces.person import IPersonSet
-from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp import (
+    LaunchpadFormView, action, canonical_url)
 
 
 __all__ = [
@@ -56,79 +58,44 @@
         return canonical_url(self.context)
 
 
-class SpecificationFeedbackClearingView:
-
-    def __init__(self, context, request):
-        """A custom little view class to process the results of this unusual
-        page. It is unusual because we want to display multiple objects with
-        checkboxes, then process the selected items, which is not the usual
-        add/edit metaphor."""
-        self.context = context
-        self.request = request
-        self.process_status = None
-
-    @property
-    def feedbackrequests(self):
-        """Return the feedback requests on this spec which have been made of
-        this user."""
+class SpecificationFeedbackClearingView(LaunchpadFormView):
+
+    schema = Interface
+    field_names = []
+
+    @property
+    def label(self):
+        return _('Give feedback on this blueprint')
+
+    @property
+    def requests(self):
+        """Return the feedback requests made of this user."""
         if self.user is None:
             return None
         return self.context.getFeedbackRequests(self.user)
 
-    @property
-    def user(self):
-        """Return the Launchpad person who is logged in."""
-        return getUtility(ILaunchBag).user
-
-    def process_form(self):
-        """Largely copied from webapp/generalform.py, without the
-        schema processing bits because we are not rendering the form in the
-        usual way. Instead, we are creating our own form in the page
-        template and interpreting it here."""
-
-        if self.process_status is not None:
-            # We've been called before. Just return the status we previously
-            # computed.
-            return self.process_status
-
-        if 'cancel' in self.request:
-            self.process_status = 'Cancelled'
-            self.request.response.redirect(canonical_url(self.context))
-            return self.process_status
-
-        if "FORM_SUBMIT" not in self.request:
-            self.process_status = ''
-            return self.process_status
-
-        if self.request.method == 'POST':
-            if 'feedbackrequest' not in self.request:
-                self.process_status = ('Please select feedback queue items '
-                                       'to clear.')
-                return self.process_status
-
-        clearedreqs = self.request['feedbackrequest']
-        if isinstance(clearedreqs, unicode):
-            # only a single item was selected, but we want to deal with a
-            # list for the general case, so convert it to a list
-            clearedreqs = [clearedreqs,]
-
-        queue_length = self.context.getFeedbackRequests(self.user).count()
-        number_cleared = 0
-        msg = 'Cleared requests from: '
-        for clearedreq in clearedreqs:
-            requester = getUtility(IPersonSet).getByName(clearedreq)
-            if requester is not None:
-                self.context.unqueue(self.user, requester)
-                if number_cleared > 0:
-                    msg += ', '
-                msg += requester.displayname
-                number_cleared += 1
-
-        self.process_status = msg
-
-        if number_cleared == queue_length:
-            # they are all done, so redirect back to the spec
-            self.request.response.redirect(canonical_url(self.context))
-
-        return self.process_status
-
+    @action(_('Save changes'), name='save')
+    def save_action(self, action, data):
+        names = self.request.form_ng.getAll('name')
+        if len(names) == 0:
+            self.request.response.addNotification(
+                'Please select feedback queue items to clear.')
+        else:
+            cleared_from = []
+            for name in names:
+                requester = getUtility(IPersonSet).getByName(name)
+                if requester is not None:
+                    self.context.unqueue(self.user, requester)
+                    cleared_from.append(requester.displayname)
+            self.request.response.addNotification(
+                'Cleared requests from: %s' % english_list(cleared_from))
+
+    @property
+    def next_url(self):
+        if self.context.getFeedbackRequests(self.user).count() == 0:
+            # No more queue items to process; return to the spec.
+            return canonical_url(self.context)
+
+    @property
+    def cancel_url(self):
+        return canonical_url(self.context)

=== modified file 'lib/lp/blueprints/stories/blueprints/05-reviews.txt'
--- lib/lp/blueprints/stories/blueprints/05-reviews.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/blueprints/stories/blueprints/05-reviews.txt	2009-09-22 08:20:33 +0000
@@ -220,7 +220,7 @@
   ... Content-Length: 43
   ... Content-Type: application/x-www-form-urlencoded
   ... 
-  ... feedbackrequest=carlos&FORM_SUBMIT=Continue""")
+  ... name=carlos&field.actions.save=Save%20changes""")
   HTTP/1.1 200 Ok
   ...Cleared requests from: Carlos Perelló Marín...
 
@@ -233,7 +233,7 @@
   ... Content-Length: 66
   ... Content-Type: application/x-www-form-urlencoded
   ... 
-  ... feedbackrequest=name12&feedbackrequest=name16&FORM_SUBMIT=Continue""")
+  ... name=name12&name=name16&field.actions.save=Save%20changes""")
   HTTP/1.1 303 See Other
   ...
   Location: http://.../firefox/+spec/canvas

=== modified file 'lib/lp/blueprints/templates/specification-givefeedback.pt'
--- lib/lp/blueprints/templates/specification-givefeedback.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/blueprints/templates/specification-givefeedback.pt	2009-09-22 09:19:45 +0000
@@ -1,77 +1,42 @@
-<html
+<specification-give-feedback
   xmlns="http://www.w3.org/1999/xhtml"
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-
-<body>
-
-<metal:leftportlets fill-slot="portlets_one">
-  <div tal:replace="structure context/@@+portlet-feedbackqueue" />
-</metal:leftportlets>
-
-<div metal:fill-slot="main">
-
-  <h1>Give feedback on this blueprint</h1>
-
-  <p class="documentDescription">
-    Select the requests which you wish to remove from your queue.
-  </p>
-
-  <p class="error message"
-     tal:define="status view/process_form"
-     tal:condition="status"
-     tal:content="status" />
-
-  <form tal:attributes="action request/URL" method="POST">
-
-    <table>
-      <tbody>
-        <tr tal:repeat="fbreq view/feedbackrequests">
-          <td>
-            <input type="checkbox" name="feedbackrequest"
-                   tal:attributes="
-                     value fbreq/requester/name;
-                     id string:req_${fbreq/requester/name}" />
-          </td>
-          <td>
-            <label tal:content="fbreq/requester/displayname"
-                   tal:attributes="
-                     for string:req_${fbreq/requester/name}">Foo
-                     Bar</label>
-            <div tal:condition="fbreq/queuemsg"
-                 tal:content="fbreq/queuemsg"
-                 class="lesser" />
-          </td>
-        </tr>
-      </tbody>
-    </table>
-
-    <div class="discreet">
-      Select the items that you have addressed in this blueprint, and
-      click "Continue" to clear those feedback requests from your queue.
-    </div>
-
-
-    <div class="actions">
-      <input type="submit"
-             name="cancel"
-             value="Cancel" />
-      <input tal:condition="view/feedbackrequests"
-             type="submit"
-             name="FORM_SUBMIT"
-             value="Continue" />
-    </div>
-
-  </form>
-
-</div>
-
-</body>
-</html>
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+
+  <div metal:fill-slot="main">
+    <div class="top-portlet">
+      <h2>Select the requests which you wish to remove from your queue</h2>
+      <div metal:use-macro="context/@@launchpad_form/form">
+        <div metal:fill-slot="extra_info">
+          <p>
+            Select the items that you have addressed in this
+            blueprint, and click "Save changes" to clear those
+            feedback requests from your queue.
+          </p>
+        </div>
+        <div metal:fill-slot="widgets">
+          <table class="form">
+            <tbody>
+              <tr tal:repeat="request view/requests">
+                <td colspan="2">
+                  <input type="checkbox" name="name"
+                         tal:attributes="value request/requester/name;
+                                         id string:req_${request/requester/name}" />
+                  <label tal:content="request/requester/displayname"
+                         tal:attributes="for string:req_${request/requester/name}" />
+                  <div tal:condition="request/queuemsg"
+                       tal:content="request/queuemsg"
+                       class="lesser" />
+                </td>
+              </tr>
+            </tbody>
+          </table>
+        </div>
+      </div>
+    </div>
+  </div>
+
+</specification-give-feedback>

=== modified file 'lib/lp/blueprints/templates/specification-goaldecide.pt'
--- lib/lp/blueprints/templates/specification-goaldecide.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/blueprints/templates/specification-goaldecide.pt	2009-09-21 20:24:36 +0000
@@ -1,79 +1,36 @@
-<html
+<specification-goal-decide
   xmlns="http://www.w3.org/1999/xhtml"
   xmlns:tal="http://xml.zope.org/namespaces/tal"
   xmlns:metal="http://xml.zope.org/namespaces/metal"
   xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  dir="ltr"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-
-<body>
-
-<div metal:fill-slot="main">
-
-      <div>
-        <img alt="" src="/@@/blueprint" />
-        <a tal:attributes="href context/fmt:url" tal:content="context/title" />
+  metal:use-macro="view/macro:page/main_only"
+  i18n:domain="launchpad">
+
+  <div metal:fill-slot="main">
+    <div class="top-portlet">
+      <h2>Blueprint summary</h2>
+      <div metal:use-macro="context/@@launchpad_form/form">
+        <div metal:fill-slot="extra_info">
+          <p tal:content="context/summary">
+            Spec summary here
+          </p>
+          <p>
+            This goal was proposed by
+            <a tal:replace="structure context/goal_proposer/fmt:link">Foo Bar</a>
+            <span tal:attributes="title context/date_goal_proposed/fmt:datetime"
+                  tal:content="context/date_goal_proposed/fmt:displaydate" />.
+            <tal:already_decided condition="context/goal_decider">
+              It was previously marked
+              "<span tal:replace="context/goalstatus/title">Approved</span>"
+              as a goal by
+              <a tal:replace="structure context/goal_decider/fmt:link">Foo Bar</a>
+              <span tal:attributes="title context/date_goal_decided/fmt:datetime"
+                    tal:content="context/date_goal_decided/fmt:displaydate" />.
+            </tal:already_decided>
+          </p>
+        </div>
       </div>
-
-  <h1>Accept as <span tal:replace="context/goal/name">1.0
-                </span> series goal?</h1>
-
-  <form method="POST" tal:attributes="action request/URL">
-
-    <div>
-
-      <h2>Blueprint summary</h2>
-
-      <p tal:content="context/summary">
-        Spec summary here
-      </p>
-
-    </div>
-
-    <p>
-      This goal was proposed by
-      <a tal:replace="structure context/goal_proposer/fmt:link">Foo Bar</a>
-      <span
-        tal:attributes="title context/date_goal_proposed/fmt:datetime"
-        tal:content="context/date_goal_proposed/fmt:displaydate">
-        2006-12-13
-      </span>.
-      <tal:already_decided condition="context/goal_decider">
-        It was previously marked "<span tal:replace="context/goalstatus/title">
-        Approved</span>" as a goal by
-        <a tal:replace="structure context/goal_decider/fmt:link">Foo Bar</a>
-        <span
-          tal:attributes="title context/date_goal_decided/fmt:datetime"
-          tal:content="context/date_goal_decided/fmt:displaydate">
-          2006-12-13
-        </span>.
-      </tal:already_decided>
-    </p>
-
-    <div class="actions">
-      <input
-        type="submit"
-        name="accept"
-        value="Accept"
-        />
-      <input
-        type="submit"
-        name="decline"
-        value="Decline"
-        />
-      <input
-        type="submit"
-        name="cancel"
-        value="Cancel"
-        />
-    </div>
-
-  </form>
-
-</div>
-</body>
-</html>
+    </div>
+  </div>
+
+</specification-goal-decide>

=== removed file 'lib/lp/blueprints/templates/specification-superseding.pt'
--- lib/lp/blueprints/templates/specification-superseding.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/blueprints/templates/specification-superseding.pt	1970-01-01 00:00:00 +0000
@@ -1,24 +0,0 @@
-<html
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  xml:lang="en"
-  lang="en"
-  metal:use-macro="context/@@main_template/master"
-  i18n:domain="launchpad"
->
-  <body>
-
-    <div metal:fill-slot="main">
-
-      <div tal:content="structure context/fmt:link" />
-      <div metal:use-macro="context/@@launchpad_form/form">
-
-      </div>
-
-    </div>
-
-  </body>
-
-</html>