launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/patcher-all-the-things into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/patcher-all-the-things/+merge/85147

Summary
=======
This branch updates the maintainer and driver fields on product and project
overview pages to use the inline edit widget, so they can be updated without
moving to a new page or requiring page refreshes.

While this is good in its own right, this is also a necessary step in
disclosure work, wherein we will warn users if they are assigning a private
team to a public role.

Preimp
======
Spoke with Curtis Hovey

Implementation
==============
New maintainer and driver widgets are created for the project and product
views. The templates are updated to use them.

Tests
=====
bin/test -vvcm lp.registery.browser.{product|project}

QA
==
Go to the project and product overviews and attempt to set or reassign the
maintainer and driver fields. You will get a person picker popup which will
then update the information on the page without need of a refresh or going to
the +edit pages.

Lint
====
This branch is lint free.
-- 
https://code.launchpad.net/~jcsackett/launchpad/patcher-all-the-things/+merge/85147
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/patcher-all-the-things into lp:launchpad.

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2011-11-30 23:31:25 +0000
+++ lib/lp/registry/browser/product.py	2011-12-09 15:53:26 +0000
@@ -128,10 +128,14 @@
     )
 from lp.app.browser.lazrjs import (
     BooleanChoiceWidget,
+    InlinePersonEditPickerWidget,
     TextLineEditorWidget,
     )
 from lp.app.browser.stringformatter import FormattersAPI
-from lp.app.browser.tales import MenuAPI
+from lp.app.browser.tales import (
+    format_link,
+    MenuAPI,
+    )
 from lp.app.enums import ServiceUsage
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.headings import IEditableContextTitle
@@ -1001,6 +1005,23 @@
 
     implements(IProductActionMenu, IEditableContextTitle)
 
+    @property
+    def maintainer_widget(self):
+        return InlinePersonEditPickerWidget(
+            self.context, IProduct['owner'],
+            format_link(self.context.owner),
+            header='Change maintainer', edit_view='+edit-people',
+            step_title='Select a new maintainer')
+
+    @property
+    def driver_widget(self):
+        return InlinePersonEditPickerWidget(
+            self.context, IProduct['driver'],
+            format_link(self.context.driver, empty_value="Not yet selected"),
+            header='Change driver', edit_view='+edit-people',
+            step_title='Select a new driver',
+            null_display_value="Not yet selected")
+
     def __init__(self, context, request):
         HasAnnouncementsView.__init__(self, context, request)
         self.form = request.form_ng
@@ -1035,7 +1056,9 @@
 
     @property
     def page_description(self):
-        return '\n'.filter(None, [self.context.summary, self.context.description])
+        return '\n'.filter(
+            None,
+            [self.context.summary, self.context.description])
 
     @property
     def show_license_status(self):

=== modified file 'lib/lp/registry/browser/project.py'
--- lib/lp/registry/browser/project.py	2011-09-18 17:35:53 +0000
+++ lib/lp/registry/browser/project.py	2011-12-09 15:53:26 +0000
@@ -70,6 +70,8 @@
     LaunchpadEditFormView,
     LaunchpadFormView,
     )
+from lp.app.browser.lazrjs import InlinePersonEditPickerWidget
+from lp.app.browser.tales import format_link
 from lp.app.errors import NotFoundError
 from lp.blueprints.browser.specificationtarget import (
     HasSpecificationsMenuMixin,
@@ -346,8 +348,27 @@
 
 
 class ProjectView(HasAnnouncementsView, FeedsMixin):
+
     implements(IProjectGroupActionMenu)
 
+    @property
+    def maintainer_widget(self):
+        return InlinePersonEditPickerWidget(
+            self.context, IProjectGroup['owner'],
+            format_link(self.context.owner, empty_value="Not yet selected"),
+            header='Change maintainer', edit_view='+reassign',
+            step_title='Select a new maintainer',
+            null_display_value="Not yet selected")
+
+    @property
+    def driver_widget(self):
+        return InlinePersonEditPickerWidget(
+            self.context, IProjectGroup['driver'],
+            format_link(self.context.driver, empty_value="Not yet selected"),
+            header='Change driver', edit_view='+driver',
+            step_title='Select a new driver',
+            null_display_value="Not yet selected")
+
     def initialize(self):
         super(ProjectView, self).initialize()
         expose_structural_subscription_data_to_js(

=== modified file 'lib/lp/registry/templates/product-index.pt'
--- lib/lp/registry/templates/product-index.pt	2011-11-26 04:03:29 +0000
+++ lib/lp/registry/templates/product-index.pt	2011-12-09 15:53:26 +0000
@@ -55,7 +55,7 @@
           <a tal:replace="structure overview_menu/review_license/fmt:icon"/>
         </p>
 
-	<div class="summary" 
+	<div class="summary"
 	  tal:content="structure context/summary/fmt:markdown">
           $Product.summary goes here. This should be quite short,
           just a single paragraph of text really, giving the project
@@ -97,10 +97,7 @@
 
               <dl id="owner" tal:condition="context/owner">
                 <dt>Maintainer:</dt>
-                <dd>
-                  <a tal:replace="structure context/owner/fmt:link" />
-                  <a tal:replace="structure overview_menu/reassign/fmt:icon" />
-                </dd>
+                <dd tal:content="structure view/maintainer_widget" />
               </dl>
 
               <dl id="aliases" tal:condition="context/aliases">
@@ -113,21 +110,7 @@
 
               <dl id="driver">
                 <dt>Driver:</dt>
-                <dd>
-
-                  <tal:driver_set condition="view/effective_driver">
-                  <a tal:replace="structure view/effective_driver/fmt:link" />
-                  <a tal:replace="structure overview_menu/reassign/fmt:icon" />
-                  </tal:driver_set>
-
-                  <tal:driver_unset condition="not: view/effective_driver">
-                  Not yet appointed.
-                  <a tal:replace="structure overview_menu/reassign/fmt:icon" />
-                  </tal:driver_unset>
-
-                  <a target="help" href="/+help-registry/driver.html"
-                     class="sprite maybe">&nbsp;<span class="invisible-link">Driver help</span></a>
-                </dd>
+                <dd tal:content="structure view/driver_widget" />
               </dl>
 
               <dl id="dev-focus"

=== modified file 'lib/lp/registry/templates/project-details.pt'
--- lib/lp/registry/templates/project-details.pt	2009-09-22 17:39:22 +0000
+++ lib/lp/registry/templates/project-details.pt	2011-12-09 15:53:26 +0000
@@ -20,24 +20,11 @@
     </dl>
     <dl id="maintainer">
       <dt>Maintainer:</dt>
-      <dd>
-        <a tal:replace="structure context/owner/fmt:link" />
-        <tal:edit-maintainer
-          content="structure overview_menu/reassign/fmt:icon" />
-      </dd>
+      <dd tal:content="structure view/maintainer_widget" />
     </dl>
     <dl id="driver">
       <dt>Driver:</dt>
-      <dd>
-        <tal:no-driver condition="not:context/driver">
-          Not yet appointed
-        </tal:no-driver>
-        <tal:has-driver condition="context/driver">
-          <a tal:replace="structure context/driver/fmt:link">Driver</a>
-        </tal:has-driver>
-        <tal:edit-maintainer
-          content="structure overview_menu/driver/fmt:icon" />
-      </dd>
+      <dd tal:content="structure view/driver_widget" />
     </dl>
     <dl id="bug-tracker">
       <dt>Bug tracker:</dt>