From 01c1708cc0ea404f21ce03c7ba48767370fe8427 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 13 2018 15:51:42 +0000 Subject: [PATCH 1/5] Small black change Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 674a370..940eb68 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -541,7 +541,7 @@ SSH_KEYS_USERNAME_EXPECT = None # Arguments to add to the SSH keys, possible replacements: # %(username)s: username owning this key SSH_KEYS_OPTIONS = ( - "restrict,command=\"/usr/libexec/pagure/aclchecker.py %(username)s\"" + 'restrict,command="/usr/libexec/pagure/aclchecker.py %(username)s"' ) # ACL Checker options From 340fdc140e5db835856f4b4cb1eb35c064d1c38d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 13 2018 15:51:54 +0000 Subject: [PATCH 2/5] Include the lines missed when showing the coverage Signed-off-by: Pierre-Yves Chibon --- diff --git a/runtests.py b/runtests.py index 0692333..15e43c1 100755 --- a/runtests.py +++ b/runtests.py @@ -549,7 +549,7 @@ def do_show_coverage(args): subprocess.check_call(cmd, env=env) print() print("Python %s coverage: " % pyver) - cmd = [cover, "report", "--include=./pagure/*"] + cmd = [cover, "report", "--include=./pagure/*", "-m"] subprocess.check_call(cmd, env=env) From 276cdbe60a15ddf6cf8c94aad90421bfba531441 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 13 2018 15:52:31 +0000 Subject: [PATCH 3/5] Update the ticket git repo after having sent the notifications On pagure.io some project fail to update their ticket git repo because of "too many files open" errors. But they also fail to send notification about the new comment added to the database. With this fix, they should be able to get the notification regardless of whether pagure could update successfully the ticket git repository. Fixes https://pagure.io/pagure/issue/3973 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/query.py b/pagure/lib/query.py index a84a3ad..7abbd8f 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -403,14 +403,14 @@ def add_issue_comment( # Make sure we won't have SQLAlchemy error before we continue session.commit() - pagure.lib.git.update_git(issue, repo=issue.project) - if not notification: log_action(session, "commented", issue, user_obj) if notify: pagure.lib.notify.notify_new_comment(issue_comment, user=user_obj) + pagure.lib.git.update_git(issue, repo=issue.project) + if not issue.private: pagure.lib.notify.log( issue.project, From a3506170645fc4e3c33c06ee47fb3c492380b0d8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 14 2018 09:56:33 +0000 Subject: [PATCH 4/5] Rename "Cancel a PR" into "Close a PR" This is friendlier for the people who opened it in the first place. Fixes https://pagure.io/pagure/issue/3983 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py index 03520c4..d0ebed5 100644 --- a/pagure/lib/notify.py +++ b/pagure/lib/notify.py @@ -804,14 +804,15 @@ Reopened pull-request: ) -def notify_cancelled_pull_request(request, user): +def notify_closed_pull_request(request, user): """ Notify the people following a project that a pull-request was - cancelled in it. + closed in it. """ text = """ -%s canceled a pull-request against the project: `%s` that you are following. +%s closed without merging a pull-request against the project: `%s` that you +are following. -Cancelled pull-request: +Closed pull-request: `` %s diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 7abbd8f..118e59a 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -3270,7 +3270,7 @@ def close_pull_request(session, request, user, merged=True): if merged is True: pagure.lib.notify.notify_merge_pull_request(request, user_obj) else: - pagure.lib.notify.notify_cancelled_pull_request(request, user_obj) + pagure.lib.notify.notify_closed_pull_request(request, user_obj) pagure.lib.git.update_git(request, repo=request.project) diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html index 78f5358..2f1bef1 100644 --- a/pagure/templates/repo_pull_request.html +++ b/pagure/templates/repo_pull_request.html @@ -71,7 +71,7 @@ by {{ pull_request.user.user }}. {% elif pull_request.status == 'Closed' %} - Cancelled {{ pull_request.closed_at |humanize }} + Closed {{ pull_request.closed_at |humanize }} by {{ pull_request.closed_by.user }}. @@ -150,7 +150,7 @@ {% endif %}
{% endif %} @@ -158,9 +158,9 @@ {% if pull_request.status == 'Open' and g.authenticated and (g.repo_committer or g.fas_user.username == pull_request.user.username) %} {{ mergeform.csrf_token }} - {% endif %} {% if pull_request.status == 'Open' and g.authenticated and @@ -944,7 +944,7 @@ $(document).ready(function() { return false; }); - $('#cancel_pr').click(function(){ + $('#close_pr').click(function(){ return window.confirm("Are you sure you want to close this requested pull?"); }); diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 1527430..c0c743e 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -1218,21 +1218,21 @@ def merge_request_pull(repo, requestid, username=None, namespace=None): ) -@UI_NS.route("//pull-request/cancel/", methods=["POST"]) +@UI_NS.route("//pull-request/close/", methods=["POST"]) @UI_NS.route( - "///pull-request/cancel/", methods=["POST"] + "///pull-request/close/", methods=["POST"] ) @UI_NS.route( - "/fork///pull-request/cancel/", + "/fork///pull-request/close/", methods=["POST"], ) @UI_NS.route( - "/fork////pull-request/cancel/", + "/fork////pull-request/close/", methods=["POST"], ) @login_required -def cancel_request_pull(repo, requestid, username=None, namespace=None): - """ Cancel a pull request. +def close_request_pull(repo, requestid, username=None, namespace=None): + """ Close a pull request without merging it. """ form = pagure.forms.ConfirmationForm() @@ -1254,7 +1254,7 @@ def cancel_request_pull(repo, requestid, username=None, namespace=None): ): flask.abort( 403, - "You are not allowed to cancel pull-request for this project", + "You are not allowed to close pull-request for this project", ) pagure.lib.query.close_pull_request( diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index 42486a6..56231c0 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -1661,8 +1661,8 @@ index 0000000..2a552bb shutil.rmtree(newpath) @patch('pagure.lib.notify.send_email') - def test_cancel_request_pull(self, send_email): - """ Test the cancel_request_pull endpoint. """ + def test_close_request_pull(self, send_email): + """ Test the close_request_pull endpoint. """ send_email.return_value = True tests.create_projects(self.session) @@ -1673,11 +1673,11 @@ index 0000000..2a552bb user = tests.FakeUser() with tests.user_set(self.app.application, user): - output = self.app.post('/test/pull-request/cancel/1') + output = self.app.post('/test/pull-request/close/1') self.assertEqual(output.status_code, 302) output = self.app.post( - '/test/pull-request/cancel/1', follow_redirects=True) + '/test/pull-request/close/1', follow_redirects=True) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) self.assertIn( @@ -1697,19 +1697,19 @@ index 0000000..2a552bb # Invalid project output = self.app.post( - '/foo/pull-request/cancel/1', data=data, + '/foo/pull-request/close/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 404) # Invalid PR id output = self.app.post( - '/test/pull-request/cancel/100', data=data, + '/test/pull-request/close/100', data=data, follow_redirects=True) self.assertEqual(output.status_code, 404) # Invalid user for this project output = self.app.post( - '/test/pull-request/cancel/1', data=data, + '/test/pull-request/close/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 403) @@ -1724,7 +1724,7 @@ index 0000000..2a552bb self.session.commit() output = self.app.post( - '/test/pull-request/cancel/1', data=data, + '/test/pull-request/close/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 404) @@ -1737,7 +1737,7 @@ index 0000000..2a552bb self.session.commit() output = self.app.post( - '/test/pull-request/cancel/1', data=data, + '/test/pull-request/close/1', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) output_text = output.get_data(as_text=True) From e9b3b27d369292c83b78a7c861971f1d3d6658bb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 14 2018 11:15:57 +0000 Subject: [PATCH 5/5] Fix the place of the import in the template alembic revision This corresponds more to pagure's coding style and pep0008 Signed-off-by: Pierre-Yves Chibon --- diff --git a/alembic/script.py.mako b/alembic/script.py.mako index 9570201..46bd73e 100644 --- a/alembic/script.py.mako +++ b/alembic/script.py.mako @@ -6,13 +6,14 @@ Create Date: ${create_date} """ +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + # revision identifiers, used by Alembic. revision = ${repr(up_revision)} down_revision = ${repr(down_revision)} -from alembic import op -import sqlalchemy as sa -${imports if imports else ""} def upgrade(): ${upgrades if upgrades else "pass"}