pagure

Clone

#4648 Fix rebase behaviour
Merged 4 years ago by pingou. Opened 4 years ago by jlanda.
jlanda/pagure rebase-tests  into  master

file modified
+11 -5 
@@ -682,15 +682,21 @@ 
 

  

      repo = _get_repo(repo, username, namespace)
 

      _check_pull_request(repo)
 

-     _check_token(repo)
 

+     _check_token(repo, project_token=False)
 

      request = _get_request(repo, requestid)
 

  

-     if not is_repo_committer(repo):
 

-         raise pagure.exceptions.APIError(403, error_code=APIERROR.ENOPRCLOSE)
 

+     can_rebase = (
 

+         not request.remote_git
 

+         and request.project_from
 

+         and is_repo_committer(request.project_from)
 

+     )
 

  

-     if not request.allow_rebase:
 

+     if not ((is_repo_committer(repo) and request.allow_rebase) or can_rebase):
 

          raise pagure.exceptions.APIError(
 

-             403, error_code=APIERROR.EREBASENOTALLOWED
 

+             403,
 

+             error_code=APIERROR.EREBASENOTALLOWED
 

+             if not request.allow_rebase
 

+             else APIERROR.ENOPRCLOSE,
 

          )
 

  

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

file modified
+94 -106 
@@ -135,121 +135,109 @@ 
 

  

        </h4>
 

      <div class="ml-auto d-flex">
 

-         {% if g.authenticated and (g.fas_user.username == pull_request.user.username
 

-           or g.repo_committer) and pull_request.status == 'Open'%}
 

-            {% if pull_request.status == 'Open' and g.authenticated and
 

-               (g.repo_committer or g.fas_user.username == pull_request.user.username) %}
 

-                 {% if mergeform and pull_request.remote %}
 

-                   <form class="inline" action="{{ url_for(
 

-                     'ui_ns.refresh_request_pull',
 

-                     username=repo.user.user if repo.is_fork else None,
 

-                     namespace=repo.namespace,
 

-                     repo=repo.name, requestid=requestid) }}" method="POST">
 

-                       <button type="submit" value="Refresh" id="refresh_pr"
 

-                               class="btn btn-outline-primary btn-sm" title="Refresh the remote pull request">
 

-                         <span class="fa fa-refresh"></span> Refresh
 

-                       </button>
 

-                       {{ mergeform.csrf_token }}
 

-                   </form>
 

-                 {% endif %}
 

- 

+         {% if pull_request.status == 'Open' %}
 

+           {% if g.authenticated and (g.fas_user.username == pull_request.user.username
 

+             or g.repo_committer) %}
 

+             {% if mergeform and pull_request.remote %}
 

                <form class="inline" action="{{ url_for(
 

-                   'ui_ns.close_request_pull',
 

+                   'ui_ns.refresh_request_pull',
 

                    username=repo.user.user if repo.is_fork else None,
 

                    namespace=repo.namespace,
 

                    repo=repo.name, requestid=requestid) }}" method="POST">
 

-            {% endif %}
 

- 

-            {% if pull_request.status == 'Open' and g.authenticated and
 

-               (g.repo_committer or g.fas_user.username == pull_request.user.username) %}
 

+                 <button type="submit" value="Refresh" id="refresh_pr"
 

+                     class="btn btn-outline-primary btn-sm" title="Refresh the remote pull request">
 

+                   <span class="fa fa-refresh"></span> Refresh
 

+                 </button>
 

+                 {{ mergeform.csrf_token }}
 

+               </form>
 

+             {% endif %}
 

+             <form class="inline" action="{{ url_for(
 

+                 'ui_ns.close_request_pull',
 

+                 username=repo.user.user if repo.is_fork else None,
 

+                 namespace=repo.namespace,
 

+                 repo=repo.name, requestid=requestid) }}" method="POST">
 

                {{ mergeform.csrf_token }}
 

-                   <button type="submit" value="Close" id="close_pr"
 

-                           class="btn btn-outline-danger btn-sm" title="Close PR without merging it" data-toggle="tooltip">
 

-                     <span class="fa fa-times"></span> Close
 

-                   </button>
 

-           {% endif %}
 

