launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04520
[Merge] lp:~wallyworld/launchpad/optimise-affiliation-implementation-1 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/optimise-affiliation-implementation-1 into lp:launchpad with lp:~wallyworld/launchpad/additional-affiliation-types-798764 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/optimise-affiliation-implementation-1/+merge/70805
Person pickers can display affiliation information to indicate how a person is affiliated with the current page context. The implementation is done via an affiliation adapter and is based on the current codebase which turns vocabulary terms into picker entries for display in the UI. The current code does something like:
for term in vocab_search_results_batch:
picker_adapter = IPickerEntry(term)
picker_entry = picker_adapter.getPickerEntry()
....
ie each vocab term is processed one at a time.
The above implementation artifact means that the processing to determine the affiliation details for people in a picker search is also done one person at a time. The affiliation check looks at team memberships of the pillar owner, drivers, bug contacts etc. It is inefficient to do these checks one person at a time, since each check requires a select from the TeamParticipation table.
NB The display of any affiliation details requires a feature flag to be provided: disclosure.personpicker_affiliation.enabled
This branch refactors the vocab term picker entry adapters to provide a set based interface, processing multiple people in a single call. This then allows the affiliation adapter to also provide a set based interface.
No new user visible functionality is provided by this branch; it is simply a refactoring. The diff looks larger than it is due to indenting some blocks of code.
== Implementation ==
This branch provides the refactoring to introduce set based interfaces for the adapters used to process vocab terms into picker entries. It does not optimise any of the business logic behind the adapters to take advantage of the new interfaces. This may be done later once more performance data is available to help drive any required optimisations. ie this branch is a necessary prerequisite step to allow performance tuning.
Main implementation details:
- IHasAffiliation.getAffiliationBadge(person) -> IHasAffiliation.getAffiliationBadges(persons)
- new IPickerEntrySource adapter, providing getPickerEntries(term_values, context_object, **kwarg). Previously there was IPickerEntry. getPickerEntry(context_object, **kwarg)
- rework HugeVocabularyJSONView.__call__ to use the new set based interfaces
The implementation work in this branch is all internal to HugeVocabularyJSONView
== Tests ==
bin/test -vvct test_vocabulary
bin/test -vvct test_pillaraffiliation
The above tests were altered to use the new interfaces, plus additional tests were added to ensure processing of more than one picker entry at a time works as expected.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/configure.zcml
lib/lp/app/browser/vocabulary.py
lib/lp/app/browser/tests/test_vocabulary.py
lib/lp/registry/model/pillaraffiliation.py
lib/lp/registry/tests/test_pillaraffiliation.py
./lib/lp/app/browser/vocabulary.py
327: E251 no spaces around keyword / parameter equals
--
https://code.launchpad.net/~wallyworld/launchpad/optimise-affiliation-implementation-1/+merge/70805
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/optimise-affiliation-implementation-1 into lp:launchpad.
=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml 2011-06-29 14:38:05 +0000
+++ lib/lp/app/browser/configure.zcml 2011-08-09 02:37:32 +0000
@@ -399,23 +399,23 @@
/>
<adapter
- factory="lp.app.browser.vocabulary.DefaultPickerEntryAdapter"
- />
-
- <adapter
- factory="lp.app.browser.vocabulary.PersonPickerEntryAdapter"
- />
-
- <adapter
- factory="lp.app.browser.vocabulary.BranchPickerEntryAdapter"
- />
-
- <adapter
- factory="lp.app.browser.vocabulary.SourcePackageNamePickerEntryAdapter"
- />
-
- <adapter
- factory="lp.app.browser.vocabulary.ArchivePickerEntryAdapter"
+ factory="lp.app.browser.vocabulary.DefaultPickerEntrySourceAdapter"
+ />
+
+ <adapter
+ factory="lp.app.browser.vocabulary.PersonPickerEntrySourceAdapter"
+ />
+
+ <adapter
+ factory="lp.app.browser.vocabulary.BranchPickerEntrySourceAdapter"
+ />
+
+ <adapter
+ factory="lp.app.browser.vocabulary.SourcePackageNamePickerEntrySourceAdapter"
+ />
+
+ <adapter
+ factory="lp.app.browser.vocabulary.ArchivePickerEntrySourceAdapter"
/>
<!-- TALES namespaces. -->
=== modified file 'lib/lp/app/browser/tests/test_vocabulary.py'
--- lib/lp/app/browser/tests/test_vocabulary.py 2011-08-09 02:37:30 +0000
+++ lib/lp/app/browser/tests/test_vocabulary.py 2011-08-09 02:37:32 +0000
@@ -29,7 +29,7 @@
)
from canonical.testing.layers import DatabaseFunctionalLayer
from lp.app.browser.vocabulary import (
- IPickerEntry,
+ IPickerEntrySource,
MAX_DESCRIPTION_LENGTH,
)
from lp.app.errors import UnexpectedFormData
@@ -42,59 +42,59 @@
from lp.testing.views import create_view
-class PersonPickerEntryAdapterTestCase(TestCaseWithFactory):
+class PersonPickerEntrySourceAdapterTestCase(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
def test_person_to_pickerentry(self):
# IPerson can be adpated to IPickerEntry.
person = self.factory.makePerson()
- adapter = IPickerEntry(person)
- self.assertTrue(IPickerEntry.providedBy(adapter))
+ adapter = IPickerEntrySource(person)
+ self.assertTrue(IPickerEntrySource.providedBy(adapter))
- def test_PersonPickerEntryAdapter_email_anonymous(self):
+ def test_PersonPickerEntrySourceAdapter_email_anonymous(self):
# Anonymous users cannot see entry email addresses.
person = self.factory.makePerson(email='snarf@xxxxxx')
- entry = IPickerEntry(person).getPickerEntry(None)
+ [entry] = IPickerEntrySource(person).getPickerEntries([person], None)
self.assertEqual('<email address hidden>', entry.description)
- def test_PersonPickerEntryAdapter_visible_email_logged_in(self):
+ def test_PersonPickerEntrySourceAdapter_visible_email_logged_in(self):
# Logged in users can see visible email addresses.
observer = self.factory.makePerson()
login_person(observer)
person = self.factory.makePerson(email='snarf@xxxxxx')
- entry = IPickerEntry(person).getPickerEntry(None)
+ [entry] = IPickerEntrySource(person).getPickerEntries([person], None)
self.assertEqual('snarf@xxxxxx', entry.description)
- def test_PersonPickerEntryAdapter_hidden_email_logged_in(self):
+ def test_PersonPickerEntrySourceAdapter_hidden_email_logged_in(self):
# Logged in users cannot see hidden email addresses.
person = self.factory.makePerson(email='snarf@xxxxxx')
login_person(person)
person.hide_email_addresses = True
observer = self.factory.makePerson()
login_person(observer)
- entry = IPickerEntry(person).getPickerEntry(None)
+ [entry] = IPickerEntrySource(person).getPickerEntries([person], None)
self.assertEqual('<email address hidden>', entry.description)
- def test_PersonPickerEntryAdapter_no_email_logged_in(self):
+ def test_PersonPickerEntrySourceAdapter_no_email_logged_in(self):
# Teams without email address have no desriptions.
team = self.factory.makeTeam()
observer = self.factory.makePerson()
login_person(observer)
- entry = IPickerEntry(team).getPickerEntry(None)
+ [entry] = IPickerEntrySource(team).getPickerEntries([team], None)
self.assertEqual(None, entry.description)
- def test_PersonPickerEntryAdapter_logged_in(self):
+ def test_PersonPickerEntrySourceAdapter_logged_in(self):
# Logged in users can see visible email addresses.
observer = self.factory.makePerson()
login_person(observer)
person = self.factory.makePerson(
email='snarf@xxxxxx', name='snarf')
- entry = IPickerEntry(person).getPickerEntry(None)
+ [entry] = IPickerEntrySource(person).getPickerEntries([person], None)
self.assertEqual('sprite person', entry.css)
self.assertEqual('sprite new-window', entry.link_css)
- def test_PersonPickerEntryAdapter_enhanced_picker_enabled_user(self):
+ def test_PersonPickerEntrySourceAdapter_enhanced_picker_user(self):
# The enhanced person picker provides more information for users.
person = self.factory.makePerson(email='snarf@xxxxxx', name='snarf')
creation_date = datetime(
@@ -102,31 +102,31 @@
removeSecurityProxy(person).datecreated = creation_date
getUtility(IIrcIDSet).new(person, 'eg.dom', 'snarf')
getUtility(IIrcIDSet).new(person, 'ex.dom', 'pting')
- entry = IPickerEntry(person).getPickerEntry(
- None, enhanced_picker_enabled=True,
+ [entry] = IPickerEntrySource(person).getPickerEntries(
+ [person], None, enhanced_picker_enabled=True,
picker_expander_enabled=True)
self.assertEqual('http://launchpad.dev/~snarf', entry.alt_title_link)
self.assertEqual(
['snarf on eg.dom, pting on ex.dom', 'Member since 2005-01-30'],
entry.details)
- def test_PersonPickerEntryAdapter_enhanced_picker_enabled_team(self):
+ def test_PersonPickerEntrySourceAdapter_enhanced_picker_team(self):
# The enhanced person picker provides more information for teams.
team = self.factory.makeTeam(email='fnord@xxxxxx', name='fnord')
- entry = IPickerEntry(team).getPickerEntry(
- None, enhanced_picker_enabled=True,
+ [entry] = IPickerEntrySource(team).getPickerEntries(
+ [team], None, enhanced_picker_enabled=True,
picker_expander_enabled=True)
self.assertEqual('http://launchpad.dev/~fnord', entry.alt_title_link)
self.assertEqual(['Team members: 1'], entry.details)
- def test_PersonPickerEntryAdapter_personpicker_affiliation_badges(self):
+ def test_PersonPickerEntrySourceAdapter_affiliation_badges(self):
# The person picker with affiliation enabled provides affilliation
# information.
person = self.factory.makePerson(email='snarf@xxxxxx', name='snarf')
project = self.factory.makeProduct(name='fnord', owner=person)
bugtask = self.factory.makeBugTask(target=project)
- entry = IPickerEntry(person).getPickerEntry(
- bugtask, enhanced_picker_enabled=True,
+ [entry] = IPickerEntrySource(person).getPickerEntries(
+ [person], bugtask, enhanced_picker_enabled=True,
picker_expander_enabled=True,
personpicker_affiliation_enabled=True)
self.assertEqual(1, len(entry.badges))
@@ -206,14 +206,18 @@
flags = FeatureFixture(feature_flag)
flags.setUp()
self.addCleanup(flags.cleanUp)
- team = self.factory.makeTeam(name='pting-team')
- TestPersonVocabulary.test_persons.append(team)
+ team = self.factory.makeTeam(name='xpting-team')
+ person = self.factory.makePerson(name='xpting-person')
+ creation_date = datetime(
+ 2005, 01, 30, 0, 0, 0, 0, pytz.timezone('UTC'))
+ removeSecurityProxy(person).datecreated = creation_date
+ TestPersonVocabulary.test_persons.extend([team, person])
product = self.factory.makeProduct(owner=team)
bugtask = self.factory.makeBugTask(target=product)
- form = dict(name='TestPerson', search_text='pting-team')
+ form = dict(name='TestPerson', search_text='xpting')
view = self.create_vocabulary_view(form, context=bugtask)
result = simplejson.loads(view())
- expected = {
+ expected = [{
"alt_title": team.name,
"alt_title_link": "http://launchpad.dev/~%s"; % team.name,
"api_uri": "/~%s" % team.name,
@@ -226,10 +230,24 @@
"metadata": "team",
"title": team.displayname,
"value": team.name
- }
+ },
+ {
+ "alt_title": person.name,
+ "alt_title_link": "http://launchpad.dev/~%s"; % person.name,
+ "api_uri": "/~%s" % person.name,
+ "css": "sprite person",
+ "description": "<email address hidden>",
+ "details": ['Member since 2005-01-30'],
+ "link_css": "sprite new-window",
+ "metadata": "person",
+ "title": person.displayname,
+ "value": person.name
+ }]
self.assertTrue('entries' in result)
self.assertContentEqual(
- expected.items(), result['entries'][0].items())
+ expected[0].items(), result['entries'][0].items())
+ self.assertContentEqual(
+ expected[1].items(), result['entries'][1].items())
def test_max_description_size(self):
# Descriptions over 120 characters are truncated and ellipsised.
=== modified file 'lib/lp/app/browser/vocabulary.py'
--- lib/lp/app/browser/vocabulary.py 2011-08-09 02:37:30 +0000
+++ lib/lp/app/browser/vocabulary.py 2011-08-09 02:37:32 +0000
@@ -7,11 +7,12 @@
__all__ = [
'HugeVocabularyJSONView',
- 'IPickerEntry',
+ 'IPickerEntrySource',
'get_person_picker_entry_metadata',
]
import simplejson
+from itertools import izip
from lazr.restful.interfaces import IWebServiceClientRequest
from zope.app.form.interfaces import MissingInputError
@@ -88,30 +89,42 @@
self.metadata = metadata
+class IPickerEntrySource(Interface):
+ """An adapter used to convert vocab terms to picker entries."""
+
+ def getPickerEntries(term_values, context_object, **kwarg):
+ """Return picker entries for the specified term values.
+
+ :param term_values: a collection of vocab term values
+ :param context_object: the current context used to determine any
+ affiliation for the resulting picker entries. eg a picker used to
+ select a bug task assignee will have context_object set to the bug
+ task.
+ """
+
+
@adapter(Interface)
-class DefaultPickerEntryAdapter(object):
- """Adapts Interface to IPickerEntry."""
+class DefaultPickerEntrySourceAdapter(object):
+ """Adapts Interface to IPickerEntrySource."""
- implements(IPickerEntry)
+ implements(IPickerEntrySource)
def __init__(self, context):
self.context = context
- def getPickerEntry(self, associated_object, **kwarg):
- """ Construct a PickerEntry for the context of this adapter.
-
- The associated_object represents the context for which the picker is
- being rendered. eg a picker used to select a bug task assignee will
- have associated_object set to the bug task.
- """
- extra = PickerEntry()
- if hasattr(self.context, 'summary'):
- extra.description = self.context.summary
- display_api = ObjectImageDisplayAPI(self.context)
- extra.css = display_api.sprite_css()
- if extra.css is None:
- extra.css = 'sprite bullet'
- return extra
+ def getPickerEntries(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ entries = []
+ for term_value in term_values:
+ extra = PickerEntry()
+ if hasattr(term_value, 'summary'):
+ extra.description = term_value.summary
+ display_api = ObjectImageDisplayAPI(term_value)
+ extra.css = display_api.sprite_css()
+ if extra.css is None:
+ extra.css = 'sprite bullet'
+ entries.append(extra)
+ return entries
def get_person_picker_entry_metadata(picker_entry):
@@ -122,111 +135,118 @@
@adapter(IPerson)
-class PersonPickerEntryAdapter(DefaultPickerEntryAdapter):
- """Adapts IPerson to IPickerEntry."""
+class PersonPickerEntrySourceAdapter(DefaultPickerEntrySourceAdapter):
+ """Adapts IPerson to IPickerEntrySource."""
- def getPickerEntry(self, associated_object, **kwarg):
- person = self.context
- extra = super(PersonPickerEntryAdapter, self).getPickerEntry(
- associated_object)
+ def getPickerEntries(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ picker_entries = super(PersonPickerEntrySourceAdapter, self)\
+ .getPickerEntries(term_values, context_object)
personpicker_affiliation_enabled = kwarg.get(
'personpicker_affiliation_enabled', False)
if personpicker_affiliation_enabled:
- # If the person is affiliated with the associated_object then we
+ # If a person is affiliated with the associated_object then we
# can display a badge.
- badge_info = IHasAffiliation(
- associated_object).getAffiliationBadge(person)
- if badge_info:
- extra.badges = [
- dict(url=badge_info.url, alt=badge_info.alt_text)]
+ badges = IHasAffiliation(
+ context_object).getAffiliationBadges(term_values)
+ for picker_entry, badge_info in izip(picker_entries, badges):
+ if badge_info:
+ picker_entry.badges = [
+ dict(url=badge_info.url, alt=badge_info.alt_text)]
picker_expander_enabled = kwarg.get('picker_expander_enabled', False)
- if picker_expander_enabled:
- extra.details = []
-
- if person.preferredemail is not None:
- if person.hide_email_addresses:
- extra.description = '<email address hidden>'
- else:
- try:
- extra.description = person.preferredemail.email
- except Unauthorized:
- extra.description = '<email address hidden>'
-
- extra.metadata = get_person_picker_entry_metadata(person)
- enhanced_picker_enabled = kwarg.get('enhanced_picker_enabled', False)
- 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
- if person.ircnicknames:
- irc_nicks = ", ".join(
- [IRCNicknameFormatterAPI(ircid).displayname()
- for ircid in person.ircnicknames])
- if irc_nicks and not picker_expander_enabled:
- if extra.description:
- extra.description = ("%s (%s)" %
- (extra.description, irc_nicks))
- else:
- extra.description = "%s" % irc_nicks
+ for person, picker_entry in izip(term_values, picker_entries):
if picker_expander_enabled:
- if irc_nicks:
- extra.details.append(irc_nicks)
- if person.is_team:
- extra.details.append(
- 'Team members: %s' % person.all_member_count)
+ picker_entry.details = []
+
+ if person.preferredemail is not None:
+ if person.hide_email_addresses:
+ picker_entry.description = '<email address hidden>'
else:
- extra.details.append(
- 'Member since %s' % DateTimeFormatterAPI(
- person.datecreated).date())
+ try:
+ picker_entry.description = person.preferredemail.email
+ except Unauthorized:
+ picker_entry.description = '<email address hidden>'
- return extra
+ picker_entry.metadata = get_person_picker_entry_metadata(person)
+ enhanced_picker_enabled = kwarg.get(
+ 'enhanced_picker_enabled', False)
+ if enhanced_picker_enabled:
+ # We will display the person's name (launchpad id) after their
+ # displayname.
+ picker_entry.alt_title = person.name
+ # We will linkify the person's name so it can be clicked to
+ # open the page for that person.
+ picker_entry.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
+ if person.ircnicknames:
+ irc_nicks = ", ".join(
+ [IRCNicknameFormatterAPI(ircid).displayname()
+ for ircid in person.ircnicknames])
+ if irc_nicks and not picker_expander_enabled:
+ if picker_entry.description:
+ picker_entry.description = ("%s (%s)" %
+ (picker_entry.description, irc_nicks))
+ else:
+ picker_entry.description = "%s" % irc_nicks
+ if picker_expander_enabled:
+ if irc_nicks:
+ picker_entry.details.append(irc_nicks)
+ if person.is_team:
+ picker_entry.details.append(
+ 'Team members: %s' % person.all_member_count)
+ else:
+ picker_entry.details.append(
+ 'Member since %s' % DateTimeFormatterAPI(
+ person.datecreated).date())
+ return picker_entries
@adapter(IBranch)
-class BranchPickerEntryAdapter(DefaultPickerEntryAdapter):
- """Adapts IBranch to IPickerEntry."""
+class BranchPickerEntrySourceAdapter(DefaultPickerEntrySourceAdapter):
+ """Adapts IBranch to IPickerEntrySource."""
- def getPickerEntry(self, associated_object, **kwarg):
- branch = self.context
- extra = super(BranchPickerEntryAdapter, self).getPickerEntry(
- associated_object)
- extra.description = branch.bzr_identity
- return extra
+ def getPickerEntries(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ entries = super(BranchPickerEntrySourceAdapter, self)\
+ .getPickerEntries(term_values, context_object, kwarg)
+ for branch, picker_entry in izip(term_values, entries):
+ picker_entry.description = branch.bzr_identity
+ return entries
@adapter(ISourcePackageName)
-class SourcePackageNamePickerEntryAdapter(DefaultPickerEntryAdapter):
- """Adapts ISourcePackageName to IPickerEntry."""
+class SourcePackageNamePickerEntrySourceAdapter(
+ DefaultPickerEntrySourceAdapter):
+ """Adapts ISourcePackageName to IPickerEntrySource."""
- def getPickerEntry(self, associated_object, **kwarg):
- sourcepackagename = self.context
- extra = super(
- SourcePackageNamePickerEntryAdapter, self).getPickerEntry(
- associated_object)
- descriptions = getSourcePackageDescriptions([sourcepackagename])
- extra.description = descriptions.get(
- sourcepackagename.name, "Not yet built")
- return extra
+ def getPickerEntry(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ entries = super(SourcePackageNamePickerEntrySourceAdapter, self)\
+ .getPickerEntries(term_values, context_object, kwarg)
+ for sourcepackagename, picker_entry in izip(term_values, entries):
+ descriptions = getSourcePackageDescriptions([sourcepackagename])
+ picker_entry.description = descriptions.get(
+ sourcepackagename.name, "Not yet built")
+ return entries
@adapter(IArchive)
-class ArchivePickerEntryAdapter(DefaultPickerEntryAdapter):
- """Adapts IArchive to IPickerEntry."""
+class ArchivePickerEntrySourceAdapter(DefaultPickerEntrySourceAdapter):
+ """Adapts IArchive to IPickerEntrySource."""
- def getPickerEntry(self, associated_object, **kwarg):
- archive = self.context
- extra = super(ArchivePickerEntryAdapter, self).getPickerEntry(
- associated_object)
- extra.description = '%s/%s' % (archive.owner.name, archive.name)
- return extra
+ def getPickerEntry(self, term_values, context_object, **kwarg):
+ """See `IPickerEntrySource`"""
+ entries = super(ArchivePickerEntrySourceAdapter, self)\
+ .getPickerEntries(term_values, context_object, kwarg)
+ for archive, picker_entry in izip(term_values, entries):
+ picker_entry.description = '%s/%s' % (
+ archive.owner.name, archive.name)
+ return entries
class HugeVocabularyJSONView:
@@ -273,6 +293,42 @@
batch_navigator = BatchNavigator(matches, self.request)
+ # We need to collate what IPickerEntrySource adapters are required for
+ # the items in the current batch. We expect that the batch will be
+ # homogenous and so only one adapter instance is required, but we
+ # allow for the case where the batch may contain disparate entries
+ # requiring different adapter implementations.
+
+ # A mapping from adapter class name -> adapter instance
+ adapter_cache = {}
+ # A mapping from adapter class name -> list of vocab terms
+ picker_entry_terms = {}
+ for term in batch_navigator.currentBatch():
+ picker_entry_source = IPickerEntrySource(term.value)
+ adapter_class = picker_entry_source.__class__.__name__
+ picker_terms = picker_entry_terms.get(adapter_class)
+ if picker_terms is None:
+ picker_terms = []
+ picker_entry_terms[adapter_class] = picker_terms
+ adapter_cache[adapter_class] = picker_entry_source
+ picker_terms.append(term.value)
+
+ # A mapping from vocab terms -> picker entries
+ picker_term_entries = {}
+
+ # For the list of terms associated with a picker adapter, we get the
+ # corresponding picker entries by calling the adapter.
+ for adapter_class, term_values in picker_entry_terms.items():
+ picker_entries = adapter_cache[adapter_class].getPickerEntries(
+ term_values,
+ self.context,
+ enhanced_picker_enabled=self.enhanced_picker_enabled,
+ picker_expander_enabled=self.picker_expander_enabled,
+ personpicker_affiliation_enabled=
+ self.personpicker_affiliation_enabled)
+ for term_value, picker_entry in izip(term_values, picker_entries):
+ picker_term_entries[term_value] = picker_entry
+
result = []
for term in batch_navigator.currentBatch():
entry = dict(value=term.token, title=term.title)
@@ -288,12 +344,7 @@
# needed for inplace editing via a REST call. The
# form picker doesn't need the api_url.
entry['api_uri'] = 'Could not find canonical url.'
- picker_entry = IPickerEntry(term.value).getPickerEntry(
- self.context,
- enhanced_picker_enabled=self.enhanced_picker_enabled,
- picker_expander_enabled=self.picker_expander_enabled,
- personpicker_affiliation_enabled=
- self.personpicker_affiliation_enabled)
+ picker_entry = picker_term_entries[term.value]
if picker_entry.description is not None:
if len(picker_entry.description) > MAX_DESCRIPTION_LENGTH:
entry['description'] = (
=== modified file 'lib/lp/registry/model/pillaraffiliation.py'
--- lib/lp/registry/model/pillaraffiliation.py 2011-08-09 02:37:30 +0000
+++ lib/lp/registry/model/pillaraffiliation.py 2011-08-09 02:37:32 +0000
@@ -39,12 +39,12 @@
class IHasAffiliation(Interface):
"""The affiliation status of a person with a context."""
- def getAffiliationBadge(person):
- """Return the badge for the type of affiliation the person has.
-
- The return value is a tuple: (url, alt).
-
- If the person has no affiliation with this object, return None.
+ def getAffiliationBadges(persons):
+ """Return the badges for the type of affiliation each person has.
+
+ The return value is a list of namedtuples: BadgeDetails(url, alt_text)
+
+ If a person has no affiliation with this object, their entry is None.
"""
BadgeDetails = namedtuple('BadgeDetails', ('url', 'alt_text'))
@@ -80,30 +80,34 @@
return pillar.displayname, 'driver'
return None
- def getAffiliationBadge(self, person):
+ def getAffiliationBadges(self, persons):
""" Return the affiliation badge details for a person given a context.
"""
pillar = self.getPillar()
- affiliation_details = self._getAffiliationDetails(person, pillar)
- if not affiliation_details:
- return None
-
- def getIconUrl(context, pillar, default_url):
- if IHasIcon.providedBy(context) and context.icon is not None:
- icon_url = context.icon.getURL()
- return icon_url
- if IHasIcon.providedBy(pillar) and pillar.icon is not None:
- icon_url = context.icon.getURL()
- return icon_url
- return default_url
-
- alt_text = "%s %s" % affiliation_details
- if IDistribution.providedBy(pillar):
- default_icon_url = "/@@/distribution-badge"
- else:
- default_icon_url = "/@@/product-badge"
- icon_url = getIconUrl(self.context, pillar, default_icon_url)
- return BadgeDetails(icon_url, alt_text)
+ result = []
+ for person in persons:
+ affiliation_details = self._getAffiliationDetails(person, pillar)
+ if not affiliation_details:
+ result.append(None)
+ continue
+
+ def getIconUrl(context, pillar, default_url):
+ if IHasIcon.providedBy(context) and context.icon is not None:
+ icon_url = context.icon.getURL()
+ return icon_url
+ if IHasIcon.providedBy(pillar) and pillar.icon is not None:
+ icon_url = context.icon.getURL()
+ return icon_url
+ return default_url
+
+ alt_text = "%s %s" % affiliation_details
+ if IDistribution.providedBy(pillar):
+ default_icon_url = "/@@/distribution-badge"
+ else:
+ default_icon_url = "/@@/product-badge"
+ icon_url = getIconUrl(self.context, pillar, default_icon_url)
+ result.append(BadgeDetails(icon_url, alt_text))
+ return result
class BugTaskPillarAffiliation(PillarAffiliation):
=== modified file 'lib/lp/registry/tests/test_pillaraffiliation.py'
--- lib/lp/registry/tests/test_pillaraffiliation.py 2011-08-09 02:37:30 +0000
+++ lib/lp/registry/tests/test_pillaraffiliation.py 2011-08-09 02:37:32 +0000
@@ -25,7 +25,7 @@
layer = DatabaseFunctionalLayer
def _check_affiliated_with_distro(self, person, distro, role):
- badge = IHasAffiliation(distro).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(distro).getAffiliationBadges([person])
self.assertEqual(
("/@@/distribution-badge", "Pting %s" % role), badge)
@@ -54,7 +54,7 @@
person = self.factory.makePerson()
distro = self.factory.makeDistribution(security_contact=person)
self.assertIs(
- None, IHasAffiliation(distro).getAffiliationBadge(person))
+ None, IHasAffiliation(distro).getAffiliationBadges([person])[0])
def test_no_distro_bug_supervisor_affiliation(self):
# A person who is the bug supervisor for a distro is not affiliated
@@ -62,10 +62,10 @@
person = self.factory.makePerson()
distro = self.factory.makeDistribution(bug_supervisor=person)
self.assertIs(
- None, IHasAffiliation(distro).getAffiliationBadge(person))
+ None, IHasAffiliation(distro).getAffiliationBadges([person])[0])
def _check_affiliated_with_product(self, person, product, role):
- badge = IHasAffiliation(product).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(product).getAffiliationBadges([person])
self.assertEqual(
("/@@/product-badge", "Pting %s" % role), badge)
@@ -95,7 +95,7 @@
person = self.factory.makePerson()
product = self.factory.makeProduct(security_contact=person)
self.assertIs(
- None, IHasAffiliation(product).getAffiliationBadge(person))
+ None, IHasAffiliation(product).getAffiliationBadges([person])[0])
def test_no_product_bug_supervisor_affiliation(self):
# A person who is the bug supervisor for a product is is not
@@ -103,7 +103,7 @@
person = self.factory.makePerson()
product = self.factory.makeProduct(bug_supervisor=person)
self.assertIs(
- None, IHasAffiliation(product).getAffiliationBadge(person))
+ None, IHasAffiliation(product).getAffiliationBadges([person])[0])
def test_product_owner_affiliation(self):
# A person who owns a product is affiliated.
@@ -111,6 +111,19 @@
product = self.factory.makeProduct(owner=person, name='pting')
self._check_affiliated_with_product(person, product, 'maintainer')
+ def test_distro_affiliation_multiple_people(self):
+ # A collection of people associated with a distro are affiliated.
+ people = [self.factory.makePerson() for x in range(3)]
+ distro = self.factory.makeDistribution(owner=people[0],
+ driver=people[1],
+ name='pting')
+ badges = IHasAffiliation(distro).getAffiliationBadges(people)
+ self.assertEqual(
+ ("/@@/distribution-badge", "Pting maintainer"), badges[0])
+ self.assertEqual(
+ ("/@@/distribution-badge", "Pting driver"), badges[1])
+ self.assertIs(None, badges[2])
+
class _TestBugTaskorBranchMixin:
@@ -151,18 +164,18 @@
def test_correct_pillar_is_used(self):
bugtask = self.factory.makeBugTask()
- badge = IHasAffiliation(bugtask)
- self.assertEqual(bugtask.pillar, badge.getPillar())
+ adapter = IHasAffiliation(bugtask)
+ self.assertEqual(bugtask.pillar, adapter.getPillar())
def _check_affiliated_with_distro(self, person, target, role):
bugtask = self.factory.makeBugTask(target=target)
- badge = IHasAffiliation(bugtask).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertEqual(
("/@@/distribution-badge", "Pting %s" % role), badge)
def _check_affiliated_with_product(self, person, target, role):
bugtask = self.factory.makeBugTask(target=target)
- badge = IHasAffiliation(bugtask).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertEqual(
("/@@/product-badge", "Pting %s" % role), badge)
@@ -174,7 +187,7 @@
bugtask = self.factory.makeBugTask(target=product)
Store.of(bugtask).invalidate()
with StormStatementRecorder() as recorder:
- IHasAffiliation(bugtask).getAffiliationBadge(person)
+ IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertThat(recorder, HasQueryCount(Equals(4)))
def test_distro_affiliation_query_count(self):
@@ -185,7 +198,7 @@
bugtask = self.factory.makeBugTask(target=distro)
Store.of(bugtask).invalidate()
with StormStatementRecorder() as recorder:
- IHasAffiliation(bugtask).getAffiliationBadge(person)
+ IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertThat(recorder, HasQueryCount(Equals(4)))
@@ -196,20 +209,20 @@
def test_correct_pillar_is_used(self):
branch = self.factory.makeBranch()
- badge = IHasAffiliation(branch)
- self.assertEqual(branch.product, badge.getPillar())
+ adapter = IHasAffiliation(branch)
+ self.assertEqual(branch.product, adapter.getPillar())
def _check_affiliated_with_distro(self, person, target, role):
distroseries = self.factory.makeDistroSeries(distribution=target)
sp = self.factory.makeSourcePackage(distroseries=distroseries)
branch = self.factory.makeBranch(sourcepackage=sp)
- badge = IHasAffiliation(branch).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(branch).getAffiliationBadges([person])
self.assertEqual(
("/@@/distribution-badge", "Pting %s" % role), badge)
def _check_affiliated_with_product(self, person, target, role):
branch = self.factory.makeBranch(product=target)
- badge = IHasAffiliation(branch).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(branch).getAffiliationBadges([person])
self.assertEqual(
("/@@/product-badge", "Pting %s" % role), badge)
@@ -220,8 +233,8 @@
def test_correct_pillar_is_used(self):
series = self.factory.makeDistroSeries()
- badge = IHasAffiliation(series)
- self.assertEqual(series.distribution, badge.getPillar())
+ adapter = IHasAffiliation(series)
+ self.assertEqual(series.distribution, adapter.getPillar())
def test_driver_affiliation(self):
# A person who is the driver for a distroseries is affiliated.
@@ -232,7 +245,7 @@
owner=owner, driver=driver, name='pting')
distroseries = self.factory.makeDistroSeries(
registrant=driver, distribution=distribution)
- badge = IHasAffiliation(distroseries).getAffiliationBadge(driver)
+ [badge] = IHasAffiliation(distroseries).getAffiliationBadges([driver])
self.assertEqual(
("/@@/distribution-badge", "Pting driver"), badge)
@@ -245,7 +258,7 @@
owner=owner, driver=driver, name='pting')
distroseries = self.factory.makeDistroSeries(
registrant=owner, distribution=distribution)
- badge = IHasAffiliation(distroseries).getAffiliationBadge(driver)
+ [badge] = IHasAffiliation(distroseries).getAffiliationBadges([driver])
self.assertEqual(
("/@@/distribution-badge", "Pting driver"), badge)
@@ -256,8 +269,8 @@
def test_correct_pillar_is_used(self):
series = self.factory.makeProductSeries()
- badge = IHasAffiliation(series)
- self.assertEqual(series.product, badge.getPillar())
+ adapter = IHasAffiliation(series)
+ self.assertEqual(series.product, adapter.getPillar())
def test_driver_affiliation(self):
# A person who is the driver for a productseries is affiliated.
@@ -268,7 +281,8 @@
owner=owner, driver=driver, name='pting')
productseries = self.factory.makeProductSeries(
owner=driver, product=product)
- badge = IHasAffiliation(productseries).getAffiliationBadge(driver)
+ [badge] = IHasAffiliation(productseries)\
+ .getAffiliationBadges([driver])
self.assertEqual(
("/@@/product-badge", "Pting driver"), badge)
@@ -281,7 +295,8 @@
owner=owner, driver=driver, name='pting')
productseries = self.factory.makeProductSeries(
owner=owner, product=product)
- badge = IHasAffiliation(productseries).getAffiliationBadge(driver)
+ [badge] = IHasAffiliation(productseries)\
+ .getAffiliationBadges([driver])
self.assertEqual(
("/@@/product-badge", "Pting driver"), badge)
@@ -295,7 +310,8 @@
owner=owner, project=project, name='pting')
productseries = self.factory.makeProductSeries(
owner=owner, product=product)
- badge = IHasAffiliation(productseries).getAffiliationBadge(driver)
+ [badge] = IHasAffiliation(productseries)\
+ .getAffiliationBadges([driver])
self.assertEqual(
("/@@/product-badge", "Pting driver"), badge)
@@ -307,14 +323,14 @@
def test_correct_pillar_is_used_for_product(self):
product = self.factory.makeProduct()
question = self.factory.makeQuestion(target=product)
- badge = IHasAffiliation(question)
- self.assertEqual(question.product, badge.getPillar())
+ adapter = IHasAffiliation(question)
+ self.assertEqual(question.product, adapter.getPillar())
def test_correct_pillar_is_used_for_distribution(self):
distribution = self.factory.makeDistribution()
question = self.factory.makeQuestion(target=distribution)
- badge = IHasAffiliation(question)
- self.assertEqual(question.distribution, badge.getPillar())
+ adapter = IHasAffiliation(question)
+ self.assertEqual(question.distribution, adapter.getPillar())
def test_correct_pillar_is_used_for_distro_sourcepackage(self):
distribution = self.factory.makeDistribution()
@@ -323,8 +339,8 @@
owner = self.factory.makePerson()
question = self.factory.makeQuestion(
target=distro_sourcepackage, owner=owner)
- badge = IHasAffiliation(question)
- self.assertEqual(distribution, badge.getPillar())
+ adapter = IHasAffiliation(question)
+ self.assertEqual(distribution, adapter.getPillar())
def test_answer_contact_affiliation_for_distro(self):
# A person is affiliated if they are an answer contact for a distro
@@ -337,7 +353,8 @@
with person_logged_in(answer_contact):
distro.addAnswerContact(answer_contact, answer_contact)
question = self.factory.makeQuestion(target=distro)
- badge = IHasAffiliation(question).getAffiliationBadge(answer_contact)
+ [badge] = IHasAffiliation(question)\
+ .getAffiliationBadges([answer_contact])
self.assertEqual(
("/@@/distribution-badge", "%s answer contact" %
distro.displayname), badge)
@@ -357,7 +374,8 @@
answer_contact, answer_contact)
question = self.factory.makeQuestion(
target=distro_sourcepackage, owner=answer_contact)
- badge = IHasAffiliation(question).getAffiliationBadge(answer_contact)
+ [badge] = IHasAffiliation(question)\
+ .getAffiliationBadges([answer_contact])
self.assertEqual(
("/@@/distribution-badge", "%s answer contact" %
distro_sourcepackage.displayname), badge)
@@ -376,7 +394,8 @@
distribution.addAnswerContact(answer_contact, answer_contact)
question = self.factory.makeQuestion(
target=distro_sourcepackage, owner=answer_contact)
- badge = IHasAffiliation(question).getAffiliationBadge(answer_contact)
+ [badge] = IHasAffiliation(question)\
+ .getAffiliationBadges([answer_contact])
self.assertEqual(
("/@@/distribution-badge", "%s answer contact" %
distribution.displayname), badge)
@@ -392,7 +411,8 @@
with person_logged_in(answer_contact):
product.addAnswerContact(answer_contact, answer_contact)
question = self.factory.makeQuestion(target=product)
- badge = IHasAffiliation(question).getAffiliationBadge(answer_contact)
+ [badge] = IHasAffiliation(question)\
+ .getAffiliationBadges([answer_contact])
self.assertEqual(
("/@@/product-badge", "%s answer contact" %
product.displayname), badge)
@@ -402,7 +422,7 @@
person = self.factory.makePerson()
product = self.factory.makeProduct(owner=person)
question = self.factory.makeQuestion(target=product)
- badge = IHasAffiliation(question).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(question).getAffiliationBadges([person])
self.assertEqual(
("/@@/product-badge", "%s maintainer" %
product.displayname), badge)
@@ -412,7 +432,7 @@
person = self.factory.makePerson()
distro = self.factory.makeDistribution(owner=person)
question = self.factory.makeQuestion(target=distro)
- badge = IHasAffiliation(question).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(question).getAffiliationBadges([person])
self.assertEqual(
("/@@/distribution-badge", "%s maintainer" %
distro.displayname), badge)
@@ -425,21 +445,22 @@
def test_correct_pillar_is_used_for_product(self):
product = self.factory.makeProduct()
specification = self.factory.makeSpecification(product=product)
- badge = IHasAffiliation(specification)
- self.assertEqual(specification.product, badge.getPillar())
+ adapter = IHasAffiliation(specification)
+ self.assertEqual(specification.product, adapter.getPillar())
def test_correct_pillar_is_used_for_distribution(self):
distro = self.factory.makeDistribution()
specification = self.factory.makeSpecification(distribution=distro)
- badge = IHasAffiliation(specification)
- self.assertEqual(specification.distribution, badge.getPillar())
+ adapter = IHasAffiliation(specification)
+ self.assertEqual(specification.distribution, adapter.getPillar())
def test_product_affiliation(self):
# A person is affiliated if they are affiliated with the pillar.
person = self.factory.makePerson()
product = self.factory.makeProduct(owner=person)
specification = self.factory.makeSpecification(product=product)
- badge = IHasAffiliation(specification).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(specification)\
+ .getAffiliationBadges([person])
self.assertEqual(
("/@@/product-badge", "%s maintainer" %
product.displayname), badge)
@@ -449,7 +470,8 @@
person = self.factory.makePerson()
distro = self.factory.makeDistribution(owner=person)
specification = self.factory.makeSpecification(distribution=distro)
- badge = IHasAffiliation(specification).getAffiliationBadge(person)
+ [badge] = IHasAffiliation(specification)\
+ .getAffiliationBadges([person])
self.assertEqual(
("/@@/distribution-badge", "%s maintainer" %
distro.displayname), badge)
