launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-register-merge-oops into lp:launchpad.

Commit message:
Fix OOPS when trying to register a git merge with a target path but no target repository.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-register-merge-oops/+merge/278790

Fix OOPS when trying to register a git merge with a target path but no target repository.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-register-merge-oops into lp:launchpad.

=== modified file 'lib/lp/code/browser/gitref.py'
--- lib/lp/code/browser/gitref.py	2015-09-24 09:58:26 +0000
+++ lib/lp/code/browser/gitref.py	2015-11-27 11:44:59 +0000
@@ -325,12 +325,16 @@
 
     def validate(self, data):
         source_ref = self.context
-        target_repository = self._validateRef(data, 'target')
-        if not target_repository.isRepositoryMergeable(source_ref.repository):
-            self.setFieldError(
-                'target_git_repository',
-                "%s is not mergeable into this repository." %
-                source_ref.repository.identity)
+        # The existence of target_git_repository is handled by the form
+        # machinery.
+        if data.get('target_git_repository') is not None:
+            target_repository = self._validateRef(data, 'target')
+            if not target_repository.isRepositoryMergeable(
+                    source_ref.repository):
+                self.setFieldError(
+                    'target_git_repository',
+                    "%s is not mergeable into this repository." %
+                    source_ref.repository.identity)
         if data.get('prerequisite_git_repository') is not None:
             prerequisite_repository = self._validateRef(data, 'prerequisite')
             if not target_repository.isRepositoryMergeable(

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2015-11-11 18:29:23 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2015-11-27 11:44:59 +0000
@@ -72,9 +72,7 @@
 from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.messages.model.message import MessageSet
 from lp.services.webapp import canonical_url
-from lp.services.webapp.interfaces import (
-    BrowserNotificationLevel,
-    )
+from lp.services.webapp.interfaces import BrowserNotificationLevel
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.testing import (
     BrowserTestCase,
@@ -885,6 +883,34 @@
             'form_wide_errors': []},
             simplejson.loads(view.form_result))
 
+    def test_register_ajax_request_with_missing_target_git_repository(self):
+        # A missing target_git_repository is a validation error.
+        owner = self.factory.makePerson()
+        extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+        with person_logged_in(owner):
+            request = LaunchpadTestRequest(
+                method='POST', principal=owner,
+                form={
+                    'field.actions.register': 'Propose Merge',
+                    'field.target_git_repository.target_git_repository': '',
+                    'field.target_git_repository-empty-marker': '1',
+                    'field.target_git_path': 'master',
+                    },
+                **extra)
+            view = create_initialized_view(
+                self.source_branch,
+                name='+register-merge',
+                request=request)
+        self.assertEqual(
+            '400 Validation', view.request.response.getStatusString())
+        self.assertEqual(
+            {'error_summary': 'There is 1 error.',
+            'errors': {
+                'field.target_git_repository': 'Required input is missing.',
+                },
+            'form_wide_errors': []},
+            simplejson.loads(view.form_result))
+
     def test_register_ajax_request_with_missing_target_git_path(self):
         # A missing target_git_path is a validation error.
         owner = self.factory.makePerson()