launchpad-reviewers team mailing list archive

[Thierry Carrez <thierry.carrez+lp@xxxxxxxxx>]

Thierry Carrez has proposed merging lp:~ttx/launchpad/lp1193389 into lp:launchpad.

Commit message:
Expose specification goal management in specification API

- Expose proposeGoal, acceptGoal, declineGoal methods and has_accepted_goal attribute
- Under wgrant advice, remove lp.blueprints.subscribers.specification_goalstatus as it's not used and its security model blocks proxied objects
- Add unit tests

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1193389 in Launchpad itself: ""Series goal" should be exported through API"
  https://bugs.launchpad.net/launchpad/+bug/1193389

For more details, see:
https://code.launchpad.net/~ttx/launchpad/lp1193389/+merge/171491

Expose specification goal management in specification API

- Expose proposeGoal, acceptGoal, declineGoal methods and has_accepted_goal attribute
- Under wgrant advice, remove lp.blueprints.subscribers.specification_goalstatus as it's not used and its security model blocks proxied objects
- Add unit tests
-- 
https://code.launchpad.net/~ttx/launchpad/lp1193389/+merge/171491
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~ttx/launchpad/lp1193389 into lp:launchpad.

=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml	2012-09-17 18:55:17 +0000
+++ lib/lp/blueprints/configure.zcml	2013-06-26 09:21:36 +0000
@@ -204,10 +204,6 @@
   <subscriber
       for="lp.blueprints.interfaces.specification.ISpecification
            lazr.lifecycle.interfaces.IObjectModifiedEvent"
-      handler="lp.blueprints.subscribers.specification_goalstatus"/>
-  <subscriber
-      for="lp.blueprints.interfaces.specification.ISpecification
-           lazr.lifecycle.interfaces.IObjectModifiedEvent"
       handler="lp.blueprints.mail.notifications.notify_specification_modified"/>
 
   <!-- SpecificationSet -->

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2013-04-11 00:51:46 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2013-06-26 09:21:36 +0000
@@ -17,6 +17,7 @@
 from lazr.restful.declarations import (
     call_with,
     export_as_webservice_entry,
+    export_operation_as,
     export_write_operation,
     exported,
     mutator_for,
@@ -70,6 +71,7 @@
     )
 from lp.blueprints.interfaces.sprint import ISprint
 from lp.bugs.interfaces.buglink import IBugLinkTarget
+from lp.bugs.interfaces.bugtarget import IBugTarget
 from lp.code.interfaces.branchlink import IHasLinkedBranches
 from lp.registry.interfaces.milestone import IMilestone
 from lp.registry.interfaces.person import IPerson
@@ -476,20 +478,39 @@
         """Return the list of email addresses that receive notifications."""
 
     # goal management
+    @call_with(proposer=REQUEST_USER)
+    @operation_parameters(
+        goal=Reference(schema=IBugTarget, title=_('Target'),
+        required=False, default=None))
+    @export_write_operation()
+    @operation_for_version("devel")
     def proposeGoal(goal, proposer):
         """Propose this spec for a series or distroseries."""
 
+    @call_with(decider=REQUEST_USER)
+    @export_operation_as('acceptGoal')
+    @export_write_operation()
+    @operation_for_version("devel")
     def acceptBy(decider):
         """Mark the spec as being accepted for its current series goal."""
 
+    @call_with(decider=REQUEST_USER)
+    @export_operation_as('declineGoal')
+    @export_write_operation()
+    @operation_for_version("devel")
     def declineBy(decider):
         """Mark the spec as being declined as a goal for the proposed
         series.
         """
 
-    has_accepted_goal = Attribute('Is true if this specification has been '
-        'proposed as a goal for a specific series, '
-        'and the drivers of that series have accepted the goal.')
+    has_accepted_goal = exported(
+        Bool(title=_('Series goal is accepted'),
+             readonly=True, required=True,
+             description=_(
+                'Is true if this specification has been '
+                'proposed as a goal for a specific series, '
+                'and the drivers of that series have accepted the goal.')),
+        as_of="devel")
 
     # lifecycle management
     def updateLifecycleStatus(user):

