Merge lp:~abentley/launchpad/bad-state-transition-2 into lp:launchpad

[Aaron Bentley]

Treat BadStateTransition as BadRequest.

= Summary =
Fix bug #820050: BadStateTransition: Invalid state transition for merge proposal: Superseded -> Approved

== Proposed fix ==
Have the web service treat BadStateTransition as a 400 Bad Request instead of a 500 Internal Server Error.

== Pre-implementation notes ==
None

== Implementation details ==
Modernized nearby tests.

== Tests ==
bin/test -t test_setStatus_invalid_transition test_branchmergeproposal

== Demo and Q/A ==
Create a merge proposal with a target that you can approvve, and resubmit it.
Using the web service, call setStatus(status='Approved) on the proposal. You should get BadRequest exception, not an Internal Server Error.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/errors.py
  lib/lp/code/model/tests/test_branchmergeproposal.py

[Graham Binns (gmb)]

review: Approve (code)

=== modified file 'lib/lp/code/errors.py'
--- lib/lp/code/errors.py	2011-08-22 17:40:02 +0000
+++ lib/lp/code/errors.py	2011-08-23 15:37:33 +0000
@@ -60,6 +60,7 @@
     """The context is not valid for a branch merge proposal search."""
 
 
+@error_status(httplib.BAD_REQUEST)
 class BadStateTransition(Exception):
     """The user requested a state transition that is not possible."""
 
 
=== modified file 'lib/lp/code/model/tests/test_branchmergeproposal.py'
--- lib/lp/code/model/tests/test_branchmergeproposal.py	2011-06-24 19:51:18 +0000
+++ lib/lp/code/model/tests/test_branchmergeproposal.py	2011-08-23 15:37:33 +0000
@@ -17,6 +17,7 @@
     )
 
 from lazr.lifecycle.event import ObjectModifiedEvent
+from lazr.restfulclient.errors import BadRequest
 from pytz import UTC
 from sqlobject import SQLObjectNotFound
 from storm.locals import Store
@@ -86,6 +87,7 @@
     person_logged_in,
     TestCaseWithFactory,
     ws_object,
+    WebServiceTestCase,
     )
 from lp.testing.factory import (
     GPGSigningContext,
@@ -2005,11 +2007,9 @@
         self.assertNotIn(r1, partial_revisions)
 
 
-class TestWebservice(TestCaseWithFactory):
+class TestWebservice(WebServiceTestCase):
     """Tests for the webservice."""
 
-    layer = AppServerLayer
-
     def test_getMergeProposals_with_merged_revnos(self):
         """Specifying merged revnos selects the correct merge proposal."""
         mp = self.factory.makeBranchMergeProposal()
@@ -2028,15 +2028,22 @@
     def test_getRelatedBugTasks(self):
         """Test the getRelatedBugTasks API."""
         db_bmp = self.factory.makeBranchMergeProposal()
-        launchpad = launchpadlib_for(
-            'test', db_bmp.registrant, version="devel",
-            service_root=self.layer.appserver_root_url('api'))
-
-        with person_logged_in(db_bmp.registrant):
-            db_bug = self.factory.makeBug()
-            db_bmp.source_branch.linkBug(db_bug, db_bmp.registrant)
-            transaction.commit()
-            bmp = ws_object(launchpad, db_bmp)
-            bugtask = ws_object(launchpad, db_bug.default_bugtask)
+        db_bug = self.factory.makeBug()
+        db_bmp.source_branch.linkBug(db_bug, db_bmp.registrant)
+        transaction.commit()
+        bmp = self.wsObject(db_bmp)
+        bugtask = self.wsObject(db_bug.default_bugtask)
         self.assertEqual(
             [bugtask], list(bmp.getRelatedBugTasks()))
+
+    def test_setStatus_invalid_transition(self):
+        """Emit BadRequest when an invalid transition is requested."""
+        bmp = self.factory.makeBranchMergeProposal()
+        with person_logged_in(bmp.registrant):
+            bmp.resubmit(bmp.registrant)
+        transaction.commit()
+        ws_bmp = self.wsObject(bmp, user=bmp.target_branch.owner)
+        with ExpectedException(
+            BadRequest,
+            '(.|\n)*Invalid state transition for merge proposal(.|\n)*'):
+            ws_bmp.setStatus(status='Approved')