-           {% if pull_request.status == 'Open' and g.authenticated and
 

-               (g.repo_committer or g.fas_user.username == pull_request.user.username) %}
 

+               <button type="submit" value="Close" id="close_pr"
 

+                   class="btn btn-outline-danger btn-sm" title="Close PR without merging it" data-toggle="tooltip">
 

+                 <span class="fa fa-times"></span> Close
 

+               </button>
 

              </form>
 

            {% endif %}
 

-         {% endif %}
 

-         {% if pull_request.status == 'Open' %}
 

-         <div class="dropdown float-right ml-1">
 

-         <span class="dropdown dropdown-btn">
 

-           <a href="#" id="merge_dropdown_btn"
 

-           class="btn btn-outline-secondary btn-sm disabled dropdown-toggle" data-toggle="dropdown">
 

-             <span class="fa fa-circle-o-notch fa-spin fa-fw"></span> <span id="merge-status-text">Merge</span>
 

-           </a>
 

-           <div id="merge-alert" class="text-xs-center dropdown-menu dropdown-menu-right p-0">
 

-              <div class="alert text-center mb-0">
 

-               {% if pull_request.status == 'Open' %}
 

-                 {% if g.repo_committer %}
 

-                   <small id="merge-alert-message"></small>
 

-                   <form action="{{ url_for('ui_ns.merge_request_pull',
 

-                           repo=repo.name,
 

-                           username=repo.user.user if repo.is_fork else None,
 

-                           namespace=repo.namespace,
 

-                           requestid=requestid)
 

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

-                     {{ mergeform.csrf_token }}
 

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

-                       class="btn btn-block my-2">Merge</button>
 

-                     {% if can_delete_branch %}
 

-                     <div class="small">
 

-                     {{ mergeform.delete_branch }} {{ mergeform.delete_branch.label }}
 

-                     </div>
 

-                     {% endif %}
 

+           <div class="dropdown float-right ml-1">
 

+             <span class="dropdown dropdown-btn">
 

+               <a href="#" id="merge_dropdown_btn"
 

+                   class="btn btn-outline-secondary btn-sm disabled dropdown-toggle" data-toggle="dropdown">
 

+                 <span class="fa fa-circle-o-notch fa-spin fa-fw"></span> <span id="merge-status-text">Merge</span>
 

+               </a>
 

+               <div id="merge-alert" class="text-xs-center dropdown-menu dropdown-menu-right p-0">
 

+                 <div class="alert text-center mb-0">
 

+                   {% if g.repo_committer %}
 

+                     <small id="merge-alert-message"></small>
 

+                     <form action="{{ url_for('ui_ns.merge_request_pull',
 

+                         repo=repo.name,
 

+                         username=repo.user.user if repo.is_fork else None,
 

+                         namespace=repo.namespace,
 

+                         requestid=requestid)
 

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

+                       {{ mergeform.csrf_token }}
 

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

+                           class="btn btn-block my-2">Merge</button>
 

+                       {% if can_delete_branch %}
 

+                         <div class="small">
 

+                           {{ mergeform.delete_branch }} {{ mergeform.delete_branch.label }}
 

+                         </div>
 

+                       {% endif %}
 

+                     </form>
 

+                   {% endif %}
 

+                   {% if g.authenticated and (g.repo_committer and pull_request.allow_rebase) or can_rebase_branch %}
 

+                   <button id="rebase_btn" type="submit"
 

+                     class="btn btn-block my-2">Rebase</button>
 

+                   {% elif g.authenticated and g.repo_committer %}
 

+                   <button id="rebase_btn" disabled
 

+                     title="PR author does not allow rebasing it"
 

+                     class="btn btn-block my-2">Rebase</button>
 

+                   {% endif %}
 

+                 </div>
 

+               </div>
 

+             </span>
 

+             {% if g.authenticated and trigger_ci %}
 

+               <span class="dropdown dropdown-btn">
 

+                 <a href="#" id="ci_dropdown_btn"
 

+                     class="btn btn-outline-primary btn-sm btn-info dropdown-toggle" data-toggle="dropdown">
 

+                   Rerun CI
 

+                 </a>
 