=== modified file 'lib/lp/blueprints/subscribers.py'
--- lib/lp/blueprints/subscribers.py	2011-12-30 06:14:56 +0000
+++ lib/lp/blueprints/subscribers.py	2013-06-26 09:21:36 +0000
@@ -9,17 +9,6 @@
 from lp.services.database.sqlbase import block_implicit_flushes
 
 
-@block_implicit_flushes
-def specification_goalstatus(spec, event):
-    """Update goalstatus if productseries or distroseries is changed."""
-    delta = spec.getDelta(
-        event.object_before_modification, IPerson(event.user))
-    if delta is None:
-        return
-    if delta.productseries is not None or delta.distroseries is not None:
-        spec.goalstatus = SpecificationGoalStatus.PROPOSED
-
-
 def specification_update_lifecycle_status(spec, event):
     """Mark the specification as started and/or complete if appropriate.
 

=== modified file 'lib/lp/blueprints/tests/test_webservice.py'
--- lib/lp/blueprints/tests/test_webservice.py	2013-01-25 03:30:08 +0000
+++ lib/lp/blueprints/tests/test_webservice.py	2013-06-26 09:21:36 +0000
@@ -401,3 +401,65 @@
 
         # Now that we've unlinked the bug, there are no linked bugs at all.
         self.assertEqual(0, spec.bugs.total_size)
+
+
+class TestSpecificationGoalHandling(SpecificationWebserviceTestCase):
+
+    layer = AppServerLayer
+
+    def setUp(self):
+        super(TestSpecificationGoalHandling, self).setUp()
+        self.driver = self.factory.makePerson()
+        self.proposer = self.factory.makePerson()
+        self.product = self.factory.makeProduct(driver=self.driver)
+        self.series = self.factory.makeProductSeries(product=self.product)
+
+    def test_goal_propose_and_accept(self):
+        # Create spec
+        db_spec = self.factory.makeBlueprint(product=self.product,
+                                             owner=self.proposer)
+        # Propose for series goal
+        with person_logged_in(self.proposer):
+            launchpad = self.factory.makeLaunchpadService(person=self.proposer)
+            spec = ws_object(launchpad, db_spec)
+            series = ws_object(launchpad, self.series)
+            spec.proposeGoal(goal=series)
+            transaction.commit()
+            self.assertEqual(db_spec.goal, self.series)
+            self.assertFalse(spec.has_accepted_goal)
+
+        # Accept series goal
+        with person_logged_in(self.driver):
+            launchpad = self.factory.makeLaunchpadService(person=self.driver)
+            spec = ws_object(launchpad, db_spec)
+            spec.acceptGoal()
+            transaction.commit()
+            self.assertTrue(spec.has_accepted_goal)
+
+    def test_goal_propose_decline_and_clear(self):
+        # Create spec
+        db_spec = self.factory.makeBlueprint(product=self.product,
+                                             owner=self.proposer)
+        # Propose for series goal
+        with person_logged_in(self.proposer):
+            launchpad = self.factory.makeLaunchpadService(person=self.proposer)
+            spec = ws_object(launchpad, db_spec)
+            series = ws_object(launchpad, self.series)
+            spec.proposeGoal(goal=series)
+            transaction.commit()
+            self.assertEqual(db_spec.goal, self.series)
+            self.assertFalse(spec.has_accepted_goal)
+
+        with person_logged_in(self.driver):
+            # Decline series goal
+            launchpad = self.factory.makeLaunchpadService(person=self.driver)
+            spec = ws_object(launchpad, db_spec)
+            spec.declineGoal()
+            transaction.commit()
+            self.assertFalse(spec.has_accepted_goal)
+            self.assertEqual(db_spec.goal, self.series)
+
+            # Clear series goal as a driver
+            spec.proposeGoal(goal=None)
+            transaction.commit()
+            self.assertIsNone(db_spec.goal)