diff --git a/doc/configuration.rst b/doc/configuration.rst index 05e9509..2467079 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -1219,14 +1219,25 @@ TRIGGER_CI ~~~~~~~~~~ A run of pagure-ci can be manually triggered if some key sentences are added -as comment to a pull-request. This allows to re-run a test that failed due -to some network outage or other unexpected issues unrelated to the test -suite. +as comment to a pull-request, either manually or via the "Rerun CI" dropdown. +This allows to re-run a test that failed due to some network outage or other +unexpected issues unrelated to the test suite. This configuration key allows to define all the sentences that can be used -to trigger this pagure-ci run. +to trigger this pagure-ci run. The format is following: ``{"": +{"name": "", "description": ""}}`` -Defaults to: ``['pretty please pagure-ci rebuild']`` +Sentences which have ``None`` as value won't show up in the "Rerun CI" +dropdown. Additionally, it's possible to add a ``requires_project_hook_attr`` +key to the dict with data about a sentence. For example, having +``"requires_project_hook_attr": ("ci_hook", "active_pr", True)`` would make +the "Rerun CI" dropdown have a button for this specific CI only if the +project has ``ci_hook`` activated and its ``active_pr`` value is ``True``. + +In versions before 5.2, this was a list containing just the sentences. + +Defaults to: ``{"pretty please pagure-ci rebuild": {"name": "Default CI", +"description": "Rerun default CI"}}`` .. note:: The sentences defined in this configuration key should be lower case only! diff --git a/pagure/default_config.py b/pagure/default_config.py index ef5a28c..e24f523 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -376,7 +376,13 @@ USER_NAMESPACE = False # unless the user has direct access to it. EXCLUDE_GROUP_INDEX = [] -TRIGGER_CI = ["pretty please pagure-ci rebuild"] +TRIGGER_CI = { + "pretty please pagure-ci rebuild": { + "name": "Default CI", + "description": "Rerun default CI", + "requires_project_hook_attr": ("ci_hook", "active_pr", True), + }, +} FLAG_STATUSES_LABELS = { "success": "badge-success", diff --git a/pagure/forms.py b/pagure/forms.py index 5c77b13..82bb9c7 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -862,3 +862,24 @@ class MergePRForm(PagureForm): [wtforms.validators.optional()], false_values=FALSE_VALUES, ) + + +class TriggerCIPRForm(PagureForm): + def __init__(self, *args, **kwargs): + # we need to instantiate dynamically because the configuration + # values may change during tests and we want to always respect + # the currently set value + super(TriggerCIPRForm, self).__init__(*args, **kwargs) + choices = [] + trigger_ci = pagure_config["TRIGGER_CI"] + if isinstance(trigger_ci, dict): + # make sure to preserver compatibility with older configs + # which had TRIGGER_CI as a list + for comment, meta in trigger_ci.items(): + if meta is not None: + choices.append((comment, comment)) + self.comment.choices = choices + + comment = wtforms.SelectField( + "comment", [wtforms.validators.Required()], choices=[] + ) diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 9ae5047..a35bfc8 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -1377,6 +1377,7 @@ def add_pull_request_comment( # Send notification to the CI server, if the comment added was a # notification and the PR is still open and project is not private + ci_triggered = False if ( notification and request.status == "Open" @@ -1391,6 +1392,7 @@ def add_pull_request_comment( branch=request.branch_from, ci_type=request.project.ci_hook.ci_type, ) + ci_triggered = True pagure.lib.notify.log( request.project, @@ -1402,7 +1404,8 @@ def add_pull_request_comment( ) if ( - trigger_ci + not ci_triggered + and trigger_ci and comment.strip().lower() in trigger_ci and pagure_config.get("PAGURE_CI_SERVICES") and request.project.ci_hook diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html index 294632e..9fac1a3 100644 --- a/pagure/templates/repo_pull_request.html +++ b/pagure/templates/repo_pull_request.html @@ -170,36 +170,60 @@ {% endif %} {% if pull_request.status == 'Open' %}
- - Merge + + + Merge - -
+
- {% if pull_request.status == 'Open' and g.repo_committer %} + {% if pull_request.status == 'Open' and g.repo_committer %} + +
+ {{ mergeform.csrf_token }} + + {% if can_delete_branch %} +
+ {{ mergeform.delete_branch }} {{ mergeform.delete_branch.label }} +
+ {% endif %} +
+ {% else %} -
- {{ mergeform.csrf_token }} - - {% if can_delete_branch %} -
- {{ mergeform.delete_branch }} {{ mergeform.delete_branch.label }} -
- {% endif %} -
- {% else %} - - {% endif %} + {% endif %} +
-
+ + {% if g.authenticated and trigger_ci %} + + + Rerun CI + +
+
+ {{ trigger_ci_pr_form.csrf_token }} + {{ trigger_ci_pr_form.comment(id="ci_pr_comment", hidden=True) }} + {% for comment, meta in trigger_ci|dictsort %} + {{ meta["name"] }} + {% endfor %} +
+
+ + {% endif %}
{% endif %} {% if pull_request.status == 'Closed' and g.authenticated and diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py index 1ffbc88..c47369b 100644 --- a/pagure/ui/fork.py +++ b/pagure/ui/fork.py @@ -30,6 +30,7 @@ import pagure import pagure.doc_utils import pagure.exceptions import pagure.lib.git +import pagure.lib.plugins import pagure.lib.query import pagure.lib.tasks import pagure.forms @@ -308,6 +309,24 @@ def request_pull(repo, requestid, username=None, namespace=None): diff.find_similar() form = pagure.forms.MergePRForm() + trigger_ci_pr_form = pagure.forms.TriggerCIPRForm() + + # we need to leave out all members of trigger_ci_conf that have + # "meta" set to False or meta["requires_project_hook_attr"] condition + # defined and it's not met + trigger_ci_conf = pagure_config["TRIGGER_CI"] + if not isinstance(trigger_ci_conf, dict): + trigger_ci_conf = {} + trigger_ci = {} + # make sure all the backrefs are set properly on repo + pagure.lib.plugins.get_enabled_plugins(repo) + for comment, meta in trigger_ci_conf.items(): + if not meta: + continue + cond = meta.get("requires_project_hook_attr", ()) + if cond and not pagure.utils.project_has_hook_attr_value(repo, *cond): + continue + trigger_ci[comment] = meta can_delete_branch = ( pagure_config.get("ALLOW_DELETE_BRANCH", True) @@ -328,6 +347,8 @@ def request_pull(repo, requestid, username=None, namespace=None): 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_delete_branch=can_delete_branch, + trigger_ci=trigger_ci, + trigger_ci_pr_form=trigger_ci_pr_form, ) @@ -619,6 +640,9 @@ def pull_request_add_comment( comment = form.comment.data try: + trigger_ci = pagure_config["TRIGGER_CI"] + if isinstance(trigger_ci, dict): + trigger_ci = list(trigger_ci.keys()) message = pagure.lib.query.add_pull_request_comment( flask.g.session, request=request, @@ -628,7 +652,7 @@ def pull_request_add_comment( row=row, comment=comment, user=flask.g.fas_user.username, - trigger_ci=pagure_config["TRIGGER_CI"], + trigger_ci=trigger_ci, ) flask.g.session.commit() if not is_js: @@ -931,6 +955,76 @@ def reopen_request_pull(repo, requestid, username=None, namespace=None): ) +@UI_NS.route( + "//pull-request//trigger-ci", + methods=["POST"] +) +@UI_NS.route( + "///pull-request//trigger-ci", + methods=["POST"] +) +@UI_NS.route( + "/fork///pull-request//trigger-ci", + methods=["POST"], +) +@UI_NS.route( + ("/fork////pull-request/" + "/trigger-ci"), + methods=["POST"], +) +@login_required +def ci_trigger_request_pull(repo, requestid, username=None, namespace=None): + """ Trigger CI testing for a PR. + """ + form = pagure.forms.TriggerCIPRForm() + if not form.validate_on_submit(): + flask.flash("Invalid input submitted", "error") + return flask.redirect( + flask.url_for( + "ui_ns.request_pull", + repo=repo, + requestid=requestid, + username=username, + namespace=namespace, + ) + ) + + repo_obj = flask.g.repo + request = pagure.lib.query.search_pull_requests( + flask.g.session, project_id=repo_obj.id, requestid=requestid + ) + + if not request: + flask.abort(404, "Pull-request not found") + + trigger_ci = pagure_config["TRIGGER_CI"] + if isinstance(trigger_ci, dict): + trigger_ci = list(trigger_ci.keys()) + pagure.lib.query.add_pull_request_comment( + flask.g.session, + request, + commit=None, + tree_id=None, + filename=None, + row=None, + comment=form.comment.data, + user=flask.g.fas_user.username, + notify=True, + notification=True, + trigger_ci=trigger_ci, + ) + + return flask.redirect( + flask.url_for( + "ui_ns.request_pull", + repo=repo, + username=username, + namespace=namespace, + requestid=requestid, + ) + ) + + @UI_NS.route("//pull-request//merge", methods=["POST"]) @UI_NS.route( "///pull-request//merge", methods=["POST"] diff --git a/pagure/utils.py b/pagure/utils.py index f1769b5..9e2b1dd 100644 --- a/pagure/utils.py +++ b/pagure/utils.py @@ -664,3 +664,27 @@ def lookup_deploykey(project, username): if key.id == keyid: return key return None + + +def project_has_hook_attr_value(project, hook, attr, value): + """ Finds out if project's hook has attribute of given value. + + :arg project: The project to inspect + :type project: pagure.lib.model.Project + :arg hook: Name of the hook to inspect + :type hook: str + :arg attr: Name of hook attribute to inspect + :type attr: str + :arg value: Value to compare project's hook attribute value with + :type value: object + :return: True if project's hook attribute value is equal with given + value, False otherwise + """ + retval = False + hook_obj = getattr(project, hook, None) + if hook_obj is not None: + attr_obj = getattr(hook_obj, attr, None) + if attr_obj == value: + retval = True + + return retval diff --git a/tests/test_pagure_flask_ui_fork.py b/tests/test_pagure_flask_ui_fork.py index baa74b8..42486a6 100644 --- a/tests/test_pagure_flask_ui_fork.py +++ b/tests/test_pagure_flask_ui_fork.py @@ -378,6 +378,156 @@ class PagureFlaskForktests(tests.Modeltests): self.assertNotEqual(stop_commit, request.commit_stop) @patch('pagure.lib.notify.send_email') + def test_request_pull_ci_dropdown(self, send_email): + """ Test presence of the "Rerun CI" dropdown with various settings. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo(new_project=None, branch_from='feature') + + user = tests.FakeUser() + user.username = 'pingou' + with tests.user_set(self.app.application, user): + # old-style TRIGGER_CI list - test backwards compatibility + with patch.dict('pagure.config.config', + {'TRIGGER_CI': ['old-style-trigger-ci']}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertNotIn('Rerun CI', output_text) + + # new-style TRIGGER_CI, but no button to show + with patch.dict('pagure.config.config', + {'TRIGGER_CI': {'no-button': None}}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertNotIn('Rerun CI', output_text) + + trigger_ci = { + 'foobar-ci': { + 'name': 'foobar-ci-name', + 'description': 'barfoo', + }, + 'spam-ci': { + 'name': 'spam-ci-name', + 'description': 'with beans and eggs', + }, + 'no-button-for-me-ci': None, + } + # new-style TRIGGER_CI, several buttons to show + with patch.dict('pagure.config.config', + {'TRIGGER_CI': trigger_ci}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('Rerun CI', output_text) + self.assertIn('foobar-ci-name', output_text) + self.assertIn('spam-ci-name', output_text) + self.assertNotIn('no-button-for-me-ci', output_text) + + trigger_ci = { + 'foobar-ci': { + 'name': 'foobar-ci-name', + 'description': 'barfoo', + 'requires_project_hook_attr': ('ci_hook', 'active_pr', True), + }, + } + # new-style TRIGGER_CI with requires_project_hook_attr that is + # not fulfilled by the project + with patch.dict('pagure.config.config', + {'TRIGGER_CI': trigger_ci}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertNotIn('Rerun CI', output_text) + # now activate the hook and try again + data = { + 'active_pr': 'y', + 'ci_url': 'https://jenkins.fedoraproject.org', + 'ci_job': 'ci_job', + 'ci_type': 'jenkins', + 'csrf_token': self.get_csrf() + } + output = self.app.post('/test/settings/Pagure CI', data=data, + follow_redirects=True) + self.assertEqual(output.status_code, 200) + with patch.dict('pagure.config.config', + {'TRIGGER_CI': trigger_ci}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn('Rerun CI', output_text) + self.assertIn('foobar-ci-name', output_text) + + # shouldn't show up if user is not logged in + with patch.dict('pagure.config.config', + {'TRIGGER_CI': trigger_ci}): + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertNotIn('Rerun CI', output_text) + + @patch('pagure.lib.notify.send_email') + @patch.dict('pagure.config.config', + {'TRIGGER_CI': {'CI1': {'name': 'CI1', 'description': 'CI1!'}}}) + def test_request_pull_ci_rerun(self, send_email): + """ Test rerunning CI using button from the "Rerun CI" dropdown. """ + send_email.return_value = True + + tests.create_projects(self.session) + tests.create_projects_git( + os.path.join(self.path, 'requests'), bare=True) + self.set_up_git_repo(new_project=None, branch_from='feature') + user = tests.FakeUser() + user.username = 'pingou' + project = pagure.lib.query.get_authorized_project(self.session, 'test') + request = project.requests[0] + + with tests.user_set(self.app.application, user): + # no csrf token + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output = self.app.post( + '/test/pull-request/1/trigger-ci', follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn('Invalid input', output.get_data(as_text=True)) + + # no such PR + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + output = self.app.post( + '/test/pull-request/2/trigger-ci', follow_redirects=True) + self.assertEqual(output.status_code, 404) + + # wrong comment + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + csrf_token = self.get_csrf(output=output) + data = {'csrf_token': csrf_token, 'comment': 'this doesnt exist'} + output = self.app.post( + '/test/pull-request/1/trigger-ci', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + self.assertIn('Invalid input', output.get_data(as_text=True)) + + # everything ok + output = self.app.get('/test/pull-request/1') + self.assertEqual(output.status_code, 200) + csrf_token = self.get_csrf(output=output) + data = {'csrf_token': csrf_token, 'comment': 'CI1'} + output = self.app.post( + '/test/pull-request/1/trigger-ci', data=data, follow_redirects=True) + output_text = output.get_data(as_text=True) + self.assertEqual(output.status_code, 200) + self.assertIn('

CI1

', output_text) + comment = request.comments[0] + self.assertTrue(comment.notification) + self.assertEqual(comment.comment, 'CI1') + + + @patch('pagure.lib.notify.send_email') def test_merge_request_pull_FF(self, send_email): """ Test the merge_request_pull endpoint with a FF PR. """ send_email.return_value = True