+                 <div class="text-xs-center dropdown-menu dropdown-menu-right p-0">
 

+                   <form action="{{ url_for('ui_ns.ci_trigger_request_pull',
 

+                       repo=repo.name,
 

+                       username=repo.username if repo.is_fork else None,
 

+                       namespace=repo.namespace,
 

+                       requestid=requestid)
 

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

+                     {{ trigger_ci_pr_form.csrf_token }}
 

+                     {{ trigger_ci_pr_form.comment(id="ci_pr_comment", hidden=True) }}
 

+                     {% for comment, meta in trigger_ci|dictsort %}
 

+                       <a class="dropdown-item trigger-ci-btn" href="#" title="{{ meta["description"] }}"
 

+                           data-comment="{{ comment }}">{{ meta["name"] }}</a>
 

+                     {% endfor %}
 

                    </form>
 

-                 {% endif %}
 

-                 {% if pull_request.allow_rebase or can_delete_branch %}
 

-                 <button id="rebase_btn" type="submit"
 

-                   class="btn btn-block my-2">Rebase</button>
 

-                 {% endif %}
 

-               {% else %}
 

-               <small id="merge-alert-message"></small>
 

-               {% endif %}
 

-             </div>
 

-           </div>
 

-         </span>
 

-         {% if g.authenticated and trigger_ci %}
 

-         <span class="dropdown dropdown-btn">
 

-           <a href="#" id="ci_dropdown_btn"
 

-           class="btn btn-outline-primary btn-sm btn-info dropdown-toggle" data-toggle="dropdown">
 

-             Rerun CI
 

-           </a>
 

-           <div class="text-xs-center dropdown-menu dropdown-menu-right p-0">
 

-             <form action="{{ url_for('ui_ns.ci_trigger_request_pull',
 

-                     repo=repo.name,
 

-                     username=repo.username if repo.is_fork else None,
 

-                     namespace=repo.namespace,
 

-                     requestid=requestid)
 

-               }}" method="POST" id="ci_pr_trigger_form">
 

-               {{ trigger_ci_pr_form.csrf_token }}
 

-               {{ trigger_ci_pr_form.comment(id="ci_pr_comment", hidden=True) }}
 

-                 {% for comment, meta in trigger_ci|dictsort %}
 

-                   <a class="dropdown-item trigger-ci-btn" href="#" title="{{ meta["description"] }}"
 

-                   data-comment="{{ comment }}">{{ meta["name"] }}</a>
 

-                 {% endfor %}
 

-             </form>
 

+                 </div>
 

+               </span>
 

+           {% endif %}
 

            </div>
 

-         </span>
 

-         {% endif %}
 

-       </div>
 

-       {% endif %}
 

-       {% if pull_request.status == 'Closed' and g.authenticated and
 

-       (g.repo_committer or g.fas_user.username == pull_request.user.username) %}
 

-       <form action="{{ url_for(
 

-             'ui_ns.reopen_request_pull',
 

-             username=repo.user.user if repo.is_fork else None,
 

-             namespace=repo.namespace,
 

-             repo=repo.name, requestid=requestid) }}" method="POST">
 

-       {{ mergeform.csrf_token }}
 

-       <button type="submit" value="Reopen" id="reopen_pr"
 

-               class="btn btn-sm btn-outline-danger" title="Reopen PR">
 

-         Reopen Pull Request
 

-       </button>
 

-       </form>
 

-    {% endif %}
 

+         {% elif pull_request.status == 'Closed' and g.authenticated and
 

+           (g.repo_committer or g.fas_user.username == pull_request.user.username) %}
 

+           <form action="{{ url_for(
 

+               'ui_ns.reopen_request_pull',
 

+               username=repo.user.user if repo.is_fork else None,
 

+               namespace=repo.namespace,
 

+               repo=repo.name, requestid=requestid) }}" method="POST">
 

+               {{ mergeform.csrf_token }}
 

+             <button type="submit" value="Reopen" id="reopen_pr"
 

+                 class="btn btn-sm btn-outline-danger" title="Reopen PR">
 

+               Reopen Pull Request
 

+             </button>
 

+           </form>
 

+        {% endif %}
 

      </div>
 

    </div>

