launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/bad-state-transition-2 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #820050 in Launchpad itself: "BadStateTransition: Invalid state transition for merge proposal: Superseded -> Approved "
  https://bugs.launchpad.net/launchpad/+bug/820050

For more details, see:
https://code.launchpad.net/~abentley/launchpad/bad-state-transition-2/+merge/72592

= 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 ==
None

== 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
-- 
https://code.launchpad.net/~abentley/launchpad/bad-state-transition-2/+merge/72592
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/bad-state-transition-2 into lp:launchpad.

=== 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:21:26 +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:21:26 +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,7 +2007,7 @@
         self.assertNotIn(r1, partial_revisions)
 
 
-class TestWebservice(TestCaseWithFactory):
+class TestWebservice(WebServiceTestCase):
     """Tests for the webservice."""
 
     layer = AppServerLayer
@@ -2040,3 +2042,15 @@
             bugtask = ws_object(launchpad, 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')