From cd52b806ce673db066300c0c38097a9549f9e425 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Sep 25 2019 20:49:55 +0000 Subject: [PATCH 1/4] Allow changing allow_rebase from pull-request edit --- diff --git a/pagure/templates/pull_request_title.html b/pagure/templates/pull_request_title.html index 9d7ee45..46bc952 100644 --- a/pagure/templates/pull_request_title.html +++ b/pagure/templates/pull_request_title.html @@ -51,7 +51,16 @@
- +
+ + + + Let the maintainer of the target project to rebase the pull-request + +

diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 6f1ecce..ca3e25a 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -538,6 +538,8 @@ def request_pull_edit(repo, requestid, username=None, namespace=None): if form.validate_on_submit(): request.title = form.title.data.strip() request.initial_comment = form.initial_comment.data.strip() + if flask.g.fas_user.username == request.user.username: + request.allow_rebase = form.allow_rebase.data flask.g.session.add(request) if not request.private and not request.project.private: pagure.lib.notify.log( From b28d02a05627b8bee57256b9a8a20a31f8d8c739 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Sep 25 2019 20:49:55 +0000 Subject: [PATCH 2/4] Edit pull request form: don't print None as initial comment when initial_comment == None --- diff --git a/pagure/templates/pull_request_title.html b/pagure/templates/pull_request_title.html index 46bc952..ecbfce7 100644 --- a/pagure/templates/pull_request_title.html +++ b/pagure/templates/pull_request_title.html @@ -45,7 +45,7 @@

From 064be0a7a856950e60489cbd978ac37f9d2cc684 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Sep 25 2019 20:49:55 +0000 Subject: [PATCH 3/4] tests: move pull request creation function from pr_no_sources test suite to __init__ --- diff --git a/tests/__init__.py b/tests/__init__.py index 5e9da59..153fdae 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -1108,6 +1108,105 @@ def remove_file_git_repo(folder, filename, branch="master"): shutil.rmtree(newfolder) +def add_pull_request_git_repo( + folder, + session, + repo, + fork, + branch_from="feature", + user="pingou", + allow_rebase=False, +): + """ Set up the git repo and create the corresponding PullRequest + object. + """ + + # Clone the main repo + gitrepo = os.path.join(folder, "repos", repo.path) + newpath = tempfile.mkdtemp(prefix="pagure-fork-test") + repopath = os.path.join(newpath, "test") + clone_repo = pygit2.clone_repository(gitrepo, repopath) + + # Create a file in that git repo + with open(os.path.join(repopath, "sources"), "w") as stream: + stream.write("foo\n bar") + clone_repo.index.add("sources") + clone_repo.index.write() + + # Commits the files added + tree = clone_repo.index.write_tree() + author = pygit2.Signature("Alice Author", "alice@authors.tld") + committer = pygit2.Signature("Cecil Committer", "cecil@committers.tld") + clone_repo.create_commit( + "refs/heads/master", # the name of the reference to update + author, + committer, + "Add sources file for testing", + # binary string representing the tree object ID + tree, + # list of binary strings representing parents of the new commit + [], + ) + refname = "refs/heads/master:refs/heads/master" + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + first_commit = clone_repo.revparse_single("HEAD") + + # Set the second repo + repopath = os.path.join(folder, "repos", fork.path) + new_gitrepo = os.path.join(newpath, "fork_test") + clone_repo = pygit2.clone_repository(repopath, new_gitrepo) + + # Add the main project as remote repo + upstream_path = os.path.join(folder, "repos", repo.path) + remote = clone_repo.create_remote("upstream", upstream_path) + remote.fetch() + + # Edit the sources file again + with open(os.path.join(new_gitrepo, "sources"), "w") as stream: + stream.write("foo\n bar\nbaz\n boose") + clone_repo.index.add("sources") + clone_repo.index.write() + + # Commits the files added + tree = clone_repo.index.write_tree() + author = pygit2.Signature("Alice Author", "alice@authors.tld") + committer = pygit2.Signature("Cecil Committer", "cecil@committers.tld") + clone_repo.create_commit( + "refs/heads/%s" % branch_from, + author, + committer, + "A commit on branch %s" % branch_from, + tree, + [first_commit.oid.hex], + ) + refname = "refs/heads/%s" % (branch_from) + ori_remote = clone_repo.remotes[0] + PagureRepo.push(ori_remote, refname) + + # Create a PR for these changes + project = pagure.lib.query.get_authorized_project(session, "test") + req = pagure.lib.query.new_pull_request( + session=session, + repo_from=fork, + branch_from=branch_from, + repo_to=project, + branch_to="master", + title="PR from the %s branch" % branch_from, + allow_rebase=allow_rebase, + user=user, + ) + session.commit() + + return req + + +def clean_pull_requests_path(): + newpath = tempfile.mkdtemp(prefix="pagure-fork-test") + shutil.rmtree(newpath) + + @contextmanager def capture_output(merge_stderr=True): oldout, olderr = sys.stdout, sys.stderr diff --git a/tests/test_pagure_flask_ui_pr_no_sources.py b/tests/test_pagure_flask_ui_pr_no_sources.py index 01eaddf..4040043 100644 --- a/tests/test_pagure_flask_ui_pr_no_sources.py +++ b/tests/test_pagure_flask_ui_pr_no_sources.py @@ -81,86 +81,14 @@ class PagureFlaskPrNoSourcestests(tests.Modeltests): object. """ - # Clone the main repo - gitrepo = os.path.join(self.path, "repos", repo.path) - newpath = tempfile.mkdtemp(prefix="pagure-fork-test") - repopath = os.path.join(newpath, "test") - clone_repo = pygit2.clone_repository(gitrepo, repopath) - - # Create a file in that git repo - with open(os.path.join(repopath, "sources"), "w") as stream: - stream.write("foo\n bar") - clone_repo.index.add("sources") - clone_repo.index.write() - - # Commits the files added - tree = clone_repo.index.write_tree() - author = pygit2.Signature("Alice Author", "alice@authors.tld") - committer = pygit2.Signature("Cecil Committer", "cecil@committers.tld") - clone_repo.create_commit( - "refs/heads/master", # the name of the reference to update - author, - committer, - "Add sources file for testing", - # binary string representing the tree object ID - tree, - # list of binary strings representing parents of the new commit - [], + req = tests.add_pull_request_git_repo( + self.path, self.session, repo, fork, branch_from ) - refname = "refs/heads/master:refs/heads/master" - ori_remote = clone_repo.remotes[0] - PagureRepo.push(ori_remote, refname) - - first_commit = clone_repo.revparse_single("HEAD") - - # Set the second repo - repopath = os.path.join(self.path, "repos", fork.path) - new_gitrepo = os.path.join(newpath, "fork_test") - clone_repo = pygit2.clone_repository(repopath, new_gitrepo) - - # Add the main project as remote repo - upstream_path = os.path.join(self.path, "repos", repo.path) - remote = clone_repo.create_remote("upstream", upstream_path) - remote.fetch() - - # Edit the sources file again - with open(os.path.join(new_gitrepo, "sources"), "w") as stream: - stream.write("foo\n bar\nbaz\n boose") - clone_repo.index.add("sources") - clone_repo.index.write() - - # Commits the files added - tree = clone_repo.index.write_tree() - author = pygit2.Signature("Alice Author", "alice@authors.tld") - committer = pygit2.Signature("Cecil Committer", "cecil@committers.tld") - clone_repo.create_commit( - "refs/heads/%s" % branch_from, - author, - committer, - "A commit on branch %s" % branch_from, - tree, - [first_commit.oid.hex], - ) - refname = "refs/heads/%s" % (branch_from) - ori_remote = clone_repo.remotes[0] - PagureRepo.push(ori_remote, refname) - # 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=fork, - branch_from=branch_from, - repo_to=project, - branch_to="master", - title="PR from the %s branch" % branch_from, - user="pingou", - ) - self.session.commit() self.assertEqual(req.id, 1) self.assertEqual(req.title, "PR from the %s branch" % branch_from) - shutil.rmtree(newpath) + tests.clean_pull_requests_path() def test_request_pull_reference(self): """ Test if there is a reference created for a new PR. """ From 89da7f43206381d51e8ecdbe1a99fa90f44abb20 Mon Sep 17 00:00:00 2001 From: Julen Landa Alustiza Date: Sep 25 2019 20:49:55 +0000 Subject: [PATCH 4/4] Add unit test for flask.ui.pull_request_edit --- diff --git a/tests/test_pagure_flask_ui_pr_edit.py b/tests/test_pagure_flask_ui_pr_edit.py new file mode 100644 index 0000000..d60db95 --- /dev/null +++ b/tests/test_pagure_flask_ui_pr_edit.py @@ -0,0 +1,313 @@ +# -*- coding: utf-8 -*- + +""" + Authors: + Julen Landa Alustiza +""" + +from __future__ import unicode_literals, absolute_import + +import sys +import os + +sys.path.insert( + 0, os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") +) + +import tests +import pagure.lib.query +import pygit2 + + +class PagureFlaskPrEditSimpletests(tests.Modeltests): + def test_pr_edit_no_project(self): + """ Test the edit pull request endpoint """ + output = self.app.get("/foo/pull-request/1/edit") + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + "Page not found :'( - Pagure", output_text + ) + self.assertIn("

Page not found (404)

", output_text) + + def test_pr_edit_no_git_repo(self): + """ Test the edit pull request endpoint """ + tests.create_projects(self.session) + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + "Page not found :'( - Pagure", output_text + ) + self.assertIn("

No git repo found

", output_text) + + def test_pr_edit_no_pull_requests_no_login(self): + """ Test the edit pull request endpoint """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 302) + + def test_pr_edit_no_pull_requests(self): + """ Test the edit pull request endpoint """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, "repos"), bare=True) + user = tests.FakeUser() + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 404) + output_text = output.get_data(as_text=True) + self.assertIn( + "Page not found :'( - Pagure", output_text + ) + self.assertIn("

Pull-request not found

", output_text) + + +class PagureFlaskPrEdittests(tests.Modeltests): + def setUp(self): + 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 + name="test", + description="test project #1", + hook_token="aaabbb", + is_fork=True, + parent_id=1, + ) + self.session.add(item) + self.session.commit() + # Create the fork's git repo + repo_path = os.path.join(self.path, "repos", item.path) + pygit2.init_repository(repo_path, bare=True) + + project = pagure.lib.query.get_authorized_project(self.session, "test") + fork = pagure.lib.query.get_authorized_project( + self.session, "test", user="foo" + ) + tests.add_pull_request_git_repo( + self.path, + self.session, + project, + fork, + user="foo", + allow_rebase=True, + ) + + def tearDown(self): + super(PagureFlaskPrEdittests, self).tearDown() + tests.clean_pull_requests_path() + + def test_pr_edit_pull_request_unauthenticated(self): + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 302) + + def test_pr_edit_pull_request_unauthorized(self): + user = tests.FakeUser() + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 403) + output_text = output.get_data(as_text=True) + self.assertIn("403 Forbidden", output_text) + self.assertIn( + "

You are not allowed to edit this pull-request

", + output_text, + ) + + def test_pr_edit_pull_request_view_author(self): + user = tests.FakeUser(username="foo") + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + # Author is editing PR #1 + self.assertIn( + "Edit PR#1: PR from the feature branch - test - " + "Pagure", + output_text, + ) + # Author has a title input + if self.get_wtforms_version() >= (2, 2): + self.assertIn( + '', + output_text, + ) + else: + self.assertIn( + '', + output_text, + ) + # Author has an initial_commit textarea + self.assertIn( + '', + output_text, + ) + # Author has an non-disabled allow_rebase input + self.assertIn( + '', + output_text, + ) + + def test_pr_edit_pull_request_post_author_no_csrf_token(self): + user = tests.FakeUser(username="foo") + with tests.user_set(self.app.application, user): + data = { + "title": "New title", + "initial_comment": "New initial comment", + "allow_rebase": False, + } + 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) + # Without CSRF token, we finish again on the form with new + # values. + self.assertIn( + "Edit PR#1: PR from the feature branch - test - " + "Pagure", + output_text, + ) + if self.get_wtforms_version() >= (2, 2): + self.assertIn( + '', + output_text, + ) + else: + self.assertIn( + '', + output_text, + ) + self.assertIn( + '', + output_text, + ) + self.assertIn( + '', + 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(True, request.allow_rebase) + + def test_pr_edit_pull_request_post_author(self): + user = tests.FakeUser(username="foo") + with tests.user_set(self.app.application, user): + data = { + "title": "New title", + "initial_comment": "New initial comment", + "allow_rebase": False, + "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( + "PR#1: New title - test\n - Pagure", output_text + ) + self.assertIn( + '\n' + " New title\n" + " ", + output_text, + ) + self.assertIn("

New initial comment

", 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) + self.assertEqual(False, request.allow_rebase) + + def test_pr_edit_pull_request_view_committer(self): + user = tests.FakeUser(username="pingou") + with tests.user_set(self.app.application, user): + output = self.app.get("/test/pull-request/1/edit") + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + # Committer is editing PR #1 + self.assertIn( + "Edit PR#1: PR from the feature branch - test - " + "Pagure", + output_text, + ) + # Committer has a title input + if self.get_wtforms_version() >= (2, 2): + self.assertIn( + '', + output_text, + ) + else: + self.assertIn( + '', + output_text, + ) + # Committer has an initial_commit textarea + self.assertIn( + '', + output_text, + ) + # Committer has an disabled allow_rebase input + self.assertIn( + '', + output_text, + ) + + def test_pr_edit_pull_request_post_committer(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, + "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( + "PR#1: New title - test\n - Pagure", output_text + ) + self.assertIn( + '\n' + " New title\n" + " ", + output_text, + ) + self.assertIn("

New initial comment

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