Code review comment for lp:~jamalta/launchpad/bug-127489

I went through the pagetest you referenced, and it will actually not fail. It is also not possible to test my changes with this because there is not enough data. The problem is that the only two icons displayed are bugs and translations. Sorted alphabetically, bugs will obviously always be first, which is the case with the replacement sort.

To test this I need to be able to add a third entry, say, Answers for example. I have this in my local environment (which I should really revert since it's going to break tests in the future), but I don't know how to properly write a pagetest for it because of this limitation. I think this is the reason salgado recommended writing a unittest.

Let me give the unittests another try to see if I can get them done. Thanks for your help so far!

> Hi Gavin,
>
> Thanks for your input regarding this bug. I have actually been working on a
> test for this. I did not have a mentor when I started working on this issue,
> but went on #launchpad-dev to get some help. Salgado had recommended that I
> write a unittest to call the view, and make sure the data was returned in the
> right order, using a LaunchpadObjectFactory. I had been struggling trying to
> figure out how to do that exactly, and haven't gotten back to it in a few days
> now. I will take a look at the test you mentioned and make sure I fix it.
> Also, I appreciate your comment regarding the depracated cmp() function. I am
> fairly new to Python and was not aware of this. I will fix that shortly as
> well, using your recommendation.
>
> > Hi Jamal,
> >
> > Thank you for working on this bug!
> >
> > As Stuart commented, this will need a pagetest. In fact, there is
> > already one that is probably broken by this change. Have a look at the
> > end of lib/lp/registry/stories/foaf/xx-person-home.txt.
> >
> > Did you have a mentor for this branch, or a pre-implementation
> > discussion? Perhaps that might have spotted the need to update the
> > pagetest. Have a look at https://dev.launchpad.net/PatchSubmission if
> > you haven't already. It's the same process as all the Launchpad devs
> > from Canonical use.
> >
> > I have a comment about the code, because the cmp argument to sorted()
> > is deprecated (and I think it's gone entirely in Python 3).
> >
> > Thanks again!
> >
> > Gavin.
> >
> >
> > > === modified file 'lib/lp/registry/browser/person.py'
> > > --- lib/lp/registry/browser/person.py 2009-07-29 15:34:46 +0000
> > > +++ lib/lp/registry/browser/person.py 2009-07-31 21:12:09 +0000
> > > @@ -2517,7 +2517,10 @@
> > > categories = set()
> > > for contrib in self.contributions:
> > > categories.update(category for category in
> > contrib['categories'])
> > > - return sorted(categories, key=attrgetter('title'))
> > > + sort = {'code': 0, 'bugs': 1, 'blueprints': 2, 'translations': 3,
> > > + 'answers': 4, 'specs': 5, 'soyuz': 6}
> > > + return sorted(categories, key=attrgetter('name'),
> > > + cmp=lambda x,y: cmp(sort[x], sort[y]))
> >
> > Because cmp is deprecated, I think this would be better as:
> >
> > return sorted(categories, key=lambda category: sort[category.name])
> >
> > >
> > > @cachedproperty
> > > def context_is_probably_a_team(self):