On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire <[email protected]>
# Date 1542402570 -3600
# Fri Nov 16 22:09:30 2018 +0100
# Node ID 4e6bfd3650ddfaf9d2bfbb166eaaec15f5194135
# Parent abef10c5c0d21af567d76d6ffaf5356cc3c3be81
controllers: align pullrequests.comment with changeset.comment
This commit purely serves to highlight the differences.
The subsequent commit will remove the duplication.
diff --git a/kallithea/controllers/changeset.py
b/kallithea/controllers/changeset.py
--- a/kallithea/controllers/changeset.py
+++ b/kallithea/controllers/changeset.py
@@ -365,48 +365,87 @@ class ChangesetController(BaseRepoContro
@LoginRequired()
@HasRepoPermissionLevelDecorator('read')
@jsonify
- def comment(self, repo_name, revision):
+ def comment(self, repo_name, revision, pull_request_id=None,
allowed_to_change_status=True):
(When extracting this as function later on, it would be nice to get a
docstring.)
assert request.environ.get('HTTP_X_PARTIAL_XHR')
+ if pull_request_id is not None:
+ pull_request = PullRequest.get_or_404(pull_request_id)
+ else:
+ pull_request = None
+
status = request.POST.get('changeset_status')
+ close_pr = request.POST.get('save_close')
+ delete = request.POST.get('save_delete')
f_path = request.POST.get('f_path')
line_no = request.POST.get('line')
- if status and (f_path or line_no):
- # status votes are only possible in general comments
+ if (status or close_pr or delete) and (f_path or line_no):
+ # status votes and closing is only possible in general comments
raise HTTPBadRequest()
+ if not allowed_to_change_status:
+ if status or close_pr:
+ h.flash(_('No permission to change status'), 'error')
+ raise HTTPForbidden()
+
+ if pull_request and delete == "delete":
+ if (pull_request.owner_id == request.authuser.user_id or
+ h.HasPermissionAny('hg.admin')() or
+
h.HasRepoPermissionLevel('admin')(pull_request.org_repo.repo_name) or
+
h.HasRepoPermissionLevel('admin')(pull_request.other_repo.repo_name)
+ ) and not pull_request.is_closed():
+ PullRequestModel().delete(pull_request)
+ Session().commit()
+ h.flash(_('Successfully deleted pull request %s') %
pull_request_id,
+ category='success')
+ return {
+ 'location': url('my_pullrequests'), # or repo pr list?
+ }
+ raise HTTPFound(location=url('my_pullrequests')) # or repo pr
list?
+ raise HTTPForbidden()
+
text = request.POST.get('text', '').strip()
- c.comment = create_comment(
+ comment = create_comment(
text,
status,
revision=revision,
+ pull_request_id=pull_request_id,
f_path=f_path,
line_no=line_no,
+ closing_pr=close_pr,
)
- # get status if set !
if status:
ChangesetStatusModel().set_status(
c.db_repo.repo_id,
status,
request.authuser.user_id,
- c.comment,
+ comment,
revision=revision,
+ pull_request=pull_request_id,
)
- action_logger(request.authuser,
- 'user_commented_revision:%s' % revision,
- c.db_repo, request.ip_addr)
+ if pull_request:
+ action = 'user_closed_pull_request:%s' % pull_request_id
+ else:
+ action = 'user_commented_revision:%s' % revision
+ action_logger(request.authuser, action, c.db_repo, request.ip_addr)
+
+ if pull_request and close_pr:
+ PullRequestModel().close_pull_request(pull_request_id)
+ action_logger(request.authuser,
+ 'user_closed_pull_request:%s' % pull_request_id,
+ c.db_repo, request.ip_addr)
Session().commit()
data = {
'target_id': h.safeid(h.safe_unicode(request.POST.get('f_path'))),
}
- if c.comment is not None:
- data.update(c.comment.get_dict())
+ if comment is not None:
+ c.comment = comment
+ data.update(comment.get_dict())
data.update({'rendered_text':
render('changeset/changeset_comment_block.html')})
diff --git a/kallithea/controllers/pullrequests.py b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -631,9 +631,13 @@ class PullrequestsController(BaseRepoCon
@LoginRequired()
@HasRepoPermissionLevelDecorator('read')
@jsonify
- def comment(self, repo_name, pull_request_id):
+ def comment(self, repo_name, pull_request_id, revision=None):
Something is fishy about adding this unused parameter and not removing
it again.
assert request.environ.get('HTTP_X_PARTIAL_XHR')
- pull_request = PullRequest.get_or_404(pull_request_id)
+
+ if pull_request_id is not None:
+ pull_request = PullRequest.get_or_404(pull_request_id)
+ else:
+ pull_request = None
status = request.POST.get('changeset_status')
close_pr = request.POST.get('save_close')
@@ -648,10 +652,10 @@ class PullrequestsController(BaseRepoCon
allowed_to_change_status =
self._is_allowed_to_change_status(pull_request)
if not allowed_to_change_status:
if status or close_pr:
- h.flash(_('No permission to change pull request status'),
'error')
+ h.flash(_('No permission to change status'), 'error')
raise HTTPForbidden()
- if delete == "delete":
+ if pull_request and delete == "delete":
(Generally, I prefer to use the slightly more verbose "is not None"
instead of relying on falsiness of None ... and thus the assumption that
objects never are "false" in other ways. But I'm also lazy and would
perhaps do it this way ...)
if (pull_request.owner_id == request.authuser.user_id or
h.HasPermissionAny('hg.admin')() or
h.HasRepoPermissionLevel('admin')(pull_request.org_repo.repo_name) or
@@ -672,26 +676,30 @@ class PullrequestsController(BaseRepoCon
comment = create_comment(
text,
status,
+ revision=revision,
pull_request_id=pull_request_id,
f_path=f_path,
line_no=line_no,
closing_pr=close_pr,
)
- action_logger(request.authuser,
- 'user_commented_pull_request:%s' % pull_request_id,
- c.db_repo, request.ip_addr)
-
if status:
ChangesetStatusModel().set_status(
c.db_repo.repo_id,
status,
request.authuser.user_id,
comment,
- pull_request=pull_request_id
+ revision=revision,
+ pull_request=pull_request_id,
)
- if close_pr:
+ if pull_request:
+ action = 'user_closed_pull_request:%s' % pull_request_id
'user_commented_pull_request' ... so apparently no coverage of this
logging ... but no big deal ...
+ else:
+ action = 'user_commented_revision:%s' % revision
+ action_logger(request.authuser, action, c.db_repo, request.ip_addr)
+
+ if pull_request and close_pr:
PullRequestModel().close_pull_request(pull_request_id)
action_logger(request.authuser,
'user_closed_pull_request:%s' % pull_request_id,
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