file modified
+7 -3 
@@ -341,12 +341,15 @@ 
 

              continue
 

          trigger_ci[comment] = meta
 

  

-     can_delete_branch = (
 

-         pagure_config.get("ALLOW_DELETE_BRANCH", True)
 

-         and not request.remote_git
 

+     can_rebase_branch = (
 

+         not request.remote_git
 

          and request.project_from
 

          and pagure.utils.is_repo_committer(request.project_from)
 

      )
 

+ 

+     can_delete_branch = (
 

+         pagure_config.get("ALLOW_DELETE_BRANCH", True) and can_rebase_branch
 

+     )
 

      return flask.render_template(
 

          "repo_pull_request.html",
 

          select="requests",
 
@@ -360,6 +363,7 @@ 
 

          mergeform=form,
 

          subscribers=pagure.lib.query.get_watch_list(flask.g.session, request),
 

          tag_list=pagure.lib.query.get_tags_of_project(flask.g.session, repo),
 

+         can_rebase_branch=can_rebase_branch,
 

          can_delete_branch=can_delete_branch,
 

          trigger_ci=trigger_ci,
 

          trigger_ci_pr_form=trigger_ci_pr_form,

file modified
+292 -15 
@@ -50,24 +50,43 @@ 
 

              content="foobarbaz",
 

              filename="testfile",
 

          )
 

+         project = pagure.lib.query.get_authorized_project(self.session, "test")
 

+         # Fork the project
 

+         task = pagure.lib.query.fork_project(
 

+             session=self.session, user="foo", repo=project
 

+         )
 

+         self.session.commit()
 

+         self.assertEqual(
 

+             task.get(),
 

+             {
 

+                 "endpoint": "ui_ns.view_repo",
 

+                 "repo": "test",
 

+                 "username": "foo",
 

+                 "namespace": None,
 

+             },
 

+         )
 

          tests.add_content_to_git(
 

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

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

              branch="test",
 

              content="foobar",
 

              filename="sources",
 

          )
 

+         fork_repo = pagure.lib.query.get_authorized_project(
 

+             self.session, "test", user="foo"
 

+         )
 

+ 

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

  

          # Create a PR for these changes
 

-         project = pagure.lib.query.get_authorized_project(self.session, "test")
 

+ 

          req = pagure.lib.query.new_pull_request(
 

              session=self.session,
 

-             repo_from=project,
 

+             repo_from=fork_repo,
 

              branch_from="test",
 

              repo_to=project,
 

              branch_to="master",
 

              title="PR from the test branch",
 

-             user="pingou",
 

+             user="foo",
 

              allow_rebase=True,
 

          )
 

          self.session.commit()
 
@@ -208,21 +227,82 @@ 
 

              output_text = output.get_data(as_text=True)
 

              self.assertIn("rebased onto", output_text)
 

              repo = pagure.lib.query._get_project(self.session, "test")
 

-             self.assertEqual(
 

-                 repo.requests[0].comments[0].user.username, "pingou"
 

-             )
 

+             # This should be pingou, but we have some bug that adds the
 

+             # rebase message as PR author instead of rebaser
 

+             self.assertEqual(repo.requests[0].comments[0].user.username, "foo")
 

  

      def test_rebase_api_ui_logged_in_different_user(self):
 

          """ Test the rebase PR API endpoint when logged in from the UI and
 

          its outcome. """
 

-         # Add 'foo' to the project 'test' so 'foo' can rebase the PR
 

+         # Add 'bar' to the project 'test' so 'bar' can rebase the PR
 

+         item = pagure.lib.model.User(
 

+             user="bar",
 

+             fullname="bar foo",
 

+             password=b"foo",
 

+             default_email="bar@foo.com",
 

+         )
 

+         self.session.add(item)
 

+         item = pagure.lib.model.UserEmail(user_id=2, email="bar@foo.com")
 

+         self.session.add(item)
 

+ 

+         self.session.commit()
 

          repo = pagure.lib.query._get_project(self.session, "test")
 

          msg = pagure.lib.query.add_user_to_project(
 

-             session=self.session, project=repo, new_user="foo", user="pingou"
 

+             session=self.session, project=repo, new_user="bar", user="pingou"
 

          )
 

          self.session.commit()
 

          self.assertEqual(msg, "User added")
 

  

+         user = tests.FakeUser(username="bar")
 

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

+             # Get the merge status first so it's cached and can be refreshed
 

+             csrf_token = self.get_csrf()
 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "MERGE",
 

+                     "message": "The pull-request can be merged with "
 

+                     "a merge commit",
 

+                     "short_code": "With merge",
 

+                 },
 

