#4035 Allow commenting on a PR when clicking on the merge button
Merged 5 years ago by pingou. Opened 5 years ago by pingou.

@@ -184,7 +184,7 @@ 

                          username=username,

                          namespace=repo.namespace,

                          requestid=requestid)

-                   }}" method="POST">

+                   }}" method="POST"  id="merge_pr_form">

                    {{ mergeform.csrf_token }}

                    <button id="merge_btn" type="submit"

                      onclick="return confirm('Confirm merging this pull-request');"
@@ -1127,6 +1127,18 @@ 

    }

  }

  

+ $("#merge_pr_form").submit( function() {

+   set_ui_for_comment(true);

+   var _c = $("#comment");

+   if (_c.val()) {

+     $('<input />').attr('type', 'hidden')

+       .attr('name', "comment")

+       .attr('value', _c.val())

+       .appendTo(this);

+   }

+   return true;

+ });

+ 

  function try_async_comment(form, inline) {

    set_ui_for_comment(true);

    var _data = $(form).serialize();

file modified
+31
@@ -1164,6 +1164,24 @@ 

      _log.info("All checks in the controller passed")

  

      try:

+         if flask.request.form.get('comment'):

+             trigger_ci = pagure_config["TRIGGER_CI"]

+             if isinstance(trigger_ci, dict):

+                 trigger_ci = list(trigger_ci.keys())

+             message = pagure.lib.query.add_pull_request_comment(

+                 flask.g.session,

+                 request=request,

+                 commit=None,

+                 tree_id=None,

+                 filename=None,

+                 row=None,

+                 comment=flask.request.form.get('comment'),

+                 user=flask.g.fas_user.username,

+                 trigger_ci=trigger_ci,

+             )

+             flask.g.session.commit()

+             flask.flash(message)

+ 

          task = pagure.lib.tasks.merge_pull_request.delay(

              repo.name,

              namespace,
@@ -1182,6 +1200,19 @@ 

                  requestid=requestid,

              ),

          )

+     except SQLAlchemyError as err:  # pragma: no cover

+         flask.g.session.rollback()

+         _log.exception(err)

+         flask.flash(str(err), "error")

+         return flask.redirect(

+             flask.url_for(

+                 "ui_ns.request_pull",

+                 repo=repo.name,

+                 requestid=requestid,

+                 username=username,

+                 namespace=namespace,

+             )

+         )

      except pygit2.GitError as err:

          _log.info("GitError exception raised")

          flask.flash("%s" % err, "error")

@@ -746,6 +746,59 @@ 

                  output.get_data(as_text=True))

  

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

+     def test_merge_request_pull_merge_with_comment(self, send_email):

+         """ Test the merge_request_pull endpoint with a merge PR. """

+         send_email.return_value = True

+ 

+         tests.create_projects(self.session)

+         tests.create_projects_git(

+             os.path.join(self.path, 'requests'), bare=True)

+         set_up_git_repo(

+             self.session, self.path, new_project=None,

+             branch_from='feature', mtype='merge')

+ 

+         self.session = pagure.lib.query.create_session(self.dbpath)

+         request = pagure.lib.query.search_pull_requests(

+             self.session, project_id=1, requestid=1)

+         self.assertEqual(len(request.comments), 0)

+ 

+         user = tests.FakeUser()

+         user.username = 'pingou'

+         with tests.user_set(self.app.application, user):

+             output = self.app.get('/test/pull-request/1')

+             self.assertEqual(output.status_code, 200)

+ 

+             csrf_token = self.get_csrf(output=output)

+ 

+             data = {

+                 'csrf_token': csrf_token,

+                 'comment': 'Thanks for the review and the suggestions!'

+             }

+ 

+             # Merge

+             output = self.app.post(

+                 '/test/pull-request/1/merge', data=data, follow_redirects=True)

+             self.assertEqual(output.status_code, 200)

+             self.assertIn(

+                 '<title>Overview - test - Pagure</title>', output.get_data(as_text=True))

+ 

+             # Check if the closing notification was added

+             output = self.app.get('/test/pull-request/1')

+             output_text = output.get_data(as_text=True)

+             self.assertIn(

+                 '<span class="text-info font-weight-bold">Merged</span> just now\n'

+                 '            </span>\n            by\n'

+                 '            <span title="PY C (pingou)">pingou.</span>\n',

+                 output_text)

+             self.assertIn(

+                 'Thanks for the review and the suggestions!', output_text)

+ 

+             self.session = pagure.lib.query.create_session(self.dbpath)

+             request = pagure.lib.query.search_pull_requests(

+                 self.session, project_id=1, requestid=1)

+             self.assertEqual(len(request.comments), 2)

+ 

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

      def test_merge_request_pull_merge_with_delete_branch(self, send_email):

          """ Test the merge_request_pull endpoint with a merge PR and delete source branch. """

          send_email.return_value = True