Merge lp:~wgrant/launchpad/export-bug-nominations into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~wgrant/launchpad/export-bug-nominations
Merge into: lp:launchpad
Diff against target: None lines
To merge this branch: bzr merge lp:~wgrant/launchpad/export-bug-nominations
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+10715@code.launchpad.net
To post a comment you must log in.
Revision history for this message
William Grant (wgrant) wrote :

= Summary =
Bug nominations want to be exported through the API.

== Proposed fix ==
Export IBugNomination and relevant methods from IBug, moving access control code from views to model classes.

== Implementation details ==
All of the relevant IBug and IBugNomination methods already took the user as an argument, so no signature changes were required.

The main non-trivial bits are the refactoring of IBug.canBeNominatedFor and IBug.addNomination to properly forbid attempts to nominate a non-series. Rather than introduce an ISeriesBugTarget as suggested in the bug, I opted to just use IBugTarget. Further type checking was going to be required anyway when dealing with an ISeriesBugTarget, so it's probably cleaner this way.

There are four significant model behaviour changes:
 - IBug.addNomination will no longer automatically approve nominations if possible. The view does this explicitly instead.
 - BugNomination.approve is now a no-op if it is already approved, as IBugNomination says. Previously it would crash or create a duplicate task.
 - BugNomination.decline now fails if it is already approved, as IBugNomination says. Previously it would successfully decline the task.
 - Bug.addNomination will refuse to create a nomination for a series on a pillar that does not already have a task. This has always been enforced in the view.

== Tests ==
Direct tests were added for IBug.canBeNominatedFor and the webservice, and a test to verify that nominations are automatically approved was dropped. Other tests required trivial changes to comply with the new model restrictions.

 $ bin/test -t test_bugnomination -t bugtask-edit-views.txt -t bug-nomination.txt -t bug-set-status.txt -t bugtask.txt -t xx-bug.txt -t test_bugchanges

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Well done!

Good branch, though there are some little warts that came up on IRC and
which you fixed interactively:

 * Don't return None when looking up a malformed id if a well-formed id
   that doesn't belong to any bug nomination reaises NotFoundError.

 * Don't over-test isApproved() in the doctest, though it was fine as a
   transitional check during development. It's already covered in
   bug-nominations.txt.

 * Better not export canApprove() for any person, because it may expose
   private team memberships. Just make it apply to the current user:
   "Can I approve this?"

 * Test tearDown belongs just below setUp.

 * Don't derive a test case from another, but lift the commonality up
   into a mixin class.

 * You inherited a few small pieces of lint; should be easy to clean up.

With that out of the way, r=me.

