From 8b8b3e6a20ba902d7b3695286131519958e9a0d5 Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jan 06 2021 16:07:48 +0000
Subject: Allow updating the target branch when editing a PR


With this commit we will be able to update a PR to change its target
branch in cases that branch disappears.
For example, you have a PR opened against the "master" branch but the
project decides to change its default branch to "main" and delete the
"master" branch to avoid confusion.
Now, the PR that was opened against "master" will error saying it cannot
find the target branch for the PR (and the diff of the PR will thus be
empty). With this commit, one will be able to edit the PR and change
the target branch from "master" to "main", thus allowing the review to
continue.

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

---

diff --git a/pagure/forms.py b/pagure/forms.py
index a7ff9d9..458a74a 100644
--- a/pagure/forms.py
+++ b/pagure/forms.py
@@ -330,6 +330,28 @@ class RequestPullForm(PagureForm):
     )
 
 
+class RequestPullEditForm(RequestPullForm):
+    """ Form to edit a pull request. """
+
+    branch_to = wtforms.SelectField(
+        "Target branch",
+        [wtforms.validators.Required()],
+        choices=[],
+        coerce=convert_value,
+    )
+
+    def __init__(self, *args, **kwargs):
+        """Calls the default constructor with the normal argument but
+        uses the list of collection provided to fill the choices of the
+        drop-down list.
+        """
+        super(RequestPullEditForm, self).__init__(*args, **kwargs)
+        if "branches" in kwargs:
+            self.branch_to.choices = [
+                (branch, branch) for branch in kwargs["branches"]
+            ]
+
+
 class RemoteRequestPullForm(RequestPullForm):
     """ Form to create a remote pull request. """
 
diff --git a/pagure/templates/pull_request_title.html b/pagure/templates/pull_request_title.html
index ecbfce7..817b77a 100644
--- a/pagure/templates/pull_request_title.html
+++ b/pagure/templates/pull_request_title.html
@@ -51,6 +51,9 @@
               <div id="preview" class="p-1">
               </div>
             </fieldset>
+            {{ render_bootstrap_field(
+              form.branch_to,
+              field_description="branch in which the pull-request should be merged") }}
             <div class="form-control">
               <label for="allow_rebase">Allow rebasing</label>
               <label class="c-input c-checkbox">
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py
index 670b75e..e4613d2 100644
--- a/pagure/ui/fork.py
+++ b/pagure/ui/fork.py
@@ -550,10 +550,11 @@ def request_pull_edit(repo, requestid, username=None, namespace=None):
             403, description="You are not allowed to edit this pull-request"
         )
 
-    form = pagure.forms.RequestPullForm()
+    form = pagure.forms.RequestPullEditForm(branches=flask.g.branches)
     if form.validate_on_submit():
         request.title = form.title.data.strip()
         request.initial_comment = form.initial_comment.data.strip()
+        request.branch = form.branch_to.data.strip()
         if flask.g.fas_user.username == request.user.username:
             request.allow_rebase = form.allow_rebase.data
         flask.g.session.add(request)
@@ -594,6 +595,7 @@ def request_pull_edit(repo, requestid, username=None, namespace=None):
     elif flask.request.method == "GET":
         form.title.data = request.title
         form.initial_comment.data = request.initial_comment
