From 8821f714086e51297b93a8a9c3482b13ea39e8f5 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Nov 25 2018 20:57:55 +0000
Subject: Allow commenting on a PR when clicking on the merge button


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

---

diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html
index 38c1b0d..ac0afcb 100644
--- a/pagure/templates/repo_pull_request.html
+++ b/pagure/templates/repo_pull_request.html
@@ -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 @@ function set_ui_for_comment(setting){
   }
 }
 
+$("#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();
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py
index 9cdd84b..9f7b053 100644
--- a/pagure/ui/fork.py
+++ b/pagure/ui/fork.py
@@ -1164,6 +1164,24 @@ def merge_request_pull(repo, requestid, username=None, namespace=None):
     _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 @@ def merge_request_pull(repo, requestid, username=None, namespace=None):
                 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")
diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py
index 957672e..311da94 100644
--- a/tests/test_pagure_flask_ui_fork.py
+++ b/tests/test_pagure_flask_ui_fork.py
@@ -746,6 +746,59 @@ class PagureFlaskForktests(tests.Modeltests):
                 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