On 04/14/2015 06:03 PM, Jan Heylen wrote:
# HG changeset patch
# User Jan Heylen <[email protected]>
# Date 1427526729 -3600
# Sat Mar 28 08:12:09 2015 +0100
# Node ID f26b18234a7731a361f75e7a68ad143fc78218bf
# Parent f48bc03627761dd62f7e7094bb0d322834da336c
changeset: draft comment on changesets
This doesn't include any template changes at all so I doubt that
description is exact?
diff -r f48bc0362776 -r f26b18234a77 kallithea/controllers/changeset.py
--- a/kallithea/controllers/changeset.py Mon Mar 30 19:13:08 2015 +0200
+++ b/kallithea/controllers/changeset.py Sat Mar 28 08:12:09 2015 +0100
@@ -222,11 +222,14 @@
comments = dict()
c.statuses = []
c.inline_comments = []
+ c.drafts = []
c.inline_cnt = 0
+ c.draft_cnt = 0
# Iterate over ranges (default changeset view is always one changeset)
for changeset in c.cs_ranges:
inlines = []
+ drafts = []
if method == 'show':
c.statuses.extend([ChangesetStatusModel().get_status(
c.db_repo.repo_id, changeset.raw_id)])
@@ -247,6 +250,11 @@
.get_inline_comments(c.db_repo.repo_id,
revision=changeset.raw_id)
c.inline_comments.extend(inlines)
+ drafts = ChangesetCommentsModel()\
+ .get_inline_drafts(c.db_repo.repo_id,
+ revision=changeset.raw_id,
+ user_id=c.authuser.user_id)
+ c.drafts.extend(drafts)
c.changes[changeset.raw_id] = []
@@ -291,6 +299,11 @@
for comments in lines.values():
c.inline_cnt += len(comments)
+ # count draft comments
+ for __, lines in c.drafts:
I like to use a _ prefix for unused values but please give it a name
that gives a hint what it is we don't use.
+ for drafts in lines.values():
+ c.draft_cnt += len(drafts)
This extra iteration is inefficient. Let's hope it doesn't touch
unloaded objects where it would have to touch the db for each iteration.
Unfortunately that is how we do it ...
+
if len(c.cs_ranges) == 1:
c.changeset = c.cs_ranges[0]
c.parent_tmpl = ''.join(['# Parent %s\n' % x.raw_id
@@ -347,6 +360,39 @@
@HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
'repository.admin')
@jsonify
+ def draft(self, repo_name, revision=None, pull_request_id=None):
Please make sure new code is better than the existing code. Add
docstrings that explains the intention with the function. That makes it
much easier to review.
Please also consider adding test coverage for this new functionality.
+ text = request.POST.get('text', '').strip() or _('No comments.')
+ c.co = comm = ChangesetCommentsModel().create(
These two variable names are not pretty. I guess the excuse/explanation
is that it is a cut'n'paste of existing code. But that is not really a
good excuse/explanation ;-)
Could the existing code perhaps be cleaned up and reused?
+ text=text,
+ repo=c.db_repo.repo_id,
+ user=c.authuser.user_id,
+ revision=revision,
+ pull_request=pull_request_id,
+ f_path=request.POST.get('f_path'),
+ line_no=request.POST.get('line'),
+ status_change=None,
+ draft=True
+ )
+
+ Session().commit()
+
+ #only ajax below
+ data = {
+ 'target_id': h.safeid(h.safe_unicode(request.POST.get('f_path'))),
+ }
+ if comm:
+ data.update(comm.get_dict())
+ data.update({'rendered_text':
+ render('changeset/changeset_comment_block.html')})
+
+ return data
+
+
+ @LoginRequired()
+ @NotAnonymous()
+ @HasRepoPermissionAnyDecorator('repository.read', 'repository.write',
+ 'repository.admin')
+ @jsonify
def comment(self, repo_name, revision):
status = request.POST.get('changeset_status')
text = request.POST.get('text', '').strip() or _('No comments.')
@@ -359,7 +405,8 @@
f_path=request.POST.get('f_path'),
line_no=request.POST.get('line'),
status_change=(ChangesetStatus.get_status_lbl(status)
- if status else None)
+ if status else None),
+ draft=True
It is always nice to leave a comma after the last entry of a list (if at
the end of the line) - that makes it easier to add and review more
entries later on.
)
/Mads
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