+             )
 

+ 

+             output = self.app.post("/api/0/test/pull-request/1/rebase")
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(data, {"message": "Pull-request rebased"})
 

+ 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "FFORWARD",
 

+                     "message": "The pull-request can be merged and "
 

+                     "fast-forwarded",
 

+                     "short_code": "Ok",
 

+                 },
 

+             )
 

+ 

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

+             self.assertEqual(output.status_code, 200)
 

+             output_text = output.get_data(as_text=True)
 

+             self.assertIn("rebased onto", output_text)
 

+             repo = pagure.lib.query._get_project(self.session, "test")
 

+             # This should be bar, but we have some bug that adds the
 

+             # rebase message as PR author instead of rebaser
 

+             self.assertEqual(repo.requests[0].comments[0].user.username, "foo")
 

+ 

+     def test_rebase_api_ui_logged_in_pull_request_author(self):
 

+         """ Test the rebase PR API endpoint when logged in from the UI and
 

+         its outcome. """
 

+ 

          user = tests.FakeUser(username="foo")
 

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

              # Get the merge status first so it's cached and can be refreshed
 
@@ -423,24 +503,43 @@ 
 

              content="foobarbaz",
 

              filename="testfile",
 

          )
 

+         project = pagure.lib.query.get_authorized_project(self.session, "test")
 

+         # Fork the project
 

+         task = pagure.lib.query.fork_project(
 

+             session=self.session, user="foo", repo=project
 

+         )
 

+         self.session.commit()
 

+         self.assertEqual(
 

+             task.get(),
 

+             {
 

+                 "endpoint": "ui_ns.view_repo",
 

+                 "repo": "test",
 

+                 "username": "foo",
 

+                 "namespace": None,
 

+             },
 

+         )
 

          tests.add_content_to_git(
 

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

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

              branch="test",
 

              content="foobar",
 

              filename="sources",
 

          )
 

+         fork_repo = pagure.lib.query.get_authorized_project(
 

+             self.session, "test", user="foo"
 

+         )
 

+ 

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

  

          # Create a PR for these changes
 

          project = pagure.lib.query.get_authorized_project(self.session, "test")
 

          req = pagure.lib.query.new_pull_request(
 

              session=self.session,
 

-             repo_from=project,
 

+             repo_from=fork_repo,
 

              branch_from="test",
 

              repo_to=project,
 

              branch_to="master",
 

              title="PR from the test branch",
 

-             user="pingou",
 

+             user="foo",
 

              allow_rebase=False,
 

          )
 

          self.session.commit()
 
@@ -486,18 +585,71 @@ 
 

                  },
 

              )
 

  

+             # Add pingou to fork repo so he can rebase while allow_rebase = False
 

+             fork = pagure.lib.query.get_authorized_project(
 

+                 self.session, "test", user="foo"
 

+             )
 

+             msg = pagure.lib.query.add_user_to_project(
 

+                 session=self.session,
 

+                 project=fork,
 

+                 new_user="pingou",
 

+                 user="foo",
 

+             )
 

+             self.session.commit()
 

+             self.assertEqual(msg, "User added")
 

+ 

+             output = self.app.post("/api/0/test/pull-request/1/rebase")
 

+             self.assertEqual(output.status_code, 200)
 

+ 

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

+             self.assertEqual(data, {"message": "Pull-request rebased"})
 

+ 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "FFORWARD",
 

+                     "message": "The pull-request can be merged and "
 

+                     "fast-forwarded",
 

+                     "short_code": "Ok",
 

+                 },
 

+             )
 

+ 

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

+             self.assertEqual(output.status_code, 200)
 

+             output_text = output.get_data(as_text=True)
 

+             self.assertIn("rebased onto", output_text)
 

