Re: [PATCH 09 of 14] controllers: changeset: always allow status changes
On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# Date 1541882097 -3600
# Sat Nov 10 21:34:57 2018 +0100
# Node ID cdb00b730e7ababba77c52198c75384075191b33
# Parent 909dc1223bf73fa46beec48fd5bfccb14a9fd17d
controllers: changeset: always allow status changes
Don't disallow status changes on changesets that are part of a pull request
and on which the last status change happened via the pull request.
This odd restriction was already previously highlighted by Mads Kiilerich as
'RLY?' in commit 7834f845505aec3086f525600c81209a40b495ef, so it seems fair
to remove it.
diff --git a/kallithea/controllers/changeset.py
b/kallithea/controllers/changeset.py
--- a/kallithea/controllers/changeset.py
+++ b/kallithea/controllers/changeset.py
@@ -388,20 +388,13 @@ class ChangesetController(BaseRepoContro
# get status if set !
if status:
-# if latest status was from pull request and it's closed
-# disallow changing status ! RLY?
-try:
-ChangesetStatusModel().set_status(
-c.db_repo.repo_id,
-status,
-request.authuser.user_id,
-c.comment,
-revision=revision,
-dont_allow_on_closed_pull_request=True,
-)
-except StatusChangeOnClosedPullRequestError:
-log.debug('cannot change status on %s with closed pull
request', revision)
-raise HTTPBadRequest()
I will also kill StatusChangeOnClosedPullRequestError.
/Mads
+ChangesetStatusModel().set_status(
+c.db_repo.repo_id,
+status,
+request.authuser.user_id,
+c.comment,
+revision=revision,
+)
action_logger(request.authuser,
'user_commented_revision:%s' % revision,
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 06 of 14] controllers: forward pullrequests.delete_comment to changeset
On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote: # HG changeset patch # User Thomas De Schampheleire # Date 1541709517 -3600 # Thu Nov 08 21:38:37 2018 +0100 # Node ID 7350230aa3578c3fc3a4aff864e35a02eea216e1 # Parent 57b9fe6c7871e1ee74eca9c6723ccc4a9dfd5c77 controllers: forward pullrequests.delete_comment to changeset Remove duplication between pullrequests and changeset. We move the code outside ChangesetController to make it callable from PullrequestsController. Note: - instead of keeping the method pullrequests.delete_comment itself and letting it forward to changeset.delete_comment, an alternative solution would have been to change the routing table directly. However, the chosen solution makes it more explicit which operations are supported on each controller. diff --git a/kallithea/controllers/changeset.py b/kallithea/controllers/changeset.py --- a/kallithea/controllers/changeset.py +++ b/kallithea/controllers/changeset.py @@ -187,6 +187,22 @@ def create_comment(text, status, f_path, return comment +def delete_cs_pr_comment(repo_name, comment_id, pr_comment=False): +co = ChangesetComment.get_or_404(comment_id) (It would be nice to have more consistent "default" naming of variables if different types, both in code and tests. "co" for ChangesetComment doesn't seem obvious. "cc" seems like a better mnemonic. (But also, ChangesetComment is perhaps badly named after it also is used for PRs (which also are badly named)...)) +if co.repo.repo_name != repo_name: +raise HTTPNotFound() +if pr_comment and co.pull_request.is_closed(): Do we need this pr_comment flag? Can't we just check co.pull_request? +# don't allow deleting comments on closed pull request +raise HTTPForbidden() /Mads ___ kallithea-general mailing list kallithea-general@sfconservancy.org https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 11 of 14] controllers: pullrequests: comments are always using AJAX
On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# Date 1542401121 -3600
# Fri Nov 16 21:45:21 2018 +0100
# Node ID abef10c5c0d21af567d76d6ffaf5356cc3c3be81
# Parent 92773b07e1244a8b1896cbf2cfb5941189112bdb
controllers: pullrequests: comments are always using AJAX
This is preparation to align commenting on changeset and pullrequests.
diff --git a/kallithea/controllers/pullrequests.py
b/kallithea/controllers/pullrequests.py
--- a/kallithea/controllers/pullrequests.py
+++ b/kallithea/controllers/pullrequests.py
@@ -632,6 +632,7 @@ class PullrequestsController(BaseRepoCon
@HasRepoPermissionLevelDecorator('read')
@jsonify
def comment(self, repo_name, pull_request_id):
+assert request.environ.get('HTTP_X_PARTIAL_XHR')
Assertion checking can be disabled and should thus perhaps not be used
for access control -ish things. But we already do that in many other
places, so ok ...
/Mads
pull_request = PullRequest.get_or_404(pull_request_id)
status = request.POST.get('changeset_status')
@@ -698,9 +699,6 @@ class PullrequestsController(BaseRepoCon
Session().commit()
-if not request.environ.get('HTTP_X_PARTIAL_XHR'):
-raise HTTPFound(location=pull_request.url())
-
data = {
'target_id': h.safeid(h.safe_unicode(request.POST.get('f_path'))),
}
___
kallithea-general mailing list
kallithea-general@sfconservancy.org
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
Re: [PATCH 12 of 14] controllers: align pullrequests.comment with changeset.comment
On 11/20/2018 09:32 PM, Thomas De Schampheleire wrote:
# HG changeset patch
# User Thomas De Schampheleire
# 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()
@HasRepoPermissionLevel
