launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04867
[Merge] lp:~wallyworld/launchpad/better-bugtask-affiliation into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/better-bugtask-affiliation into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #841425 in Launchpad itself: "Person picker should show multiple affiliations for bug tasks"
https://bugs.launchpad.net/launchpad/+bug/841425
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/better-bugtask-affiliation/+merge/74033
This branch allows the person picker to display affiliation with multiple pillars and modifies the affiliation adaptors to look up multiple affiliations if they exist. because multiple affiliations are now displayed, the affiliation display for each pillar is restricted to the most important affiliated role instead of displaying all of the roles as happened with previous versions of this functionality.
== Implementation ==
The biggest change is to pillaraffiliation.py. The affiliation adaptors needed to be restructured such that they would look up multiple pillars for a given context. Only bug tasks currently can have multiple pillars, which are all the pillars of the bug tasks associated with the parent bug. The affiliation lookup for the multiple pillars is essentially the same as that for a single pillar.
As well as the above, the general business logic was optimised to accommodate the new requirements. eg getIconUrl was moved to be an instance method. The changes still take advantage of previous work which allows team based affiliations for maintainer, drivers, etc to be retrieved with a single query.
The structure of the data returned in the BadgeDetails also needed to be changed. Instead of (icon_url, text) we now have (icon_url, pillar label, role). This allows the client javascript to have the information it needs to determine the unique affiliations for each pillar from the list of badge details retrieved from the server.
== Demo and QA ==
The screenshow shows how affiliation details for more than one pillar are displayed.
http://people.canonical.com/~ianb/affiliation-multiple-badges.png
== Tests ==
test_pillaraffiliation:
The existing tests were ported to use the new BadgeDetails attributes (url, label,role), plus a new test was added:
test_affiliated_with_multiple_bugtasks()
The query count tests were moved also to the TestPillarAffiliation test case.
lp/app/browser/teststest_vocabulary
The existing tests were ported to use the new BadgeDetails attributes
test_picker.js
The existing tests were ported to use the new BadeDetails attribute and the tests modified to reflect the different data being displayed.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/app/browser/vocabulary.py
lib/lp/app/browser/tests/test_vocabulary.py
lib/lp/app/javascript/picker/picker.js
lib/lp/app/javascript/picker/tests/test_picker.js
lib/lp/registry/model/pillaraffiliation.py
lib/lp/registry/tests/test_pillaraffiliation.py
--
https://code.launchpad.net/~wallyworld/launchpad/better-bugtask-affiliation/+merge/74033
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/better-bugtask-affiliation into lp:launchpad.
=== modified file 'lib/lp/app/browser/tests/test_vocabulary.py'
--- lib/lp/app/browser/tests/test_vocabulary.py 2011-08-29 16:13:43 +0000
+++ lib/lp/app/browser/tests/test_vocabulary.py 2011-09-05 00:24:26 +0000
@@ -132,11 +132,14 @@
personpicker_affiliation_enabled=True)
self.assertEqual(3, len(entry.badges))
self.assertEqual('/@@/product-badge', entry.badges[0]['url'])
- self.assertEqual('Fnord maintainer', entry.badges[0]['alt'])
+ self.assertEqual('Fnord', entry.badges[0]['label'])
+ self.assertEqual('maintainer', entry.badges[0]['role'])
self.assertEqual('/@@/product-badge', entry.badges[1]['url'])
- self.assertEqual('Fnord driver', entry.badges[1]['alt'])
+ self.assertEqual('Fnord', entry.badges[1]['label'])
+ self.assertEqual('driver', entry.badges[1]['role'])
self.assertEqual('/@@/product-badge', entry.badges[2]['url'])
- self.assertEqual('Fnord bug supervisor', entry.badges[2]['alt'])
+ self.assertEqual('Fnord', entry.badges[2]['label'])
+ self.assertEqual('bug supervisor', entry.badges[2]['role'])
def test_PersonPickerEntryAdapter_badges_without_IHasAffiliation(self):
# The enhanced person picker handles objects that do not support
@@ -320,9 +323,11 @@
"alt_title_link": "http://launchpad.dev/~%s"; % team.name,
"api_uri": "/~%s" % team.name,
"badges":
- [{"alt": "%s maintainer" % product.displayname,
+ [{"label": product.displayname,
+ "role": "maintainer",
"url": "/@@/product-badge"},
- {"alt": "%s driver" % product.displayname,
+ {"label": product.displayname,
+ "role": "driver",
"url": "/@@/product-badge"}],
"css": "sprite team",
"details": ['Team members: 1'],
=== modified file 'lib/lp/app/browser/vocabulary.py'
--- lib/lp/app/browser/vocabulary.py 2011-08-30 15:06:35 +0000
+++ lib/lp/app/browser/vocabulary.py 2011-09-05 00:24:26 +0000
@@ -161,7 +161,9 @@
picker_entry.badges = []
for badge_info in badges:
picker_entry.badges.append(
- dict(url=badge_info.url, alt=badge_info.alt_text))
+ dict(url=badge_info.url,
+ label=badge_info.label,
+ role=badge_info.role))
picker_expander_enabled = kwarg.get('picker_expander_enabled', False)
for person, picker_entry in izip(term_values, picker_entries):
=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js 2011-08-23 16:11:55 +0000
+++ lib/lp/app/javascript/picker/picker.js 2011-09-05 00:24:26 +0000
@@ -444,13 +444,13 @@
var already_processed = [];
Y.each(data.badges, function(badge_info) {
var badge_url = badge_info.url;
- var badge_alt = badge_info.alt;
- if (already_processed.indexOf(badge_url)<0) {
- already_processed.push(badge_url);
+ var badge_alt = badge_info.label + ' ' + badge_info.role;
+ if (already_processed.indexOf(badge_info.label)<0) {
+ already_processed.push(badge_info.label);
var badge = Y.Node.create('<img></img>')
.addClass('badge')
.set('src', badge_url)
- .set('alt', badge_alt);
+ .set('alt', Y.Escape.html(badge_alt));
badges.appendChild(badge);
}
});
@@ -495,29 +495,25 @@
details_node.append(data_node);
}
if (Y.Lang.isArray(data.badges)) {
- var badge_url = null;
- var affiliation = Y.Node.create('<div></div>')
- .addClass('affiliation');
- var affiliation_details = [];
+ var already_processed = [];
Y.each(data.badges, function(badge_info) {
- var badge_alt = badge_info.alt;
- if (badge_url === null) {
- badge_url = badge_info.url;
+ if (already_processed.indexOf(badge_info.label)<0) {
+ already_processed.push(badge_info.label);
+ var affiliation = Y.Node.create('<div></div>')
+ .addClass('affiliation');
+ var badge_text = badge_info.label + ' ' + badge_info.role;
var badge = Y.Node.create('<img></img>')
- .set('src', badge_url)
- .set('alt', badge_alt);
+ .set('src', badge_info.url)
+ .set('alt', Y.Escape.html(badge_text));
affiliation.appendChild(badge);
affiliation.appendChild(Y.Node.create('Affiliation'));
details_node.append(affiliation);
+ var affiliation_text = Y.Node.create('<div></div>')
+ .addClass('affiliation-text');
+ affiliation_text.appendChild(Y.Node.create(badge_text));
+ details_node.append(affiliation_text);
}
- affiliation_details.push(Y.Escape.html(badge_alt));
});
- var affiliation_text = Y.Node.create('<div></div>')
- .addClass('affiliation-text');
- affiliation_text.appendChild(
- Y.Node.create(affiliation_details.join('<br />')));
- details_node.append(affiliation_text);
-
}
var links = [];
links.push(Y.Node.create(
=== modified file 'lib/lp/app/javascript/picker/tests/test_picker.js'
--- lib/lp/app/javascript/picker/tests/test_picker.js 2011-08-19 06:08:49 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker.js 2011-09-05 00:24:26 +0000
@@ -327,9 +327,17 @@
test_title_badges: function () {
this.picker.render();
var badge_info = [
- {url: '../../lazr/assets/skins/sam/search.png', alt: 'alt 1'},
- {url: '../../lazr/assets/skins/sam/spinner.png', alt: 'alt 2'},
- {url: '../../lazr/assets/skins/sam/spinner.png', alt: 'alt 3'}];
+ {
+ url: '../../lazr/assets/skins/sam/search.png',
+ label: 'product 1',
+ role: 'driver'},
+ { url: '../../lazr/assets/skins/sam/spinner.png',
+ label: 'product 2',
+ role: 'maintainer'},
+ { url: '../../lazr/assets/skins/sam/spinner.png',
+ label: 'product 2',
+ role: 'driver'
+ }];
this.picker.set('results', [
{
badges: badge_info,
@@ -353,8 +361,9 @@
Assert.areEqual(
badge_info[i].url, img_node.getAttribute('src'),
'Unexpected badge url');
+ var badge_text = badge_info[i].label + ' ' + badge_info[i].role;
Assert.areEqual(
- badge_info[i].alt, img_node.get('alt'),
+ badge_text, img_node.get('alt'),
'Unexpected badge alt text');
}
},
@@ -364,8 +373,14 @@
// of the expander.
this.picker.render();
var badge_info = [
- {url: '../../lazr/assets/skins/sam/spinner.png', alt: 'alt 1'},
- {url: '../../lazr/assets/skins/sam/spinner.png', alt: 'alt 2'}];
+ {
+ url: '../../lazr/assets/skins/sam/spinner.png',
+ label: 'product 1',
+ role: 'maintainer'},
+ {
+ url: '../../lazr/assets/skins/sam/search.png',
+ label: 'product 2',
+ role: 'driver'}];
this.picker.set('results', [
{
badges: badge_info,
@@ -378,15 +393,26 @@
var bb = this.picker.get('boundingBox');
var li = bb.one('.yui3-picker-results li');
var details = li.expander.content_node;
- var affiliation_header = details.one('div.affiliation');
+ var affiliation_header = details.one('div.affiliation:nth-child(1)');
+ Assert.areEqual('Affiliation', affiliation_header.get('text'));
var badge_img = affiliation_header.one('img');
- Assert.areEqual('Affiliation', affiliation_header.get('text'));
- Assert.areEqual('alt 1', badge_img.get('alt'));
+ Assert.areEqual('product 1 maintainer', badge_img.get('alt'));
Assert.areEqual(
'../../lazr/assets/skins/sam/spinner.png',
badge_img.getAttribute('src'));
- var affiliation_text = details.one('div.affiliation-text');
- Assert.areEqual('alt 1<br>alt 2', affiliation_text.get('innerHTML'));
+ affiliation_header = details.one('div.affiliation:nth-child(3)');
+ badge_img = affiliation_header.one('img');
+ Assert.areEqual('product 2 driver', badge_img.get('alt'));
+ Assert.areEqual(
+ '../../lazr/assets/skins/sam/search.png',
+ badge_img.getAttribute('src'));
+ var affiliation_text = details.one(
+ 'div.affiliation-text:nth-child(2)');
+ Assert.areEqual(
+ 'product 1 maintainer', affiliation_text.get('innerHTML'));
+ affiliation_text = details.one('div.affiliation-text:nth-child(4)');
+ Assert.areEqual(
+ 'product 2 driver', affiliation_text.get('innerHTML'));
},
test_results_display_escaped: function () {
=== modified file 'lib/lp/registry/model/pillaraffiliation.py'
--- lib/lp/registry/model/pillaraffiliation.py 2011-09-01 06:18:57 +0000
+++ lib/lp/registry/model/pillaraffiliation.py 2011-09-05 00:24:26 +0000
@@ -43,21 +43,23 @@
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)
+ The return value is a list of namedtuples:
+ BadgeDetails(url, label, role)
If a person has no affiliation with this object, their entry is None.
"""
-BadgeDetails = namedtuple('BadgeDetails', ('url', 'alt_text'))
+BadgeDetails = namedtuple('BadgeDetails', ('url', 'label', 'role'))
@adapter(Interface)
class PillarAffiliation(object):
"""Default affiliation adapter.
- Subclasses may need to override getPillar() in order to provide the pillar
- entity for which affiliation is to be determined. The default is just to
- use the context object directly.
+ Subclasses may need to override getPillars() in order to provide the
+ pillar entities for which affiliation is to be determined. A given context
+ may supply for than one pillar for which affiliation can be determined.
+ The default is just to use the context object directly.
"""
implements(IHasAffiliation)
@@ -74,27 +76,45 @@
def __init__(self, context):
self.context = context
- def getPillar(self):
- return self.context
-
- def _getAffiliation(self, person, pillar):
+ def getPillars(self):
+ return [self.context]
+
+ def getIconUrl(self, pillar):
+ if (IHasIcon.providedBy(self.context)
+ and self.context.icon is not None):
+ icon_url = self.context.icon.getURL()
+ return icon_url
+ if IHasIcon.providedBy(pillar) and pillar.icon is not None:
+ icon_url = pillar.icon.getURL()
+ return icon_url
+ if IDistribution.providedBy(pillar):
+ return "/@@/distribution-badge"
+ else:
+ return "/@@/product-badge"
+
+ def _getAffiliation(self, person, pillars):
""" Return the affiliation information for a person, if any.
Subclasses will override this method to perform specific affiliation
checks.
- The return result is a list of tuples (pillar displayanme, role).
+ The return result is a list of AffiliationRecord.
"""
return []
- def _getAffiliationTeamRoles(self, pillar):
+ def _getAffiliationTeamRoles(self, pillars):
""" Return teams for which a person needs to belong, if affiliated.
A person is affiliated with a pillar if they are in the list of
drivers or are the maintainer.
"""
- result = {
- (pillar.displayname, 'maintainer'): [pillar.owner],
- (pillar.displayname, 'driver'): pillar.drivers}
+ result = {}
+ for pillar in pillars:
+ result[BadgeDetails(
+ self.getIconUrl(pillar),
+ pillar.displayname, 'maintainer')] = [pillar.owner]
+ result[BadgeDetails(
+ self.getIconUrl(pillar),
+ pillar.displayname, 'driver')] = pillar.drivers
return result
def getAffiliationBadges(self, persons):
@@ -105,11 +125,11 @@
_getAffiliationTeamRoles
2. Specific affiliation checks as performed by _getAffiliation
"""
- pillar = self.getPillar()
+ pillars = self.getPillars()
result = []
# We find the teams to check for participation..
- affiliation_team_details = self._getAffiliationTeamRoles(pillar)
+ affiliation_team_details = self._getAffiliationTeamRoles(pillars)
teams_to_check = set()
for teams in affiliation_team_details.values():
teams_to_check.update(teams)
@@ -118,52 +138,37 @@
for person in persons:
# Specific affiliations
- affiliations = self._getAffiliation(person, pillar)
+ badges = self._getAffiliation(person, pillars)
# Generic, team based affiliations
affiliated_teams = people_teams.get(person, [])
for affiliated_team in affiliated_teams:
- for affiliation, teams in affiliation_team_details.items():
+ for badge, teams in affiliation_team_details.items():
if affiliated_team in teams:
- affiliations.append(affiliation)
+ badges.append(badge)
- if not affiliations:
+ if not badges:
result.append([])
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 = pillar.icon.getURL()
- return icon_url
- return default_url
-
- if IDistribution.providedBy(pillar):
- default_icon_url = "/@@/distribution-badge"
- else:
- default_icon_url = "/@@/product-badge"
- icon_url = getIconUrl(self.context, pillar, default_icon_url)
-
# Sort the affiliation list according the the importance of each
# affiliation role.
- affiliations.sort(
- key=lambda affiliation_rec:
- self.affiliation_priorities.get(affiliation_rec[1], 10))
- badges = []
- for affiliation in affiliations:
- alt_text = "%s %s" % affiliation
- badges.append(BadgeDetails(icon_url, alt_text))
+ badges.sort(
+ key=lambda badge:
+ self.affiliation_priorities.get(badge.role, 10))
result.append(badges)
return result
class BugTaskPillarAffiliation(PillarAffiliation):
"""An affiliation adapter for bug tasks."""
- def getPillar(self):
- return self.context.pillar
+ def getPillars(self):
+ result = []
+ bug = self.context.bug
+ for bugtask in bug.bugtasks:
+ result.append(bugtask.pillar)
+ return result
- def _getAffiliationTeamRoles(self, pillar):
+ def _getAffiliationTeamRoles(self, pillars):
""" A person is affiliated with a bugtask based on (in order):
- owner of bugtask pillar
- driver of bugtask pillar
@@ -171,38 +176,46 @@
- security contact of bugtask pillar
"""
super_instance = super(BugTaskPillarAffiliation, self)
- result = super_instance._getAffiliationTeamRoles(pillar)
- result[(pillar.displayname, 'bug supervisor')] = (
- [pillar.bug_supervisor])
- result[(pillar.displayname, 'security contact')] = (
- [pillar.security_contact])
+ result = super_instance._getAffiliationTeamRoles(pillars)
+ for pillar in pillars:
+ result[BadgeDetails(
+ self.getIconUrl(pillar),
+ pillar.displayname,
+ 'bug supervisor')] = [pillar.bug_supervisor]
+ result[BadgeDetails(
+ self.getIconUrl(pillar),
+ pillar.displayname,
+ 'security contact')] = [pillar.security_contact]
return result
class BranchPillarAffiliation(BugTaskPillarAffiliation):
"""An affiliation adapter for branches."""
- def getPillar(self):
- return self.context.product or self.context.distribution
+ def getPillars(self):
+ return [self.context.product or self.context.distribution]
def getBranch(self):
return self.context
- def _getAffiliation(self, person, pillar):
+ def _getAffiliation(self, person, pillars):
super_instance = super(BranchPillarAffiliation, self)
- result = super_instance._getAffiliation(person, pillar)
- if self.getBranch().isPersonTrustedReviewer(person):
- result.append((pillar.displayname, 'trusted reviewer'))
+ result = super_instance._getAffiliation(person, pillars)
+ for pillar in pillars:
+ if self.getBranch().isPersonTrustedReviewer(person):
+ result.append(BadgeDetails(
+ self.getIconUrl(pillar),
+ pillar.displayname, 'trusted reviewer'))
return result
class CodeReviewVotePillarAffiliation(BranchPillarAffiliation):
"""An affiliation adapter for CodeReviewVotes."""
- def getPillar(self):
+ def getPillars(self):
"""Return the target branch'pillar."""
branch = self.getBranch()
- return branch.product or branch.distribution
+ return [branch.product or branch.distribution]
def getBranch(self):
return self.context.branch_merge_proposal.target_branch
@@ -210,20 +223,20 @@
class DistroSeriesPillarAffiliation(PillarAffiliation):
"""An affiliation adapter for distroseries."""
- def getPillar(self):
- return self.context.distribution
+ def getPillars(self):
+ return [self.context.distribution]
class ProductSeriesPillarAffiliation(PillarAffiliation):
"""An affiliation adapter for productseries."""
- def getPillar(self):
- return self.context.product
+ def getPillars(self):
+ return [self.context.product]
class SpecificationPillarAffiliation(PillarAffiliation):
"""An affiliation adapter for blueprints."""
- def getPillar(self):
- return (self.context.target)
+ def getPillars(self):
+ return [self.context.target]
class QuestionPillarAffiliation(PillarAffiliation):
@@ -235,12 +248,12 @@
- driver of question target
"""
- def getPillar(self):
- return self.context.product or self.context.distribution
+ def getPillars(self):
+ return [self.context.product or self.context.distribution]
- def _getAffiliation(self, person, pillar):
- result = (super(QuestionPillarAffiliation, self)
- ._getAffiliation(person, pillar))
+ def _getAffiliation(self, person, pillars):
+ super_instance = super(QuestionPillarAffiliation, self)
+ result = super_instance._getAffiliation(person, pillars)
target = self.context.target
if IDistributionSourcePackage.providedBy(target):
question_targets = (target, target.distribution)
@@ -249,8 +262,14 @@
questions_person = IQuestionsPerson(person)
for target in questions_person.getDirectAnswerQuestionTargets():
if target in question_targets:
- result.append((target.displayname, 'answer contact'))
+ result.append(
+ BadgeDetails(
+ self.getIconUrl(pillars[0]),
+ target.displayname, 'answer contact'))
for target in questions_person.getTeamAnswerQuestionTargets():
if target in question_targets:
- result.append((target.displayname, 'answer contact'))
+ result.append(
+ BadgeDetails(
+ self.getIconUrl(pillars[0]),
+ target.displayname, 'answer contact'))
return result
=== modified file 'lib/lp/registry/tests/test_pillaraffiliation.py'
--- lib/lp/registry/tests/test_pillaraffiliation.py 2011-08-18 20:56:37 +0000
+++ lib/lp/registry/tests/test_pillaraffiliation.py 2011-09-05 00:24:26 +0000
@@ -35,12 +35,12 @@
distro = self.factory.makeDistribution(
owner=person, name='pting', icon=icon)
[badges] = IHasAffiliation(distro).getAffiliationBadges([person])
- self.assertEqual((icon.getURL(), "Pting maintainer"), badges[0])
+ self.assertEqual((icon.getURL(), "Pting", "maintainer"), badges[0])
def _check_affiliated_with_distro(self, person, distro, role):
[badges] = IHasAffiliation(distro).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/distribution-badge", "Pting %s" % role), badges[0])
+ ("/@@/distribution-badge", "Pting", role), badges[0])
def test_distro_owner_affiliation(self):
# A person who owns a distro is affiliated.
@@ -85,7 +85,7 @@
product = self.factory.makeProduct(
owner=person, name='pting', icon=icon)
[badges] = IHasAffiliation(product).getAffiliationBadges([person])
- self.assertEqual((icon.getURL(), "Pting maintainer"), badges[0])
+ self.assertEqual((icon.getURL(), "Pting", "maintainer"), badges[0])
def test_pillar_badge_icon(self):
# A pillar's icon is used for the badge if the context has no icon.
@@ -96,11 +96,11 @@
owner=person, name='pting', icon=icon)
bugtask = self.factory.makeBugTask(target=product)
[badges] = IHasAffiliation(bugtask).getAffiliationBadges([person])
- self.assertEqual((icon.getURL(), "Pting maintainer"), badges[0])
+ self.assertEqual((icon.getURL(), "Pting", "maintainer"), badges[0])
def _check_affiliated_with_product(self, person, product, role):
[badges] = IHasAffiliation(product).getAffiliationBadges([person])
- self.assertEqual(("/@@/product-badge", "Pting %s" % role), badges[0])
+ self.assertEqual(("/@@/product-badge", "Pting", role), badges[0])
def test_product_driver_affiliation(self):
# A person who is the driver for a product is affiliated.
@@ -152,12 +152,33 @@
name='pting')
person_badges = IHasAffiliation(distro).getAffiliationBadges(people)
self.assertEqual(
- [("/@@/distribution-badge", "Pting maintainer")],
+ [("/@@/distribution-badge", "Pting", "maintainer")],
person_badges[0])
self.assertEqual(
- [("/@@/distribution-badge", "Pting driver")], person_badges[1])
+ [("/@@/distribution-badge", "Pting", "driver")], person_badges[1])
self.assertEqual([], person_badges[2])
+ def test_product_affiliation_query_count(self):
+ # Only 2 queries are expected, selects from:
+ # - Product, Person
+ person = self.factory.makePerson()
+ product = self.factory.makeProduct(owner=person, name='pting')
+ Store.of(product).invalidate()
+ with StormStatementRecorder() as recorder:
+ IHasAffiliation(product).getAffiliationBadges([person])
+ self.assertThat(recorder, HasQueryCount(Equals(2)))
+
+ def test_distro_affiliation_query_count(self):
+ # Only 2 business queries are expected, selects from:
+ # - Distribution, Person
+ # plus an additional query to create a PublisherConfig record.
+ person = self.factory.makePerson()
+ distro = self.factory.makeDistribution(owner=person, name='pting')
+ Store.of(distro).invalidate()
+ with StormStatementRecorder() as recorder:
+ IHasAffiliation(distro).getAffiliationBadges([person])
+ self.assertThat(recorder, HasQueryCount(Equals(3)))
+
class _TestBugTaskorBranchMixin:
@@ -196,44 +217,48 @@
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used(self):
+ def test_correct_pillars_are_used(self):
bugtask = self.factory.makeBugTask()
adapter = IHasAffiliation(bugtask)
- self.assertEqual(bugtask.pillar, adapter.getPillar())
+ pillars = [bugtask.pillar for bugtask in bugtask.bug.bugtasks]
+ self.assertEqual(pillars, adapter.getPillars())
def _check_affiliated_with_distro(self, person, target, role):
bugtask = self.factory.makeBugTask(target=target)
[badges] = IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/distribution-badge", "Pting %s" % role), badges[0])
+ ("/@@/distribution-badge", "Pting", role), badges[0])
def _check_affiliated_with_product(self, person, target, role):
bugtask = self.factory.makeBugTask(target=target)
[badges] = IHasAffiliation(bugtask).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/product-badge", "Pting %s" % role), badges[0])
-
- def test_product_affiliation_query_count(self):
- # Only 4 queries are expected, selects from:
- # - Bug, BugTask, Product, Person
- person = self.factory.makePerson()
- product = self.factory.makeProduct(owner=person, name='pting')
- bugtask = self.factory.makeBugTask(target=product)
- Store.of(bugtask).invalidate()
- with StormStatementRecorder() as recorder:
- IHasAffiliation(bugtask).getAffiliationBadges([person])
- self.assertThat(recorder, HasQueryCount(Equals(4)))
-
- def test_distro_affiliation_query_count(self):
- # Only 4 queries are expected, selects from:
- # - Bug, BugTask, Distribution, Person
- person = self.factory.makePerson()
- distro = self.factory.makeDistribution(owner=person, name='pting')
- bugtask = self.factory.makeBugTask(target=distro)
- Store.of(bugtask).invalidate()
- with StormStatementRecorder() as recorder:
- IHasAffiliation(bugtask).getAffiliationBadges([person])
- self.assertThat(recorder, HasQueryCount(Equals(4)))
+ ("/@@/product-badge", "Pting", role), badges[0])
+
+ def test_affiliated_with_multiple_bugtasks(self):
+ # When a bugtask belongs to a bug which has other bugtasks, all such
+ # bugtasks are checked for affiliation.
+ person = self.factory.makePerson()
+ bug = self.factory.makeBug()
+ expected_affiliations = []
+ for x in range(3):
+ bug_supervisor = None
+ if x == 0:
+ bug_supervisor = person
+ product = self.factory.makeProduct(
+ owner=person, bug_supervisor=bug_supervisor)
+ self.factory.makeBugTask(bug=bug, target=product)
+ expected_affiliations.append(
+ ("/@@/product-badge", product.displayname, "maintainer"))
+ expected_affiliations.append(
+ ("/@@/product-badge", product.displayname, "driver"))
+ if x == 0:
+ expected_affiliations.append(
+ ("/@@/product-badge",
+ product.displayname, "bug supervisor"))
+ [badges] = IHasAffiliation(
+ bug.default_bugtask).getAffiliationBadges([person])
+ self.assertContentEqual(expected_affiliations, badges)
class TestBranchPillarAffiliation(_TestBugTaskorBranchMixin,
@@ -241,10 +266,10 @@
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used(self):
+ def test_correct_pillars_are_used(self):
branch = self.factory.makeBranch()
adapter = IHasAffiliation(branch)
- self.assertEqual(branch.product, adapter.getPillar())
+ self.assertEqual([branch.product], adapter.getPillars())
def test_getBranch(self):
# The branch is the context.
@@ -265,7 +290,7 @@
branch = self.factory.makeBranch(sourcepackage=sp)
[badges] = IHasAffiliation(branch).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/distribution-badge", "Pting %s" % role), badges[0])
+ ("/@@/distribution-badge", "Pting", role), badges[0])
def _check_affiliated_with_product(self, person, target, role):
branch = self.factory.makeBranch(product=target)
@@ -273,7 +298,7 @@
branch.reviewer = person
[badges] = IHasAffiliation(branch).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/product-badge", "Pting %s" % role), badges[0])
+ ("/@@/product-badge", "Pting", role), badges[0])
class CodeReviewVotePillarAffiliationTestCase(TestBranchPillarAffiliation):
@@ -289,11 +314,11 @@
reviewer, merge_proposal.registrant)
return vote
- def test_correct_pillar_is_used(self):
+ def test_correct_pillars_are_used(self):
branch = self.factory.makeBranch()
vote = self.makeCodeReviewVote(branch)
adapter = IHasAffiliation(vote)
- self.assertEqual(branch.product, adapter.getPillar())
+ self.assertEqual([branch.product], adapter.getPillars())
def test_getBranch(self):
# The code review vote's target branch is the branch.
@@ -309,7 +334,7 @@
vote = self.makeCodeReviewVote(branch)
[badges] = IHasAffiliation(vote).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/distribution-badge", "Pting %s" % role), badges[0])
+ ("/@@/distribution-badge", "Pting", role), badges[0])
def _check_affiliated_with_product(self, person, target, role):
branch = self.factory.makeBranch(product=target)
@@ -318,17 +343,17 @@
vote = self.makeCodeReviewVote(branch)
[badges] = IHasAffiliation(vote).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/product-badge", "Pting %s" % role), badges[0])
+ ("/@@/product-badge", "Pting", role), badges[0])
class TestDistroSeriesPillarAffiliation(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used(self):
+ def test_correct_pillars_are_used(self):
series = self.factory.makeDistroSeries()
adapter = IHasAffiliation(series)
- self.assertEqual(series.distribution, adapter.getPillar())
+ self.assertEqual([series.distribution], adapter.getPillars())
def test_driver_affiliation(self):
# A person who is the driver for a distroseries is affiliated.
@@ -342,7 +367,7 @@
[badges] = IHasAffiliation(
distroseries).getAffiliationBadges([driver])
self.assertEqual(
- ("/@@/distribution-badge", "Pting driver"), badges[0])
+ ("/@@/distribution-badge", "Pting", "driver"), badges[0])
def test_distro_driver_affiliation(self):
# A person who is the driver for a distroseries' distro is affiliated.
@@ -356,17 +381,17 @@
[badges] = IHasAffiliation(
distroseries).getAffiliationBadges([driver])
self.assertEqual(
- ("/@@/distribution-badge", "Pting driver"), badges[0])
+ ("/@@/distribution-badge", "Pting", "driver"), badges[0])
class TestProductSeriesPillarAffiliation(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used(self):
+ def test_correct_pillars_are_used(self):
series = self.factory.makeProductSeries()
adapter = IHasAffiliation(series)
- self.assertEqual(series.product, adapter.getPillar())
+ self.assertEqual([series.product], adapter.getPillars())
def test_driver_affiliation(self):
# A person who is the driver for a productseries is affiliated.
@@ -380,7 +405,7 @@
[badges] = (
IHasAffiliation(productseries).getAffiliationBadges([driver]))
self.assertEqual(
- ("/@@/product-badge", "Pting driver"), badges[0])
+ ("/@@/product-badge", "Pting", "driver"), badges[0])
def test_product_driver_affiliation(self):
# A person who is the driver for a productseries' product is
@@ -394,7 +419,7 @@
[badges] = (
IHasAffiliation(productseries).getAffiliationBadges([driver]))
self.assertEqual(
- ("/@@/product-badge", "Pting driver"), badges[0])
+ ("/@@/product-badge", "Pting", "driver"), badges[0])
def test_product_group_driver_affiliation(self):
# A person who is the driver for a productseries' product's group is
@@ -409,26 +434,26 @@
[badges] = (
IHasAffiliation(productseries).getAffiliationBadges([driver]))
self.assertEqual(
- ("/@@/product-badge", "Pting driver"), badges[0])
+ ("/@@/product-badge", "Pting", "driver"), badges[0])
class TestQuestionPillarAffiliation(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used_for_product(self):
+ def test_correct_pillars_are_used_for_product(self):
product = self.factory.makeProduct()
question = self.factory.makeQuestion(target=product)
adapter = IHasAffiliation(question)
- self.assertEqual(question.product, adapter.getPillar())
+ self.assertEqual([question.product], adapter.getPillars())
- def test_correct_pillar_is_used_for_distribution(self):
+ def test_correct_pillars_are_used_for_distribution(self):
distribution = self.factory.makeDistribution()
question = self.factory.makeQuestion(target=distribution)
adapter = IHasAffiliation(question)
- self.assertEqual(question.distribution, adapter.getPillar())
+ self.assertEqual([question.distribution], adapter.getPillars())
- def test_correct_pillar_is_used_for_distro_sourcepackage(self):
+ def test_correct_pillars_are_used_for_distro_sourcepackage(self):
distribution = self.factory.makeDistribution()
distro_sourcepackage = self.factory.makeDistributionSourcePackage(
distribution=distribution)
@@ -436,7 +461,7 @@
question = self.factory.makeQuestion(
target=distro_sourcepackage, owner=owner)
adapter = IHasAffiliation(question)
- self.assertEqual(distribution, adapter.getPillar())
+ self.assertEqual([distribution], adapter.getPillars())
def test_answer_contact_affiliation_for_distro(self):
# A person is affiliated if they are an answer contact for a distro
@@ -451,14 +476,14 @@
[badges] = (
IHasAffiliation(question).getAffiliationBadges([answer_contact]))
self.assertEqual(
- ("/@@/distribution-badge", "%s maintainer" %
- distro.displayname), badges[0])
- self.assertEqual(
- ("/@@/distribution-badge", "%s driver" %
- distro.displayname), badges[1])
- self.assertEqual(
- ("/@@/distribution-badge", "%s answer contact" %
- distro.displayname), badges[2])
+ ("/@@/distribution-badge", distro.displayname,
+ "maintainer"), badges[0])
+ self.assertEqual(
+ ("/@@/distribution-badge", distro.displayname,
+ "driver"), badges[1])
+ self.assertEqual(
+ ("/@@/distribution-badge", distro.displayname,
+ "answer contact"), badges[2])
def test_answer_contact_affiliation_for_distro_sourcepackage(self):
# A person is affiliated if they are an answer contact for a dsp
@@ -477,14 +502,14 @@
[badges] = (
IHasAffiliation(question).getAffiliationBadges([answer_contact]))
self.assertEqual(
- ("/@@/distribution-badge", "%s maintainer" %
- distribution.displayname), badges[0])
- self.assertEqual(
- ("/@@/distribution-badge", "%s driver" %
- distribution.displayname), badges[1])
- self.assertEqual(
- ("/@@/distribution-badge", "%s answer contact" %
- distro_sourcepackage.displayname), badges[2])
+ ("/@@/distribution-badge", distribution.displayname,
+ "maintainer"), badges[0])
+ self.assertEqual(
+ ("/@@/distribution-badge", distribution.displayname,
+ "driver"), badges[1])
+ self.assertEqual(
+ ("/@@/distribution-badge", distro_sourcepackage.displayname,
+ "answer contact"), badges[2])
def test_answer_contact_affiliation_for_distro_sourcepackage_distro(self):
# A person is affiliated if they are an answer contact for a dsp
@@ -502,14 +527,14 @@
[badges] = (
IHasAffiliation(question).getAffiliationBadges([answer_contact]))
self.assertEqual(
- ("/@@/distribution-badge", "%s maintainer" %
- distribution.displayname), badges[0])
- self.assertEqual(
- ("/@@/distribution-badge", "%s driver" %
- distribution.displayname), badges[1])
- self.assertEqual(
- ("/@@/distribution-badge", "%s answer contact" %
- distribution.displayname), badges[2])
+ ("/@@/distribution-badge", distribution.displayname,
+ "maintainer"), badges[0])
+ self.assertEqual(
+ ("/@@/distribution-badge", distribution.displayname,
+ "driver"), badges[1])
+ self.assertEqual(
+ ("/@@/distribution-badge", distribution.displayname,
+ "answer contact"), badges[2])
def test_answer_contact_affiliation_for_product(self):
# A person is affiliated if they are an answer contact for a product
@@ -524,8 +549,8 @@
[badges] = (
IHasAffiliation(question).getAffiliationBadges([answer_contact]))
self.assertEqual(
- ("/@@/product-badge", "%s answer contact" %
- product.displayname), badges[0])
+ ("/@@/product-badge", product.displayname, "answer contact"),
+ badges[0])
def test_product_affiliation(self):
# A person is affiliated if they are affiliated with the product.
@@ -534,8 +559,8 @@
question = self.factory.makeQuestion(target=product)
[badges] = IHasAffiliation(question).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/product-badge", "%s maintainer" %
- product.displayname), badges[0])
+ ("/@@/product-badge", product.displayname, "maintainer"),
+ badges[0])
def test_distribution_affiliation(self):
# A person is affiliated if they are affiliated with the distribution.
@@ -544,25 +569,25 @@
question = self.factory.makeQuestion(target=distro)
[badges] = IHasAffiliation(question).getAffiliationBadges([person])
self.assertEqual(
- ("/@@/distribution-badge", "%s maintainer" %
- distro.displayname), badges[0])
+ ("/@@/distribution-badge", distro.displayname, "maintainer"),
+ badges[0])
class TestSpecificationPillarAffiliation(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
- def test_correct_pillar_is_used_for_product(self):
+ def test_correct_pillars_are_used_for_product(self):
product = self.factory.makeProduct()
specification = self.factory.makeSpecification(product=product)
adapter = IHasAffiliation(specification)
- self.assertEqual(specification.product, adapter.getPillar())
+ self.assertEqual([specification.product], adapter.getPillars())
- def test_correct_pillar_is_used_for_distribution(self):
+ def test_correct_pillars_are_used_for_distribution(self):
distro = self.factory.makeDistribution()
specification = self.factory.makeSpecification(distribution=distro)
adapter = IHasAffiliation(specification)
- self.assertEqual(specification.distribution, adapter.getPillar())
+ self.assertEqual([specification.distribution], adapter.getPillars())
def test_product_affiliation(self):
# A person is affiliated if they are affiliated with the pillar.
@@ -572,8 +597,8 @@
[badges] = (
IHasAffiliation(specification).getAffiliationBadges([person]))
self.assertEqual(
- ("/@@/product-badge", "%s maintainer" %
- product.displayname), badges[0])
+ ("/@@/product-badge", product.displayname, "maintainer"),
+ badges[0])
def test_distribution_affiliation(self):
# A person is affiliated if they are affiliated with the distribution.
@@ -583,5 +608,5 @@
[badges] = (
IHasAffiliation(specification).getAffiliationBadges([person]))
self.assertEqual(
- ("/@@/distribution-badge", "%s maintainer" %
- distro.displayname), badges[0])
+ ("/@@/distribution-badge", distro.displayname, "maintainer"),
+ badges[0])
