#4178 Fix the output of the merge PR API endpoint when the PR conflicts
Merged 4 months ago by pingou. Opened 4 months ago by pingou.

file modified
+1

@@ -90,6 +90,7 @@ 

          "This request does not have the minimum review score "

          "necessary to be merged"

      )

+     EPRCONFLICTS = "This pull-request conflicts and thus cannot be merged"

      ENOTASSIGNEE = "Only the assignee can merge this review"

      ENOTASSIGNED = "This request must be assigned to be merged"

      ENOUSER = "No such user found"

file modified
+7 -2

@@ -498,8 +498,13 @@ 

      output = {"message": "Merging queued", "taskid": task.id}

  

      if get_request_data().get("wait", True):

-         task.get()

-         output = {"message": "Changes merged!"}

+         try:

+             task.get()

+             output = {"message": "Changes merged!"}

+         except pagure.exceptions.PagureException:

+             raise pagure.exceptions.APIError(

+                 409, error_code=APIERROR.EPRCONFLICTS

+             )

  

      jsonout = flask.jsonify(output)

      return jsonout

@@ -235,7 +235,7 @@ 

          output = self.app.get('/api/0/-/error_codes')

          self.assertEqual(output.status_code, 200)

          data = json.loads(output.get_data(as_text=True))

-         self.assertEqual(len(data), 33)

+         self.assertEqual(len(data), 34)

          self.assertEqual(

              sorted(data.keys()),

              [

@@ -248,8 +248,9 @@ 

                  u'ENOISSUE', u'ENOPRCLOSE', u'ENOPROJECT', u'ENOPROJECTS',

                  u'ENOPRSTATS', u'ENOREQ', u'ENOSIGNEDOFF', u'ENOTASSIGNED',

                  u'ENOTASSIGNEE', u'ENOTHIGHENOUGH', u'ENOTMAINADMIN',

-                 u'ENOUSER', u'EPRSCORE', u'EPULLREQUESTSDISABLED',

-                 u'ETIMESTAMP', u'ETRACKERDISABLED', u'ETRACKERREADONLY'

+                 u'ENOUSER', u'EPRCONFLICTS', u'EPRSCORE',

+                 u'EPULLREQUESTSDISABLED', u'ETIMESTAMP', u'ETRACKERDISABLED',

+                 u'ETRACKERREADONLY'

              ]

          )

  

@@ -1370,6 +1370,81 @@ 

          )

  

      @patch('pagure.lib.notify.send_email')

+     def test_api_pull_request_merge_conflicting(self, send_email):

+         """ Test the api_pull_request_merge method of the flask api. """

+         send_email.return_value = True

+ 

+         tests.create_projects(self.session)

+         tests.add_content_git_repo(

+             os.path.join(self.path, "repos", "test.git"))

+ 

+         # Fork

+         project = pagure.lib.query.get_authorized_project(

+             self.session, 'test')

+         task = pagure.lib.query.fork_project(

+             session=self.session,

+             user='pingou',

+             repo=project,

+         )

+         self.session.commit()

+         self.assertEqual(

+             task.get(),

+             {'endpoint': 'ui_ns.view_repo',

+              'repo': 'test',

+              'namespace': None,

+              'username': 'pingou'})

+ 

+         # Add content to the fork

+         tests.add_content_to_git(

+             os.path.join(self.path, "repos", "forks", "pingou", "test.git"),

+             filename="foobar", content="content from the fork")

+ 

+         # Add content to the main repo, so they conflict

+         tests.add_content_to_git(

+             os.path.join(self.path, "repos", "test.git"),

+             filename="foobar", content="content from the main repo")

+ 

+         project = pagure.lib.query.get_authorized_project(

+             self.session, 'test')

+         fork = pagure.lib.query.get_authorized_project(

+             self.session,

+             'test',

+             user='pingou',

+         )

+ 

+         tests.create_tokens(self.session)

+         tests.create_tokens_acl(self.session)

+ 

+         # Create the pull-request to close

+         req = pagure.lib.query.new_pull_request(

+             session=self.session,

+             repo_from=fork,

+             branch_from='master',

+             repo_to=project,

+             branch_to='master',

+             title='test pull-request',

+             user='pingou',

+         )

+         self.session.commit()

+         self.assertEqual(req.id, 1)

+         self.assertEqual(req.title, 'test pull-request')

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}

+ 

+         # Merge PR

+         output = self.app.post(

+             '/api/0/test/pull-request/1/merge', headers=headers)

+         self.assertEqual(output.status_code, 409)

+         data = json.loads(output.get_data(as_text=True))

+         self.assertDictEqual(

+             data,

+             {

+                 'error': 'This pull-request conflicts and thus cannot be merged',

+                 'error_code': 'EPRCONFLICTS'

+             }

+         )

+ 

+     @patch('pagure.lib.notify.send_email')

      def test_api_pull_request_merge_user_token(self, send_email):

          """ Test the api_pull_request_merge method of the flask api. """

          send_email.return_value = True

Before this it was returning a 500 error code with HTML, while now it
returns a 409 error code with our usual JSON payload.

Fixes https://pagure.io/pagure/issue/3999

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

Pretty please pagure-ci rebuild

rebased onto 2c0e55f5d9c47f8df21865fe3f17f4d6b08d2dac

4 months ago

rebased onto 457b8ed

4 months ago

Pull-Request has been merged by pingou

4 months ago