launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-click-item-detail into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #791116 in Launchpad itself: "Linkify launchpad id in picker results"
  https://bugs.launchpad.net/launchpad/+bug/791116

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-click-item-detail/+merge/63096

This branch uses infrastructure added to lazr-js pickers to display links to a person's home page in launchpad when displaying search results in a person picker.

== Implementation ==

The lazr-js changes do all the work. This branch simply adds the required attributes to the json data structure passed to the picker. It also reverses an earlier implementation used to display lp id in the picker results since the lazr-js modifications allow it to be implemented much better.

The lazr-js branch is lp:~wallyworld/lazr-js/picker-entry-extra-title-links
When this lazr-js mp is approved and a new lazr-js egg added to the download cache, this mp will update versions.cfg accordingly before landing.

== Demo ==

See screenshot:
http://people.canonical.com/~ianb/person-picker-links.png

When clicking on the links, a new window is opened and the picker is left in place. The user can configure their browser to open the link in a new tab instead of a new window if they wish.

== Tests ==

Enhance the vocabulary-json.txt tests.

== Lint ==


Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/doc/vocabulary-json.txt
  lib/lp/app/browser/vocabulary.py
  lib/lp/registry/vocabularies.py

./lib/canonical/launchpad/doc/vocabulary-json.txt
       1: narrative uses a moin header.
     176: want exceeds 78 characters.
./lib/lp/registry/vocabularies.py
     592: E261 at least two spaces before inline comment

-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-click-item-detail/+merge/63096
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-click-item-detail into lp:launchpad.

=== modified file 'lib/canonical/launchpad/doc/vocabulary-json.txt'
--- lib/canonical/launchpad/doc/vocabulary-json.txt	2011-05-27 01:06:07 +0000
+++ lib/canonical/launchpad/doc/vocabulary-json.txt	2011-06-01 10:38:28 +0000
@@ -56,9 +56,12 @@
     {
         "entries": [
             {
+                "alt_title": "guadamen",
+                "alt_title_link": "http://launchpad.dev/~guadamen";,
                 "api_uri": "/~guadamen",
                 "css": "sprite team",
-                "title": "GuadaMen (guadamen)",
+                "link_css": "js-action",
+                "title": "GuadaMen",
                 "value": "guadamen"
             }
         ],
@@ -84,9 +87,12 @@
     {
         "entries": [
             {
+                "alt_title": "commercial-admins",
+                "alt_title_link": "http://launchpad.dev/~commercial-admins";,
                 "api_uri": "/~commercial-admins",
                 "css": "sprite team",
-                "title": "Commercial Subscription Admins (commercial-admins)",
+                "link_css": "js-action",
+                "title": "Commercial Subscription Admins",
                 "value": "commercial-admins"
             }
         ],
@@ -100,10 +106,13 @@
     {
         "entries": [
             {
+                "alt_title": "name16",
+                "alt_title_link": "http://launchpad.dev/~name16";,
                 "api_uri": "/~name16",
                 "css": "sprite person",
                 "description": "<email address hidden>",
-                "title": "Foo Bar (name16)",
+                "link_css": "js-action",
+                "title": "Foo Bar",
                 "value": "name16"
             }
         ],
@@ -138,10 +147,13 @@
     {
         "entries": [
             {
+                "alt_title": "name12",
+                "alt_title_link": "http://launchpad.dev/~name12";,
                 "api_uri": "/~name12",
                 "css": "sprite person",
                 "description": "<email address hidden>",
-                "title": "Sample Person (name12)",
+                "link_css": "js-action",
+                "title": "Sample Person",
                 "value": "name12"
             }
         ],
@@ -157,10 +169,13 @@
     {
         "entries": [
             {
+                "alt_title": "mark",
+                "alt_title_link": "http://launchpad.dev/~mark";,
                 "api_uri": "/~mark",
                 "css": "sprite person",
                 "description": "<email address hidden> (mark on irc.freenode.net)",
-                "title": "Mark Shuttleworth (mark)",
+                "link_css": "js-action",
+                "title": "Mark Shuttleworth",
                 "value": "mark"
             }
         ],

=== modified file 'lib/lp/app/browser/vocabulary.py'
--- lib/lp/app/browser/vocabulary.py	2011-05-28 15:08:55 +0000
+++ lib/lp/app/browser/vocabulary.py	2011-06-01 10:38:28 +0000
@@ -57,16 +57,26 @@
     description = Attribute('Description')
     image = Attribute('Image URL')
     css = Attribute('CSS Class')
+    alt_title = Attribute('Alternative title')
+    title_link = Attribute('URL used for anchor on title')
+    alt_title_link = Attribute('URL used for anchor on alt title')
+    link_css = Attribute('CSS Class for links')
 
 
 class PickerEntry:
     """See `IPickerEntry`."""
     implements(IPickerEntry)
 
-    def __init__(self, description=None, image=None, css=None, api_uri=None):
+    def __init__(self, description=None, image=None, css=None, alt_title=None,
+                 title_link=None, alt_title_link=None, link_css='js-action',
+                 api_uri=None):
         self.description = description
         self.image = image
         self.css = css
+        self.alt_title = alt_title
+        self.title_link = title_link
+        self.alt_title_link = alt_title_link
+        self.link_css = link_css
 
 
 @adapter(Interface)
@@ -123,6 +133,12 @@
                     extra.description = '<email address hidden>'
 
         if enhanced_picker_enabled:
+            # We will display the person's name (launchpad id) after their
+            # displayname.
+            extra.alt_title = person.name
+            # We will linkify the person's name so it can be clicked to open
+            # the page for that person.
+            extra.alt_title_link = canonical_url(person, rootsite='mainsite')
             # We will display the person's irc nick(s) after their email
             # address in the description text.
             irc_nicks = None
@@ -236,6 +252,14 @@
                 entry['image'] = picker_entry.image
             if picker_entry.css is not None:
                 entry['css'] = picker_entry.css
+            if picker_entry.alt_title is not None:
+                entry['alt_title'] = picker_entry.alt_title
+            if picker_entry.title_link is not None:
+                entry['title_link'] = picker_entry.title_link
+            if picker_entry.alt_title_link is not None:
+                entry['alt_title_link'] = picker_entry.alt_title_link
+            if picker_entry.link_css is not None:
+                entry['link_css'] = picker_entry.link_css
             result.append(entry)
 
         self.request.response.setHeader('Content-type', 'application/json')

=== modified file 'lib/lp/registry/vocabularies.py'
--- lib/lp/registry/vocabularies.py	2011-05-28 04:09:11 +0000
+++ lib/lp/registry/vocabularies.py	2011-06-01 10:38:28 +0000
@@ -178,7 +178,6 @@
 from lp.registry.model.sourcepackagename import SourcePackageName
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.database import bulk
-from lp.services.features import getFeatureFlag
 from lp.services.propertycache import cachedproperty
 
 
@@ -187,19 +186,10 @@
 
     _table = Person
 
-    def __init__(self, context=None):
-        self.enhanced_picker_enabled = bool(
-            getFeatureFlag('disclosure.picker_enhancements.enabled'))
-
     def toTerm(self, obj):
         """Return the term for this object."""
         try:
-            if self.enhanced_picker_enabled:
-                # Display the person's Launchpad id next to their name.
-                title = "%s (%s)" % (obj.displayname, obj.name)
-            else:
-                title = obj.displayname
-            return SimpleTerm(obj, obj.name, title)
+            return SimpleTerm(obj, obj.name, obj.displayname)
         except Unauthorized:
             return None