Merge lp:~wallyworld/launchpad/additional-affiliation-types-798764 into lp:launchpad
- additional-affiliation-types-798764
- Merge into devel
Proposed by
Ian Booth
| Status: | Merged | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Approved by: | Curtis Hovey | ||||||||||||||||
| Approved revision: | no longer in the source branch. | ||||||||||||||||
| Merged at revision: | 13636 | ||||||||||||||||
| Proposed branch: | lp:~wallyworld/launchpad/additional-affiliation-types-798764 | ||||||||||||||||
| Merge into: | lp:launchpad | ||||||||||||||||
| Diff against target: |
886 lines (+622/-57) 7 files modified
lib/lp/app/browser/tests/test_vocabulary.py (+16/-7) lib/lp/app/browser/vocabulary.py (+9/-3) lib/lp/registry/configure.zcml (+28/-3) lib/lp/registry/model/pillaraffiliation.py (+110/-21) lib/lp/registry/tests/test_pillaraffiliation.py (+437/-15) lib/lp/services/features/flags.py (+4/-0) lib/lp/testing/factory.py (+18/-8) |
||||||||||||||||
| To merge this branch: | bzr merge lp:~wallyworld/launchpad/additional-affiliation-types-798764 | ||||||||||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Curtis Hovey (community) | code | Approve | |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2011-08-04.
Commit message
[r=sinzui][bug=798764,820210,
Description of the change
This branch adds to the affiliation adaptor to provide affiliation for other entity types:
- Specification
- Question
- Distribution
- DistroSeries
- ProductSeries
- Product
== Implementation ==
Add extra affiliation adaptors for the additional context types. Checks are done for:
owner
driver
security contact
bug supervisor
Some contexts have pillars eg BugTask has a product or distribution; a distroseries has a distribution. The context is checked first. If the context doesn't match on the given attribute (eg owner), then the pillar is checked. If there is no match on owner, then driver is checked etc.
I think this mp also covers the intent of bug 81692, although that bug talks about additional checks eg registrant. Do we consider the work done here sufficient to address that bug?
== Tests ==
Add a bunch of tests to test_pillaraffi
== Lint ==
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
To post a comment you must log in.
Revision history for this message
| Curtis Hovey (sinzui) wrote : Posted in a previous version of this proposal | # |
review:
Needs Information
(code)
Revision history for this message
| Ian Booth (wallyworld) wrote : Posted in a previous version of this proposal | # |
Download full text (9.8 KiB)
Thanks for the review. Clearly I'm missing some knowledge.
On 05/08/11 05:22, Curtis Hovey wrote:
> Review: Needs Information code
>> === modified file 'lib/lp/
>> --- lib/lp/
>> +++ lib/lp/
> ...
>> + def getPillar(self):
>> + return self.context
>> +
>> + def getAffiliationB
>> + """ Return the affiliation information for a person given a context.
>> +
>> + The context is a Distribution, Product etc and is associated with a
>> + pillar. Checks are done to see if the person is associated with the
>> + context first (owner, driver etc) and if not, then the pillar.
>> + """
>> + pillar = self.getPillar()
>> +
>> + def checkAffiliatio
>> + # Check the affiliation defined by the specified capability on the
>> + # context and then pillar. Capability is IHasOwner etc. WHatever
>> + # matches first (context or pillar) is used for the display name.
>> + affiliated_entity = None
>> + capabilityProvi
>> + if capabilityProvi
>> + if person.
>> + affiliated_entity = self.context.
>> + if (affiliated_entity is None and capabilityProvi
>> + if person.
>> + affiliated_entity = pillar.displayname
>> + if affiliated_entity is None:
>> + return None
>> + return affiliated_entity, role
>
> I do not see why this is an inner function. This could be simpler too if
Me either.
> we decide that all we care about is product or distribution. We know how to
> check owner, drivers, and other roles. The other kinds of items I
> see returned, notably for specification and question are probably wrong.
> We do not need the interface checks if we are certain we are getting a
> distro or product.
>
Will we always be getting a distro or product though? If I have a
Specification, wouldn't I want to possibly first see if the person in
question is affiliated with the specification itself and then check the
affiliation with the target (product/distro) only if the specification
check turned up empty?
> I have some doubts about the universality of these checks. I think
> owner and driver are universal. bug_supervisor and security contact are
> only useful in bugs and branches cases that deal with privacy and security.
> eg assigning a branch reviewer, bug assignee, subscriber.
>
This suggests we need another piece of context information to properly
determine the affiliation - the name of the attribute that is being
updated with the person, not just the person alone. So instead of saying
"we are associating person fred with bugtask 4 in some capacity" we are
saying "we are associating person fred with bugtask 4 as an assignee"
and that this distinction possibly affect...
Revision history for this message
| Ian Booth (wallyworld) wrote : | # |
The mp diff screwed up so I resubmitted and it looks better now.
I think I have addressed the main issues raised in the review.
- the base pillaraffiliation class performs owner and driver checks
- subclasses perform additional checks as required (eg bug supervisor, security contact for bugtasks)
- drivers attribute is used to get the drivers
- answer contacts used to check for question affiliation
- tests added for distroseries and productseries to test for driver affiliation
- tests added for questions to check for answer affiliation
- other test cleanup
The affiliation adapters don't take into account the attribute being set (eg assignee, approver etc). This can be done is another branch if required.
Revision history for this message
| Curtis Hovey (sinzui) wrote : | # |
Download full text (4.0 KiB)
Hi Ian.
I think this branch is good to land. I have a few remarks you may want to consider.
=== modified file 'lib/lp/
--- lib/lp/
+++ lib/lp/
...
> +@adapter(IBugTask)
> +class BugTaskPillarAf
> + """An affiliation adapter for bug tasks."""
> + def getPillar(self):
> + return self.context.pillar
> +
> + def _getAffiliation
> + """ A person is affiliated with a bugtask based on (in order):
> + - owner of bugtask pillar
> + - driver of bugtask pillar
> + - bug supervisor of bugtask pillar
> + - security contact of bugtask pillar
> + """
> + result = super(BugTaskPi
> + person, pillar)
> + if result is not None:
> + return result
> + if person.
> + return pillar.displayname, 'bug supervisor'
> + if person.
> + return pillar.displayname, 'security contact'
I think this will also work for branches. Could we do this as well?
@adapter(IBranch)
class BranchPillarAff
"""An affiliation adapter for branches."""
^ An extra class would not be be necessary using ZCML to register the adapter,
but we ouls still have two separate ZCML entried for both IBugTask
and IBranch.
> +@adapter(
> > +class SpecificationPi
> > + """An affiliation adapter for blueprints."""
> > + def getPillar(self):
> > + return (self.context.
You suggested that this adapter could check if the user is assigned a roll
for the specification. I think that could misinform users. Anyone can register
a specification and assign users to rolls. They are not in anyway affilliated
with the project. Most of Launchpad's visible blueprints are bogus, and they
where created by people other than ~launchpad. I think this implementation
is good.
> +@adapter(
> +class QuestionPillarA
> + """An affiliation adapter for questions."""
> + def getPillar(self):
> + return self.context.
> +
> + def _getAffiliation
> + """ A person is affiliated with a question based on (in order):
> + - answer contact for question target
> + - owner of question target
> + - driver of question target
> + """
> + target = self.context.target
> + answer_contacts = target.
> + for answer_contact in answer_contacts:
> + if person.
> + return target.displayname, 'answer contact'
> + return super(QuestionP
> + person, pillar)
This looks a lot like code I wrote in 2007. In 2010 I updated security.py
to use an alternate approach that was much faster (fewer sql queries)...
review:
Approve
(code)
Revision history for this message
| Ian Booth (wallyworld) wrote : | # |
Thanks fir the review and suggestions.
I added a BranchAffiliati
QuestionAffilia
added new tests for the branch adapter and question adapter
On 06/08/11 00:21, Curtis Hovey wrote:
> Review: Approve code
> Hi Ian.
>
> I think this branch is good to land. I have a few remarks you may want to consider.
>
>
<snip>
Revision history for this message
| Ian Booth (wallyworld) wrote : | # |
I have put this new functionality behind a new eature flag: disclosure.
Previously, this work was behind the existing disclosure.
The reason is that there are questions over the performance in terms of the number of selects from the TeamParticipation table. The enhance_picker feature flag is on for production and I don't want to risk breaking lp.net with this new work.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'lib/lp/app/browser/tests/test_vocabulary.py' |
| 2 | --- lib/lp/app/browser/tests/test_vocabulary.py 2011-08-04 23:49:37 +0000 |
| 3 | +++ lib/lp/app/browser/tests/test_vocabulary.py 2011-08-09 00:14:32 +0000 |
| 4 | @@ -119,17 +119,19 @@ |
| 5 | self.assertEqual('http://launchpad.dev/~fnord', entry.alt_title_link) |
| 6 | self.assertEqual(['Team members: 1'], entry.details) |
| 7 | |
| 8 | - def test_PersonPickerEntryAdapter_enhanced_picker_enabled_badges(self): |
| 9 | - # The enhanced person picker provides affilliation information. |
| 10 | + def test_PersonPickerEntryAdapter_personpicker_affiliation_badges(self): |
| 11 | + # The person picker with affiliation enabled provides affilliation |
| 12 | + # information. |
| 13 | person = self.factory.makePerson(email='snarf@eg.dom', name='snarf') |
| 14 | project = self.factory.makeProduct(name='fnord', owner=person) |
| 15 | bugtask = self.factory.makeBugTask(target=project) |
| 16 | entry = IPickerEntry(person).getPickerEntry( |
| 17 | bugtask, enhanced_picker_enabled=True, |
| 18 | - picker_expander_enabled=True) |
| 19 | + picker_expander_enabled=True, |
| 20 | + personpicker_affiliation_enabled=True) |
| 21 | self.assertEqual(1, len(entry.badges)) |
| 22 | self.assertEqual('/@@/product-badge', entry.badges[0]['url']) |
| 23 | - self.assertEqual('Affiliated with Fnord', entry.badges[0]['alt']) |
| 24 | + self.assertEqual('Fnord maintainer', entry.badges[0]['alt']) |
| 25 | |
| 26 | |
| 27 | class TestPersonVocabulary: |
| 28 | @@ -171,8 +173,9 @@ |
| 29 | self.addCleanup(TestPersonVocabulary.setTestData, []) |
| 30 | |
| 31 | @staticmethod |
| 32 | - def create_vocabulary_view(form): |
| 33 | - context = getUtility(ILaunchpadRoot) |
| 34 | + def create_vocabulary_view(form, context=None): |
| 35 | + if context is None: |
| 36 | + context = getUtility(ILaunchpadRoot) |
| 37 | query_string = urlencode(form) |
| 38 | return create_view( |
| 39 | context, '+huge-vocabulary', form=form, query_string=query_string) |
| 40 | @@ -198,19 +201,25 @@ |
| 41 | feature_flag = { |
| 42 | 'disclosure.picker_enhancements.enabled': 'on', |
| 43 | 'disclosure.picker_expander.enabled': 'on', |
| 44 | + 'disclosure.personpicker_affiliation.enabled': 'on', |
| 45 | } |
| 46 | flags = FeatureFixture(feature_flag) |
| 47 | flags.setUp() |
| 48 | self.addCleanup(flags.cleanUp) |
| 49 | team = self.factory.makeTeam(name='pting-team') |
| 50 | TestPersonVocabulary.test_persons.append(team) |
| 51 | + product = self.factory.makeProduct(owner=team) |
| 52 | + bugtask = self.factory.makeBugTask(target=product) |
| 53 | form = dict(name='TestPerson', search_text='pting-team') |
| 54 | - view = self.create_vocabulary_view(form) |
| 55 | + view = self.create_vocabulary_view(form, context=bugtask) |
| 56 | result = simplejson.loads(view()) |
| 57 | expected = { |
| 58 | "alt_title": team.name, |
| 59 | "alt_title_link": "http://launchpad.dev/~%s" % team.name, |
| 60 | "api_uri": "/~%s" % team.name, |
| 61 | + "badges": |
| 62 | + [{"alt": "%s maintainer" % product.displayname, |
| 63 | + "url": "/@@/product-badge"}], |
| 64 | "css": "sprite team", |
| 65 | "details": ['Team members: 1'], |
| 66 | "link_css": "sprite new-window", |
| 67 | |
| 68 | === modified file 'lib/lp/app/browser/vocabulary.py' |
| 69 | --- lib/lp/app/browser/vocabulary.py 2011-08-04 15:18:04 +0000 |
| 70 | +++ lib/lp/app/browser/vocabulary.py 2011-08-09 00:14:32 +0000 |
| 71 | @@ -130,8 +130,9 @@ |
| 72 | extra = super(PersonPickerEntryAdapter, self).getPickerEntry( |
| 73 | associated_object) |
| 74 | |
| 75 | - enhanced_picker_enabled = kwarg.get('enhanced_picker_enabled', False) |
| 76 | - if enhanced_picker_enabled: |
| 77 | + personpicker_affiliation_enabled = kwarg.get( |
| 78 | + 'personpicker_affiliation_enabled', False) |
| 79 | + if personpicker_affiliation_enabled: |
| 80 | # If the person is affiliated with the associated_object then we |
| 81 | # can display a badge. |
| 82 | badge_info = IHasAffiliation( |
| 83 | @@ -154,6 +155,7 @@ |
| 84 | extra.description = '<email address hidden>' |
| 85 | |
| 86 | extra.metadata = get_person_picker_entry_metadata(person) |
| 87 | + enhanced_picker_enabled = kwarg.get('enhanced_picker_enabled', False) |
| 88 | if enhanced_picker_enabled: |
| 89 | # We will display the person's name (launchpad id) after their |
| 90 | # displayname. |
| 91 | @@ -242,6 +244,8 @@ |
| 92 | getFeatureFlag('disclosure.picker_enhancements.enabled')) |
| 93 | self.picker_expander_enabled = bool( |
| 94 | getFeatureFlag('disclosure.picker_expander.enabled')) |
| 95 | + self.personpicker_affiliation_enabled = bool( |
| 96 | + getFeatureFlag('disclosure.personpicker_affiliation.enabled')) |
| 97 | |
| 98 | def __call__(self): |
| 99 | name = self.request.form.get('name') |
| 100 | @@ -287,7 +291,9 @@ |
| 101 | picker_entry = IPickerEntry(term.value).getPickerEntry( |
| 102 | self.context, |
| 103 | enhanced_picker_enabled=self.enhanced_picker_enabled, |
| 104 | - picker_expander_enabled=self.picker_expander_enabled) |
| 105 | + picker_expander_enabled=self.picker_expander_enabled, |
| 106 | + personpicker_affiliation_enabled= |
| 107 | + self.personpicker_affiliation_enabled) |
| 108 | if picker_entry.description is not None: |
| 109 | if len(picker_entry.description) > MAX_DESCRIPTION_LENGTH: |
| 110 | entry['description'] = ( |
| 111 | |
| 112 | === modified file 'lib/lp/registry/configure.zcml' |
| 113 | --- lib/lp/registry/configure.zcml 2011-08-02 05:35:39 +0000 |
| 114 | +++ lib/lp/registry/configure.zcml 2011-08-09 00:14:32 +0000 |
| 115 | @@ -883,11 +883,36 @@ |
| 116 | permission="zope.Public"/> |
| 117 | |
| 118 | <adapter |
| 119 | + for="lp.registry.interfaces.distribution.IDistribution" |
| 120 | + factory="lp.registry.model.pillaraffiliation.PillarAffiliation" |
| 121 | + /> |
| 122 | + <adapter |
| 123 | + for="lp.registry.interfaces.product.IProduct" |
| 124 | + factory="lp.registry.model.pillaraffiliation.PillarAffiliation" |
| 125 | + /> |
| 126 | + <adapter |
| 127 | + for="lp.bugs.interfaces.bugtask.IBugTask" |
| 128 | factory="lp.registry.model.pillaraffiliation.BugTaskPillarAffiliation" |
| 129 | /> |
| 130 | - |
| 131 | - <adapter |
| 132 | - factory="lp.registry.model.pillaraffiliation.PillarAffiliation" |
| 133 | + <adapter |
| 134 | + for="lp.code.interfaces.branch.IBranch" |
| 135 | + factory="lp.registry.model.pillaraffiliation.BranchPillarAffiliation" |
| 136 | + /> |
| 137 | + <adapter |
| 138 | + for="lp.answers.interfaces.question.IQuestion" |
| 139 | + factory="lp.registry.model.pillaraffiliation.QuestionPillarAffiliation" |
| 140 | + /> |
| 141 | + <adapter |
| 142 | + for="lp.blueprints.interfaces.specification.ISpecification" |
| 143 | + factory="lp.registry.model.pillaraffiliation.SpecificationPillarAffiliation" |
| 144 | + /> |
| 145 | + <adapter |
| 146 | + for="lp.registry.interfaces.distroseries.IDistroSeries" |
| 147 | + factory="lp.registry.model.pillaraffiliation.DistroSeriesPillarAffiliation" |
| 148 | + /> |
| 149 | + <adapter |
| 150 | + for="lp.registry.interfaces.productseries.IProductSeries" |
| 151 | + factory="lp.registry.model.pillaraffiliation.ProductSeriesPillarAffiliation" |
| 152 | /> |
| 153 | |
| 154 | <!-- Using |
| 155 | |
| 156 | === modified file 'lib/lp/registry/model/pillaraffiliation.py' |
| 157 | --- lib/lp/registry/model/pillaraffiliation.py 2011-08-04 13:56:55 +0000 |
| 158 | +++ lib/lp/registry/model/pillaraffiliation.py 2011-08-09 00:14:32 +0000 |
| 159 | @@ -29,8 +29,11 @@ |
| 160 | ) |
| 161 | |
| 162 | from canonical.launchpad.interfaces.launchpad import IHasIcon |
| 163 | -from lp.bugs.interfaces.bugtask import IBugTask |
| 164 | +from lp.answers.interfaces.questionsperson import IQuestionsPerson |
| 165 | from lp.registry.interfaces.distribution import IDistribution |
| 166 | +from lp.registry.interfaces.distributionsourcepackage import ( |
| 167 | + IDistributionSourcePackage, |
| 168 | + ) |
| 169 | |
| 170 | |
| 171 | class IHasAffiliation(Interface): |
| 172 | @@ -51,7 +54,9 @@ |
| 173 | class PillarAffiliation(object): |
| 174 | """Default affiliation adapter. |
| 175 | |
| 176 | - No affiliation is returned. |
| 177 | + Subclasses may need to override getPillar() in order to provide the pillar |
| 178 | + entity for which affiliation is to be determined. The default is just to |
| 179 | + use the context object directly. |
| 180 | """ |
| 181 | |
| 182 | implements(IHasAffiliation) |
| 183 | @@ -59,32 +64,116 @@ |
| 184 | def __init__(self, context): |
| 185 | self.context = context |
| 186 | |
| 187 | - def getAffiliationBadge(self, person): |
| 188 | - return None |
| 189 | - |
| 190 | - |
| 191 | -# XXX: wallyworld 2011-05-24 bug=81692: TODO Work is required to determine |
| 192 | -# exactly what is required in terms of figuring out affiliation.. |
| 193 | - |
| 194 | -@adapter(IBugTask) |
| 195 | -class BugTaskPillarAffiliation(PillarAffiliation): |
| 196 | - """An affiliation adapter for bug tasks.""" |
| 197 | - |
| 198 | - def getAffiliationBadge(self, person): |
| 199 | - pillar = self.context.pillar |
| 200 | - affiliated = person.inTeam(pillar.owner) |
| 201 | - if not affiliated: |
| 202 | + def getPillar(self): |
| 203 | + return self.context |
| 204 | + |
| 205 | + def _getAffiliationDetails(self, person, pillar): |
| 206 | + """ Return the affiliation information for a person, if any. |
| 207 | + |
| 208 | + A person is affiliated with a pillar if they are in the list of |
| 209 | + drivers or are the maintainer. |
| 210 | + """ |
| 211 | + if person.inTeam(pillar.owner): |
| 212 | + return pillar.displayname, 'maintainer' |
| 213 | + for driver in pillar.drivers: |
| 214 | + if person.inTeam(driver): |
| 215 | + return pillar.displayname, 'driver' |
| 216 | + return None |
| 217 | + |
| 218 | + def getAffiliationBadge(self, person): |
| 219 | + """ Return the affiliation badge details for a person given a context. |
| 220 | + """ |
| 221 | + pillar = self.getPillar() |
| 222 | + affiliation_details = self._getAffiliationDetails(person, pillar) |
| 223 | + if not affiliation_details: |
| 224 | return None |
| 225 | |
| 226 | - def getIconUrl(context, default_url): |
| 227 | + def getIconUrl(context, pillar, default_url): |
| 228 | if IHasIcon.providedBy(context) and context.icon is not None: |
| 229 | icon_url = context.icon.getURL() |
| 230 | return icon_url |
| 231 | + if IHasIcon.providedBy(pillar) and pillar.icon is not None: |
| 232 | + icon_url = context.icon.getURL() |
| 233 | + return icon_url |
| 234 | return default_url |
| 235 | |
| 236 | - alt_text = "Affiliated with %s" % pillar.displayname |
| 237 | + alt_text = "%s %s" % affiliation_details |
| 238 | if IDistribution.providedBy(pillar): |
| 239 | - icon_url = getIconUrl(pillar, "/@@/distribution-badge") |
| 240 | + default_icon_url = "/@@/distribution-badge" |
| 241 | else: |
| 242 | - icon_url = getIconUrl(pillar, "/@@/product-badge") |
| 243 | + default_icon_url = "/@@/product-badge" |
| 244 | + icon_url = getIconUrl(self.context, pillar, default_icon_url) |
| 245 | return BadgeDetails(icon_url, alt_text) |
| 246 | + |
| 247 | + |
| 248 | +class BugTaskPillarAffiliation(PillarAffiliation): |
| 249 | + """An affiliation adapter for bug tasks.""" |
| 250 | + def getPillar(self): |
| 251 | + return self.context.pillar |
| 252 | + |
| 253 | + def _getAffiliationDetails(self, person, pillar): |
| 254 | + """ A person is affiliated with a bugtask based on (in order): |
| 255 | + - owner of bugtask pillar |
| 256 | + - driver of bugtask pillar |
| 257 | + - bug supervisor of bugtask pillar |
| 258 | + - security contact of bugtask pillar |
| 259 | + """ |
| 260 | + result = super(BugTaskPillarAffiliation, self)._getAffiliationDetails( |
| 261 | + person, pillar) |
| 262 | + if result is not None: |
| 263 | + return result |
| 264 | + if person.inTeam(pillar.bug_supervisor): |
| 265 | + return pillar.displayname, 'bug supervisor' |
| 266 | + if person.inTeam(pillar.security_contact): |
| 267 | + return pillar.displayname, 'security contact' |
| 268 | + |
| 269 | + |
| 270 | +class BranchPillarAffiliation(BugTaskPillarAffiliation): |
| 271 | + """An affiliation adapter for branches.""" |
| 272 | + def getPillar(self): |
| 273 | + return self.context.product or self.context.distribution |
| 274 | + |
| 275 | + |
| 276 | +class DistroSeriesPillarAffiliation(PillarAffiliation): |
| 277 | + """An affiliation adapter for distroseries.""" |
| 278 | + def getPillar(self): |
| 279 | + return self.context.distribution |
| 280 | + |
| 281 | + |
| 282 | +class ProductSeriesPillarAffiliation(PillarAffiliation): |
| 283 | + """An affiliation adapter for productseries.""" |
| 284 | + def getPillar(self): |
| 285 | + return self.context.product |
| 286 | + |
| 287 | + |
| 288 | +class SpecificationPillarAffiliation(PillarAffiliation): |
| 289 | + """An affiliation adapter for blueprints.""" |
| 290 | + def getPillar(self): |
| 291 | + return (self.context.target) |
| 292 | + |
| 293 | + |
| 294 | +class QuestionPillarAffiliation(PillarAffiliation): |
| 295 | + """An affiliation adapter for questions.""" |
| 296 | + def getPillar(self): |
| 297 | + return self.context.product or self.context.distribution |
| 298 | + |
| 299 | + def _getAffiliationDetails(self, person, pillar): |
| 300 | + """ A person is affiliated with a question based on (in order): |
| 301 | + - answer contact for question target |
| 302 | + - owner of question target |
| 303 | + - driver of question target |
| 304 | + """ |
| 305 | + target = self.context.target |
| 306 | + if IDistributionSourcePackage.providedBy(target): |
| 307 | + question_targets = (target, target.distribution) |
| 308 | + else: |
| 309 | + question_targets = (target, ) |
| 310 | + questions_person = IQuestionsPerson(person) |
| 311 | + for target in questions_person.getDirectAnswerQuestionTargets(): |
| 312 | + if target in question_targets: |
| 313 | + return target.displayname, 'answer contact' |
| 314 | + for target in questions_person.getTeamAnswerQuestionTargets(): |
| 315 | + if target in question_targets: |
| 316 | + return target.displayname, 'answer contact' |
| 317 | + return super(QuestionPillarAffiliation, self)._getAffiliationDetails( |
| 318 | + person, pillar) |
| 319 | |
| 320 | === modified file 'lib/lp/registry/tests/test_pillaraffiliation.py' |
| 321 | --- lib/lp/registry/tests/test_pillaraffiliation.py 2011-08-04 13:56:55 +0000 |
| 322 | +++ lib/lp/registry/tests/test_pillaraffiliation.py 2011-08-09 00:14:32 +0000 |
| 323 | @@ -5,29 +5,451 @@ |
| 324 | |
| 325 | __metaclass__ = type |
| 326 | |
| 327 | +from storm.store import Store |
| 328 | +from testtools.matchers import Equals |
| 329 | +from zope.component import getUtility |
| 330 | + |
| 331 | from canonical.testing.layers import DatabaseFunctionalLayer |
| 332 | from lp.registry.model.pillaraffiliation import IHasAffiliation |
| 333 | -from lp.testing import TestCaseWithFactory |
| 334 | +from lp.services.worlddata.interfaces.language import ILanguageSet |
| 335 | +from lp.testing import ( |
| 336 | + person_logged_in, |
| 337 | + StormStatementRecorder, |
| 338 | + TestCaseWithFactory, |
| 339 | + ) |
| 340 | +from lp.testing.matchers import HasQueryCount |
| 341 | |
| 342 | |
| 343 | class TestPillarAffiliation(TestCaseWithFactory): |
| 344 | |
| 345 | layer = DatabaseFunctionalLayer |
| 346 | |
| 347 | - def test_bugtask_distro_affiliation(self): |
| 348 | - # A person who owns a bugtask distro is affiliated. |
| 349 | + def _check_affiliated_with_distro(self, person, distro, role): |
| 350 | + badge = IHasAffiliation(distro).getAffiliationBadge(person) |
| 351 | + self.assertEqual( |
| 352 | + ("/@@/distribution-badge", "Pting %s" % role), badge) |
| 353 | + |
| 354 | + def test_distro_owner_affiliation(self): |
| 355 | + # A person who owns a distro is affiliated. |
| 356 | + person = self.factory.makePerson() |
| 357 | + distro = self.factory.makeDistribution(owner=person, name='pting') |
| 358 | + self._check_affiliated_with_distro(person, distro, 'maintainer') |
| 359 | + |
| 360 | + def test_distro_driver_affiliation(self): |
| 361 | + # A person who is a distro driver is affiliated. |
| 362 | + person = self.factory.makePerson() |
| 363 | + distro = self.factory.makeDistribution(driver=person, name='pting') |
| 364 | + self._check_affiliated_with_distro(person, distro, 'driver') |
| 365 | + |
| 366 | + def test_distro_team_driver_affiliation(self): |
| 367 | + # A person who is a member of the distro driver team is affiliated. |
| 368 | + person = self.factory.makePerson() |
| 369 | + team = self.factory.makeTeam(members=[person]) |
| 370 | + distro = self.factory.makeDistribution(driver=team, name='pting') |
| 371 | + self._check_affiliated_with_distro(person, distro, 'driver') |
| 372 | + |
| 373 | + def test_no_distro_security_contact_affiliation(self): |
| 374 | + # A person who is the security contact for a distro is not affiliated |
| 375 | + # for simple distro affiliation checks. |
| 376 | + person = self.factory.makePerson() |
| 377 | + distro = self.factory.makeDistribution(security_contact=person) |
| 378 | + self.assertIs( |
| 379 | + None, IHasAffiliation(distro).getAffiliationBadge(person)) |
| 380 | + |
| 381 | + def test_no_distro_bug_supervisor_affiliation(self): |
| 382 | + # A person who is the bug supervisor for a distro is not affiliated |
| 383 | + # for simple distro affiliation checks. |
| 384 | + person = self.factory.makePerson() |
| 385 | + distro = self.factory.makeDistribution(bug_supervisor=person) |
| 386 | + self.assertIs( |
| 387 | + None, IHasAffiliation(distro).getAffiliationBadge(person)) |
| 388 | + |
| 389 | + def _check_affiliated_with_product(self, person, product, role): |
| 390 | + badge = IHasAffiliation(product).getAffiliationBadge(person) |
| 391 | + self.assertEqual( |
| 392 | + ("/@@/product-badge", "Pting %s" % role), badge) |
| 393 | + |
| 394 | + def test_product_driver_affiliation(self): |
| 395 | + # A person who is the driver for a product is affiliated. |
| 396 | + person = self.factory.makePerson() |
| 397 | + product = self.factory.makeProduct(driver=person, name='pting') |
| 398 | + self._check_affiliated_with_product(person, product, 'driver') |
| 399 | + |
| 400 | + def test_product_team_driver_affiliation(self): |
| 401 | + # A person who is a member of the product driver team is affiliated. |
| 402 | + person = self.factory.makePerson() |
| 403 | + team = self.factory.makeTeam(members=[person]) |
| 404 | + product = self.factory.makeProduct(driver=team, name='pting') |
| 405 | + self._check_affiliated_with_product(person, product, 'driver') |
| 406 | + |
| 407 | + def test_product_group_driver_affiliation(self): |
| 408 | + # A person who is the driver for a product's group is affiliated. |
| 409 | + person = self.factory.makePerson() |
| 410 | + project = self.factory.makeProject(driver=person) |
| 411 | + product = self.factory.makeProduct(project=project, name='pting') |
| 412 | + self._check_affiliated_with_product(person, product, 'driver') |
| 413 | + |
| 414 | + def test_no_product_security_contact_affiliation(self): |
| 415 | + # A person who is the security contact for a product is is not |
| 416 | + # affiliated for simple product affiliation checks. |
| 417 | + person = self.factory.makePerson() |
| 418 | + product = self.factory.makeProduct(security_contact=person) |
| 419 | + self.assertIs( |
| 420 | + None, IHasAffiliation(product).getAffiliationBadge(person)) |
| 421 | + |
| 422 | + def test_no_product_bug_supervisor_affiliation(self): |
| 423 | + # A person who is the bug supervisor for a product is is not |
| 424 | + # affiliated for simple product affiliation checks. |
| 425 | + person = self.factory.makePerson() |
| 426 | + product = self.factory.makeProduct(bug_supervisor=person) |
| 427 | + self.assertIs( |
| 428 | + None, IHasAffiliation(product).getAffiliationBadge(person)) |
| 429 | + |
| 430 | + def test_product_owner_affiliation(self): |
| 431 | + # A person who owns a product is affiliated. |
| 432 | + person = self.factory.makePerson() |
| 433 | + product = self.factory.makeProduct(owner=person, name='pting') |
| 434 | + self._check_affiliated_with_product(person, product, 'maintainer') |
| 435 | + |
| 436 | + |
| 437 | +class _TestBugTaskorBranchMixin: |
| 438 | + |
| 439 | + def test_distro_security_contact_affiliation(self): |
| 440 | + # A person who is the security contact for a distro is affiliated. |
| 441 | + person = self.factory.makePerson() |
| 442 | + distro = self.factory.makeDistribution( |
| 443 | + security_contact=person, name='pting') |
| 444 | + self._check_affiliated_with_distro(person, distro, 'security contact') |
| 445 | + |
| 446 | + def test_distro_bug_supervisor_affiliation(self): |
| 447 | + # A person who is the bug supervisor for a distro is affiliated. |
| 448 | + person = self.factory.makePerson() |
| 449 | + distro = self.factory.makeDistribution( |
| 450 | + bug_supervisor=person, name='pting') |
| 451 | + self._check_affiliated_with_distro(person, distro, 'bug supervisor') |
| 452 | + |
| 453 | + def test_product_security_contact_affiliation(self): |
| 454 | + # A person who is the security contact for a distro is affiliated. |
| 455 | + person = self.factory.makePerson() |
| 456 | + product = self.factory.makeProduct( |
| 457 | + security_contact=person, name='pting') |
| 458 | + self._check_affiliated_with_product( |
| 459 | + person, product, 'security contact') |
| 460 | + |
| 461 | + def test_product_bug_supervisor_affiliation(self): |
| 462 | + # A person who is the bug supervisor for a distro is affiliated. |
| 463 | + person = self.factory.makePerson() |
| 464 | + product = self.factory.makeProduct( |
| 465 | + bug_supervisor=person, name='pting') |
| 466 | + self._check_affiliated_with_product(person, product, 'bug supervisor') |
| 467 | + |
| 468 | + |
| 469 | +class TestBugTaskPillarAffiliation(_TestBugTaskorBranchMixin, |
| 470 | + TestCaseWithFactory): |
| 471 | + |
| 472 | + layer = DatabaseFunctionalLayer |
| 473 | + |
| 474 | + def test_correct_pillar_is_used(self): |
| 475 | + bugtask = self.factory.makeBugTask() |
| 476 | + badge = IHasAffiliation(bugtask) |
| 477 | + self.assertEqual(bugtask.pillar, badge.getPillar()) |
| 478 | + |
| 479 | + def _check_affiliated_with_distro(self, person, target, role): |
| 480 | + bugtask = self.factory.makeBugTask(target=target) |
| 481 | + badge = IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 482 | + self.assertEqual( |
| 483 | + ("/@@/distribution-badge", "Pting %s" % role), badge) |
| 484 | + |
| 485 | + def _check_affiliated_with_product(self, person, target, role): |
| 486 | + bugtask = self.factory.makeBugTask(target=target) |
| 487 | + badge = IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 488 | + self.assertEqual( |
| 489 | + ("/@@/product-badge", "Pting %s" % role), badge) |
| 490 | + |
| 491 | + def test_product_affiliation_query_count(self): |
| 492 | + # Only 4 queries are expected, selects from: |
| 493 | + # - Bug, BugTask, Product, Person |
| 494 | + person = self.factory.makePerson() |
| 495 | + product = self.factory.makeProduct(owner=person, name='pting') |
| 496 | + bugtask = self.factory.makeBugTask(target=product) |
| 497 | + Store.of(bugtask).invalidate() |
| 498 | + with StormStatementRecorder() as recorder: |
| 499 | + IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 500 | + self.assertThat(recorder, HasQueryCount(Equals(4))) |
| 501 | + |
| 502 | + def test_distro_affiliation_query_count(self): |
| 503 | + # Only 4 queries are expected, selects from: |
| 504 | + # - Bug, BugTask, Distribution, Person |
| 505 | person = self.factory.makePerson() |
| 506 | distro = self.factory.makeDistribution(owner=person, name='pting') |
| 507 | bugtask = self.factory.makeBugTask(target=distro) |
| 508 | - badge = IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 509 | - self.assertEqual( |
| 510 | - badge, ("/@@/distribution-badge", "Affiliated with Pting")) |
| 511 | - |
| 512 | - def test_bugtask_product_affiliation(self): |
| 513 | - # A person who owns a bugtask product is affiliated. |
| 514 | - person = self.factory.makePerson() |
| 515 | - product = self.factory.makeProduct(owner=person, name='pting') |
| 516 | - bugtask = self.factory.makeBugTask(target=product) |
| 517 | - badge = IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 518 | - self.assertEqual( |
| 519 | - badge, ("/@@/product-badge", "Affiliated with Pting")) |
| 520 | + Store.of(bugtask).invalidate() |
| 521 | + with StormStatementRecorder() as recorder: |
| 522 | + IHasAffiliation(bugtask).getAffiliationBadge(person) |
| 523 | + self.assertThat(recorder, HasQueryCount(Equals(4))) |
| 524 | + |
| 525 | + |
| 526 | +class TestBranchPillarAffiliation(_TestBugTaskorBranchMixin, |
| 527 | + TestCaseWithFactory): |
| 528 | + |
| 529 | + layer = DatabaseFunctionalLayer |
| 530 | + |
| 531 | + def test_correct_pillar_is_used(self): |
| 532 | + branch = self.factory.makeBranch() |
| 533 | + badge = IHasAffiliation(branch) |
| 534 | + self.assertEqual(branch.product, badge.getPillar()) |
| 535 | + |
| 536 | + def _check_affiliated_with_distro(self, person, target, role): |
| 537 | + distroseries = self.factory.makeDistroSeries(distribution=target) |
| 538 | + sp = self.factory.makeSourcePackage(distroseries=distroseries) |
| 539 | + branch = self.factory.makeBranch(sourcepackage=sp) |
| 540 | + badge = IHasAffiliation(branch).getAffiliationBadge(person) |
| 541 | + self.assertEqual( |
| 542 | + ("/@@/distribution-badge", "Pting %s" % role), badge) |
| 543 | + |
| 544 | + def _check_affiliated_with_product(self, person, target, role): |
| 545 | + branch = self.factory.makeBranch(product=target) |
| 546 | + badge = IHasAffiliation(branch).getAffiliationBadge(person) |
| 547 | + self.assertEqual( |
| 548 | + ("/@@/product-badge", "Pting %s" % role), badge) |
| 549 | + |
| 550 | + |
| 551 | +class TestDistroSeriesPillarAffiliation(TestCaseWithFactory): |
| 552 | + |
| 553 | + layer = DatabaseFunctionalLayer |
| 554 | + |
| 555 | + def test_correct_pillar_is_used(self): |
| 556 | + series = self.factory.makeDistroSeries() |
| 557 | + badge = IHasAffiliation(series) |
| 558 | + self.assertEqual(series.distribution, badge.getPillar()) |
| 559 | + |
| 560 | + def test_driver_affiliation(self): |
| 561 | + # A person who is the driver for a distroseries is affiliated. |
| 562 | + # Here, the affiliation is with the distribution of the series. |
| 563 | + owner = self.factory.makePerson() |
| 564 | + driver = self.factory.makePerson() |
| 565 | + distribution = self.factory.makeDistribution( |
| 566 | + owner=owner, driver=driver, name='pting') |
| 567 | + distroseries = self.factory.makeDistroSeries( |
| 568 | + registrant=driver, distribution=distribution) |
| 569 | + badge = IHasAffiliation(distroseries).getAffiliationBadge(driver) |
| 570 | + self.assertEqual( |
| 571 | + ("/@@/distribution-badge", "Pting driver"), badge) |
| 572 | + |
| 573 | + def test_distro_driver_affiliation(self): |
| 574 | + # A person who is the driver for a distroseries' distro is affiliated. |
| 575 | + # Here, the affiliation is with the distribution of the series. |
| 576 | + owner = self.factory.makePerson() |
| 577 | + driver = self.factory.makePerson() |
| 578 | + distribution = self.factory.makeDistribution( |
| 579 | + owner=owner, driver=driver, name='pting') |
| 580 | + distroseries = self.factory.makeDistroSeries( |
| 581 | + registrant=owner, distribution=distribution) |
| 582 | + badge = IHasAffiliation(distroseries).getAffiliationBadge(driver) |
| 583 | + self.assertEqual( |
| 584 | + ("/@@/distribution-badge", "Pting driver"), badge) |
| 585 | + |
| 586 | + |
| 587 | +class TestProductSeriesPillarAffiliation(TestCaseWithFactory): |
| 588 | + |
| 589 | + layer = DatabaseFunctionalLayer |
| 590 | + |
| 591 | + def test_correct_pillar_is_used(self): |
| 592 | + series = self.factory.makeProductSeries() |
| 593 | + badge = IHasAffiliation(series) |
| 594 | + self.assertEqual(series.product, badge.getPillar()) |
| 595 | + |
| 596 | + def test_driver_affiliation(self): |
| 597 | + # A person who is the driver for a productseries is affiliated. |
| 598 | + # Here, the affiliation is with the product. |
| 599 | + owner = self.factory.makePerson() |
| 600 | + driver = self.factory.makePerson() |
| 601 | + product = self.factory.makeProduct( |
| 602 | + owner=owner, driver=driver, name='pting') |
| 603 | + productseries = self.factory.makeProductSeries( |
| 604 | + owner=driver, product=product) |
| 605 | + badge = IHasAffiliation(productseries).getAffiliationBadge(driver) |
| 606 | + self.assertEqual( |
| 607 | + ("/@@/product-badge", "Pting driver"), badge) |
| 608 | + |
| 609 | + def test_product_driver_affiliation(self): |
| 610 | + # A person who is the driver for a productseries' product is |
| 611 | + # affiliated. Here, the affiliation is with the product. |
| 612 | + owner = self.factory.makePerson() |
| 613 | + driver = self.factory.makePerson() |
| 614 | + product = self.factory.makeProduct( |
| 615 | + owner=owner, driver=driver, name='pting') |
| 616 | + productseries = self.factory.makeProductSeries( |
| 617 | + owner=owner, product=product) |
| 618 | + badge = IHasAffiliation(productseries).getAffiliationBadge(driver) |
| 619 | + self.assertEqual( |
| 620 | + ("/@@/product-badge", "Pting driver"), badge) |
| 621 | + |
| 622 | + def test_product_group_driver_affiliation(self): |
| 623 | + # A person who is the driver for a productseries' product's group is |
| 624 | + # affiliated. Here, the affiliation is with the product. |
| 625 | + owner = self.factory.makePerson() |
| 626 | + driver = self.factory.makePerson() |
| 627 | + project = self.factory.makeProject(driver=driver) |
| 628 | + product = self.factory.makeProduct( |
| 629 | + owner=owner, project=project, name='pting') |
| 630 | + productseries = self.factory.makeProductSeries( |
| 631 | + owner=owner, product=product) |
| 632 | + badge = IHasAffiliation(productseries).getAffiliationBadge(driver) |
| 633 | + self.assertEqual( |
| 634 | + ("/@@/product-badge", "Pting driver"), badge) |
| 635 | + |
| 636 | + |
| 637 | +class TestQuestionPillarAffiliation(TestCaseWithFactory): |
| 638 | + |
| 639 | + layer = DatabaseFunctionalLayer |
| 640 | + |
| 641 | + def test_correct_pillar_is_used_for_product(self): |
| 642 | + product = self.factory.makeProduct() |
| 643 | + question = self.factory.makeQuestion(target=product) |
| 644 | + badge = IHasAffiliation(question) |
| 645 | + self.assertEqual(question.product, badge.getPillar()) |
| 646 | + |
| 647 | + def test_correct_pillar_is_used_for_distribution(self): |
| 648 | + distribution = self.factory.makeDistribution() |
| 649 | + question = self.factory.makeQuestion(target=distribution) |
| 650 | + badge = IHasAffiliation(question) |
| 651 | + self.assertEqual(question.distribution, badge.getPillar()) |
| 652 | + |
| 653 | + def test_correct_pillar_is_used_for_distro_sourcepackage(self): |
| 654 | + distribution = self.factory.makeDistribution() |
| 655 | + distro_sourcepackage = self.factory.makeDistributionSourcePackage( |
| 656 | + distribution=distribution) |
| 657 | + owner = self.factory.makePerson() |
| 658 | + question = self.factory.makeQuestion( |
| 659 | + target=distro_sourcepackage, owner=owner) |
| 660 | + badge = IHasAffiliation(question) |
| 661 | + self.assertEqual(distribution, badge.getPillar()) |
| 662 | + |
| 663 | + def test_answer_contact_affiliation_for_distro(self): |
| 664 | + # A person is affiliated if they are an answer contact for a distro |
| 665 | + # target. Even if they also own the distro, the answer contact |
| 666 | + # affiliation takes precedence. |
| 667 | + answer_contact = self.factory.makePerson() |
| 668 | + english = getUtility(ILanguageSet)['en'] |
| 669 | + answer_contact.addLanguage(english) |
| 670 | + distro = self.factory.makeDistribution(owner=answer_contact) |
| 671 | + with person_logged_in(answer_contact): |
| 672 | + distro.addAnswerContact(answer_contact, answer_contact) |
| 673 | + question = self.factory.makeQuestion(target=distro) |
| 674 | + badge = IHasAffiliation(question).getAffiliationBadge(answer_contact) |
| 675 | + self.assertEqual( |
| 676 | + ("/@@/distribution-badge", "%s answer contact" % |
| 677 | + distro.displayname), badge) |
| 678 | + |
| 679 | + def test_answer_contact_affiliation_for_distro_sourcepackage(self): |
| 680 | + # A person is affiliated if they are an answer contact for a dsp |
| 681 | + # target. Even if they also own the distro, the answer contact |
| 682 | + # affiliation takes precedence. |
| 683 | + answer_contact = self.factory.makePerson() |
| 684 | + english = getUtility(ILanguageSet)['en'] |
| 685 | + answer_contact.addLanguage(english) |
| 686 | + distribution = self.factory.makeDistribution(owner=answer_contact) |
| 687 | + distro_sourcepackage = self.factory.makeDistributionSourcePackage( |
| 688 | + distribution=distribution) |
| 689 | + with person_logged_in(answer_contact): |
| 690 | + distro_sourcepackage.addAnswerContact( |
| 691 | + answer_contact, answer_contact) |
| 692 | + question = self.factory.makeQuestion( |
| 693 | + target=distro_sourcepackage, owner=answer_contact) |
| 694 | + badge = IHasAffiliation(question).getAffiliationBadge(answer_contact) |
| 695 | + self.assertEqual( |
| 696 | + ("/@@/distribution-badge", "%s answer contact" % |
| 697 | + distro_sourcepackage.displayname), badge) |
| 698 | + |
| 699 | + def test_answer_contact_affiliation_for_distro_sourcepackage_distro(self): |
| 700 | + # A person is affiliated if they are an answer contact for a dsp |
| 701 | + # target's distro. Even if they also own the distro, the answer |
| 702 | + # contact affiliation takes precedence. |
| 703 | + answer_contact = self.factory.makePerson() |
| 704 | + english = getUtility(ILanguageSet)['en'] |
| 705 | + answer_contact.addLanguage(english) |
| 706 | + distribution = self.factory.makeDistribution(owner=answer_contact) |
| 707 | + distro_sourcepackage = self.factory.makeDistributionSourcePackage( |
| 708 | + distribution=distribution) |
| 709 | + with person_logged_in(answer_contact): |
| 710 | + distribution.addAnswerContact(answer_contact, answer_contact) |
| 711 | + question = self.factory.makeQuestion( |
| 712 | + target=distro_sourcepackage, owner=answer_contact) |
| 713 | + badge = IHasAffiliation(question).getAffiliationBadge(answer_contact) |
| 714 | + self.assertEqual( |
| 715 | + ("/@@/distribution-badge", "%s answer contact" % |
| 716 | + distribution.displayname), badge) |
| 717 | + |
| 718 | + def test_answer_contact_affiliation_for_product(self): |
| 719 | + # A person is affiliated if they are an answer contact for a product |
| 720 | + # target. Even if they also own the product, the answer contact |
| 721 | + # affiliation takes precedence. |
| 722 | + answer_contact = self.factory.makePerson() |
| 723 | + english = getUtility(ILanguageSet)['en'] |
| 724 | + answer_contact.addLanguage(english) |
| 725 | + product = self.factory.makeProduct(owner=answer_contact) |
| 726 | + with person_logged_in(answer_contact): |
| 727 | + product.addAnswerContact(answer_contact, answer_contact) |
| 728 | + question = self.factory.makeQuestion(target=product) |
| 729 | + badge = IHasAffiliation(question).getAffiliationBadge(answer_contact) |
| 730 | + self.assertEqual( |
| 731 | + ("/@@/product-badge", "%s answer contact" % |
| 732 | + product.displayname), badge) |
| 733 | + |
| 734 | + def test_product_affiliation(self): |
| 735 | + # A person is affiliated if they are affiliated with the product. |
| 736 | + person = self.factory.makePerson() |
| 737 | + product = self.factory.makeProduct(owner=person) |
| 738 | + question = self.factory.makeQuestion(target=product) |
| 739 | + badge = IHasAffiliation(question).getAffiliationBadge(person) |
| 740 | + self.assertEqual( |
| 741 | + ("/@@/product-badge", "%s maintainer" % |
| 742 | + product.displayname), badge) |
| 743 | + |
| 744 | + def test_distribution_affiliation(self): |
| 745 | + # A person is affiliated if they are affiliated with the distribution. |
| 746 | + person = self.factory.makePerson() |
| 747 | + distro = self.factory.makeDistribution(owner=person) |
| 748 | + question = self.factory.makeQuestion(target=distro) |
| 749 | + badge = IHasAffiliation(question).getAffiliationBadge(person) |
| 750 | + self.assertEqual( |
| 751 | + ("/@@/distribution-badge", "%s maintainer" % |
| 752 | + distro.displayname), badge) |
| 753 | + |
| 754 | + |
| 755 | +class TestSpecificationPillarAffiliation(TestCaseWithFactory): |
| 756 | + |
| 757 | + layer = DatabaseFunctionalLayer |
| 758 | + |
| 759 | + def test_correct_pillar_is_used_for_product(self): |
| 760 | + product = self.factory.makeProduct() |
| 761 | + specification = self.factory.makeSpecification(product=product) |
| 762 | + badge = IHasAffiliation(specification) |
| 763 | + self.assertEqual(specification.product, badge.getPillar()) |
| 764 | + |
| 765 | + def test_correct_pillar_is_used_for_distribution(self): |
| 766 | + distro = self.factory.makeDistribution() |
| 767 | + specification = self.factory.makeSpecification(distribution=distro) |
| 768 | + badge = IHasAffiliation(specification) |
| 769 | + self.assertEqual(specification.distribution, badge.getPillar()) |
| 770 | + |
| 771 | + def test_product_affiliation(self): |
| 772 | + # A person is affiliated if they are affiliated with the pillar. |
| 773 | + person = self.factory.makePerson() |
| 774 | + product = self.factory.makeProduct(owner=person) |
| 775 | + specification = self.factory.makeSpecification(product=product) |
| 776 | + badge = IHasAffiliation(specification).getAffiliationBadge(person) |
| 777 | + self.assertEqual( |
| 778 | + ("/@@/product-badge", "%s maintainer" % |
| 779 | + product.displayname), badge) |
| 780 | + |
| 781 | + def test_distribution_affiliation(self): |
| 782 | + # A person is affiliated if they are affiliated with the distribution. |
| 783 | + person = self.factory.makePerson() |
| 784 | + distro = self.factory.makeDistribution(owner=person) |
| 785 | + specification = self.factory.makeSpecification(distribution=distro) |
| 786 | + badge = IHasAffiliation(specification).getAffiliationBadge(person) |
| 787 | + self.assertEqual( |
| 788 | + ("/@@/distribution-badge", "%s maintainer" % |
| 789 | + distro.displayname), badge) |
| 790 | |
| 791 | === modified file 'lib/lp/services/features/flags.py' |
| 792 | --- lib/lp/services/features/flags.py 2011-08-05 06:40:47 +0000 |
| 793 | +++ lib/lp/services/features/flags.py 2011-08-09 00:14:32 +0000 |
| 794 | @@ -121,6 +121,10 @@ |
| 795 | 'boolean', |
| 796 | ('Enables the expanding of extra details in the person picker.'), |
| 797 | ''), |
| 798 | + ('disclosure.personpicker_affiliation.enabled', |
| 799 | + 'boolean', |
| 800 | + ('Enables display of affiliation details in the person picker.'), |
| 801 | + ''), |
| 802 | ('disclosure.person_affiliation_rank.enabled', |
| 803 | 'boolean', |
| 804 | ('Enables ranking by pillar affiliation in the person picker.'), |
| 805 | |
| 806 | === modified file 'lib/lp/testing/factory.py' |
| 807 | --- lib/lp/testing/factory.py 2011-08-05 06:40:47 +0000 |
| 808 | +++ lib/lp/testing/factory.py 2011-08-09 00:14:32 +0000 |
| 809 | @@ -954,7 +954,7 @@ |
| 810 | licenses=None, owner=None, registrant=None, |
| 811 | title=None, summary=None, official_malone=None, |
| 812 | translations_usage=None, bug_supervisor=None, |
| 813 | - driver=None): |
| 814 | + driver=None, security_contact=None): |
| 815 | """Create and return a new, arbitrary Product.""" |
| 816 | if owner is None: |
| 817 | owner = self.makePerson() |
| 818 | @@ -990,6 +990,8 @@ |
| 819 | naked_product.bug_supervisor = bug_supervisor |
| 820 | if driver is not None: |
| 821 | naked_product.driver = driver |
| 822 | + if security_contact is not None: |
| 823 | + naked_product.security_contact = security_contact |
| 824 | return product |
| 825 | |
| 826 | def makeProductSeries(self, product=None, name=None, owner=None, |
| 827 | @@ -1023,7 +1025,7 @@ |
| 828 | return ProxyFactory(series) |
| 829 | |
| 830 | def makeProject(self, name=None, displayname=None, title=None, |
| 831 | - homepageurl=None, summary=None, owner=None, |
| 832 | + homepageurl=None, summary=None, owner=None, driver=None, |
| 833 | description=None): |
| 834 | """Create and return a new, arbitrary ProjectGroup.""" |
| 835 | if owner is None: |
| 836 | @@ -1038,7 +1040,7 @@ |
| 837 | description = self.getUniqueString('description') |
| 838 | if title is None: |
| 839 | title = self.getUniqueString('title') |
| 840 | - return getUtility(IProjectGroupSet).new( |
| 841 | + project = getUtility(IProjectGroupSet).new( |
| 842 | name=name, |
| 843 | displayname=displayname, |
| 844 | title=title, |
| 845 | @@ -1046,6 +1048,9 @@ |
| 846 | summary=summary, |
| 847 | description=description, |
| 848 | owner=owner) |
| 849 | + if driver is not None: |
| 850 | + removeSecurityProxy(project).driver = driver |
| 851 | + return project |
| 852 | |
| 853 | def makeSprint(self, title=None, name=None): |
| 854 | """Make a sprint.""" |
| 855 | @@ -2324,9 +2329,10 @@ |
| 856 | |
| 857 | def makeDistribution(self, name=None, displayname=None, owner=None, |
| 858 | registrant=None, members=None, title=None, |
| 859 | - aliases=None, bug_supervisor=None, |
| 860 | - publish_root_dir=None, publish_base_url=None, |
| 861 | - publish_copy_base_url=None, no_pubconf=False): |
| 862 | + aliases=None, bug_supervisor=None, driver=None, |
| 863 | + security_contact=None, publish_root_dir=None, |
| 864 | + publish_base_url=None, publish_copy_base_url=None, |
| 865 | + no_pubconf=False): |
| 866 | """Make a new distribution.""" |
| 867 | if name is None: |
| 868 | name = self.getUniqueString(prefix="distribution") |
| 869 | @@ -2346,11 +2352,15 @@ |
| 870 | distro = getUtility(IDistributionSet).new( |
| 871 | name, displayname, title, description, summary, domainname, |
| 872 | members, owner, registrant) |
| 873 | + naked_distro = removeSecurityProxy(distro) |
| 874 | if aliases is not None: |
| 875 | - removeSecurityProxy(distro).setAliases(aliases) |
| 876 | + naked_distro.setAliases(aliases) |
| 877 | + if driver is not None: |
| 878 | + naked_distro.driver = driver |
| 879 | if bug_supervisor is not None: |
| 880 | - naked_distro = removeSecurityProxy(distro) |
| 881 | naked_distro.bug_supervisor = bug_supervisor |
| 882 | + if security_contact is not None: |
| 883 | + naked_distro.security_contact = security_contact |
| 884 | if not no_pubconf: |
| 885 | self.makePublisherConfig( |
| 886 | distro, publish_root_dir, publish_base_url, |
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
...
> + def getPillar(self):
> + return self.context
> +
> + def getAffiliationB
> + """ Return the affiliation information for a person given a context.
> +
> + The context is a Distribution, Product etc and is associated with a
> + pillar. Checks are done to see if the person is associated with the
> + context first (owner, driver etc) and if not, then the pillar.
> + """
> + pillar = self.getPillar()
> +
> + def checkAffiliatio
> + # Check the affiliation defined by the specified capability on the
> + # context and then pillar. Capability is IHasOwner etc. WHatever
> + # matches first (context or pillar) is used for the display name.
> + affiliated_entity = None
> + capabilityProvi
> + if capabilityProvi
> + if person.
> + affiliated_entity = self.context.
> + if (affiliated_entity is None and capabilityProvi
> + if person.
> + affiliated_entity = pillar.displayname
> + if affiliated_entity is None:
> + return None
> + return affiliated_entity, role
I do not see why this is an inner function. This could be simpler too if
we decide that all we care about is product or distribution. We know how to
check owner, drivers, and other roles. The other kinds of items I
see returned, notably for specification and question are probably wrong.
We do not need the interface checks if we are certain we are getting a
distro or product.
I have some doubts about the universality of these checks. I think
owner and driver are universal. bug_supervisor and security contact are
only useful in bugs and branches cases that deal with privacy and security.
eg assigning a branch reviewer, bug assignee, subscriber.
Questions might care more about answer contacts which are more likely to
be assigned. Consider Ubuntu. The owners and drivers are rarely assigned.
when I assign a question, I care about the answer contact for the package
first, then the contacts for Ubuntu.
Specifications care about the people who are assignees, drafters, approvers,
and need feedback. In most cases these people are owners and drivers. There
is a rare case for the approver who is for the series goal...
The series is the problem. Maybe a separate branch becuase it is a rule
that may not prove very important. The series driver (release manager) is
the most important person for bugs that affect a series or blueprints with
a series goal. Those driver are assumed to be a subset of owner or drivers
who have the authority to define the work for a series. I happen to know
that This is not an issue for Ubuntu or any of our major pro...