Jeroen

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-08-08 06:45:42 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-08-24 12:06:17 +0000
@@ -24,6 +24,7 @@
2424
25from lp.bugs.interfaces.bug import IBug25from lp.bugs.interfaces.bug import IBug
26from lp.bugs.interfaces.bugbranch import IBugBranch26from lp.bugs.interfaces.bugbranch import IBugBranch
27from lp.bugs.interfaces.bugnomination import IBugNomination
27from lp.bugs.interfaces.bugtask import IBugTask28from lp.bugs.interfaces.bugtask import IBugTask
28from lp.bugs.interfaces.bugtarget import IHasBugs29from lp.bugs.interfaces.bugtarget import IHasBugs
29from lp.soyuz.interfaces.build import (30from lp.soyuz.interfaces.build import (
@@ -118,6 +119,12 @@
118 IBug, 'unlinkHWSubmission', 'submission', IHWSubmission)119 IBug, 'unlinkHWSubmission', 'submission', IHWSubmission)
119patch_collection_return_type(120patch_collection_return_type(
120 IBug, 'getHWSubmissions', IHWSubmission)121 IBug, 'getHWSubmissions', IHWSubmission)
122IBug['getNominations'].queryTaggedValue(
123 LAZR_WEBSERVICE_EXPORTED)['params']['nominations'].value_type.schema = (
124 IBugNomination)
125patch_entry_return_type(IBug, 'addNomination', IBugNomination)
126patch_entry_return_type(IBug, 'getNominationFor', IBugNomination)
127patch_collection_return_type(IBug, 'getNominations', IBugNomination)
121128
122patch_choice_parameter_type(129patch_choice_parameter_type(
123 IHasBugs, 'searchTasks', 'hardware_bus', HWBus)130 IHasBugs, 'searchTasks', 'hardware_bus', HWBus)
124131
=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py 2009-08-21 15:33:18 +0000
+++ lib/lp/bugs/browser/bug.py 2009-08-23 04:53:33 +0000
@@ -58,6 +58,7 @@
58from lp.bugs.interfaces.bugwatch import IBugWatchSet58from lp.bugs.interfaces.bugwatch import IBugWatchSet
59from lp.bugs.interfaces.cve import ICveSet59from lp.bugs.interfaces.cve import ICveSet
60from lp.bugs.interfaces.bugattachment import IBugAttachmentSet60from lp.bugs.interfaces.bugattachment import IBugAttachmentSet
61from lp.bugs.interfaces.bugnomination import IBugNominationSet
6162
62from canonical.launchpad.mailnotification import (63from canonical.launchpad.mailnotification import (
63 MailWrapper, format_rfc2822_date)64 MailWrapper, format_rfc2822_date)
@@ -126,6 +127,13 @@
126 if attachment is not None and attachment.bug == self.context:127 if attachment is not None and attachment.bug == self.context:
127 return attachment128 return attachment
128129
130 @stepthrough('nominations')
131 def traverse_nominations(self, nomination_id):
132 """Traverse to a nomination by id."""
133 if not nomination_id.isdigit():
134 return None
135 return getUtility(IBugNominationSet).get(nomination_id)
136
129137
130class BugFacets(StandardLaunchpadFacets):138class BugFacets(StandardLaunchpadFacets):
131 """The links that will appear in the facet menu for an `IBug`.139 """The links that will appear in the facet menu for an `IBug`.
132140
=== modified file 'lib/lp/bugs/browser/bugnomination.py'
--- lib/lp/bugs/browser/bugnomination.py 2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/browser/bugnomination.py 2009-08-24 10:45:09 +0000
@@ -105,8 +105,9 @@
105 target=series, owner=self.user)105 target=series, owner=self.user)
106106
107 # If the user has the permission to approve the nomination,107 # If the user has the permission to approve the nomination,
108 # then nomination was approved automatically.108 # we approve it automatically.
109 if nomination.isApproved():109 if nomination.canApprove(self.user):
110 nomination.approve(self.user)
110 approved_nominations.append(111 approved_nominations.append(
111 nomination.target.bugtargetdisplayname)112 nomination.target.bugtargetdisplayname)
112 else:113 else:
113114
=== modified file 'lib/lp/bugs/browser/tests/bugtask-edit-views.txt'
--- lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2009-08-13 19:03:36 +0000
+++ lib/lp/bugs/browser/tests/bugtask-edit-views.txt 2009-08-24 10:45:09 +0000
@@ -126,8 +126,7 @@
126 >>> ubuntu_grumpy = ubuntu.getSeries('grumpy')126 >>> ubuntu_grumpy = ubuntu.getSeries('grumpy')
127 >>> nomination = bug_two.addNomination(127 >>> nomination = bug_two.addNomination(
128 ... target=ubuntu_grumpy, owner=getUtility(ILaunchBag).user)128 ... target=ubuntu_grumpy, owner=getUtility(ILaunchBag).user)
129 >>> nomination.isApproved()129 >>> nomination.approve(getUtility(ILaunchBag).user)
130 True
131 >>> transaction.commit()130 >>> transaction.commit()
132131
133 >>> ubuntu_grumpy_task = bug_two.bugtasks[5]132 >>> ubuntu_grumpy_task = bug_two.bugtasks[5]
134133
=== modified file 'lib/lp/bugs/doc/bug-nomination.txt'
--- lib/lp/bugs/doc/bug-nomination.txt 2009-06-13 19:58:26 +0000
+++ lib/lp/bugs/doc/bug-nomination.txt 2009-08-26 02:33:29 +0000
@@ -296,8 +296,8 @@
296 thunderbird (Ubuntu Grumpy)296 thunderbird (Ubuntu Grumpy)
297 thunderbird (Ubuntu)297 thunderbird (Ubuntu)
298298
299If the one nominating a goal is a driver of the series, the299Let's now nominate for Warty. no_privs is the driver, so will have
300nomination will be automatically approved.300no problems.
301301
302 >>> ubuntu_warty = ubuntu.getSeries("warty")302 >>> ubuntu_warty = ubuntu.getSeries("warty")
303 >>> login("foo.bar@canonical.com")303 >>> login("foo.bar@canonical.com")
@@ -306,6 +306,7 @@
306306
307 >>> warty_nomination = bug_one.addNomination(307 >>> warty_nomination = bug_one.addNomination(
308 ... target=ubuntu_warty, owner=no_privs)308 ... target=ubuntu_warty, owner=no_privs)
309 >>> warty_nomination.approve(no_privs)
309310
310 >>> print warty_nomination.status.title311 >>> print warty_nomination.status.title
311 Approved312 Approved
@@ -636,18 +637,18 @@
636 NominationError: ...637 NominationError: ...
637638
638Nominating a bug for an obsolete distroseries raises a639Nominating a bug for an obsolete distroseries raises a
639NominationSeriesObsoleteError. Let's mark warty obsolete to640NominationSeriesObsoleteError. Let's make a new obsolete distroseries
640demonstrate.641to demonstrate.
641642
642 >>> from canonical.launchpad.interfaces import DistroSeriesStatus643 >>> from canonical.launchpad.interfaces import DistroSeriesStatus
643644
644(Temporarily log in as an admin user to change the series status.)
645
646 >>> login("foo.bar@canonical.com")645 >>> login("foo.bar@canonical.com")
647 >>> ubuntu_warty.status = DistroSeriesStatus.OBSOLETE646 >>> ubuntu_edgy = factory.makeDistroRelease(
647 ... distribution=ubuntu, version='6.10',
648 ... status=DistroSeriesStatus.OBSOLETE)
648 >>> login("no-priv@canonical.com")649 >>> login("no-priv@canonical.com")
649650
650 >>> bug_one.addNomination(target=ubuntu_warty, owner=no_privs)651 >>> bug_one.addNomination(target=ubuntu_edgy, owner=no_privs)
651 Traceback (most recent call last):652 Traceback (most recent call last):
652 ..653 ..
653 NominationSeriesObsoleteError: ...654 NominationSeriesObsoleteError: ...
654655
=== modified file 'lib/lp/bugs/doc/bug-set-status.txt'
--- lib/lp/bugs/doc/bug-set-status.txt 2009-06-12 16:36:02 +0000
+++ lib/lp/bugs/doc/bug-set-status.txt 2009-08-24 10:45:09 +0000
@@ -225,6 +225,9 @@
225 >>> nomination = bug.addNomination(225 >>> nomination = bug.addNomination(
226 ... getUtility(ILaunchBag).user, ubuntu_hoary)226 ... getUtility(ILaunchBag).user, ubuntu_hoary)
227 >>> nomination.isApproved()227 >>> nomination.isApproved()
228 False
229 >>> nomination.approve(getUtility(ILaunchBag).user)
230 >>> nomination.isApproved()
228 True231 True
229 >>> login('no-priv@canonical.com')232 >>> login('no-priv@canonical.com')
230233
@@ -247,6 +250,9 @@
247 >>> nomination = bug.addNomination(250 >>> nomination = bug.addNomination(
248 ... getUtility(ILaunchBag).user, ubuntu_warty)251 ... getUtility(ILaunchBag).user, ubuntu_warty)
249 >>> nomination.isApproved()252 >>> nomination.isApproved()
253 False
254 >>> nomination.approve(getUtility(ILaunchBag).user)
255 >>> nomination.isApproved()
250 True256 True
251 >>> login('no-priv@canonical.com')257 >>> login('no-priv@canonical.com')
252258
253259
=== modified file 'lib/lp/bugs/doc/bugtask.txt'
--- lib/lp/bugs/doc/bugtask.txt 2009-08-13 19:03:36 +0000
+++ lib/lp/bugs/doc/bugtask.txt 2009-08-24 10:45:09 +0000
@@ -1123,8 +1123,7 @@
1123 ... sourcepackagenameset.queryByName('mozilla-firefox')))1123 ... sourcepackagenameset.queryByName('mozilla-firefox')))
1124 <BugTask at ...>1124 <BugTask at ...>
11251125
1126 >>> new_bug.addNomination(mark, ubuntu.currentseries)1126 >>> new_bug.addNomination(mark, ubuntu.currentseries).approve(mark)
1127 <BugNomination at ...>
11281127
1129The first task has been created and successfully nominated to Hoary.1128The first task has been created and successfully nominated to Hoary.
11301129
11311130
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2009-08-06 14:36:45 +0000
+++ lib/lp/bugs/interfaces/bug.py 2009-08-24 12:06:17 +0000
@@ -559,20 +559,24 @@
559 distroseries=None):559 distroseries=None):
560 """Create an INullBugTask and return it for the given parameters."""560 """Create an INullBugTask and return it for the given parameters."""
561561
562 @operation_parameters(
563 target=Reference(schema=IBugTarget, title=_('Target')))
564 @call_with(owner=REQUEST_USER)
565 @export_factory_operation(Interface, [])
562 def addNomination(owner, target):566 def addNomination(owner, target):
563 """Nominate a bug for an IDistroSeries or IProductSeries.567 """Nominate a bug for an IDistroSeries or IProductSeries.
564568
565 :owner: An IPerson.569 :owner: An IPerson.
566 :target: An IDistroSeries or IProductSeries.570 :target: An IDistroSeries or IProductSeries.
567571
568 The nomination will be automatically approved, if the user has
569 permission to approve it.
570
571 This method creates and returns a BugNomination. (See572 This method creates and returns a BugNomination. (See
572 lp.bugs.model.bugnomination.BugNomination.)573 lp.bugs.model.bugnomination.BugNomination.)
573 """574 """
574575
575 def canBeNominatedFor(nomination_target):576 @operation_parameters(
577 target=Reference(schema=IBugTarget, title=_('Target')))
578 @export_read_operation()
579 def canBeNominatedFor(target):
576 """Can this bug nominated for this target?580 """Can this bug nominated for this target?
577581
578 :nomination_target: An IDistroSeries or IProductSeries.582 :nomination_target: An IDistroSeries or IProductSeries.
@@ -580,7 +584,11 @@
580 Returns True or False.584 Returns True or False.
581 """585 """
582586
583 def getNominationFor(nomination_target):587 @operation_parameters(
588 target=Reference(schema=IBugTarget, title=_('Target')))
589 @operation_returns_entry(Interface)
590 @export_read_operation()
591 def getNominationFor(target):
584 """Return the IBugNomination for the target.592 """Return the IBugNomination for the target.
585593
586 If no nomination is found, a NotFoundError is raised.594 If no nomination is found, a NotFoundError is raised.
@@ -588,6 +596,15 @@
588 :param nomination_target: An IDistroSeries or IProductSeries.596 :param nomination_target: An IDistroSeries or IProductSeries.
589 """597 """
590598
599 @operation_parameters(
600 target=Reference(
601 schema=IBugTarget, title=_('Target'), required=False),
602 nominations=List(
603 title=_("Nominations to search through."),
604 value_type=Reference(schema=Interface), # IBugNomination
605 required=False))
606 @operation_returns_collection_of(Interface) # IBugNomination
607 @export_read_operation()
591 def getNominations(target=None, nominations=None):608 def getNominations(target=None, nominations=None):
592 """Return a list of all IBugNominations for this bug.609 """Return a list of all IBugNominations for this bug.
593610
594611
=== modified file 'lib/lp/bugs/interfaces/bugnomination.py'
--- lib/lp/bugs/interfaces/bugnomination.py 2009-07-17 00:26:05 +0000
+++ lib/lp/bugs/interfaces/bugnomination.py 2009-08-26 03:56:55 +0000
@@ -19,24 +19,38 @@
19from zope.schema import Int, Datetime, Choice, Set19from zope.schema import Int, Datetime, Choice, Set
20from zope.interface import Interface, Attribute20from zope.interface import Interface, Attribute
21from lazr.enum import DBEnumeratedType, DBItem21from lazr.enum import DBEnumeratedType, DBItem
22from lazr.restful.declarations import (
23 REQUEST_USER, call_with, export_as_webservice_entry,
24 export_read_operation, export_write_operation, exported,
25 operation_parameters, webservice_error)
26from lazr.restful.fields import Reference, ReferenceChoice
2227
23from canonical.launchpad import _28from canonical.launchpad import _
24from canonical.launchpad.fields import PublicPersonChoice29from canonical.launchpad.fields import PublicPersonChoice
25from canonical.launchpad.interfaces.launchpad import IHasBug, IHasDateCreated30from canonical.launchpad.interfaces.launchpad import IHasBug, IHasDateCreated
31from lp.bugs.interfaces.bug import IBug
32from lp.bugs.interfaces.bugtarget import IBugTarget
33from lp.registry.interfaces.distroseries import IDistroSeries
34from lp.registry.interfaces.productseries import IProductSeries
35from lp.registry.interfaces.person import IPerson
26from lp.registry.interfaces.role import IHasOwner36from lp.registry.interfaces.role import IHasOwner
27from canonical.launchpad.interfaces.validation import (37from canonical.launchpad.interfaces.validation import (
28 can_be_nominated_for_serieses)38 can_be_nominated_for_serieses)
2939
40
30class NominationError(Exception):41class NominationError(Exception):
31 """The bug cannot be nominated for this release."""42 """The bug cannot be nominated for this release."""
43 webservice_error(400)
3244
3345
34class NominationSeriesObsoleteError(Exception):46class NominationSeriesObsoleteError(Exception):
35 """A bug cannot be nominated for an obsolete series."""47 """A bug cannot be nominated for an obsolete series."""
48 webservice_error(400)
3649
3750
38class BugNominationStatusError(Exception):51class BugNominationStatusError(Exception):
39 """A error occurred while trying to set a bug nomination status."""52 """A error occurred while trying to set a bug nomination status."""
53 webservice_error(400)
4054
4155
42class BugNominationStatus(DBEnumeratedType):56class BugNominationStatus(DBEnumeratedType):
@@ -72,38 +86,43 @@
7286
73 A nomination can apply to an IDistroSeries or an IProductSeries.87 A nomination can apply to an IDistroSeries or an IProductSeries.
74 """88 """
89 export_as_webservice_entry()
90
75 # We want to customize the titles and descriptions of some of the91 # We want to customize the titles and descriptions of some of the
76 # attributes of our parent interfaces, so we redefine those specific92 # attributes of our parent interfaces, so we redefine those specific
77 # attributes below.93 # attributes below.
78 id = Int(title=_("Bug Nomination #"))94 id = Int(title=_("Bug Nomination #"))
79 bug = Int(title=_("Bug #"))95 bug = exported(Reference(schema=IBug, readonly=True))
80 date_created = Datetime(96 date_created = exported(Datetime(
81 title=_("Date Submitted"),97 title=_("Date Submitted"),
82 description=_("The date on which this nomination was submitted."),98 description=_("The date on which this nomination was submitted."),
83 required=True, readonly=True)99 required=True, readonly=True))
84 date_decided = Datetime(100 date_decided = exported(Datetime(
85 title=_("Date Decided"),101 title=_("Date Decided"),
86 description=_(102 description=_(
87 "The date on which this nomination was approved or declined."),103 "The date on which this nomination was approved or declined."),
88 required=False, readonly=True)104 required=False, readonly=True))
89 distroseries = Choice(105 distroseries = exported(ReferenceChoice(
90 title=_("Series"), required=False,106 title=_("Series"), required=False, readonly=True,
91 vocabulary="DistroSeries")107 vocabulary="DistroSeries", schema=IDistroSeries))
92 productseries = Choice(108 productseries = exported(ReferenceChoice(
93 title=_("Series"), required=False,109 title=_("Series"), required=False, readonly=True,
94 vocabulary="ProductSeries")110 vocabulary="ProductSeries", schema=IProductSeries))
95 owner = PublicPersonChoice(111 owner = exported(PublicPersonChoice(
96 title=_('Submitter'), required=True, readonly=True,112 title=_('Submitter'), required=True, readonly=True,
97 vocabulary='ValidPersonOrTeam')113 vocabulary='ValidPersonOrTeam'))
98 decider = PublicPersonChoice(114 decider = exported(PublicPersonChoice(
99 title=_('Decided By'), required=False, readonly=True,115 title=_('Decided By'), required=False, readonly=True,
100 vocabulary='ValidPersonOrTeam')116 vocabulary='ValidPersonOrTeam'))
101 target = Attribute(117 target = exported(Reference(
102 "The IProductSeries or IDistroSeries of this nomination.")118 schema=IBugTarget,
103 status = Choice(119 title=_("The IProductSeries or IDistroSeries of this nomination.")))
120 status = exported(Choice(
104 title=_("Status"), vocabulary=BugNominationStatus,121 title=_("Status"), vocabulary=BugNominationStatus,
105 default=BugNominationStatus.PROPOSED)122 default=BugNominationStatus.PROPOSED, readonly=True))
106123
124 @call_with(approver=REQUEST_USER)
125 @export_write_operation()
107 def approve(approver):126 def approve(approver):
108 """Approve this bug for fixing in a series.127 """Approve this bug for fixing in a series.
109128
@@ -117,6 +136,8 @@
117 /already/ approved, this method is a noop.136 /already/ approved, this method is a noop.
118 """137 """
119138
139 @call_with(decliner=REQUEST_USER)
140 @export_write_operation()
120 def decline(decliner):141 def decline(decliner):
121 """Decline this bug for fixing in a series.142 """Decline this bug for fixing in a series.
122143
@@ -139,6 +160,8 @@
139 def isApproved():160 def isApproved():
140 """Is this nomination in Approved state?"""161 """Is this nomination in Approved state?"""
141162
163 @operation_parameters(person=Reference(schema=IPerson))
164 @export_read_operation()
142 def canApprove(person):165 def canApprove(person):
143 """Is this person allowed to approve the nomination?"""166 """Is this person allowed to approve the nomination?"""
144167
145168
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2009-08-04 12:36:46 +0000
+++ lib/lp/bugs/model/bug.py 2009-08-26 01:54:39 +0000
@@ -1073,74 +1073,79 @@
10731073
1074 def addNomination(self, owner, target):1074 def addNomination(self, owner, target):
1075 """See `IBug`."""1075 """See `IBug`."""
1076 if not self.canBeNominatedFor(target):
1077 raise NominationError(
1078 "This bug cannot be nominated for %s." %
1079 target.bugtargetdisplayname)
1080
1076 distroseries = None1081 distroseries = None
1077 productseries = None1082 productseries = None
1078 if IDistroSeries.providedBy(target):1083 if IDistroSeries.providedBy(target):
1079 distroseries = target1084 distroseries = target
1080 target_displayname = target.fullseriesname
1081 if target.status == DistroSeriesStatus.OBSOLETE:1085 if target.status == DistroSeriesStatus.OBSOLETE:
1082 raise NominationSeriesObsoleteError(1086 raise NominationSeriesObsoleteError(
1083 "%s is an obsolete series." % target_displayname)1087 "%s is an obsolete series." % target.bugtargetdisplayname)
1084 else:1088 else:
1085 assert IProductSeries.providedBy(target)1089 assert IProductSeries.providedBy(target)
1086 productseries = target1090 productseries = target
1087 target_displayname = target.title
1088
1089 if not self.canBeNominatedFor(target):
1090 raise NominationError(
1091 "This bug cannot be nominated for %s." % target_displayname)
10921091
1093 nomination = BugNomination(1092 nomination = BugNomination(
1094 owner=owner, bug=self, distroseries=distroseries,1093 owner=owner, bug=self, distroseries=distroseries,
1095 productseries=productseries)1094 productseries=productseries)
1096 if nomination.canApprove(owner):1095 self.addChange(SeriesNominated(UTC_NOW, owner, target))
1097 nomination.approve(owner)
1098 else:
1099 self.addChange(SeriesNominated(UTC_NOW, owner, target))
1100 return nomination1096 return nomination
11011097
1102 def canBeNominatedFor(self, nomination_target):1098 def canBeNominatedFor(self, target):
1103 """See `IBug`."""1099 """See `IBug`."""
1104 try:1100 try:
1105 self.getNominationFor(nomination_target)1101 self.getNominationFor(target)
1106 except NotFoundError:1102 except NotFoundError:
1107 # No nomination exists. Let's see if the bug is already1103 # No nomination exists. Let's see if the bug is already
1108 # directly targeted to this nomination_target.1104 # directly targeted to this nomination target.
1109 if IDistroSeries.providedBy(nomination_target):1105 if IDistroSeries.providedBy(target):
1110 target_getter = operator.attrgetter("distroseries")1106 series_getter = operator.attrgetter("distroseries")
1111 elif IProductSeries.providedBy(nomination_target):1107 pillar_getter = operator.attrgetter("distribution")
1112 target_getter = operator.attrgetter("productseries")1108 elif IProductSeries.providedBy(target):
1109 series_getter = operator.attrgetter("productseries")
1110 pillar_getter = operator.attrgetter("product")
1113 else:1111 else:
1114 raise AssertionError(1112 return False
1115 "Expected IDistroSeries or IProductSeries target. "
1116 "Got %r." % nomination_target)
11171113
1118 for task in self.bugtasks:1114 for task in self.bugtasks:
1119 if target_getter(task) == nomination_target:1115 if series_getter(task) == target:
1120 # The bug is already targeted at this1116 # The bug is already targeted at this
1121 # nomination_target.1117 # nomination target.
1122 return False1118 return False
11231119
1124 # No nomination or tasks are targeted at this1120 # No nomination or tasks are targeted at this
1125 # nomination_target.1121 # nomination target. But we also don't want to nominate for a
1126 return True1122 # series of a product or distro for which we don't have a
1123 # plain pillar task.
1124 for task in self.bugtasks:
1125 if pillar_getter(task) == pillar_getter(target):
1126 return True
1127
1128 # No tasks match the candidate's pillar. We must refuse.
1129 return False
1127 else:1130 else:
1128 # The bug is already nominated for this nomination_target.1131 # The bug is already nominated for this nomination target.
1129 return False1132 return False
11301133
1131 def getNominationFor(self, nomination_target):1134 def getNominationFor(self, target):
1132 """See `IBug`."""1135 """See `IBug`."""
1133 if IDistroSeries.providedBy(nomination_target):1136 if IDistroSeries.providedBy(target):
1134 filter_args = dict(distroseriesID=nomination_target.id)1137 filter_args = dict(distroseriesID=target.id)
1138 elif IProductSeries.providedBy(target):
1139 filter_args = dict(productseriesID=target.id)
1135 else:1140 else:
1136 filter_args = dict(productseriesID=nomination_target.id)1141 return None
11371142
1138 nomination = BugNomination.selectOneBy(bugID=self.id, **filter_args)1143 nomination = BugNomination.selectOneBy(bugID=self.id, **filter_args)
11391144
1140 if nomination is None:1145 if nomination is None:
1141 raise NotFoundError(1146 raise NotFoundError(
1142 "Bug #%d is not nominated for %s." % (1147 "Bug #%d is not nominated for %s." % (
1143 self.id, nomination_target.displayname))1148 self.id, target.displayname))
11441149
1145 return nomination1150 return nomination
11461151
11471152
=== modified file 'lib/lp/bugs/model/bugnomination.py'
--- lib/lp/bugs/model/bugnomination.py 2009-06-25 00:40:31 +0000
+++ lib/lp/bugs/model/bugnomination.py 2009-08-23 07:15:30 +0000
@@ -33,7 +33,8 @@
33from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities33from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
34from canonical.launchpad.webapp.interfaces import NotFoundError34from canonical.launchpad.webapp.interfaces import NotFoundError
35from lp.bugs.interfaces.bugnomination import (35from lp.bugs.interfaces.bugnomination import (
36 BugNominationStatus, IBugNomination, IBugNominationSet)36 BugNominationStatus, BugNominationStatusError, IBugNomination,
37 IBugNominationSet)
37from lp.registry.interfaces.person import validate_public_person38from lp.registry.interfaces.person import validate_public_person
3839
39class BugNomination(SQLBase):40class BugNomination(SQLBase):
@@ -66,6 +67,9 @@
6667
67 def approve(self, approver):68 def approve(self, approver):
68 """See IBugNomination."""69 """See IBugNomination."""
70 if self.isApproved():
71 # Approving an approved nomination shouldn't duplicate tasks.
72 return
69 self.status = BugNominationStatus.APPROVED73 self.status = BugNominationStatus.APPROVED
70 self.decider = approver74 self.decider = approver
71 self.date_decided = datetime.now(pytz.timezone('UTC'))75 self.date_decided = datetime.now(pytz.timezone('UTC'))
@@ -91,6 +95,9 @@
9195
92 def decline(self, decliner):96 def decline(self, decliner):
93 """See IBugNomination."""97 """See IBugNomination."""
98 if self.isApproved():
99 raise BugNominationStatusError(
100 "Cannot decline an approved nomination.")
94 self.status = BugNominationStatus.DECLINED101 self.status = BugNominationStatus.DECLINED
95 self.decider = decliner102 self.decider = decliner
96 self.date_decided = datetime.now(pytz.timezone('UTC'))103 self.date_decided = datetime.now(pytz.timezone('UTC'))
97104
=== modified file 'lib/lp/bugs/stories/webservice/xx-bug.txt'
--- lib/lp/bugs/stories/webservice/xx-bug.txt 2009-08-13 15:12:16 +0000
+++ lib/lp/bugs/stories/webservice/xx-bug.txt 2009-08-26 03:56:55 +0000
@@ -562,6 +562,213 @@
562 "id": ... "title": "Test bug"...562 "id": ... "title": "Test bug"...
563563
564564
565Bug nominations
566---------------
567
568A bug may be nominated for any number of distro or product series.
569Nominations can be inspected, created, approved and declined through
570the webservice.
571
572First we'll create Fooix 0.1 and 0.2. Eric is the owner.
573
574 >>> login('foo.bar@canonical.com')
575 >>> eric = factory.makePerson(name='eric')
576 >>> fooix = factory.makeProduct(name='fooix', owner=eric)
577 >>> fx01 = fooix.newSeries(eric, '0.1', 'The 0.1.x series')
578 >>> fx02 = fooix.newSeries(eric, '0.2', 'The 0.2.x series')
579 >>> debuntu = factory.makeDistribution(name='debuntu', owner=eric)
580 >>> debuntu50 = debuntu.newSeries(
581 ... '5.0', '5.0', '5.0', '5.0', '5.0', '5.0', None, eric)
582 >>> bug = factory.makeBug(product=fooix)
583 >>> logout()
584
585Initially there are no nominations.
586
587 >>> pprint_collection(webservice.named_get(
588 ... '/bugs/%d' % bug.id, 'getNominations').jsonBody())
589 start: None
590 total_size: 0
591 ---
592
593 >>> login('foo.bar@canonical.com')
594 >>> john = factory.makePerson(name='john')
595 >>> logout()
596
597 >>> from canonical.launchpad.testing.pages import webservice_for_person
598 >>> from canonical.launchpad.webapp.interfaces import OAuthPermission
599
600 >>> john_webservice = webservice_for_person(
601 ... john, permission=OAuthPermission.WRITE_PRIVATE)
602
603But John, an unprivileged user, wants it fixed in Fooix 0.1.1.
604
605 >>> print john_webservice.named_post(
606 ... '/bugs/%d' % bug.id, 'addNomination',
607 ... target=john_webservice.getAbsoluteUrl('/fooix/0.1'))
608 HTTP/1.1 201 Created
609 ...
610 Location: http://.../bugs/.../nominations/...
611 ...
612
613 >>> nominations = webservice.named_get(
614 ... '/bugs/%d' % bug.id, 'getNominations').jsonBody()
615 >>> pprint_collection(nominations)
616 start: 0
617 total_size: 1
618 ---
619 bug_link: u'http://.../bugs/...'
620 date_created: u'...'
621 date_decided: None
622 decider_link: None
623 distroseries_link: None
624 owner_link: u'http://.../~john'
625 productseries_link: u'http://.../fooix/0.1'
626 resource_type_link: u'http://.../#bug_nomination'
627 self_link: u'http://.../bugs/.../nominations/...'
628 status: u'Nominated'
629 target_link: u'http://.../fooix/0.1'
630 ---
631
632John cannot approve or decline the nomination.
633
634 >>> nom_url = nominations['entries'][0]['self_link']
635
636 >>> print john_webservice.named_get(
637 ... nom_url, 'canApprove',
638 ... person=john_webservice.getAbsoluteUrl('/~john')).jsonBody()
639 False
640
641 >>> print john_webservice.named_post(nom_url, 'approve')
642 HTTP/1.1 401 Unauthorized...
643
644 >>> print john_webservice.named_post(nom_url, 'decline')
645 HTTP/1.1 401 Unauthorized...
646
647 >>> login('foo.bar@canonical.com')
648 >>> len(bug.bugtasks)
649 1
650 >>> logout()
651
652The owner, however, can decline the nomination.
653
654 >>> print john_webservice.named_get(
655 ... nom_url, 'canApprove',
656 ... person=john_webservice.getAbsoluteUrl('/~eric')).jsonBody()
657 True
658
659 >>> eric_webservice = webservice_for_person(
660 ... eric, permission=OAuthPermission.WRITE_PRIVATE)
661 >>> print eric_webservice.named_post(nom_url, 'decline')
662 HTTP/1.1 200 Ok...
663
664 >>> login('foo.bar@canonical.com')
665 >>> len(bug.bugtasks)
666 1
667 >>> logout()
668
669We can see that the nomination was declined.
670
671 >>> nominations = webservice.named_get(
672 ... '/bugs/%d' % bug.id, 'getNominations').jsonBody()
673 >>> pprint_collection(nominations)
674 start: 0
675 total_size: 1
676 ---
677 bug_link: u'http://.../bugs/...'
678 date_created: u'...'
679 date_decided: u'...'
680 decider_link: u'http://.../~eric'
681 distroseries_link: None
682 owner_link: u'http://.../~john'
683 productseries_link: u'http://.../fooix/0.1'
684 resource_type_link: u'http://.../#bug_nomination'
685 self_link: u'http://.../bugs/.../nominations/...'
686 status: u'Declined'
687 target_link: u'http://.../fooix/0.1'
688 ---
689
690The owner can approve a previously declined nomination.
691
692 >>> print eric_webservice.named_post(nom_url, 'approve')
693 HTTP/1.1 200 Ok...
694
695This marks the nomination as Approved, and creates a new task.
696
697 >>> nominations = webservice.named_get(
698 ... '/bugs/%d' % bug.id, 'getNominations').jsonBody()
699 >>> pprint_collection(nominations)
700 start: 0
701 total_size: 1
702 ---
703 bug_link: u'http://.../bugs/...'
704 date_created: u'...'
705 date_decided: u'...'
706 decider_link: u'http://.../~eric'
707 distroseries_link: None
708 owner_link: u'http://.../~john'
709 productseries_link: u'http://.../fooix/0.1'
710 resource_type_link: u'http://.../#bug_nomination'
711 self_link: u'http://.../bugs/.../nominations/...'
712 status: u'Approved'
713 target_link: u'http://.../fooix/0.1'
714 ---
715
716 >>> login('foo.bar@canonical.com')
717 >>> len(bug.bugtasks)
718 2
719 >>> logout()
720
721The owner cannot change his mind and decline an approved task.
722
723 >>> print eric_webservice.named_post(nom_url, 'decline')
724 HTTP/1.1 400 Bad Request
725 ...
726 Cannot decline an approved nomination.
727
728 >>> login('foo.bar@canonical.com')
729 >>> len(bug.bugtasks)
730 2
731 >>> logout()
732
733While he can approve it again, it's a no-op.
734
735 >>> print eric_webservice.named_post(nom_url, 'approve')
736 HTTP/1.1 200 Ok...
737
738 >>> login('foo.bar@canonical.com')
739 >>> len(bug.bugtasks)
740 2
741 >>> logout()
742
743A bug cannot be nominated for a non-series.
744
745 >>> print john_webservice.named_get(
746 ... '/bugs/%d' % bug.id, 'canBeNominatedFor',
747 ... target=john_webservice.getAbsoluteUrl('/fooix')).jsonBody()
748 False
749
750 >>> print john_webservice.named_post(
751 ... '/bugs/%d' % bug.id, 'addNomination',
752 ... target=john_webservice.getAbsoluteUrl('/fooix'))
753 HTTP/1.1 400 Bad Request
754 ...
755 NominationError: This bug cannot be nominated for Fooix.
756
757The bug also can't be nominated for Debuntu 5.0, as it has no
758Debuntu tasks.
759
760 >>> print john_webservice.named_get(
761 ... '/bugs/%d' % bug.id, 'canBeNominatedFor',
762 ... target=john_webservice.getAbsoluteUrl('/debuntu/5.0')).jsonBody()
763 False
764
765 >>> print john_webservice.named_post(
766 ... '/bugs/%d' % bug.id, 'addNomination',
767 ... target=john_webservice.getAbsoluteUrl('/debuntu/5.0'))
768 HTTP/1.1 400 Bad Request
769 ...
770 NominationError: This bug cannot be nominated for Debuntu 5.0.
771
565Bug subscriptions772Bug subscriptions
566-----------------773-----------------
567774
@@ -639,9 +846,6 @@
639Once we have a member, a web service must be created for that user.846Once we have a member, a web service must be created for that user.
640Then, the user can unsubsribe the group from the bug.847Then, the user can unsubsribe the group from the bug.
641848
642 >>> from canonical.launchpad.testing.pages import webservice_for_person
643 >>> from canonical.launchpad.webapp.interfaces import OAuthPermission
644
645 >>> member_webservice = webservice_for_person(849 >>> member_webservice = webservice_for_person(
646 ... ubuntu_team_member, permission=OAuthPermission.WRITE_PRIVATE)850 ... ubuntu_team_member, permission=OAuthPermission.WRITE_PRIVATE)
647851
648852
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2009-07-17 18:46:25 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2009-08-24 10:45:09 +0000
@@ -1209,38 +1209,6 @@
12091209
1210 self.assertRecordedChange(expected_activity=expected_activity)1210 self.assertRecordedChange(expected_activity=expected_activity)
12111211
1212 def test_series_nominated_and_approved(self):
1213 # When adding a nomination that is approved automatically, it's
1214 # like adding a new bug task for the series directly.
1215 product = self.factory.makeProduct(owner=self.user)
1216 product.driver = self.user
1217 series = self.factory.makeProductSeries(product=product)
1218 self.bug.addTask(self.user, product)
1219 self.saveOldChanges()
1220
1221 nomination = self.bug.addNomination(self.user, series)
1222 self.assertTrue(nomination.isApproved())
1223
1224 expected_activity = {
1225 'person': self.user,
1226 'newvalue': series.bugtargetname,
1227 'whatchanged': 'bug task added',
1228 'newvalue': series.bugtargetname,
1229 }
1230
1231 task_added_notification = {
1232 'person': self.user,
1233 'text': (
1234 '** Also affects: %s\n'
1235 ' Importance: Undecided\n'
1236 ' Status: New' % (
1237 series.bugtargetname)),
1238 }
1239
1240 self.assertRecordedChange(
1241 expected_activity=expected_activity,
1242 expected_notification=task_added_notification)
1243
1244 def test_nomination_approved(self):1212 def test_nomination_approved(self):
1245 # When a nomination is approved, it's like adding a new bug1213 # When a nomination is approved, it's like adding a new bug
1246 # task for the series directly.1214 # task for the series directly.
12471215
=== added file 'lib/lp/bugs/tests/test_bugnomination.py'
--- lib/lp/bugs/tests/test_bugnomination.py 1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/tests/test_bugnomination.py 2009-08-26 02:17:17 +0000
@@ -0,0 +1,100 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Tests related to bug nominations."""
5
6__metaclass__ = type
7
8import unittest
9
10from canonical.launchpad.ftests import login, logout
11from canonical.testing import DatabaseFunctionalLayer
12from lp.testing import TestCaseWithFactory
13
14
15class TestBugCanBeNominatedForProductSeries(TestCaseWithFactory):
16 """Test IBug.canBeNominated for IProductSeries nominations."""
17
18 layer = DatabaseFunctionalLayer
19
20 def setUp(self):
21 super(TestBugCanBeNominatedForProductSeries, self).setUp()
22 login('foo.bar@canonical.com')
23 self.eric = self.factory.makePerson(name='eric')
24 self.setUpTarget()
25
26
27 def setUpTarget(self):
28 self.series = self.factory.makeProductSeries()
29 self.bug = self.factory.makeBug(product=self.series.product)
30 self.milestone = self.factory.makeMilestone(productseries=self.series)
31 self.random_series = self.factory.makeProductSeries()
32
33 def test_canBeNominatedFor_series(self):
34 # A bug may be nominated for a series of a product with an existing
35 # task.
36 self.assertTrue(self.bug.canBeNominatedFor(self.series))
37
38 def test_not_canBeNominatedFor_already_nominated_series(self):
39 # A bug may not be nominated for a series with an existing nomination.
40 self.assertTrue(self.bug.canBeNominatedFor(self.series))
41 self.bug.addNomination(self.eric, self.series)
42 self.assertFalse(self.bug.canBeNominatedFor(self.series))
43
44 def test_not_canBeNominatedFor_non_series(self):
45 # A bug may not be nominated for something other than a series.
46 self.assertFalse(self.bug.canBeNominatedFor(self.milestone))
47
48 def test_not_canBeNominatedFor_already_targeted_series(self):
49 # A bug may not be nominated for a series if a task already exists.
50 # This case should be caught by the check for an existing nomination,
51 # but there are some historical cases where a series task exists
52 # without a nomination.
53 self.assertTrue(self.bug.canBeNominatedFor(self.series))
54 self.bug.addTask(self.eric, self.series)
55 self.assertFalse(self.bug.canBeNominatedFor(self.series))
56
57 def test_not_canBeNominatedFor_random_series(self):
58 # A bug may only be nominated for a series if that series' pillar
59 # already has a task.
60 self.assertFalse(self.bug.canBeNominatedFor(self.random_series))
61
62 def tearDown(self):
63 logout()
64 super(TestBugCanBeNominatedForProductSeries, self).tearDown()
65
66
67class TestBugCanBeNominatedForDistroSeries(
68 TestBugCanBeNominatedForProductSeries):
69 """Test IBug.canBeNominated for IDistroSeries nominations."""
70
71 def setUpTarget(self):
72 self.series = self.factory.makeDistroRelease()
73 # The factory can't create a distro bug directly.
74 self.bug = self.factory.makeBug()
75 self.bug.addTask(self.eric, self.series.distribution)
76 self.milestone = self.factory.makeMilestone(
77 distribution=self.series.distribution)
78 self.random_series = self.factory.makeDistroRelease()
79
80 def test_not_canBeNominatedFor_source_package(self):
81 # A bug may not be nominated directly for a source package. The
82 # distroseries must be nominated instead.
83 spn = self.factory.makeSourcePackageName()
84 source_package = self.series.getSourcePackage(spn)
85 self.assertFalse(self.bug.canBeNominatedFor(source_package))
86
87 def test_canBeNominatedFor_with_only_distributionsourcepackage(self):
88 # A distribution source package task is sufficient to allow nomination
89 # to a series of that distribution.
90 sp_bug = self.factory.makeBug()
91 spn = self.factory.makeSourcePackageName()
92
93 self.assertFalse(sp_bug.canBeNominatedFor(self.series))
94 sp_bug.addTask(
95 self.eric, self.series.distribution.getSourcePackage(spn))
96 self.assertTrue(sp_bug.canBeNominatedFor(self.series))
97
98
99def test_suite():
100 return unittest.TestLoader().loadTestsFromName(__name__)