launchpad-reviewers team mailing list archive

[Matthew Revell <matthew.revell@xxxxxxxxxxxxx>]

Matthew Revell has proposed merging lp:~matthew.revell/launchpad/top-right-name-bug134957 into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


When logged into Launchpad, your display name is shown at the top-right of the page. This is not unique and bug 134957 raises this as a problem. This branch removes the display name and replaces it with the user's unique Launchpad id.

Rather than change fmt:link globally for Person, we (Jono and I) implemented a new nameLink formatter.

You can view this change by logging into launchpad.dev and checking that the name shown in the top-right is the user's Launchpad id, rather than the display name.

You can test this by running:

bin/test -cvv test_tales



-- 
https://code.launchpad.net/~matthew.revell/launchpad/top-right-name-bug134957/+merge/32466
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~matthew.revell/launchpad/top-right-name-bug134957 into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/templates/launchpad-loginstatus.pt'
--- lib/canonical/launchpad/templates/launchpad-loginstatus.pt	2009-07-17 17:59:07 +0000
+++ lib/canonical/launchpad/templates/launchpad-loginstatus.pt	2010-08-12 16:37:55 +0000
@@ -9,7 +9,7 @@
 <div id="logincontrol" tal:condition="view/logged_in">
   <form action="+logout" method="post">
     <input type="hidden" name="loggingout" value="1" />
-    <a tal:replace="structure request/lp:person/fmt:link:mainsite" /> &bull;
+    <a tal:replace="structure request/lp:person/fmt:name_link" /> &bull;
     <input type="submit" name="logout" value="Log Out" />
   </form>
 </div>

=== modified file 'lib/canonical/launchpad/webapp/tales.py'
--- lib/canonical/launchpad/webapp/tales.py	2010-07-30 06:08:54 +0000
+++ lib/canonical/launchpad/webapp/tales.py	2010-08-12 16:37:55 +0000
@@ -1061,6 +1061,7 @@
                          'icon': 'icon',
                          'displayname': 'displayname',
                          'unique_displayname': 'unique_displayname',
+                         'name_link': 'nameLink',
                          }
 
     final_traversable_names = {'local-time': 'local_time'}
@@ -1080,24 +1081,27 @@
         """
         return super(PersonFormatterAPI, self).url(view_name, rootsite)
 
-    def link(self, view_name, rootsite='mainsite'):
-        """See `ObjectFormatterAPI`.
-
-        Return an HTML link to the person's page containing an icon
-        followed by the person's name. The default URL for a person is to
-        the mainsite.
-        """
+    def _makeLink(self, view_name, rootsite, text):
         person = self._context
         url = self.url(view_name, rootsite)
         custom_icon = ObjectImageDisplayAPI(person)._get_custom_icon_url()
         if custom_icon is None:
             css_class = ObjectImageDisplayAPI(person).sprite_css()
             return (u'<a href="%s" class="%s">%s</a>') % (
-                url, css_class, cgi.escape(person.displayname))
+                url, css_class, cgi.escape(text))
         else:
             return (u'<a href="%s" class="bg-image" '
                      'style="background-image: url(%s)">%s</a>') % (
-                url, custom_icon, cgi.escape(person.displayname))
+                url, custom_icon, cgi.escape(text))
+
+    def link(self, view_name, rootsite='mainsite'):
+        """See `ObjectFormatterAPI`.
+
+        Return an HTML link to the person's page containing an icon
+        followed by the person's name. The default URL for a person is to
+        the mainsite.
+        """
+        return self._makeLink(view_name, rootsite, self._context.displayname)
 
     def displayname(self, view_name, rootsite=None):
         """Return the displayname as a string."""
@@ -1119,6 +1123,10 @@
         else:
             return '<img src="%s" width="14" height="14" />' % custom_icon
 
+    def nameLink(self, view_name):
+        """Return the Launchpad id of the person, linked to their profile."""
+        return self._makeLink(view_name, None, self._context.name)
+
 
 class TeamFormatterAPI(PersonFormatterAPI):
     """Adapter for `ITeam` objects to a formatted string."""

=== modified file 'lib/canonical/launchpad/webapp/tests/test_tales.py'
--- lib/canonical/launchpad/webapp/tests/test_tales.py	2010-07-14 14:11:15 +0000
+++ lib/canonical/launchpad/webapp/tests/test_tales.py	2010-08-12 16:37:55 +0000
@@ -3,9 +3,14 @@
 
 """tales.py doctests."""
 
+from doctest import DocTestSuite
 import unittest
 
-from doctest import DocTestSuite
+from zope.component import getAdapter
+from zope.traversing.interfaces import IPathAdapter
+
+from canonical.testing import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
 
 
 def test_requestapi():
@@ -98,6 +103,21 @@
     """
 
 
+class TestPersonFormatterAPI(TestCaseWithFactory):
+    """Tests for PersonFormatterAPI"""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_nameLink(self):
+        """The nameLink links to the URL with the person name as the text."""
+        person = self.factory.makePerson()
+        formatter = getAdapter(person, IPathAdapter, 'fmt')
+        result = formatter.nameLink(None)
+        expected = '<a href="%s" class="sprite person">%s</a>' % (
+            formatter.url(), person.name)
+        self.assertEqual(expected, result)
+
+
 
 def test_suite():
     """Return this module's doctest Suite. Unit tests are also run."""