+             repo = pagure.lib.query._get_project(self.session, "test")
 

+             # This should be pingou, but we have some bug that adds the
 

+             # rebase message as PR author instead of rebaser
 

+             self.assertEqual(repo.requests[0].comments[0].user.username, "foo")
 

+ 

      def test_rebase_api_ui_logged_in_different_user(self):
 

          """ Test the rebase PR API endpoint when logged in from the UI and
 

          its outcome. """
 

-         # Add 'foo' to the project 'test' so 'foo' can rebase the PR
 

+         # Add 'bar' to the project 'test' so 'bar' can rebase the PR
 

+         item = pagure.lib.model.User(
 

+             user="bar",
 

+             fullname="bar foo",
 

+             password=b"foo",
 

+             default_email="bar@foo.com",
 

+         )
 

+         self.session.add(item)
 

+         item = pagure.lib.model.UserEmail(user_id=2, email="bar@foo.com")
 

+         self.session.add(item)
 

+ 

+         self.session.commit()
 

          repo = pagure.lib.query._get_project(self.session, "test")
 

          msg = pagure.lib.query.add_user_to_project(
 

-             session=self.session, project=repo, new_user="foo", user="pingou"
 

+             session=self.session, project=repo, new_user="bar", user="pingou"
 

          )
 

          self.session.commit()
 

          self.assertEqual(msg, "User added")
 

  

-         user = tests.FakeUser(username="foo")
 

+         user = tests.FakeUser(username="bar")
 

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

              # Get the merge status first so it's cached and can be refreshed
 

              csrf_token = self.get_csrf()
 
@@ -526,6 +678,45 @@ 
 

                  },
 

              )
 

  

+             # Add bar to fork repo so he can rebase while allow_rebase = False
 

+             fork = pagure.lib.query.get_authorized_project(
 

+                 self.session, "test", user="foo"
 

+             )
 

+             msg = pagure.lib.query.add_user_to_project(
 

+                 session=self.session, project=fork, new_user="bar", user="foo"
 

+             )
 

+             self.session.commit()
 

+             self.assertEqual(msg, "User added")
 

+ 

+             output = self.app.post("/api/0/test/pull-request/1/rebase")
 

+             self.assertEqual(output.status_code, 200)
 

+ 

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

+             self.assertEqual(data, {"message": "Pull-request rebased"})
 

+ 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "FFORWARD",
 

+                     "message": "The pull-request can be merged and "
 

+                     "fast-forwarded",
 

+                     "short_code": "Ok",
 

+                 },
 

+             )
 

+ 

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

+             self.assertEqual(output.status_code, 200)
 

+             output_text = output.get_data(as_text=True)
 

+             self.assertIn("rebased onto", output_text)
 

+             repo = pagure.lib.query._get_project(self.session, "test")
 

+             # This should be bar, but we have some bug that adds the
 

+             # rebase message as PR author instead of rebaser
 

+             self.assertEqual(repo.requests[0].comments[0].user.username, "foo")
 

+ 

      def test_rebase_api_api_logged_in(self):
 

          """ Test the rebase PR API endpoint when using an API token and
 

          its outcome. """
 
@@ -548,6 +739,92 @@ 
 

              },
 

          )
 

  

+         # Add pingou to fork repo so he can rebase while allow_rebase = False
 

+         fork = pagure.lib.query.get_authorized_project(
 

+             self.session, "test", user="foo"
 

+         )
 

+         msg = pagure.lib.query.add_user_to_project(
 

+             session=self.session, project=fork, new_user="pingou", user="foo"
 

+         )
 

+         self.session.commit()
 

+         self.assertEqual(msg, "User added")
 

+ 

+         output = self.app.post(
 

+             "/api/0/test/pull-request/1/rebase", headers=headers
 

+         )
 

+         self.assertEqual(output.status_code, 200)
 

+ 

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

+         self.assertEqual(data, {"message": "Pull-request rebased"})
 

+ 

+         user = tests.FakeUser(username="pingou")
 

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

+ 

+             data = {
 

+                 "requestid": self.request.uid,
 

+                 "csrf_token": self.get_csrf(),
 

+             }
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "FFORWARD",
 

