launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/distro-page-ajax-835020 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #835020 in Launchpad itself: "AJAX control to change maintainer, members and driver on distro page don't work"
  https://bugs.launchpad.net/launchpad/+bug/835020

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/distro-page-ajax-835020/+merge/106321

== Implementation ==

Modify the distribution view to provide inline pickers for maintainer, driver, members, mirror_admin properties. 
Modify the TAL to use the pickers.

== Tests ==

The pickers provide progressive enhancement to the base html. There are existing tests for the base html but in general lp does not explicitly test the picker wiring.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/distribution.py
  lib/lp/registry/templates/distribution-details.pt

-- 
https://code.launchpad.net/~wallyworld/launchpad/distro-page-ajax-835020/+merge/106321
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/distro-page-ajax-835020 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distribution.py'
--- lib/lp/registry/browser/distribution.py	2012-03-19 16:12:40 +0000
+++ lib/lp/registry/browser/distribution.py	2012-05-18 07:55:22 +0000
@@ -49,6 +49,7 @@
 from zope.interface import implements
 from zope.lifecycleevent import ObjectCreatedEvent
 from zope.schema import Bool
+from zope.security.checker import canWrite
 from zope.security.interfaces import Unauthorized
 
 from lp.answers.browser.faqtarget import FAQTargetNavigationMixin
@@ -56,11 +57,13 @@
     QuestionTargetFacetMixin,
     QuestionTargetTraversalMixin,
     )
+from lp.app.browser.lazrjs import InlinePersonEditPickerWidget
 from lp.app.browser.launchpadform import (
     action,
     custom_widget,
     LaunchpadFormView,
     )
+from lp.app.browser.tales import format_link
 from lp.app.errors import NotFoundError
 from lp.app.widgets.image import ImageChangeWidget
 from lp.app.widgets.itemswidgets import LabeledMultiCheckBoxWidget
@@ -657,6 +660,53 @@
     def page_title(self):
         return '%s in Launchpad' % self.context.displayname
 
+    @property
+    def maintainer_widget(self):
+        return InlinePersonEditPickerWidget(
+            self.context, IDistribution['owner'],
+            format_link(self.context.owner),
+            header='Change maintainer', edit_view='+reassign',
+            step_title='Select a new maintainer')
+
+    @property
+    def driver_widget(self):
+        if canWrite(self.context, 'driver'):
+            empty_value = 'Specify a driver'
+        else:
+            empty_value = 'None'
+        return InlinePersonEditPickerWidget(
+            self.context, IDistribution['driver'],
+            format_link(self.context.driver, empty_value=empty_value),
+            header='Change driver', edit_view='+driver',
+            null_display_value=empty_value,
+            step_title='Select a new driver')
+
+    @property
+    def members_widget(self):
+        if canWrite(self.context, 'members'):
+            empty_value = ' Specify the members team'
+        else:
+            empty_value = 'None'
+        return InlinePersonEditPickerWidget(
+            self.context, IDistribution['members'],
+            format_link(self.context.members, empty_value=empty_value),
+            header='Change the members team', edit_view='+selectmemberteam',
+            null_display_value=empty_value,
+            step_title='Select a new members team')
+
+    @property
+    def mirror_admin_widget(self):
+        if canWrite(self.context, 'mirror_admin'):
+            empty_value = ' Specify a mirror administrator'
+        else:
+            empty_value = 'None'
+        return InlinePersonEditPickerWidget(
+            self.context, IDistribution['mirror_admin'],
+            format_link(self.context.mirror_admin, empty_value=empty_value),
+            header='Change the mirror administrator',
+            edit_view='+selectmirroradmins', null_display_value=empty_value,
+            step_title='Select a new mirror administrator')
+
     def linkedMilestonesForSeries(self, series):
         """Return a string of linkified milestones in the series."""
         # Listify to remove repeated queries.

=== modified file 'lib/lp/registry/templates/distribution-details.pt'
--- lib/lp/registry/templates/distribution-details.pt	2010-09-30 16:54:32 +0000
+++ lib/lp/registry/templates/distribution-details.pt	2012-05-18 07:55:22 +0000
@@ -9,64 +9,22 @@
   <div id="table-details" class="two-column-list">
     <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="driver">
       <dt>Driver:</dt>
-      <dd tal:define="link overview_menu/driver">
-        <tal:no-driver condition="not:context/driver">
-          <tal:anyone condition="not: link/enabled">
-            None
-          </tal:anyone>
-          <tal:editor condition="link/enabled">
-            Specify a driver
-          </tal:editor>
-        </tal:no-driver>
-        <a
-          tal:condition="context/driver"
-          tal:replace="structure context/driver/fmt:link" />
-        <a tal:replace="structure link/fmt:icon" />
-      </dd>
+      <dd tal:content="structure view/driver_widget" />
     </dl>
 
     <dl id="members">
       <dt>Members:</dt>
-      <dd tal:define="link overview_menu/members">
-        <tal:no-members condition="not:context/members">
-          <tal:anyone condition="not: link/enabled">
-            None
-          </tal:anyone>
-          <tal:editor condition="link/enabled">
-            Specify the members team
-          </tal:editor>
-        </tal:no-members>
-        <a
-         tal:condition="context/members"
-         tal:replace="structure context/members/fmt:link" />
-        <a tal:replace="structure link/fmt:icon" />
-      </dd>
+      <dd tal:content="structure view/members_widget" />
     </dl>
 
     <dl id="mirror-admins">
       <dt>Mirror admins:</dt>
-      <dd tal:define="link overview_menu/mirror_admin">
-        <tal:no-mirror-admin condition="not:context/mirror_admin">
-          <tal:anyone condition="not: link/enabled">
-            None
-          </tal:anyone>
-          <tal:editor condition="link/enabled">
-            Specify a mirror admin
-          </tal:editor>
-        </tal:no-mirror-admin>
-        <a
-          tal:condition="context/mirror_admin"
-          tal:replace="structure context/mirror_admin/fmt:link" />
-        <a tal:replace="structure link/fmt:icon" />
-      </dd>
+      <dd tal:content="structure view/mirror_admin_widget" />
     </dl>
   </div>