diff --git a/rhodecode/config/routing.py b/rhodecode/config/routing.py --- a/rhodecode/config/routing.py +++ b/rhodecode/config/routing.py @@ -694,8 +694,8 @@ def make_map(config): requirements=URL_NAME_REQUIREMENTS, jsroute=True) rmap.connect('repo_refs_data', '/{repo_name}/refs-data', - controller='summary', action='repo_refs_data', jsroute=True, - requirements=URL_NAME_REQUIREMENTS) + controller='summary', action='repo_refs_data', + requirements=URL_NAME_REQUIREMENTS, jsroute=True) rmap.connect('repo_refs_changelog_data', '/{repo_name}/refs-data-changelog', controller='summary', action='repo_refs_changelog_data', requirements=URL_NAME_REQUIREMENTS, jsroute=True) @@ -704,9 +704,9 @@ def make_map(config): jsroute=True, requirements=URL_NAME_REQUIREMENTS) rmap.connect('changeset_home', '/{repo_name}/changeset/{revision}', - controller='changeset', revision='tip', jsroute=True, + controller='changeset', revision='tip', conditions={'function': check_repo}, - requirements=URL_NAME_REQUIREMENTS) + requirements=URL_NAME_REQUIREMENTS, jsroute=True) rmap.connect('changeset_children', '/{repo_name}/changeset_children/{revision}', controller='changeset', revision='tip', action='changeset_children', conditions={'function': check_repo}, @@ -923,7 +923,7 @@ def make_map(config): controller='pullrequests', action='show', conditions={'function': check_repo, 'method': ['GET']}, - requirements=URL_NAME_REQUIREMENTS) + requirements=URL_NAME_REQUIREMENTS, jsroute=True) rmap.connect('pullrequest_update', '/{repo_name}/pull-request/{pull_request_id}', diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -904,6 +904,8 @@ class PullrequestsController(BaseRepoCon status = request.POST.get('changeset_status', None) text = request.POST.get('text') comment_type = request.POST.get('comment_type') + resolves_comment_id = request.POST.get('resolves_comment_id') + if status and '_closed' in status: close_pr = True status = status.replace('_closed', '') @@ -936,7 +938,8 @@ class PullrequestsController(BaseRepoCon status_change_type=(status if status and allowed_to_change_status else None), closing_pr=close_pr, - comment_type=comment_type + comment_type=comment_type, + resolves_comment_id=resolves_comment_id ) if allowed_to_change_status: diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -84,9 +84,10 @@ class CommentsModel(BaseModel): return global_renderer def create(self, text, repo, user, commit_id=None, pull_request=None, - f_path=None, line_no=None, status_change=None, comment_type=None, - status_change_type=None, closing_pr=False, - send_email=True, renderer=None): + f_path=None, line_no=None, status_change=None, + status_change_type=None, comment_type=None, + resolves_comment_id=None, closing_pr=False, send_email=True, + renderer=None): """ Creates new comment for commit or pull request. IF status_change is not none this comment is associated with a @@ -113,6 +114,8 @@ class CommentsModel(BaseModel): if not renderer: renderer = self._get_renderer() + repo = self._get_repo(repo) + user = self._get_user(user) schema = comment_schema.CommentSchema() validated_kwargs = schema.deserialize(dict( @@ -121,15 +124,12 @@ class CommentsModel(BaseModel): comment_file=f_path, comment_line=line_no, renderer_type=renderer, - status_change=status_change, - - repo=repo, - user=user, + status_change=status_change_type, + resolves_comment_id=resolves_comment_id, + repo=repo.repo_id, + user=user.user_id, )) - repo = self._get_repo(validated_kwargs['repo']) - user = self._get_user(validated_kwargs['user']) - comment = ChangesetComment() comment.renderer = validated_kwargs['renderer_type'] comment.text = validated_kwargs['comment_body'] @@ -139,6 +139,8 @@ class CommentsModel(BaseModel): comment.repo = repo comment.author = user + comment.resolved_comment = self.__get_commit_comment( + validated_kwargs['resolves_comment_id']) pull_request_id = pull_request diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -2917,7 +2917,7 @@ class ChangesetComment(Base, BaseModel): comment_type = Column('comment_type', Unicode(128), nullable=True, default=COMMENT_TYPE_NOTE) resolved_comment_id = Column('resolved_comment_id', Integer(), ForeignKey('changeset_comments.comment_id'), nullable=True) - resolved_comment = relationship('ChangesetComment', remote_side=comment_id) + resolved_comment = relationship('ChangesetComment', remote_side=comment_id, backref='resolved_by') author = relationship('User', lazy='joined') repo = relationship('Repository') status_change = relationship('ChangesetStatus', cascade="all, delete, delete-orphan") @@ -2959,6 +2959,10 @@ class ChangesetComment(Base, BaseModel): """ return self.outdated and self.pull_request_version_id != version + @property + def resolved(self): + return self.resolved_by[0] if self.resolved_by else None + def get_index_version(self, versions): return self.get_index_from_version( self.pull_request_version_id, versions) diff --git a/rhodecode/model/validation_schema/schemas/comment_schema.py b/rhodecode/model/validation_schema/schemas/comment_schema.py --- a/rhodecode/model/validation_schema/schemas/comment_schema.py +++ b/rhodecode/model/validation_schema/schemas/comment_schema.py @@ -53,18 +53,22 @@ comment_types = ['note', 'todo'] class CommentSchema(colander.MappingSchema): - from rhodecode.model.db import ChangesetComment + from rhodecode.model.db import ChangesetComment, ChangesetStatus comment_body = colander.SchemaNode(colander.String()) comment_type = colander.SchemaNode( colander.String(), - validator=colander.OneOf(ChangesetComment.COMMENT_TYPES)) + validator=colander.OneOf(ChangesetComment.COMMENT_TYPES), + missing=ChangesetComment.COMMENT_TYPE_NOTE) comment_file = colander.SchemaNode(colander.String(), missing=None) comment_line = colander.SchemaNode(colander.String(), missing=None) - status_change = colander.SchemaNode(colander.String(), missing=None) + status_change = colander.SchemaNode( + colander.String(), missing=None, + validator=colander.OneOf([x[0] for x in ChangesetStatus.STATUSES])) renderer_type = colander.SchemaNode(colander.String()) - # do those ? + resolves_comment_id = colander.SchemaNode(colander.Integer(), missing=None) + user = colander.SchemaNode(types.StrOrIntType()) repo = colander.SchemaNode(types.StrOrIntType()) diff --git a/rhodecode/public/css/comments.less b/rhodecode/public/css/comments.less --- a/rhodecode/public/css/comments.less +++ b/rhodecode/public/css/comments.less @@ -66,6 +66,7 @@ tr.inline-comments div { white-space: nowrap; text-transform: uppercase; + min-width: 40px; &.todo { color: @color5; @@ -366,7 +367,7 @@ form.comment-form { display: inline-block; } - .comment-button .comment-button-input { + .comment-button-input { margin-right: 0; } diff --git a/rhodecode/public/js/rhodecode/routes.js b/rhodecode/public/js/rhodecode/routes.js --- a/rhodecode/public/js/rhodecode/routes.js +++ b/rhodecode/public/js/rhodecode/routes.js @@ -37,6 +37,7 @@ function registerRCRoutes() { pyroutes.register('pullrequest', '/%(repo_name)s/pull-request/new', ['repo_name']); pyroutes.register('pullrequest_repo_refs', '/%(repo_name)s/pull-request/refs/%(target_repo_name)s', ['repo_name', 'target_repo_name']); pyroutes.register('pullrequest_repo_destinations', '/%(repo_name)s/pull-request/repo-destinations', ['repo_name']); + pyroutes.register('pullrequest_show', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']); pyroutes.register('pullrequest_update', '/%(repo_name)s/pull-request/%(pull_request_id)s', ['repo_name', 'pull_request_id']); pyroutes.register('pullrequest_show_all', '/%(repo_name)s/pull-request', ['repo_name']); pyroutes.register('pullrequest_comment', '/%(repo_name)s/pull-request-comment/%(pull_request_id)s', ['repo_name', 'pull_request_id']); diff --git a/rhodecode/public/js/src/rhodecode/comments.js b/rhodecode/public/js/src/rhodecode/comments.js --- a/rhodecode/public/js/src/rhodecode/comments.js +++ b/rhodecode/public/js/src/rhodecode/comments.js @@ -79,7 +79,7 @@ var bindToggleButtons = function() { }; var linkifyComments = function(comments) { - /* TODO: dan: remove this - it should no longer needed */ + /* TODO: marcink: remove this - it should no longer needed */ for (var i = 0; i < comments.length; i++) { var comment_id = $(comments[i]).data('comment-id'); var prev_comment_id = $(comments[i - 1]).data('comment-id'); @@ -96,6 +96,8 @@ var linkifyComments = function(comments) $('#next_c_' + comment_id + " a.arrow_comment_link").attr( 'href', '#comment-' + next_comment_id).removeClass('disabled'); } + /* TODO(marcink): end removal here */ + // place a first link to the total counter if (i === 0) { $('#inline-comments-counter').attr('href', '#comment-' + comment_id); @@ -106,10 +108,23 @@ var linkifyComments = function(comments) /* Comment form for main and inline comments */ -var CommentForm = (function() { + +(function(mod) { + if (typeof exports == "object" && typeof module == "object") // CommonJS + module.exports = mod(); + else // Plain browser env + (this || window).CommentForm = mod(); + +})(function() { "use strict"; - function CommentForm(formElement, commitId, pullRequestId, lineNo, initAutocompleteActions) { + function CommentForm(formElement, commitId, pullRequestId, lineNo, initAutocompleteActions, resolvesCommentId) { + if (!(this instanceof CommentForm)) { + return new CommentForm(formElement, commitId, pullRequestId, lineNo, initAutocompleteActions, resolvesCommentId); + } + + // bind the element instance to our Form + $(formElement).get(0).CommentForm = this; this.withLineNo = function(selector) { var lineNo = this.lineNo; @@ -135,6 +150,9 @@ var CommentForm = (function() { this.cancelButton = this.withLineNo('#cancel-btn'); this.commentType = this.withLineNo('#comment_type'); + this.resolvesId = null; + this.resolvesActionId = null; + this.cmBox = this.withLineNo('#text'); this.cm = initCommentBoxCodeMirror(this.cmBox, this.initAutocompleteActions); @@ -147,17 +165,38 @@ var CommentForm = (function() { this.previewUrl = pyroutes.url('changeset_comment_preview', {'repo_name': templateContext.repo_name}); - // based on commitId, or pullReuqestId decide where do we submit + if (resolvesCommentId){ + this.resolvesId = '#resolve_comment_{0}'.format(resolvesCommentId); + this.resolvesActionId = '#resolve_comment_action_{0}'.format(resolvesCommentId); + $(this.commentType).prop('disabled', true); + $(this.commentType).addClass('disabled'); + + var resolvedInfo = ( + '
  • ' + + '' + + '' + + '
  • ' + ).format(resolvesCommentId, _gettext('resolve comment')); + $(resolvedInfo).insertAfter($(this.commentType).parent()); + } + + // based on commitId, or pullRequestId decide where do we submit // out data if (this.commitId){ this.submitUrl = pyroutes.url('changeset_comment', {'repo_name': templateContext.repo_name, 'revision': this.commitId}); + this.selfUrl = pyroutes.url('changeset_home', + {'repo_name': templateContext.repo_name, + 'revision': this.commitId}); } else if (this.pullRequestId) { this.submitUrl = pyroutes.url('pullrequest_comment', {'repo_name': templateContext.repo_name, 'pull_request_id': this.pullRequestId}); + this.selfUrl = pyroutes.url('pullrequest_show', + {'repo_name': templateContext.repo_name, + 'pull_request_id': this.pullRequestId}); } else { throw new Error( @@ -168,6 +207,13 @@ var CommentForm = (function() { return this.cm }; + this.setPlaceholder = function(placeholder) { + var cm = this.getCmInstance(); + if (cm){ + cm.setOption('placeholder', placeholder); + } + }; + var self = this; this.getCommentStatus = function() { @@ -176,6 +222,15 @@ var CommentForm = (function() { this.getCommentType = function() { return $(this.submitForm).find(this.commentType).val(); }; + + this.getResolvesId = function() { + return $(this.submitForm).find(this.resolvesId).val() || null; + }; + this.markCommentResolved = function(resolvedCommentId){ + $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolved').show(); + $('#comment-label-{0}'.format(resolvedCommentId)).find('.resolve').hide(); + }; + this.isAllowedToSubmit = function() { return !$(this.submitButton).prop('disabled'); }; @@ -205,12 +260,12 @@ var CommentForm = (function() { }); $(this.submitForm).find(this.statusChange).on('change', function() { var status = self.getCommentStatus(); - if (status && !self.lineNo) { + if (status && self.lineNo == 'general') { $(self.submitButton).prop('disabled', false); } - //todo, fix this name + var placeholderText = _gettext('Comment text will be set automatically based on currently selected status ({0}) ...').format(status); - self.cm.setOption('placeholder', placeholderText); + self.setPlaceholder(placeholderText) }) }; @@ -260,6 +315,7 @@ var CommentForm = (function() { var text = self.cm.getValue(); var status = self.getCommentStatus(); var commentType = self.getCommentType(); + var resolvesCommentId = self.getResolvesId(); if (text === "" && !status) { return; @@ -275,7 +331,9 @@ var CommentForm = (function() { 'comment_type': commentType, 'csrf_token': CSRF_TOKEN }; - + if (resolvesCommentId){ + postData['resolves_comment_id'] = resolvesCommentId; + } var submitSuccessCallback = function(o) { if (status) { location.reload(true); @@ -284,10 +342,15 @@ var CommentForm = (function() { self.resetCommentFormState(); bindDeleteCommentButtons(); timeagoActivate(); + + //mark visually which comment was resolved + if (resolvesCommentId) { + this.markCommentResolved(resolvesCommentId); + } } }; var submitFailCallback = function(){ - self.resetCommentFormState(text) + self.resetCommentFormState(text); }; self.submitAjaxPOST( self.submitUrl, postData, submitSuccessCallback, submitFailCallback); @@ -367,7 +430,7 @@ var CommentForm = (function() { var postData = { 'text': text, - 'renderer': DEFAULT_RENDERER, + 'renderer': templateContext.visual.default_renderer, 'csrf_token': CSRF_TOKEN }; @@ -404,15 +467,17 @@ var CommentForm = (function() { } return CommentForm; -})(); +}); -var CommentsController = function() { /* comments controller */ +/* comments controller */ +var CommentsController = function() { + var mainComment = '#text'; var self = this; this.cancelComment = function(node) { var $node = $(node); var $td = $node.closest('td'); - $node.closest('.comment-inline-form').removeClass('comment-inline-form-open'); + $node.closest('.comment-inline-form').remove(); return false; }; @@ -530,7 +595,8 @@ var CommentsController = function() { /* $node.closest('tr').toggleClass('hide-line-comments'); }; - this.createComment = function(node) { + this.createComment = function(node, resolutionComment) { + var resolvesCommentId = resolutionComment || null; var $node = $(node); var $td = $node.closest('td'); var $form = $td.find('.comment-inline-form'); @@ -556,14 +622,16 @@ var CommentsController = function() { /* var pullRequestId = templateContext.pull_request_data.pull_request_id; var commitId = templateContext.commit_data.commit_id; - var _form = $form[0]; - var commentForm = new CommentForm(_form, commitId, pullRequestId, lineno, false); + var _form = $($form[0]).find('form'); + + var commentForm = new CommentForm(_form, commitId, pullRequestId, lineno, false, resolvesCommentId); var cm = commentForm.getCmInstance(); // set a CUSTOM submit handler for inline comments. commentForm.setHandleFormSubmit(function(o) { var text = commentForm.cm.getValue(); var commentType = commentForm.getCommentType(); + var resolvesCommentId = commentForm.getResolvesId(); if (text === "") { return; @@ -589,6 +657,10 @@ var CommentsController = function() { /* 'comment_type': commentType, 'csrf_token': CSRF_TOKEN }; + if (resolvesCommentId){ + postData['resolves_comment_id'] = resolvesCommentId; + } + var submitSuccessCallback = function(json_data) { $form.remove(); try { @@ -598,6 +670,11 @@ var CommentsController = function() { /* $comments.find('.cb-comment-add-button').before(html); + //mark visually which comment was resolved + if (resolvesCommentId) { + commentForm.markCommentResolved(resolvesCommentId); + } + } catch (e) { console.error(e); } @@ -616,26 +693,81 @@ var CommentsController = function() { /* commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback); }); + if (resolvesCommentId){ + var placeholderText = _gettext('Leave a comment, or click resolve button to resolve TODO comment #{0}').format(resolvesCommentId); + + } else { + var placeholderText = _gettext('Leave a comment on line {0}.').format(lineno); + } + setTimeout(function() { // callbacks if (cm !== undefined) { - cm.setOption('placeholder', _gettext('Leave a comment on line {0}.').format(lineno)); + commentForm.setPlaceholder(placeholderText); cm.focus(); cm.refresh(); } }, 10); - $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({ - form: _form, - parent: $td[0], - lineno: lineno, - f_path: f_path} - ); + $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({ + form: _form, + parent: $td[0], + lineno: lineno, + f_path: f_path} + ); + + // trigger hash + if (resolvesCommentId){ + var resolveAction = $(commentForm.resolvesActionId); + setTimeout(function() { + $('body, html').animate({ scrollTop: resolveAction.offset().top }, 10); + }, 100); + } } $form.addClass('comment-inline-form-open'); }; + this.createResolutionComment = function(commentId){ + // hide the trigger text + $('#resolve-comment-{0}'.format(commentId)).hide(); + + var comment = $('#comment-'+commentId); + var commentData = comment.data(); + + if (commentData.commentInline) { + var resolutionComment = true; + this.createComment(comment, commentId) + } else { + + this.createComment(comment, commentId) + + console.log('TODO') + console.log(commentId) + } + + return false; + }; + + this.submitResolution = function(commentId){ + var form = $('#resolve_comment_{0}'.format(commentId)).closest('form'); + var commentForm = form.get(0).CommentForm; + + var cm = commentForm.getCmInstance(); + var renderer = templateContext.visual.default_renderer; + if (renderer == 'rst'){ + var commentUrl = '`#{0} <{1}#comment-{0}>`_'.format(commentId, commentForm.selfUrl); + } else if (renderer == 'markdown') { + var commentUrl = '[#{0}]({1}#comment-{0})'.format(commentId, commentForm.selfUrl); + } else { + var commentUrl = '{1}#comment-{0}'.format(commentId, commentForm.selfUrl); + } + + cm.setValue(_gettext('TODO from comment {0} was fixed.').format(commentUrl)); + form.submit(); + return false; + }; + this.renderInlineComments = function(file_comments) { show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true; diff --git a/rhodecode/templates/changeset/changeset_file_comment.mako b/rhodecode/templates/changeset/changeset_file_comment.mako --- a/rhodecode/templates/changeset/changeset_file_comment.mako +++ b/rhodecode/templates/changeset/changeset_file_comment.mako @@ -15,12 +15,33 @@ id="comment-${comment.comment_id}" line="${comment.line_no}" data-comment-id="${comment.comment_id}" + data-comment-type="${comment.comment_type}" + data-comment-inline=${h.json.dumps(inline)} style="${'display: none;' if outdated_at_ver else ''}">
    -
    -
    - ${comment.comment_type or 'note'} +
    +
    + % if comment.comment_type == 'todo': + % if comment.resolved: + + % else: + +
    + ${comment.comment_type} +
    + % endif + % else: + % if comment.resolved_comment: + fix + % else: + ${comment.comment_type or 'note'} + % endif + % endif
    @@ -120,6 +141,7 @@
    + ## generate main comments <%def name="generate_comments(include_pull_request=False, is_pull_request=False)">
    @@ -137,16 +159,10 @@
    -## MAIN COMMENT FORM + <%def name="comments(post_url, cur_status, is_pull_request=False, is_compare=False, change_status=True, form_extras=None)"> -%if is_compare: - <% form_id = "comments_form_compare" %> -%else: - <% form_id = "comments_form" %> -%endif - - +## merge status, and merge action %if is_pull_request:
    %if c.allowed_to_merge: @@ -168,6 +184,7 @@ %endif
    %endif +
    <% if is_pull_request: @@ -177,78 +194,28 @@ else: placeholder = _('Leave a comment on this Commit.') %> + % if c.rhodecode_user.username != h.DEFAULT_USER:
    - ${h.secure_form(post_url, id_=form_id)} -
    -
    - -
    - -
    -
    - -
    - -
    + ## inject form here + ${comment_form(form_type='general', form_id='general_comment', lineno_id='general', review_statuses=c.commit_statuses, form_extras=form_extras)} +
    + -
    - %if form_extras and isinstance(form_extras, (list, tuple)): - % for form_ex_el in form_extras: - ${form_ex_el|n} - % endfor - %endif -
    - - ${h.end_form()} -
    + % else: + ## form state when not logged in
    @@ -289,22 +256,98 @@
    % endif +
    + + + +<%def name="comment_form(form_type, form_id='', lineno_id='{1}', review_statuses=None, form_extras=None)"> + ## comment injected based on assumption that user is logged in + +
    - - + ## inject extra inputs into the form + % if form_extras and isinstance(form_extras, (list, tuple)): +
    + % for form_ex_el in form_extras: + ${form_ex_el|n} + % endfor +
    + % endif + +
    + ## inline for has a file, and line-number together with cancel hide button. + % if form_type == 'inline': + + + + % endif + ${h.submit('save', _('Comment'), class_='btn btn-success comment-button-input')} + +
    +
    + + + + \ No newline at end of file diff --git a/rhodecode/templates/codeblocks/diffs.mako b/rhodecode/templates/codeblocks/diffs.mako --- a/rhodecode/templates/codeblocks/diffs.mako +++ b/rhodecode/templates/codeblocks/diffs.mako @@ -1,3 +1,5 @@ +<%namespace name="commentblock" file="/changeset/changeset_file_comment.mako"/> + <%def name="diff_line_anchor(filename, line, type)"><% return '%s_%s_%i' % (h.safeid(filename), type, line) %> @@ -60,59 +62,8 @@ return h.url('', **new_args)
    %if c.rhodecode_user.username != h.DEFAULT_USER: - ${h.form('#', method='get')} -
    -
    - -
    - -
    -
    - -
    - -
    - - -
    - - + ## render template for inline comments + ${commentblock.comment_form(form_type='inline')} %else: ${h.form('', class_='inline-form comment-form-login', method='get')}
    @@ -521,7 +472,6 @@ from rhodecode.lib.diffs import NEW_FILE -<%namespace name="commentblock" file="/changeset/changeset_file_comment.mako"/> <%def name="inline_comments_container(comments)">
    %for comment in comments: