comment-history: fixes/ui changes

- show explicit version conflicts
- use new select2 based dropdown with nicer formatting
- enabled editing of content for TODO comments
This commit is contained in:
Marcin Kuzminski 2020-06-23 17:43:06 +02:00
parent 29356473c4
commit 40505633d1
12 changed files with 268 additions and 103 deletions

View file

@ -378,9 +378,9 @@ class TestRepoCommitCommentsView(TestController):
'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

View file

@ -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

View file

@ -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 OrderedDict
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',

View file

@ -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 RepoAppView, DataGridAppView
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, DataGridAppView):
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, DataGridAppView):
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, DataGridAppView):
)
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()

View file

@ -177,3 +177,7 @@ class ArtifactMetadataDuplicate(ValueError):
class ArtifactMetadataBadValueType(ValueError):
pass
class CommentVersionMismatch(ValueError):
pass

View file

@ -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):

View file

@ -33,6 +33,7 @@ from sqlalchemy.sql.functions import coalesce
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

View file

@ -148,6 +148,38 @@ select.select2{height:28px;visibility:hidden}
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 {

View file

@ -496,6 +496,43 @@ var _submitAjaxPOST = function(url, postData, successHandler, failHandler) {
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);
var version = dropDown.children().last().val()
if(!version){
version = 0;
// Read last version known
var versionSelector = $('#comment_versions_{0}'.format(comment_id));
var version = versionSelector.data('lastVersion');
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);

View file

@ -130,10 +130,13 @@ function formatErrorMessage(jqXHR, textStatus, errorThrown, prefix) {
}
}
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: '<span style="white-space: pre-line">{0}</span>'.format(message),
showClass: {
popup: 'swal2-noanimation',

View file

@ -66,31 +66,47 @@
<div class="date">
${h.age_component(comment.modified_at, time_is_local=True)}
</div>
<%
comment_version_selector = 'comment_versions_{}'.format(comment.comment_id)
%>
% if comment.history:
<div class="date">
<span class="comment-area-text">${_('Edited')}:</span>
<select class="comment-version-select" id="comment_history_for_comment_${comment.comment_id}"
onchange="return Rhodecode.comments.showVersion(this)"
name="comment_type">
<option style="display: none" value="0">---</option>
% for comment_history in comment.history:
<option data-comment-history-id="${comment_history.comment_history_id}"
data-comment-id="${comment.comment_id}"
value="${comment_history.version}">
${comment_history.version}
</option>
% endfor
</select>
<input id="${comment_version_selector}" name="${comment_version_selector}"
type="hidden"
data-last-version="${comment.history[-1].version}">
<script type="text/javascript">
var preLoadVersionData = [
% for comment_history in comment.history:
{
id: ${comment_history.comment_history_id},
text: 'v${comment_history.version}',
action: function () {
Rhodecode.comments.showVersion(
"${comment.comment_id}",
"${comment_history.comment_history_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)}',
},
% endfor
]
initVersionSelector("#${comment_version_selector}", {results: preLoadVersionData});
</script>
</div>
% else:
<div class="date" style="display: none">
<span class="comment-area-text">${_('Edited')}</span>
<select class="comment-version-select" id="comment_history_for_comment_${comment.comment_id}"
onchange="return Rhodecode.comments.showVersion(this)"
name="comment_type">
<option style="display: none" value="0">---</option>
</select>
<input id="${comment_version_selector}" name="${comment_version_selector}"
type="hidden"
data-last-version="0">
</div>
%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':
<a onclick="return Rhodecode.comments.editComment(this);"
class="edit-comment">${_('Edit')}</a>
%else:
<a class="tooltip edit-comment link-disabled" disabled="disabled" title="${_('Action unavailable')}">${_('Edit')}</a>
%endif
| <a onclick="return Rhodecode.comments.deleteComment(this);"
class="delete-comment">${_('Delete')}</a>
%else:

View file

@ -130,6 +130,34 @@ var CG = new ColorGenerator();
</script>
<script id="ejs_commentVersion" type="text/template" class="ejsTemplate">
<%
if (size > 16) {
var gravatar_class = 'gravatar gravatar-large';
} else {
var gravatar_class = 'gravatar';
}
%>
<%
if (show_disabled) {
var user_cls = 'user user-disabled';
} else {
var user_cls = 'user';
}
%>
<div style='line-height: 20px'>
<img style="margin: -3px 0" class="<%= gravatar_class %>" height="<%= size %>" width="<%= size %>" src="<%- gravatar_url -%>">
<strong><%- user_name -%></strong>, <code>v<%- version -%></code> edited <%- timeago_component -%>
</div>
</script>
</div>
<script>