From 4d961f80509c9279d563674132dd51dbb040f3e3 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 18 2019 08:46:23 +0000 Subject: [PATCH 1/3] Do not allow `,` in tags This break the UI when adding tags to a ticket or a PR, there are split as if they were two tags while they are one in fact. Fixes https://pagure.io/pagure/issue/4212 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/forms.py b/pagure/forms.py index fcc6ac1..52d8395 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -34,7 +34,7 @@ from pagure.config import config as pagure_config from pagure.utils import urlpattern, is_admin STRICT_REGEX = "^[a-zA-Z0-9-_]+$" -TAGS_REGEX = "^[a-zA-Z0-9-_, .:]+$" +TAGS_REGEX = "^[a-zA-Z0-9-_ .:]+$" FALSE_VALUES = ("false", "", False, "False", 0, "0") WTF_VERSION = tuple() From d86b5c7b3dbb8001e52a46282d3cd47198c9e5b8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 18 2019 08:46:23 +0000 Subject: [PATCH 2/3] Add a new decorator checking that either PR or issues are enabled This is needed for updating, editing or deleting tags. Issue tracking may be disabled for the project but PRs not and thus we need to keep tag-related actions available to the project. Fixes https://pagure.io/pagure/issue/4204 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/decorators.py b/pagure/decorators.py index 3e6e329..c00d958 100644 --- a/pagure/decorators.py +++ b/pagure/decorators.py @@ -58,6 +58,27 @@ def has_pr_enabled(function): return check_trackers +def has_issue_or_pr_enabled(function): + """ + Decorator that checks if the current pagure project has either their + issue tracker or their PR active. If both of them are disabled, it + returns a 404 page. + """ + + @wraps(function) + def check_issue_pr_trackers(*args, **kwargs): + repo = flask.g.repo + if not flask.g.issues_enabled and not repo.settings.get( + "pull_requests", True + ): + flask.abort( + 404, "Issue tracker and Pull-Request disabled for this project" + ) + return function(*args, **kwargs) + + return check_issue_pr_trackers + + def is_repo_admin(function): """ Decorator that checks if the current user is the admin of diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index deeb315..15afad5 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -60,6 +60,7 @@ from pagure.decorators import ( is_repo_admin, is_admin_sess_timedout, has_issue_tracker, + has_issue_or_pr_enabled, has_pr_enabled, ) @@ -3134,8 +3135,7 @@ def view_stats(repo, username=None, namespace=None): @UI_NS.route("///update/tags", methods=["POST"]) @login_required @is_repo_admin -@has_issue_tracker -@has_pr_enabled +@has_issue_or_pr_enabled def update_tags(repo, username=None, namespace=None): """ Update the tags of a project. """ @@ -3240,8 +3240,7 @@ def update_tags(repo, username=None, namespace=None): @UI_NS.route("/fork////droptag/", methods=["POST"]) @login_required @is_repo_admin -@has_issue_tracker -@has_pr_enabled +@has_issue_or_pr_enabled def remove_tag(repo, username=None, namespace=None): """ Remove the specified tag, associated with the issues, from the project. """ @@ -3294,8 +3293,7 @@ def remove_tag(repo, username=None, namespace=None): ) @login_required @is_repo_admin -@has_issue_tracker -@has_pr_enabled +@has_issue_or_pr_enabled def edit_tag(repo, tag, username=None, namespace=None): """ Edit the specified tag associated with the issues of a project. """ From f98d1430791c6ce5011fdae845dca1d2d78df1f2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 18 2019 08:46:23 +0000 Subject: [PATCH 3/3] Add tests ensuring we can add/edit/delete tags even when issues are off This commit adds tests for the newly introduced decorator ensuring that one can add, edit or delete tags which can then be used in PRs. Signed-off-by: Pierre-Yves Chibon --- diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index bb86064..6a73e24 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -3025,6 +3025,69 @@ class PagureFlaskIssuestests(tests.Modeltests): self.assertEqual(output_text.count( '

We should work on this

