From 3448fc31f9aea874aff714448eebb3f2cbc8f698 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 13 2018 16:02:21 +0000 Subject: [PATCH 1/3] Add the possibility to give a group away Fixes https://pagure.io/pagure/issue/4086 Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/edit_group.html b/pagure/templates/edit_group.html index 688a766..b367366 100644 --- a/pagure/templates/edit_group.html +++ b/pagure/templates/edit_group.html @@ -1,13 +1,20 @@ {% extends "master.html" %} {% from "_formhelper.html" import render_bootstrap_field %} +{% block header %} + +{% endblock %} + {% set tag = "groups" %} {% block title %}Edit group: {{ group.group_name }}{% endblock %} {% block content %} -
+ +
-
+
+
Edit group: {{ group.group_name }} @@ -24,7 +31,10 @@ field_description="Short description of the group") }}

- + + + {{ form.csrf_token }}

@@ -32,7 +42,61 @@
+ + {% if group.creator.user == g.fas_user.username or g.admin %} +
+
+
+ Give group away: {{ group.group_name }} +
+
+
+ {{ form.csrf_token }} + + +
+
+
+
+ {% endif %} +
{% endblock %} + + +{% block jscripts %} +{{ super() }} + + + + +{% endblock %} diff --git a/pagure/ui/groups.py b/pagure/ui/groups.py index bb7139e..bcb023e 100644 --- a/pagure/ui/groups.py +++ b/pagure/ui/groups.py @@ -180,6 +180,80 @@ def edit_group(group): return flask.render_template("edit_group.html", group=group, form=form) +@UI_NS.route("/group//give", methods=["POST"]) +@login_required +def give_group(group): + """ Allows giving away a group. """ + if not pagure_config.get("ENABLE_USER_MNGT", True): + flask.abort(404) + + group_type = "user" + is_admin = pagure.utils.is_admin() + if is_admin: + group_type = None + group = pagure.lib.query.search_groups( + flask.g.session, group_name=group, group_type=group_type + ) + + if not group: + flask.abort(404, "Group not found") + + if group.creator.user != flask.g.fas_user.username and not flask.g.admin: + flask.abort(403, "You are not allowed to give away this group") + + # Give away group + form = pagure.forms.ConfirmationForm() + if form.validate_on_submit(): + + username = flask.request.form.get("username") + if not username: + flask.flash( + "No user %s found to give this group to" % username, "error" + ) + return flask.redirect( + flask.url_for("ui_ns.view_group", group=group.group_name) + ) + + user = pagure.lib.query.search_user(flask.g.session, username=username) + if not user: + flask.flash( + "No user %s found to give this group to" % username, "error" + ) + return flask.redirect( + flask.url_for("ui_ns.view_group", group=group.group_name) + ) + + try: + if user not in group.users: + pagure.lib.query.add_user_to_group( + session=flask.g.session, + username=username, + group=group, + user=flask.g.fas_user.username, + is_admin=flask.g.admin, + from_external=False, + ) + group.user_id = user.id + flask.g.session.add(group) + flask.g.session.commit() + flask.flash("Group given") + return flask.redirect( + flask.url_for("ui_ns.view_group", group=group.group_name) + ) + except SQLAlchemyError: # pragma: no cover + flask.g.session.rollback() + flask.flash( + "Could not give away group `%s`." % (group.group_name), "error" + ) + _log.exception( + "Could not give away group `%s`." % (group.group_name) + ) + + return flask.redirect( + flask.url_for("ui_ns.view_group", group=group.group_name) + ) + + @UI_NS.route("/group///delete", methods=["POST"]) @login_required def group_user_delete(user, group): diff --git a/tests/test_pagure_flask_ui_groups.py b/tests/test_pagure_flask_ui_groups.py index e73b0e2..5833985 100644 --- a/tests/test_pagure_flask_ui_groups.py +++ b/tests/test_pagure_flask_ui_groups.py @@ -223,6 +223,76 @@ class PagureFlaskGroupstests(tests.Modeltests): 'Group "Test Group edited" (test_group) edited', output.get_data(as_text=True)) + def test_give_group(self): + """ Test the give_group endpoint. """ + + output = self.app.post('/group/test_group/give') + self.assertEqual(output.status_code, 302) + + user = tests.FakeUser() + with tests.user_set(self.app.application, user): + output = self.app.post('/group/test_group/give') + self.assertEqual(output.status_code, 404) + self.assertIn('