+        form.branch_to.data = request.branch
 
     return flask.render_template(
         "pull_request_title.html",
diff --git a/tests/test_pagure_flask_ui_pr_edit.py b/tests/test_pagure_flask_ui_pr_edit.py
index f3683e6..0baa541 100644
--- a/tests/test_pagure_flask_ui_pr_edit.py
+++ b/tests/test_pagure_flask_ui_pr_edit.py
@@ -72,6 +72,7 @@ class PagureFlaskPrEdittests(tests.Modeltests):
         super(PagureFlaskPrEdittests, self).setUp()
         tests.create_projects(self.session)
         tests.create_projects_git(os.path.join(self.path, "repos"), bare=True)
+
         # Create foo's fork of pingou's test project
         item = pagure.lib.model.Project(
             user_id=2,  # foo
@@ -100,6 +101,12 @@ class PagureFlaskPrEdittests(tests.Modeltests):
             allow_rebase=True,
         )
 
+        # Create a "main" branch in addition to the default "master" one
+        repo_obj = pygit2.Repository(
+            os.path.join(self.path, "repos", "test.git")
+        )
+        repo_obj.branches.local.create("main", repo_obj.head.peel())
+
     def tearDown(self):
         try:
             tests.clean_pull_requests_path()
@@ -168,6 +175,7 @@ class PagureFlaskPrEdittests(tests.Modeltests):
             data = {
                 "title": "New title",
                 "initial_comment": "New initial comment",
+                "branch_to": "master",
                 "allow_rebase": False,
             }
             output = self.app.post(
@@ -223,6 +231,7 @@ class PagureFlaskPrEdittests(tests.Modeltests):
                 "title": "New title",
                 "initial_comment": "New initial comment",
                 "allow_rebase": False,
+                "branch_to": "master",
                 "csrf_token": self.get_csrf(),
             }
             with testing.mock_sends(
@@ -492,6 +501,74 @@ class PagureFlaskPrEdittests(tests.Modeltests):
                 "title": "New title",
                 "initial_comment": "New initial comment",
                 "allow_rebase": False,
+                "branch_to": "master",
+                "csrf_token": self.get_csrf(),
+            }
+            output = self.app.post(
+                "/test/pull-request/1/edit", data=data, follow_redirects=True
+            )
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+            # After successful edit, we end on pull_request view with new data
+            self.assertIn(
+                "<title>PR#1: New title - test\n - Pagure</title>", output_text
+            )
+            self.assertIn(
+                '<span class="font-weight-bold">\n'
+                "                  New title\n"
+                "            </span>",
+                output_text,
+            )
+            self.assertIn("<p>New initial comment</p>", output_text)
+            request = pagure.lib.query.search_pull_requests(
+                self.session, project_id=1, requestid=1
+            )
+            # DB model has been changed
+            self.assertEqual("New title", request.title)
+            self.assertEqual("New initial comment", request.initial_comment)
+            # But allow_rebase remains unchanged
+            self.assertEqual(True, request.allow_rebase)
+
+    def test_pr_edit_pull_request_invalid_branch_to(self):
+        user = tests.FakeUser(username="pingou")
+        with tests.user_set(self.app.application, user):
+            data = {
+                "title": "New title",
+                "initial_comment": "New initial comment",
+                "allow_rebase": False,
+                "branch_to": "invalid",
+                "csrf_token": self.get_csrf(),
+            }
+            output = self.app.post(
+                "/test/pull-request/1/edit", data=data, follow_redirects=True
+            )
+            self.assertEqual(output.status_code, 200)
+            output_text = output.get_data(as_text=True)
+            # Edit failed - we're back on the same page
+            self.assertIn(
+                "<title>Edit PR#1: PR from the feature branch - test - Pagure"
+                "</title>",
+                output_text,
+            )
+            self.assertIn("Not a valid choice", output_text)
+            request = pagure.lib.query.search_pull_requests(
+                self.session, project_id=1, requestid=1
+            )
+            # DB model has not been changed
+            self.assertEqual("PR from the feature branch", request.title)
+            self.assertEqual(None, request.initial_comment)
+            self.assertEqual("master", request.branch)
+            # But allow_rebase remains unchanged
+            self.assertEqual(True, request.allow_rebase)
+
+    def test_pr_edit_pull_request_valid_branch_to(self):
+        user = tests.FakeUser(username="pingou")
+        with tests.user_set(self.app.application, user):
+            data = {
+                "title": "New title",
+                "initial_comment": "New initial comment",
+                "allow_rebase": False,
+                "branch_to": "main",
                 "csrf_token": self.get_csrf(),
             }
             output = self.app.post(
@@ -516,5 +593,6 @@ class PagureFlaskPrEdittests(tests.Modeltests):
             # DB model has been changed
             self.assertEqual("New title", request.title)
             self.assertEqual("New initial comment", request.initial_comment)
+            self.assertEqual("main", request.branch)
             # But allow_rebase remains unchanged
             self.assertEqual(True, request.allow_rebase)