'), 1) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_edit_tag_issue_disabled(self): + """ Test the edit_tag endpoint when issues are disabled. """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + + # disable issue tracker + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + repo.settings = {'issue_tracker': False} + self.session.add(repo) + self.session.commit() + + # Create issues to play with + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Add a tag to the issue + issue = pagure.lib.query.search_issues(self.session, repo, issueid=1) + msg = pagure.lib.query.add_tag_obj( + session=self.session, + obj=issue, + tags='tag1', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg, 'Issue tagged with: tag1') + + # Before edit, list tags + tags = pagure.lib.query.get_tags_of_project(self.session, repo) + self.assertEqual([tag.tag for tag in tags], ['tag1']) + + # Edit tag + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + + output = self.app.get('/test/tag/tag1/edit') + self.assertEqual(output.status_code, 200) + self.assertTrue('Edit tag: tag1' in output.get_data(as_text=True)) + + csrf_token = self.get_csrf(output=output) + + data = {'tag': 'tag2', + 'tag_description': 'lorem ipsum', + 'tag_color': 'DeepSkyBlue'} + + output = self.app.post('/test/tag/tag1/edit', data=data) + self.assertEqual(output.status_code, 200) + self.assertTrue('Edit tag: tag1' in output.get_data(as_text=True)) + + # After edit, list tags + self.session.commit() + tags = pagure.lib.query.get_tags_of_project(self.session, repo) + self.assertEqual([tag.tag for tag in tags], ['tag1']) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_edit_tag(self, p_send_email, p_ugt): @@ -3129,6 +3192,63 @@ class PagureFlaskIssuestests(tests.Modeltests): tags = pagure.lib.query.get_tags_of_project(self.session, repo) self.assertEqual([tag.tag for tag in tags], ['tag2']) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_remove_tag_issue_disabled(self): + """ Test the remove_tag endpoint. """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + + # disable issue tracker + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + repo.settings = {'issue_tracker': False} + self.session.add(repo) + self.session.commit() + + # Create issues to play with + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Add a tag to the issue + issue = pagure.lib.query.search_issues(self.session, repo, issueid=1) + msg = pagure.lib.query.add_tag_obj( + session=self.session, + obj=issue, + tags='tag1', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg, 'Issue tagged with: tag1') + + # Before edit, list tags + tags = pagure.lib.query.get_tags_of_project(self.session, repo) + self.assertEqual([tag.tag for tag in tags], ['tag1']) + + # Edit tag + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + data = { + 'tag': 'tag1', + 'csrf_token': self.get_csrf(), + } + output = self.app.post( + '/test/droptag/', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + '
Project ' + 'Settings
', output_text) + self.assertIn( + 'Tag: tag1 has been deleted', output_text) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_remove_tag(self, p_send_email, p_ugt): @@ -3603,6 +3723,71 @@ class PagureFlaskIssuestests(tests.Modeltests): 'value="ssh://git@localhost.localdomain/tickets/test.git', output_text) + @patch('pagure.lib.git.update_git', MagicMock(return_value=True)) + @patch('pagure.lib.notify.send_email', MagicMock(return_value=True)) + def test_update_tags_issue_disabled(self): + """ Test the update_tags endpoint. """ + tests.create_projects(self.session) + tests.create_projects_git(os.path.join(self.path, 'repos')) + + # disable issue tracker + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + repo.settings = {'issue_tracker': False} + self.session.add(repo) + self.session.commit() + + # Create issues to play with + repo = pagure.lib.query.get_authorized_project(self.session, 'test') + msg = pagure.lib.query.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + + # Before update, list tags + tags = pagure.lib.query.get_tags_of_project(self.session, repo) + self.assertEqual([tag.tag for tag in tags], []) + + user = tests.FakeUser(username='pingou') + with tests.user_set(self.app.application, user): + csrf_token = self.get_csrf() + # Valid query + data = { + 'tag': ['red1', 'green'], + 'tag_description': ['lorem ipsum', 'sample description'], + 'tag_color': ['#ff0000', '#00ff00'], + 'csrf_token': csrf_token, + } + output = self.app.post( + '/test/update/tags', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Settings - test - Pagure', output_text) + self.assertIn( + 'green\n' + '  ' + 'sample description', output_text) + self.assertIn( + '', + output_text) + self.assertIn( + 'red1\n' + '  ' + 'lorem ipsum', output_text) + + # After update, list tags + tags = pagure.lib.query.get_tags_of_project(self.session, repo) + self.assertEqual( + sorted([tag.tag for tag in tags]), + ['green', 'red1']) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_update_tags(self, p_send_email, p_ugt):