launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/resubmit-target-prerequisite-same into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #820035 in Launchpad itself: "InvalidBranchMergeProposal: Target and prerequisite branches must be different. "
  https://bugs.launchpad.net/launchpad/+bug/820035

For more details, see:
https://code.launchpad.net/~abentley/launchpad/resubmit-target-prerequisite-same/+merge/70615

= Summary =
Fix bug 820035: InvalidBranchMergeProposal: Target and prerequisite branches must be different.
== Proposed fix ==
Handle InvalidBranchMergeProposal exceptions as user errors.

== Pre-implementation notes ==
None

== Implementation details ==
None

== Tests ==
bin/test -t test_resubmit_same_target_prerequisite

== Demo and Q/A ==
1. Create a new merge proposal (e.g. https://code.qastaging.launchpad.net/~matsubara/launchpad/lp-tour-only/+register-merge)
2. Go to +resubmit page (e.g. https://code.qastaging.launchpad.net/~matsubara/launchpad/lp-tour-only/+merge/65295/+resubmit)
3. Fill in Prerequisite Branch the same branch in Target Branch. (e.g. ~launchpad-pqm/launchpad/devel)
4. Click Resubmit


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/code/browser/tests/test_branchmergeproposal.py
  lib/lp/code/browser/branchmergeproposal.py
-- 
https://code.launchpad.net/~abentley/launchpad/resubmit-target-prerequisite-same/+merge/70615
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/resubmit-target-prerequisite-same into lp:launchpad.

=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py	2011-07-11 05:21:08 +0000
+++ lib/lp/code/browser/branchmergeproposal.py	2011-08-05 18:49:25 +0000
@@ -105,6 +105,7 @@
 from lp.code.errors import (
     BranchMergeProposalExists,
     ClaimReviewFailed,
+    InvalidBranchMergeProposal,
     WrongBranchMergeProposal,
     )
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
@@ -1040,6 +1041,9 @@
             self.request.response.addErrorNotification(message)
             self.next_url = canonical_url(self.context)
             return None
+        except InvalidBranchMergeProposal as e:
+            self.addError(str(e))
+            return None
         self.next_url = canonical_url(proposal)
         return proposal
 

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2011-06-24 17:47:52 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2011-08-05 18:49:25 +0000
@@ -595,12 +595,14 @@
         self.assertIs(None, new_proposal.supersedes)
 
     @staticmethod
-    def resubmitDefault(view, break_link=False):
+    def resubmitDefault(view, break_link=False, prerequisite_branch=None):
         context = view.context
+        if prerequisite_branch is None:
+            prerequisite_branch = context.prerequisite_branch
         return view.resubmit_action.success(
             {'source_branch': context.source_branch,
              'target_branch': context.target_branch,
-             'prerequisite_branch': context.prerequisite_branch,
+             'prerequisite_branch': prerequisite_branch,
              'description': None,
              'break_link': break_link,
             })
@@ -618,6 +620,16 @@
             ' <a href=.*>a similar merge proposal</a> is already active.'))
         self.assertEqual(BrowserNotificationLevel.ERROR, notification.level)
 
+    def test_resubmit_same_target_prerequisite(self):
+        """User error if same branch is target and prerequisite."""
+        view = self.createView()
+        first_bmp = view.context
+        self.resubmitDefault(
+            view, prerequisite_branch=first_bmp.target_branch)
+        self.assertEqual(
+            view.errors,
+            ['Target and prerequisite branches must be different.'])
+
 
 class TestResubmitBrowser(BrowserTestCase):
     """Browser tests for resubmitting branch merge proposals."""