Merge lp:~andrea.corbellini/launchpad/bug-630302 into lp:launchpad

[Andrea Corbellini]

This is my attempt to fix bug #630302. I've set up a new URL: lp.net/people/+id/<person-id> that redirects to the person's homepage (e.g. lp.net/~name). <person-id> is the hexadecimal representation of the ID of the person.

The permalink is shown on the person's homepage, under "User information".

[Curtis Hovey (sinzui)]

I am not certain of this approach. Who advised it? I think Bug #630302 is a dupe of bug #655565 which cites several issues regarding the use of Person.id.

review: Needs Information

[Andrea Corbellini (andrea.corbellini)]

Hi. I though this was an easy fix, so I didn't have a pre-imp chat with anybody. Thank you for having pointed me to bug #655565: it shows many important use cases for this feature.

About the issues regarding Person.id, I can't find them. I have no problems to add a new database column "public_id" if needed, but I'd like to know the nature of such issues, just to be sure to avoid them.

Solved the Person.id problem, here's what I'm willing to do with this branch, if you agree:

* Expand my work to the API: adding the redirect, creating a new method "people.getById()" (or similar), adding a new attribute "person.id".
* Remove the "Permalink" <dl> from people pages.

[Curtis Hovey (sinzui)]

I think people.getById() should return if Person.merged is None. Otherwise, it needs to enter a recursive call to get an unmerged person.
    if person.merged is None:
        return person
    else:
        return people.getById(person.merged)

Suspension and deactivation are not permanent. This case is ambiguous and I do not think it should block this branch.

I wonder if this should return None of the person is a team? Teams are deleted by merging them with ~registry. There are hundreds of deleted teams that will ultimately return ~registry.

Supporting the merge case is probably all we needs to do to close this bug.

[Robert Collins (lifeless)]

This branch has a functional defect:
 - the redirect will OOPS if the resulting url is unusable for any reason (e.g. merged, suspended, deactivated).

Stylistically I think that showing the permalink in the homepage is cruft and irrelevant for most people.

So I propose the following to make it landable:
 - add tests that getting the permalink *as a normal unprivileged user* returns a 404 for the following cases:
  - private teams/persons
  - suspended users
  - deactivated users
  - deleted teams
  - deleted persons
 - add tests that getting the permalink as a user which can see a private team returns the current ~foo url in a redirect (302)
 - Add tests that getting the permalink as a normal unprivileged user returns the current ~foo url in a 302 when the (person,team) has been merged.

There may be data model limits making deleted teams impossible to handle (because that is implemented as a merge) - if so thats fine, lets make sure there is a bug about that and ignore it for this branch.

Secondly, export the permalink attribute on the API in the 'devel' version. You can choose to export more, but with its removal from the users homepage we need to supply some way to access it.

Lastly please remove it from the users homepage for now (unless Curtis or Huw jump in and say I'm wrong about this aspect).

I'm going to put this back in WIP for now, but I think the things I'm asking for here are a pretty small amount of work, and will prevent us having critical bugs shortly after landing this.

review: Needs Fixing

Unmerged revisions

12092. By Andrea Corbellini on 2010-12-17

Add the tests.

12091. By Andrea Corbellini on 2010-12-17

Show the permalink on the person homepage.

12090. By Andrea Corbellini on 2010-12-17

Handle the people/+id/123 redirect.

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2010-12-17 15:01:53 +0000
+++ lib/lp/registry/browser/person.py	2010-12-17 15:36:18 +0000
@@ -204,6 +204,8 @@
 from canonical.launchpad.webapp.login import logoutPerson
 from canonical.launchpad.webapp.menu import get_current_view
 from canonical.launchpad.webapp.publisher import LaunchpadView
+from canonical.launchpad.webapp.url import urlappend
+from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr.utils import smartquote
 from canonical.widgets import (
     LaunchpadDropdownWidget,
@@ -306,6 +308,7 @@
     Milestone,
     milestone_sort_key,
     )
+from lp.registry.model.person import Person
 from lp.services.fields import LocationField
 from lp.services.geoip.interfaces import IRequestPreferredLanguages
 from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint
@@ -778,6 +781,18 @@
         return self.redirectSubTree(
             canonical_url(person, request=self.request))
 
+    @stepthrough('+id')
+    def redirect_id(self, person_id_hex):
+        """Redirect to the canonical_url of the person with the given ID."""
+        try:
+            person_id = int(person_id_hex, 16)
+        except ValueError:
+            raise NotFoundError
+        person = Person.selectOne(Person.q.id == person_id)
+        if person:
+            return self.redirectSubTree(canonical_url(person))
+        raise NotFoundError
+
     @stepto('+me')
     def me(self):
         me = getUtility(ILaunchBag).user
@@ -2863,6 +2878,13 @@
             return formatter(content).text_to_html()
 
     @cachedproperty
+    def permalink(self):
+        """Return the permalink that points to this account."""
+        rooturl = allvhosts.configs['mainsite'].rooturl
+        link = 'people/+id/%x' % self.context.id
+        return urlappend(rooturl, link)
+
+    @cachedproperty
     def recently_approved_members(self):
         members = self.context.getMembersByStatus(
             TeamMembershipStatus.APPROVED,

=== modified file 'lib/lp/registry/stories/person/xx-person-home.txt'
--- lib/lp/registry/stories/person/xx-person-home.txt	2010-08-27 22:40:36 +0000
+++ lib/lp/registry/stories/person/xx-person-home.txt	2010-12-17 15:36:18 +0000
@@ -138,6 +138,18 @@
     English
 
 
+Permalink
+---------
+
+Each person has a permalink, which is shown on the homepage and is
+visible by everybody:
+
+    >>> browser.open('http://launchpad.dev/~name16')
+    >>> print extract_text(find_tag_by_id(browser.contents, 'permalink'))
+    Permalink:
+    http://launchpad.dev/people/+id/10
+
+
 Summary Pagelets
 ----------------
 

=== modified file 'lib/lp/registry/templates/person-portlet-contact-details.pt'
--- lib/lp/registry/templates/person-portlet-contact-details.pt	2010-12-01 22:05:51 +0000
+++ lib/lp/registry/templates/person-portlet-contact-details.pt	2010-12-17 15:36:18 +0000
@@ -200,6 +200,11 @@
              tal:content="context/karma">342</a>
       </dd>
     </dl>
+
+    <dl id="permalink">
+      <dt>Permalink:</dt>
+      <dd tal:content="view/permalink">None</dd>
+    </dl>
   </div>
 
 </div>