diff --git a/rhodecode/apps/repository/tests/test_repo_commit_comments.py b/rhodecode/apps/repository/tests/test_repo_commit_comments.py --- a/rhodecode/apps/repository/tests/test_repo_commit_comments.py +++ b/rhodecode/apps/repository/tests/test_repo_commit_comments.py @@ -378,9 +378,9 @@ class TestRepoCommitCommentsView(TestCon 'text': test_text_v2, 'version': '0', }, - status=404, + status=409, ) - assert response.status_int == 404 + assert response.status_int == 409 text_form_db = ChangesetComment.query().filter( ChangesetComment.comment_id == comment_id).first().text diff --git a/rhodecode/apps/repository/tests/test_repo_pullrequests.py b/rhodecode/apps/repository/tests/test_repo_pullrequests.py --- a/rhodecode/apps/repository/tests/test_repo_pullrequests.py +++ b/rhodecode/apps/repository/tests/test_repo_pullrequests.py @@ -480,9 +480,7 @@ class TestPullrequestsView(object): ) assert response.status_int == 404 - def test_comment_and_try_edit_already_edited( - self, pr_util, csrf_token, xhr_header - ): + def test_comment_and_try_edit_already_edited(self, pr_util, csrf_token, xhr_header): pull_request = pr_util.create_pull_request() response = self.app.post( route_path( @@ -498,8 +496,9 @@ class TestPullrequestsView(object): assert response.json comment_id = response.json.get('comment_id', None) assert comment_id + test_text = 'test' - response = self.app.post( + self.app.post( route_path( 'pullrequest_comment_edit', repo_name=pull_request.target_repo.scm_instance().name, @@ -528,9 +527,9 @@ class TestPullrequestsView(object): 'text': test_text_v2, 'version': '0', }, - status=404, + status=409, ) - assert response.status_int == 404 + assert response.status_int == 409 text_form_db = ChangesetComment.query().filter( ChangesetComment.comment_id == comment_id).first().text diff --git a/rhodecode/apps/repository/views/repo_commits.py b/rhodecode/apps/repository/views/repo_commits.py --- a/rhodecode/apps/repository/views/repo_commits.py +++ b/rhodecode/apps/repository/views/repo_commits.py @@ -20,9 +20,9 @@ import logging -import collections -from pyramid.httpexceptions import HTTPNotFound, HTTPBadRequest, HTTPFound, HTTPForbidden +from pyramid.httpexceptions import ( + HTTPNotFound, HTTPBadRequest, HTTPFound, HTTPForbidden, HTTPConflict) from pyramid.view import view_config from pyramid.renderers import render from pyramid.response import Response @@ -39,7 +39,7 @@ from rhodecode.lib.compat import Ordered from rhodecode.lib.diffs import ( cache_diff, load_cached_diff, diff_cache_exist, get_diff_context, get_diff_whitespace_flag) -from rhodecode.lib.exceptions import StatusChangeOnClosedPullRequestError +from rhodecode.lib.exceptions import StatusChangeOnClosedPullRequestError, CommentVersionMismatch import rhodecode.lib.helpers as h from rhodecode.lib.utils2 import safe_unicode, str2bool from rhodecode.lib.vcs.backends.base import EmptyCommit @@ -595,6 +595,8 @@ class RepoCommitsView(RepoAppView): route_name='repo_commit_comment_edit', request_method='POST', renderer='json_ext') def repo_commit_comment_edit(self): + self.load_default_context() + comment_id = self.request.matchdict['comment_id'] comment = ChangesetComment.get_or_404(comment_id) @@ -615,11 +617,12 @@ class RepoCommitsView(RepoAppView): log.warning( 'Comment(repo): ' 'Trying to create new version ' - 'of existing comment {}'.format( + 'with the same comment body {}'.format( comment_id, ) ) raise HTTPNotFound() + if version.isdigit(): version = int(version) else: @@ -633,19 +636,28 @@ class RepoCommitsView(RepoAppView): ) raise HTTPNotFound() - comment_history = CommentsModel().edit( - comment_id=comment_id, - text=text, - auth_user=self._rhodecode_user, - version=version, - ) + try: + comment_history = CommentsModel().edit( + comment_id=comment_id, + text=text, + auth_user=self._rhodecode_user, + version=version, + ) + except CommentVersionMismatch: + raise HTTPConflict() + if not comment_history: raise HTTPNotFound() + Session().commit() return { 'comment_history_id': comment_history.comment_history_id, 'comment_id': comment.comment_id, 'comment_version': comment_history.version, + 'comment_author_username': comment_history.author.username, + 'comment_author_gravatar': h.gravatar_url(comment_history.author.email, 16), + 'comment_created_on': h.age_component(comment_history.created_on, + time_is_local=True), } else: log.warning('No permissions for user %s to edit comment_id: %s', diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -25,7 +25,7 @@ import formencode import formencode.htmlfill import peppercorn from pyramid.httpexceptions import ( - HTTPFound, HTTPNotFound, HTTPForbidden, HTTPBadRequest) + HTTPFound, HTTPNotFound, HTTPForbidden, HTTPBadRequest, HTTPConflict) from pyramid.view import view_config from pyramid.renderers import render @@ -34,6 +34,7 @@ from rhodecode.apps._base import RepoApp from rhodecode.lib import helpers as h, diffs, codeblocks, channelstream from rhodecode.lib.base import vcs_operation_context from rhodecode.lib.diffs import load_cached_diff, cache_diff, diff_cache_exist +from rhodecode.lib.exceptions import CommentVersionMismatch from rhodecode.lib.ext_json import json from rhodecode.lib.auth import ( LoginRequired, HasRepoPermissionAny, HasRepoPermissionAnyDecorator, @@ -1528,6 +1529,8 @@ class RepoPullRequestsView(RepoAppView, route_name='pullrequest_comment_edit', request_method='POST', renderer='json_ext') def pull_request_comment_edit(self): + self.load_default_context() + pull_request = PullRequest.get_or_404( self.request.matchdict['pull_request_id'] ) @@ -1566,11 +1569,12 @@ class RepoPullRequestsView(RepoAppView, log.warning( 'Comment(PR): ' 'Trying to create new version ' - 'of existing comment {}'.format( + 'with the same comment body {}'.format( comment_id, ) ) raise HTTPNotFound() + if version.isdigit(): version = int(version) else: @@ -1584,24 +1588,30 @@ class RepoPullRequestsView(RepoAppView, ) raise HTTPNotFound() - comment_history = CommentsModel().edit( - comment_id=comment_id, - text=text, - auth_user=self._rhodecode_user, - version=version, - ) + try: + comment_history = CommentsModel().edit( + comment_id=comment_id, + text=text, + auth_user=self._rhodecode_user, + version=version, + ) + except CommentVersionMismatch: + raise HTTPConflict() + if not comment_history: raise HTTPNotFound() + Session().commit() return { 'comment_history_id': comment_history.comment_history_id, 'comment_id': comment.comment_id, 'comment_version': comment_history.version, + 'comment_author_username': comment_history.author.username, + 'comment_author_gravatar': h.gravatar_url(comment_history.author.email, 16), + 'comment_created_on': h.age_component(comment_history.created_on, + time_is_local=True), } else: - log.warning( - 'No permissions for user {} to edit comment_id: {}'.format( - self._rhodecode_db_user, comment_id - ) - ) + log.warning('No permissions for user %s to edit comment_id: %s', + self._rhodecode_db_user, comment_id) raise HTTPNotFound() diff --git a/rhodecode/lib/exceptions.py b/rhodecode/lib/exceptions.py --- a/rhodecode/lib/exceptions.py +++ b/rhodecode/lib/exceptions.py @@ -177,3 +177,7 @@ class ArtifactMetadataDuplicate(ValueErr class ArtifactMetadataBadValueType(ValueError): pass + + +class CommentVersionMismatch(ValueError): + pass diff --git a/rhodecode/lib/helpers.py b/rhodecode/lib/helpers.py --- a/rhodecode/lib/helpers.py +++ b/rhodecode/lib/helpers.py @@ -24,6 +24,7 @@ Helper functions Consists of functions to typically be used within templates, but also available to Controllers. This module is available to both as 'h'. """ +import base64 import os import random @@ -1352,7 +1353,7 @@ class InitialsGravatar(object): def generate_svg(self, svg_type=None): img_data = self.get_img_data(svg_type) - return "data:image/svg+xml;base64,%s" % img_data.encode('base64') + return "data:image/svg+xml;base64,%s" % base64.b64encode(img_data) def initials_gravatar(email_address, first_name, last_name, size=30): diff --git a/rhodecode/model/comment.py b/rhodecode/model/comment.py --- a/rhodecode/model/comment.py +++ b/rhodecode/model/comment.py @@ -33,6 +33,7 @@ from sqlalchemy.sql.functions import coa from rhodecode.lib import helpers as h, diffs, channelstream, hooks_utils from rhodecode.lib import audit_logger +from rhodecode.lib.exceptions import CommentVersionMismatch from rhodecode.lib.utils2 import extract_mentioned_users, safe_str from rhodecode.model import BaseModel from rhodecode.model.db import ( @@ -507,13 +508,13 @@ class CommentsModel(BaseModel): comment_version = ChangesetCommentHistory.get_version(comment_id) if (comment_version - version) != 1: log.warning( - 'Version mismatch, skipping... ' - 'version {} but should be {}'.format( - (version - 1), + 'Version mismatch comment_version {} submitted {}, skipping'.format( comment_version, + version ) ) - return + raise CommentVersionMismatch() + comment_history = ChangesetCommentHistory() comment_history.comment_id = comment_id comment_history.version = comment_version diff --git a/rhodecode/public/css/select2.less b/rhodecode/public/css/select2.less --- a/rhodecode/public/css/select2.less +++ b/rhodecode/public/css/select2.less @@ -148,6 +148,38 @@ select.select2{height:28px;visibility:hi margin: 0; } + +.drop-menu-comment-history { + .drop-menu-core; + border: none; + padding: 0 6px 0 0; + width: auto; + min-width: 0; + margin: 0; + position: relative; + display: inline-block; + line-height: 1em; + z-index: 2; + cursor: pointer; + + a { + display:block; + padding: 0; + position: relative; + + &:after { + position: absolute; + content: "\00A0\25BE"; + right: -0.80em; + line-height: 1em; + top: -0.20em; + width: 1em; + font-size: 16px; + } + } + +} + .field-sm .drop-menu { padding: 1px 0 0 0; a { 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 @@ -496,6 +496,43 @@ var _submitAjaxPOST = function(url, post return CommentForm; }); +/* selector for comment versions */ +var initVersionSelector = function(selector, initialData) { + + var formatResult = function(result, container, query, escapeMarkup) { + + return renderTemplate('commentVersion', { + show_disabled: true, + version: result.comment_version, + user_name: result.comment_author_username, + gravatar_url: result.comment_author_gravatar, + size: 16, + timeago_component: result.comment_created_on, + }) + }; + + $(selector).select2({ + placeholder: "Edited", + containerCssClass: "drop-menu-comment-history", + dropdownCssClass: "drop-menu-dropdown", + dropdownAutoWidth: true, + minimumResultsForSearch: -1, + data: initialData, + formatResult: formatResult, + }); + + $(selector).on('select2-selecting', function (e) { + // hide the mast as we later do preventDefault() + $("#select2-drop-mask").click(); + e.preventDefault(); + e.choice.action(); + }); + + $(selector).on("select2-open", function() { + timeagoActivate(); + }); +}; + /* comments controller */ var CommentsController = function() { var mainComment = '#text'; @@ -521,21 +558,8 @@ var CommentsController = function() { return false; }; - this.showVersion = function (node) { - var $node = $(node); - var selectedIndex = $node.context.selectedIndex; - var option = $node.find('option[value="'+ selectedIndex +'"]'); - var zero_option = $node.find('option[value="0"]'); - if (!option){ - return; - } + this.showVersion = function (comment_id, comment_history_id) { - // little trick to cheat onchange and allow to display the same version again - $node.context.selectedIndex = 0; - zero_option.text(selectedIndex); - - var comment_history_id = option.attr('data-comment-history-id'); - var comment_id = option.attr('data-comment-id'); var historyViewUrl = pyroutes.url( 'repo_commit_comment_history_view', { @@ -557,7 +581,8 @@ var CommentsController = function() { }); }; _submitAjaxPOST( - historyViewUrl, {'csrf_token': CSRF_TOKEN}, successRenderCommit, + historyViewUrl, {'csrf_token': CSRF_TOKEN}, + successRenderCommit, failRenderCommit ); }; @@ -863,6 +888,7 @@ var CommentsController = function() { return commentForm; }; + this.editComment = function(node) { var $node = $(node); var $comment = $(node).closest('.comment'); @@ -917,15 +943,16 @@ var CommentsController = function() { lineno: lineno, f_path: f_path} ); + // 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; } + if (old_comment_text == text) { SwalNoAnimation.fire({ title: 'Unable to edit comment', @@ -937,19 +964,22 @@ var CommentsController = function() { var submitEvent = true; commentForm.setActionButtonsDisabled(true, excludeCancelBtn, submitEvent); commentForm.cm.setOption("readOnly", true); - var dropDown = $('#comment_history_for_comment_'+comment_id); + + // Read last version known + var versionSelector = $('#comment_versions_{0}'.format(comment_id)); + var version = versionSelector.data('lastVersion'); - var version = dropDown.children().last().val() - if(!version){ - version = 0; + if (!version) { + version = 0; } + var postData = { 'text': text, 'f_path': f_path, 'line': lineno, 'comment_type': commentType, - 'csrf_token': CSRF_TOKEN, 'version': version, + 'csrf_token': CSRF_TOKEN }; var submitSuccessCallback = function(json_data) { @@ -961,32 +991,61 @@ var CommentsController = function() { 'csrf_token': CSRF_TOKEN }; + /* Inject new edited version selector */ var updateCommentVersionDropDown = function () { - var dropDown = $('#comment_history_for_comment_'+comment_id); + var versionSelectId = '#comment_versions_'+comment_id; + var preLoadVersionData = [ + { + id: json_data['comment_version'], + text: "v{0}".format(json_data['comment_version']), + action: function () { + Rhodecode.comments.showVersion( + json_data['comment_id'], + json_data['comment_history_id'] + ) + }, + comment_version: json_data['comment_version'], + comment_author_username: json_data['comment_author_username'], + comment_author_gravatar: json_data['comment_author_gravatar'], + comment_created_on: json_data['comment_created_on'], + }, + ] + + + if ($(versionSelectId).data('select2')) { + var oldData = $(versionSelectId).data('select2').opts.data.results; + $(versionSelectId).select2("destroy"); + preLoadVersionData = oldData.concat(preLoadVersionData) + } + + initVersionSelector(versionSelectId, {results: preLoadVersionData}); + $comment.attr('data-comment-text', btoa(text)); - var version = json_data['comment_version'] - var option = new Option(version, version); - var $option = $(option); - $option.attr('data-comment-history-id', json_data['comment_history_id']); - $option.attr('data-comment-id', json_data['comment_id']); - dropDown.append(option); - dropDown.parent().show(); + + var versionSelector = $('#comment_versions_'+comment_id); + + // set lastVersion so we know our last edit version + versionSelector.data('lastVersion', json_data['comment_version']) + versionSelector.parent().show(); } updateCommentVersionDropDown(); + // by default we reset state of comment preserving the text var failRenderCommit = function(jqXHR, textStatus, errorThrown) { - var prefix = "Error while editing of comment.\n" + var prefix = "Error while editing this comment.\n" var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix); ajaxErrorSwal(message); + }; - }; var successRenderCommit = function(o){ $comment.show(); $comment[0].lastElementChild.innerHTML = o; - } - var previewUrl = pyroutes.url('repo_commit_comment_preview', - {'repo_name': templateContext.repo_name, - 'commit_id': templateContext.commit_data.commit_id}); + }; + + var previewUrl = pyroutes.url( + 'repo_commit_comment_preview', + {'repo_name': templateContext.repo_name, + 'commit_id': templateContext.commit_data.commit_id}); _submitAjaxPOST( previewUrl, postData, successRenderCommit, @@ -1000,11 +1059,6 @@ var CommentsController = function() { $comments.find('.cb-comment-add-button').before(html); - //mark visually which comment was resolved - if (resolvesCommentId) { - commentForm.markCommentResolved(resolvesCommentId); - } - // run global callback on submit commentForm.globalSubmitSuccessCallback(); @@ -1029,16 +1083,25 @@ var CommentsController = function() { var submitFailCallback = function(jqXHR, textStatus, errorThrown) { var prefix = "Error while editing comment.\n" var message = formatErrorMessage(jqXHR, textStatus, errorThrown, prefix); - ajaxErrorSwal(message); + if (jqXHR.status == 409){ + message = 'This comment was probably changed somewhere else. Please reload the content of this comment.' + ajaxErrorSwal(message, 'Comment version mismatch.'); + } else { + ajaxErrorSwal(message); + } + commentForm.resetCommentFormState(text) }; commentForm.submitAjaxPOST( - commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback); + commentForm.submitUrl, postData, + submitSuccessCallback, + submitFailCallback); }); } $form.addClass('comment-inline-form-open'); }; + this.createComment = function(node, resolutionComment) { var resolvesCommentId = resolutionComment || null; var $node = $(node); diff --git a/rhodecode/public/js/src/rhodecode/utils/ajax.js b/rhodecode/public/js/src/rhodecode/utils/ajax.js --- a/rhodecode/public/js/src/rhodecode/utils/ajax.js +++ b/rhodecode/public/js/src/rhodecode/utils/ajax.js @@ -130,10 +130,13 @@ function formatErrorMessage(jqXHR, textS } } -function ajaxErrorSwal(message) { +function ajaxErrorSwal(message, title) { + + var title = (typeof title !== 'undefined') ? title : _gettext('Ajax Request Error'); + SwalNoAnimation.fire({ icon: 'error', - title: _gettext('Ajax Request Error'), + title: title, html: '{0}'.format(message), showClass: { popup: 'swal2-noanimation', 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 @@ -66,31 +66,47 @@
${h.age_component(comment.modified_at, time_is_local=True)}
+ <% + comment_version_selector = 'comment_versions_{}'.format(comment.comment_id) + %> + % if comment.history:
- ${_('Edited')}: - + + +
% else: %endif % if inline: @@ -169,12 +185,8 @@ %if not outdated_at_ver and (not comment.pull_request or (comment.pull_request and not comment.pull_request.is_closed())): ## permissions to delete %if comment.immutable is False and (c.is_super_admin or h.HasRepoPermissionAny('repository.admin')(c.repo_name) or comment.author.user_id == c.rhodecode_user.user_id): - %if comment.comment_type == 'note': ${_('Edit')} - %else: - ${_('Edit')} - %endif | ${_('Delete')} %else: diff --git a/rhodecode/templates/ejs_templates/templates.html b/rhodecode/templates/ejs_templates/templates.html --- a/rhodecode/templates/ejs_templates/templates.html +++ b/rhodecode/templates/ejs_templates/templates.html @@ -130,6 +130,34 @@ var CG = new ColorGenerator(); + + +