Group not found

', output.get_data(as_text=True)) + + self.test_add_group() + + user.username = 'foo' + with tests.user_set(self.app.application, user): + output = self.app.post('/group/foo/give') + self.assertEqual(output.status_code, 404) + self.assertIn('

Group not found

', output.get_data(as_text=True)) + + output = self.app.post('/group/test_group/give') + self.assertEqual(output.status_code, 403) + + csrf_token = self.get_csrf() + + user.username = 'pingou' + with tests.user_set(self.app.application, user): + # Missing CSRF + data = { + 'username': 'invalid', + } + + output = self.app.post( + '/group/test_group/give', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Group test_group - Pagure', output_text) + self.assertIn( + 'administered by pingou', + output_text) + + # User not found + data['csrf_token'] = csrf_token + output = self.app.post( + '/group/test_group/give', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Group test_group - Pagure', output_text) + self.assertIn( + ' No user invalid found to give this group to
', + output_text) + self.assertIn( + 'administered by pingou', + output_text) + + # Working + data["username"] = "foo" + + output = self.app.post( + '/group/test_group/give', data=data, follow_redirects=True) + self.assertEqual(output.status_code, 200) + output_text = output.get_data(as_text=True) + self.assertIn( + 'Group test_group - Pagure', output_text) + self.assertIn(' Group given
', output_text) + self.assertIn( + 'administered by foo', output_text) + def test_group_delete(self): """ Test the group_delete endpoint. """ output = self.app.post('/group/foo/delete') From 58b8e8f01a011f42592f13d6662aa5d77e5e8b2d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 13 2018 16:02:21 +0000 Subject: [PATCH 2/3] Adjust the UI to not say the group was created by someone Since the main admin of the group can now change, saying the group was created by someone can be wrong, so instead just say that the group is administered by this person. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/templates/group_info.html b/pagure/templates/group_info.html index d3bf4f8..e16846a 100644 --- a/pagure/templates/group_info.html +++ b/pagure/templates/group_info.html @@ -21,8 +21,8 @@

{{group.display_name }}

{% if group.description %}
{{ group.description }}
{% endif %}
- formed {{ group.created |humanize }} - by {{ group.creator.user }} + formed {{ group.created |humanize }}, + administered by {{ group.creator.user }}
{% if g.authenticated and (member or g.admin) From accad94c37e51da36ce9098d2e0429b62a585eb9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 13 2018 16:02:21 +0000 Subject: [PATCH 3/3] Project wide black fixes Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/lib/git.py b/pagure/lib/git.py index f1cbe86..ba0de43 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -1824,7 +1824,8 @@ def rebase_pull_request(request, username): else: _log.info( "PR is neither from a remote git repo or an existing local " - "repo, bailing") + "repo, bailing" + ) return if not request.project or not os.path.exists( @@ -1832,7 +1833,8 @@ def rebase_pull_request(request, username): ): _log.info( "Could not find the targeted git repository for %s", - request.project.fullname) + request.project.fullname, + ) raise pagure.exceptions.PagureException( "Could not find the targeted git repository for %s" % request.project.fullname diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index 6b3d200..dc6ea36 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -1046,7 +1046,8 @@ def link_pr_to_ticket(self, session, pr_uid): else: _log.info( "LINK_PR_TO_TICKET: PR neither remote, nor with a " - "project_from, bailing: %s" % pr_uid) + "project_from, bailing: %s" % pr_uid + ) return repo_obj = pygit2.Repository(repopath)