+                     "message": "The pull-request can be merged and "
 

+                     "fast-forwarded",
 

+                     "short_code": "Ok",
 

+                 },
 

+             )
 

+ 

+     def test_rebase_api_ui_logged_in_pull_request_author(self):
 

+         """ Test the rebase PR API endpoint when logged in from the UI and
 

+         its outcome. """
 

+ 

+         user = tests.FakeUser(username="foo")
 

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

+             # Get the merge status first so it's cached and can be refreshed
 

+             csrf_token = self.get_csrf()
 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "MERGE",
 

+                     "message": "The pull-request can be merged with "
 

+                     "a merge commit",
 

+                     "short_code": "With merge",
 

+                 },
 

+             )
 

+ 

+             output = self.app.post("/api/0/test/pull-request/1/rebase")
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(data, {"message": "Pull-request rebased"})
 

+ 

+             data = {"requestid": self.request.uid, "csrf_token": csrf_token}
 

+             output = self.app.post("/pv/pull-request/merge", data=data)
 

+             self.assertEqual(output.status_code, 200)
 

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

+             self.assertEqual(
 

+                 data,
 

+                 {
 

+                     "code": "FFORWARD",
 

+                     "message": "The pull-request can be merged and "
 

+                     "fast-forwarded",
 

+                     "short_code": "Ok",
 

+                 },
 

+             )
 

+ 

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

+             self.assertEqual(output.status_code, 200)
 

+             output_text = output.get_data(as_text=True)
 

+             self.assertIn("rebased onto", output_text)
 

+             repo = pagure.lib.query._get_project(self.session, "test")
 

+             self.assertEqual(repo.requests[0].comments[0].user.username, "foo")
 

+ 

  

  if __name__ == "__main__":
 

      unittest.main(verbosity=2)

Some fixes for rebase

1 new commit added

  • pagure/templates/repo_pull_request: remove double identical jinga2 {%if } block
4 years ago

2 new commits added

  • templates/repo_pull_request.html: simplify jinga2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

2 new commits added

  • templates/repo_pull_request.html: simplify jinja2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

rebased onto 6bec4c1b46929b98762a3c142553640083a774b9
4 years ago

6bec4c1 : actually I'm not sure if this is correct, can a user rebase a branch on a instance that does not allow removing them? tbh, no idea :)

35e711e: Current templating code is quite unreadable due to nested ifs that duplicate conditionals: https://pagure.io/pagure/blob/master/f/pagure/templates/repo_pull_request.html#_138-141 so this reworks that part of the template to be clean and readable. Any html output change is unintentional except the removing of an <small> block inside an impossible {% else %} block
Edited 4 years ago by jlanda

1 new commit added

  • pagure/api/fork.py: allow rebasing with non project tokens
4 years ago

b838981: right now rebase api endpoint is limited to project tokens, but on frontend we have an actor (those who are not upstream repo committers but have enough perms to rebase the fork) that can not have a project specific token for the upstream repo, so allow everyone to use global tokens to rebase

pretty please pagure-ci rebuild
4 years ago

rebased onto af38c8135daf163c395635c9a9bdb37812b63c5e
4 years ago

1 new commit added

  • api/fork: fix authorization conditionals
4 years ago

1 new commit added

  • tests/tests_pagure_flask_rebase: rename and fix test suite to align
4 years ago

After talking with @pingou we think that when allow_rebase is set to false, those who have enough rights to rebase branches on pull request's origin project should be allowed to rebase from ui since they can do it via git. 84ce6f1 fixes this.

With this modified scenario, our current allow_rebase = false test scenario is allowed to rebase since the test users are also committers on pull_request.project_from . efd15b7 modifies this.

Now we just need tests for the denied cases :)
Edited 4 years ago by jlanda

5 new commits added

  • tests/tests_pagure_flask_rebase: rename and fix test suite to align
  • api/fork: fix authorization conditionals
  • pagure/api/fork.py: allow rebasing with non project tokens
  • templates/repo_pull_request.html: simplify jinja2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

1 new commit added

  • templates/repo_pull_request.html UX: add a disabled button to committers when
4 years ago

I think I finished the refactoring|fixing process here. Any thoughts? I'll continue with tests asap

