Merge lp:~wgrant/launchpad/dedupe-bugnotification-canApprove into lp:launchpad
Status: | Merged |
---|---|
Approved by: | William Grant |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13011 |
Proposed branch: | lp:~wgrant/launchpad/dedupe-bugnotification-canApprove |
Merge into: | lp:launchpad |
Diff against target: |
429 lines (+168/-151) 6 files modified
database/schema/security.cfg (+5/-0) lib/lp/bugs/doc/bug-nomination.txt (+0/-125) lib/lp/bugs/model/bugnomination.py (+16/-19) lib/lp/bugs/tests/test_bugnomination.py (+116/-1) lib/lp/soyuz/model/component.py (+3/-2) lib/lp/testing/factory.py (+28/-4) |
To merge this branch: | bzr merge lp:~wgrant/launchpad/dedupe-bugnotification-canApprove |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email:
|
Commit message
[r=benji][bug=451390] BugNomination.
Description of the change
BugNomination.
This branch changes BugNomination.
I replaced the canApprove doctest with unittests, and added a new one to ensure that packageset permissions are respected.
This branch looks good.
I do have one question: I haven't seen "with logged_ in('admin' ):" used in non-test code before (and a grep
celebrity_
through the LP code doesn't turn any up). That doesn't mean it's wrong,
but perhaps something that warrants some thought. I'm a little worried
that logging or other audit trail might be polluted by faking the active
user. Perhaps judicious use of removeSecurityProxy is an alternative.