#5074 Allow updating the target branch when editing a PR
Merged 3 years ago by pingou. Opened 3 years ago by pingou.

file modified
+22
@@ -330,6 +330,28 @@ 

      )

  

  

+ 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. """

  

@@ -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">

file modified
+3 -1
@@ -550,10 +550,11 @@ 

              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 @@ 

      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",

@@ -72,6 +72,7 @@ 

          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 @@ 

              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 @@ 

              data = {

                  "title": "New title",

                  "initial_comment": "New initial comment",

+                 "branch_to": "master",

                  "allow_rebase": False,

              }

              output = self.app.post(
@@ -223,6 +231,7 @@ 

                  "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 @@ 

                  "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 @@ 

              # 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)

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

rebased onto 3fb0e3891aba0dbce7ee3cba906d5e4cbefd35e9

3 years ago

pretty please pagure-ci rebuild

3 years ago

pretty please pagure-ci rebuild

3 years ago

rebased onto d6bd53a6004acbb91264aff9878a90be342ba043

3 years ago

The test failure is due to the new black landing on updates-testing

rebased onto 3aa3662c06e57cc3f8edd0519dec365e066e3d3f

3 years ago

rebased onto 96e5bbc41980b07e917418da5cea39ec578508af

3 years ago

rebased onto 8b8b3e6

3 years ago

pretty please pagure-ci rebuild

3 years ago

I'm going to get this in anyway, py3 rpm and py2 rpm are working it looks like pip is failing because it has a version of pip too old in the tox venv... Not quite sure how to address this tbh

Pull-Request has been merged by pingou

3 years ago