Review: Needs Fixing code Hi Ian. I reviewed your branch minus the IRC code. I think the implementation is good. I like the IAffilaition stub code. This branch is only missing feature flags:
=== modified file 'lib/canonical/launchpad/browser/vocabulary.py' --- lib/canonical/launchpad/browser/vocabulary.py 2011-01-18 18:33:21 +0000 +++ lib/canonical/launchpad/browser/vocabulary.py 2011-05-24 02:25:53 +0000 ... > + def getPickerEntry(self, associated_object): > + person = self.context > + extra = super(PersonPickerEntryAdapter, self).getPickerEntry( > + associated_object) > + > + # If the person is affiliated with the associated_object then we can > + # display a badge. > + badge_name = IHasAffiliation( > + associated_object).getAffiliationBadge(person) > + if badge_name is not None: > + extra.image = "/@@/%s" % badge_name > + if person.preferredemail is not None: > + try: > + extra.description = person.preferredemail.email > + except Unauthorized: > + extra.description = '<email address hidden>' I am surprised this works. Clearly it must since we use this code all the time, but I expected a check of IPerson.hide_email_addresses. This code looks like a team without an email address will be listed as '<email address hidden>', which is not true. > + # We will display the person's irc nick(s) after their email address > + # in the description text. > + irc_nicks = None > + if person.ircnicknames: > + irc_nicks = ", ".join( > + [IRCNicknameFormatterAPI(ircid).displayname() > + for ircid in person.ircnicknames]) > + if irc_nicks: > + extra.description = "%s (%s)" % (extra.description, irc_nicks) > + return extra The new rules here must be guarded with a feature flag to ensure only testers see our mistakes. > === added file 'lib/lp/registry/model/pillaraffiliation.py' > --- lib/lp/registry/model/pillaraffiliation.py 1970-01-01 00:00:00 > +0000 > +++ lib/lp/registry/model/pillaraffiliation.py 2011-05-24 02:25:53 > +0000 > @@ -0,0 +1,66 @@ > +# Copyright 2011 Canonical Ltd. This software is licensed under the > +# GNU Affero General Public License version 3 (see the file LICENSE). > + > +"""Adaptors to figure out affiliations between people and pillars/bugs etc. > + > +When using a person in a given context, for example as a selection item in a > +picker used to choose a bug task assignee, it is important to provide an > +indication as to how that person may be affiliated with the context. Amongst > +other reasons, this provides a visual cue that the correct person is being > +selected for example. > + > +The adaptors herein are provided for various contexts so that for a given > +person, the relevant affiliation details may be determined. > + > +""" Match the spelling of used by the code below: s/Adaptors/Adapters/ s/adaptors/adapters/ > === modified file 'lib/lp/registry/vocabularies.py' > --- lib/lp/registry/vocabularies.py 2011-05-14 15:02:13 +0000 > +++ lib/lp/registry/vocabularies.py 2011-05-24 02:25:53 +0000 > ... > @@ -186,7 +189,9 @@ > def toTerm(self, obj): > """Return the term for this object.""" > try: > - return SimpleTerm(obj, obj.name, obj.displayname) > + # Display the person's Launchpad id next to their name. > + title = "%s (~%s)" % (obj.displayname, obj.name) > + return SimpleTerm(obj, obj.name, title) Only alpha testers should see this. -- https://code.launchpad.net/~wallyworld/launchpad/person-picker-extra-detail/+merge/61973 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/person-picker-extra-detail into lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