1 new commit added

  • tests/test_pagure_flask_rebase: leverage test suit to use a fork
4 years ago

7 new commits added

  • tests/test_pagure_flask_rebase: leverage test suit to use a fork when allow_rebase = True
  • templates/repo_pull_request.html UX: add a disabled button to committers when
  • tests/tests_pagure_flask_rebase: rename and fix test suite to align
  • api/fork: fix authorization conditionals
  • pagure/api/fork.py: allow rebasing with non project tokens
  • templates/repo_pull_request.html: simplify jinja2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

This change raised a different rebase bug :S

Now the allow_rebase = True test case is using a fork repo instead of pull requesting from one branch to other on the same repo.

This allows us to test the case where the pull_request author can rebase the pull request without being committer on the destination repo.

We need to leverage the allow_rebase = False case on the same way so we can test all the rebase button use cases
Edited 4 years ago by jlanda

7 new commits added

  • tests/test_pagure_flask_rebase: leverage test suit to use a fork when allow_rebase = True
  • templates/repo_pull_request.html UX: add a disabled button to committers when
  • tests/tests_pagure_flask_rebase: rename and fix test suite to align
  • api/fork: fix authorization conditionals
  • pagure/api/fork.py: allow rebasing with non project tokens
  • templates/repo_pull_request.html: simplify jinja2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

6 new commits added

  • tests/tests_pagure_flask_rebase: align to code and add more test cases
  • templates/repo_pull_request.html UX: add a disabled button to committers when
  • api/fork: fix authorization conditionals
  • pagure/api/fork.py: allow rebasing with non project tokens
  • templates/repo_pull_request.html: simplify jinja2 conditionals
  • small modifications to pull request rebase button showing logic:
4 years ago

This is ready for review

pretty please pagure-ci rebuild
4 years ago

Broken pipeline

pretty please pagure-ci rebuild
4 years ago

psutil and pip-30 container thing seems fixed, let rerun the pipeline

rebased onto 45cb749
4 years ago

I've reviewed the changes commit by commit because doing them all at once it a pretty big diff ^_^.

This is looking good. Let's give it a final run of CI and get it in :)

Pull-Request has been merged by pingou
4 years ago
Metadata
None
No Tags
Flags
jenkins
success (100%)
Build #2944 successful (commit: b7612196)
4 years ago
jenkins
success (100%)
Build #2941 successful (commit: ff749206)
4 years ago
jenkins
failure
Build #2935 failed (commit: ff749206)
4 years ago
jenkins
failure
Build #2934 failed (commit: ff749206)
4 years ago
jenkins
success (100%)
Build #2933 successful (commit: 83d79faf)
4 years ago
jenkins
failure
Build #2932 failed (commit: 83d79faf)
4 years ago
jenkins
failure
Build #2931 failed (commit: 98fefe40)
4 years ago
jenkins
success (100%)
Build #2930 successful (commit: 25868457)
4 years ago
jenkins
success (100%)
Build #2929 successful (commit: efd15b7d)
4 years ago
jenkins
failure
Build #2928 failed (commit: ba2bfc80)
4 years ago
jenkins
failure
Build #2927 failed (commit: 84ce6f1e)
4 years ago
jenkins
success (100%)
Build #2926 successful (commit: 13cb9259)
4 years ago
jenkins
failure
Build #2923 failed (commit: b8389811)
4 years ago
jenkins
failure
Build #2922 failed (commit: b8389811)
4 years ago
jenkins
success (100%)
Build #2921 successful (commit: 35e711ea)
4 years ago
jenkins
failure
Build #2920 failed (commit: ced5dd98)
4 years ago
jenkins
failure
Build #2919 failed (commit: ced5dd98)
4 years ago
jenkins
failure
Build #2918 failed (commit: fd9d528c)
4 years ago
jenkins
failure
Build #2917 failed (commit: fd9d528c)
4 years ago
Changes Summary 4
+11 -5
file changed
pagure/api/fork.py
+94 -106
file changed
pagure/templates/repo_pull_request.html
+7 -3
file changed
pagure/ui/fork.py
+292 -15
file changed
tests/test_pagure_flask_rebase.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.