From 2f373641cfde5e105eb8b3d9a9aa8a3ac20e49ca Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 14 2016 13:59:29 +0000 Subject: [PATCH 1/3] Small code cleaning and moving around We had a bunch of ``if repo_admin`` one after the other, that wasn't quite needed, one is enough. We moved updating the status near the top since that's one action that is allowed for admins and the person who created the ticket. Everything else will only be allowed for admins. While this piece of code could still use a nice refactoring, this already helps a little on readability and logic. --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index d9334c7..b2e9f02 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -251,6 +251,27 @@ def update_issue(repo, issueid, username=None, namespace=None): if message and not is_js: messages.add(message) + # The status field can be updated by bot the admin and the person + # who opened the ticket. + # Update status + if repo_admin or flask.g.fas_user.username == issue.user.user: + if new_status in status: + message = pagure.lib.edit_issue( + SESSION, + issue=issue, + status=new_status, + close_status=close_status, + private=issue.private, + user=flask.g.fas_user.username, + ticketfolder=APP.config['TICKETS_FOLDER'], + ) + SESSION.commit() + if message: + messages.add(message) + + # All the other meta-data can be changed only by admins + # while other field will be missing for non-admin and thus + # reset if we let them if repo_admin: # Adjust (add/remove) tags messages.union(set(pagure.lib.update_tags( @@ -258,11 +279,6 @@ def update_issue(repo, issueid, username=None, namespace=None): username=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'] ))) - - # The meta-data can be changed by admins and issue creator, - # where issue creators can only change status of their issue while - # other fields will be missing for non-admin and thus reset if we let them - if repo_admin: # Assign or update assignee of the ticket message = pagure.lib.add_issue_assignee( SESSION, @@ -275,24 +291,7 @@ def update_issue(repo, issueid, username=None, namespace=None): if message: messages.add(message) - # Update status - if repo_admin or flask.g.fas_user.username == issue.user.user: - if new_status in status: - message = pagure.lib.edit_issue( - SESSION, - issue=issue, - status=new_status, - close_status=close_status, - private=issue.private, - user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'], - ) - SESSION.commit() - if message: - messages.add(message) - # Update priority - if repo_admin: if str(new_priority) in repo.priorities: message = pagure.lib.edit_issue( SESSION, @@ -307,7 +306,6 @@ def update_issue(repo, issueid, username=None, namespace=None): messages.add(message) # Update milestone and privacy setting - if repo_admin: message = pagure.lib.edit_issue( SESSION, issue=issue, @@ -321,7 +319,6 @@ def update_issue(repo, issueid, username=None, namespace=None): messages.add(message) # Update the custom keys/fields - if repo_admin: for key in repo.issue_keys: value = flask.request.form.get(key.name) if value: @@ -341,7 +338,6 @@ def update_issue(repo, issueid, username=None, namespace=None): ) # Update ticket this one depends on - if repo_admin: messages.union(set(pagure.lib.update_dependency_issue( SESSION, repo, issue, depends, username=flask.g.fas_user.username, @@ -349,7 +345,6 @@ def update_issue(repo, issueid, username=None, namespace=None): ))) # Update ticket(s) depending on this one - if repo_admin: messages.union(set(pagure.lib.update_blocked_issue( SESSION, repo, issue, blocks, username=flask.g.fas_user.username, From eb971dbd76241bc026a0c3f69eeeb88ed1688227 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 15 2016 11:38:50 +0000 Subject: [PATCH 2/3] Fix typo in the doc as pointed out by @farhaan --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index b2e9f02..d58a2f8 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -251,8 +251,8 @@ def update_issue(repo, issueid, username=None, namespace=None): if message and not is_js: messages.add(message) - # The status field can be updated by bot the admin and the person - # who opened the ticket. + # The status field can be updated by bot hthe admin and the + # person who opened the ticket. # Update status if repo_admin or flask.g.fas_user.username == issue.user.user: if new_status in status: From 4aaaf1e5f6634be5c7a2c729a899eb4457530dc8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 15 2016 11:45:27 +0000 Subject: [PATCH 3/3] Fix typo in the doc as pointed out by @farhaan - take 2 --- diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index d58a2f8..3e0f94d 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -251,7 +251,7 @@ def update_issue(repo, issueid, username=None, namespace=None): if message and not is_js: messages.add(message) - # The status field can be updated by bot hthe admin and the + # The status field can be updated by both the admin and the # person who opened the ticket. # Update status if repo_admin or flask.g.fas_user.username == issue.user.user: