diff --git a/web/api/v6/report_server.thrift b/web/api/v6/report_server.thrift index f3e9703623..8b2a7dff96 100644 --- a/web/api/v6/report_server.thrift +++ b/web/api/v6/report_server.thrift @@ -93,6 +93,11 @@ enum ExtendedReportDataType { FIXIT = 20, } +enum CommentKind { + USER, // User-given comments. + SYSTEM // System events. +} + struct SourceFileData { 1: i64 fileId, 2: string filePath, @@ -280,7 +285,8 @@ struct CommentData { 1: i64 id, 2: string author, 3: string message, - 4: string createdAt + 4: string createdAt, + 5: CommentKind kind } typedef list CommentDataList diff --git a/web/codechecker_web/shared/webserver_context.py b/web/codechecker_web/shared/webserver_context.py index db39efc9e3..b6d30ba5ed 100644 --- a/web/codechecker_web/shared/webserver_context.py +++ b/web/codechecker_web/shared/webserver_context.py @@ -56,6 +56,8 @@ def __init__(self, package_root, pckg_layout, cfg_dict): self._package_root = package_root self._severity_map = SeverityMap( load_json_or_empty(self.checkers_severity_map_file, {})) + self.__system_comment_map = \ + load_json_or_empty(self.system_comment_map_file, {}) self.__package_version = None self.__package_build_date = None self.__package_git_hash = None @@ -110,6 +112,15 @@ def package_git_tag(self): def version_file(self): return os.path.join(self._package_root, 'config', 'web_version.json') + @property + def system_comment_map(self): + return self.__system_comment_map + + @property + def system_comment_map_file(self): + return os.path.join(self._package_root, 'config', + 'system_comment_kinds.json') + @property def path_plist_to_html_dist(self): return os.path.join(self.package_root, 'lib', 'python2.7', diff --git a/web/config/system_comment_kinds.json b/web/config/system_comment_kinds.json new file mode 100644 index 0000000000..f03d2c349f --- /dev/null +++ b/web/config/system_comment_kinds.json @@ -0,0 +1,5 @@ +{ + "rev_st_changed": "%author% changed review status from {0} to {1} %date%", + "rev_st_changed_msg": "%author% changed review status from {0} to {1} with {2} message %date%", + "comment_changed": "%author% changed comment message from {0} to {1} %date%" +} diff --git a/web/server/codechecker_server/api/report_server.py b/web/server/codechecker_server/api/report_server.py index 9a5147604b..19dc6631db 100644 --- a/web/server/codechecker_server/api/report_server.py +++ b/web/server/codechecker_server/api/report_server.py @@ -17,6 +17,7 @@ import io import os import re +import shlex import tempfile import zipfile import zlib @@ -40,6 +41,8 @@ from codechecker_common.logger import get_logger from codechecker_common.report import get_report_path_hash +from codechecker_web.shared import webserver_context + from codechecker_server.profiler import timeit from .. import permissions @@ -445,6 +448,20 @@ def extended_data_db_to_api(erd): fileId=erd.file_id) +def comment_kind_str(status): + if status == ttypes.CommentKind.USER: + return 'user' + elif status == ttypes.CommentKind.SYSTEM: + return 'system' + + +def comment_kind_enum(kind): + if kind == 'user': + return ttypes.CommentKind.USER + elif kind == 'system': + return ttypes.CommentKind.SYSTEM + + def unzip(b64zip, output_dir): """ This function unzips the base64 encoded zip file. This zip is extracted @@ -629,6 +646,17 @@ def sort_run_data_query(query, sort_mode): return query +def escape_whitespaces(s, whitespaces=None): + if not whitespaces: + whitespaces = [' ', '\n', '\t', '\r'] + + escaped = s + for w in whitespaces: + escaped = escaped.replace(w, '\\{0}'.format(w)) + + return escaped + + class ThriftRequestHandler(object): """ Connect to database and handle thrift client requests. @@ -712,7 +740,14 @@ def __get_run_ids_to_query(session, cmp_data=None): return run_ids - @exc_to_thrift_reqfail + def __add_comment(self, bug_id, message, kind='user'): + user = self.__get_username() + return Comment(bug_id, + user, + message, + kind, + datetime.now()) + @timeit def getRunData(self, run_filter, limit, offset, sort_mode): self.__require_access() @@ -1307,12 +1342,29 @@ def _setReviewStatus(self, report_id, status, message, session): user = self.__get_username() + old_status = review_status.status if review_status.status \ + else review_status_str(ttypes.ReviewStatus.UNREVIEWED) + review_status.status = review_status_str(status) review_status.author = user review_status.message = message.encode('utf8') review_status.date = datetime.now() - session.add(review_status) + + if message: + system_comment_msg = 'rev_st_changed_msg {0} {1} {2}'.format( + escape_whitespaces(old_status.capitalize()), + escape_whitespaces(review_status.status.capitalize()), + escape_whitespaces(message)) + else: + system_comment_msg = 'rev_st_changed {0} {1}'.format( + escape_whitespaces(old_status.capitalize()), + escape_whitespaces(review_status.status.capitalize())) + + system_comment = self.__add_comment(review_status.bug_hash, + system_comment_msg, + 'system') + session.add(system_comment) session.flush() return True @@ -1373,12 +1425,27 @@ def getComments(self, report_id): .order_by(Comment.created_at.desc()) \ .all() + context = webserver_context.get_context() for comment in comments: + message = comment.message + + sys_comment = comment_kind_str(ttypes.CommentKind.SYSTEM) + if comment.kind == sys_comment: + elements = shlex.split(message) + system_comment = context.system_comment_map.get( + elements[0]) + if system_comment: + for idx, value in enumerate(elements[1:]): + system_comment = system_comment.replace( + '{' + str(idx) + '}', value) + message = system_comment + result.append(CommentData( comment.id, comment.author, - comment.message, - str(comment.created_at))) + message, + str(comment.created_at), + comment_kind_enum(comment.kind))) return result else: @@ -1416,12 +1483,8 @@ def addComment(self, report_id, comment_data): with DBSession(self.__Session) as session: report = session.query(Report).get(report_id) if report: - user = self.__get_username() - comment = Comment(report.bug_id, - user, - comment_data.message, - datetime.now()) - + comment = self.__add_comment(report.bug_id, + comment_data.message) session.add(comment) session.commit() @@ -1452,8 +1515,18 @@ def updateComment(self, comment_id, content): raise codechecker_api_shared.ttypes.RequestFailed( codechecker_api_shared.ttypes.ErrorCode.UNAUTHORIZED, 'Unathorized comment modification!') + # Create system comment. + system_comment_msg = 'comment_changed {0} {1}'.format( + escape_whitespaces(comment.message), + escape_whitespaces(content)) + system_comment = self.__add_comment(comment.bug_hash, + system_comment_msg, + 'system') + session.add(system_comment) + comment.message = content session.add(comment) + session.commit() return True else: @@ -2371,6 +2444,7 @@ def checker_is_unavailable(checker_name): detection_status = 'new' detected_at = run_history_time + old_status = None if bug_id in hash_map_reports: old_report = hash_map_reports[bug_id][0] old_status = old_report.detection_status diff --git a/web/server/codechecker_server/database/run_db_model.py b/web/server/codechecker_server/database/run_db_model.py index 5a378104d3..56057f5948 100644 --- a/web/server/codechecker_server/database/run_db_model.py +++ b/web/server/codechecker_server/database/run_db_model.py @@ -360,12 +360,18 @@ class Comment(Base): bug_hash = Column(String, nullable=False, index=True) author = Column(String, nullable=False) message = Column(Binary, nullable=False) + kind = Column(Enum('user', + 'system', + name='comment_kind'), + nullable=False, + server_default='user') created_at = Column(DateTime, nullable=False) - def __init__(self, bug_hash, author, message, created_at): + def __init__(self, bug_hash, author, message, kind, created_at): self.bug_hash = bug_hash self.author = author self.message = message + self.kind = kind self.created_at = created_at diff --git a/web/server/codechecker_server/migrations/report/versions/6cb6a3a41967_system_comments.py b/web/server/codechecker_server/migrations/report/versions/6cb6a3a41967_system_comments.py new file mode 100644 index 0000000000..80cc4fcf59 --- /dev/null +++ b/web/server/codechecker_server/migrations/report/versions/6cb6a3a41967_system_comments.py @@ -0,0 +1,28 @@ +"""System comments + +Revision ID: 6cb6a3a41967 +Revises: 3e91d0612422 +Create Date: 2019-04-02 16:12:46.794131 + +""" + +# revision identifiers, used by Alembic. +revision = '6cb6a3a41967' +down_revision = '3e91d0612422' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('comments', sa.Column('kind', sa.Enum(u'user', + u'system', + name='comment_kind'), + server_default=u'user', + nullable=False)) + + +def downgrade(): + op.drop_column('comments', 'kind') diff --git a/web/server/www/scripts/codecheckerviewer/CommentView.js b/web/server/www/scripts/codecheckerviewer/CommentView.js index 1d2f054837..8f728cf8b3 100644 --- a/web/server/www/scripts/codecheckerviewer/CommentView.js +++ b/web/server/www/scripts/codecheckerviewer/CommentView.js @@ -55,25 +55,22 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, } }); - var Comment = declare(ContentPane, { - constructor : function (args) { - dojo.safeMixin(this, args); - + var UserComment = declare(ContentPane, { + postCreate : function () { var that = this; //--- Header section ---// - this._header = new ContentPane(); - var header = dom.create('div', { class : 'header'}, this._header.domNode); + var header = dom.create('div', { class : 'header'}, this.domNode); - var avatar = util.createAvatar(this.author); + var avatar = util.createAvatar(this.commentData.author); dom.place(avatar, header); var vb = dom.create('div', { class : 'vb'}, header); - dom.create('span', { class : 'author', innerHTML: this.author }, vb); + dom.create('span', { class : 'author', innerHTML: this.commentData.author }, vb); - var time = util.timeAgo(new Date(this.time.replace(/ /g,'T'))); + var time = util.timeAgo(new Date(this.commentData.createdAt.replace(/ /g,'T'))); dom.create('span', { class : 'time', innerHTML: time }, vb); //--- Comment operations (edit, remove) ---// @@ -82,7 +79,9 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, user = CC_AUTH_SERVICE.getLoggedInUser(); } catch (ex) { util.handleThriftException(ex); } - if (this.author == 'Anonymous' || user == this.author) { + if (this.commentData.author === 'Anonymous' || + user === this.commentData.author + ) { var operations = dom.create('div', { class : 'operations'}, header); dom.create('span', { class : 'customIcon edit', @@ -96,10 +95,10 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, } //--- Message section ---// - this._message = new ContentPane({ - class : 'message', - content : this.message.replace(/(?:\r\n|\r|\n)/g, '
') - }); + this._message = dom.create('div', { + class : 'message', + innerHTML : this.commentData.message.replace(/(?:\r\n|\r|\n)/g, '
') + }, this.domNode); //--- Remove comment confirmation dialog ---// @@ -108,7 +107,7 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, content : 'Are you sure you want to delete this?', onExecute : function () { try { - CC_SERVICE.removeComment(that.cId); + CC_SERVICE.removeComment(that.commentData.id); } catch (ex) { util.handleThriftException(ex); } topic.publish('showComments', that.reportId, that.sender); } @@ -121,8 +120,9 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, }); this._commentContent = new SimpleTextarea({ - value : that.message + value : this.commentData.message }); + this._editDialog.addChild(this._commentContent); this._saveButton = new Button({ label : 'Save', @@ -130,22 +130,15 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, var newContent = that._commentContent.get('value'); try { - CC_SERVICE.updateComment(that.cId, newContent); + CC_SERVICE.updateComment(that.commentData.id, newContent); } catch (ex) { util.handleThriftException(ex); } - that._message.set('content', - newContent.replace(/(?:\r\n|\r|\n)/g, '
')); + that._message.innerHTML = + newContent.replace(/(?:\r\n|\r|\n)/g, '
'); that._editDialog.hide(); } }); - }, - - postCreate : function () { - this.addChild(this._header); - this.addChild(this._message); - - this._editDialog.addChild(this._commentContent); this._editDialog.addChild(this._saveButton); }, @@ -158,6 +151,54 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, } }); + var SystemComment = declare(ContentPane, { + postCreate : function () { + var wrapper = dom.create('div', { class : 'wrapper'}, this.domNode); + + dom.create('span', { class : 'system-comment-icon' }, wrapper); + + // Create comment time. + var timeAgo = util.timeAgo(new Date(this.commentData.createdAt.replace(/ /g,'T'))); + var time = dom.create('span', { class : 'time', innerHTML: timeAgo }); + + // Create comment author. + var author = dom.create('span', { + class : 'author', + innerHTML: this.commentData.author + }); + + // Replace special string in comment message. + var message = this.commentData.message; + message = message + .replace('%author%', author.outerHTML) + .replace('%date%', time.outerHTML); + + var divMsg = dom.create('div', { + class : 'message', + innerHTML: message + }, wrapper); + + this.addStatusIcons(divMsg); + }, + + addStatusIcons : function (domElement) { + ['detection-status', 'review-status'].forEach(function (type) { + ['old-' + type, 'new-' + type].forEach(function (className) { + var domStatuses = + domElement.getElementsByClassName(className); + + for (var i = 0; i < domStatuses.length; i++) { + var domStatus = domStatuses[i]; + var status = domStatus.innerHTML.toLowerCase(); + domElement.insertBefore( + dom.create('i', { class : 'customIcon ' + type + '-' + status}), + domStatus); + } + }); + }); + } + }); + return declare(ContentPane, { constructor : function () { this._reply = new ContentPane(); @@ -209,15 +250,19 @@ function (declare, dom, style, topic, Memory, Observable, ConfirmDialog, } catch (ex) { util.handleThriftException(ex); } comments.forEach(function (comment) { - that._comments.addChild(new Comment({ - class : 'comment', - reportId : reportId, - cId : comment.id, - author : comment.author, - time : comment.createdAt, - message : comment.message, - sender : sender - })); + if (comment.kind === CC_OBJECTS.CommentKind.SYSTEM) { + that._comments.addChild(new SystemComment({ + class : 'system-comment', + commentData : comment + })); + } else { + that._comments.addChild(new UserComment({ + class : 'comment', + reportId : reportId, + commentData : comment, + sender : sender + })); + } }); }); } diff --git a/web/server/www/style/codecheckerviewer.css b/web/server/www/style/codecheckerviewer.css index 314b7ec666..713d15107d 100644 --- a/web/server/www/style/codecheckerviewer.css +++ b/web/server/www/style/codecheckerviewer.css @@ -140,7 +140,7 @@ html, body { font-weight: 600; } -#header-menu { +#header-menu { /* position: absolute; */ right: 10px; top: 10px; diff --git a/web/server/www/style/comment.css b/web/server/www/style/comment.css index f2c8e5aa6c..f9cbe66faa 100644 --- a/web/server/www/style/comment.css +++ b/web/server/www/style/comment.css @@ -9,18 +9,26 @@ } .comment-view .comment { - border: 1px solid #fff; + background-color: #fbfbfb; + border: 1px solid #f1f1f1; + + border-radius: 5px; + padding: 5px; } .comment-view .comment:hover { - background-color: #fbfbfb; - border: 1px solid #f1f1f1; + border: 1px solid #b5bcc8; + background-color: #f8fdff; } .comment-view .header { position: relative; } +.comment-view .message { + padding: 8px 0px; +} + .comment-view .reply { overflow: initial; } @@ -73,3 +81,49 @@ .comment .customIcon:hover { color: #000; } + +/* System comments */ +.comment-view .system-comment { + padding: 0px; + overflow: hidden; + font-size: smaller; +} + +.system-comment .wrapper { + position: relative; + margin-left: 20px; + padding-left: 15px; +} + +.system-comment-icon { + content: ""; + display: block; + width: 0; + height: 100%; + border: 1px solid #82a7bb; + position: absolute; + top: 0; + left: 0px; +} + +.system-comment-icon:before { + content: ""; + display: block; + width: 10px; + height: 10px; + border-radius: 50%; + background: #82a7bb; + border: 2px solid #fff; + position: absolute; + left: -7.5px; + top: 5px; +} + +.system-comment .time { + color: #767676; +} + +.system-comment .author { + color: #403d3d; + font-weight: 600; +} diff --git a/web/tests/functional/comment/test_comment.py b/web/tests/functional/comment/test_comment.py index 811d56e888..6719f86b73 100644 --- a/web/tests/functional/comment/test_comment.py +++ b/web/tests/functional/comment/test_comment.py @@ -16,12 +16,25 @@ import unittest from codechecker_api_shared.ttypes import RequestFailed -from codeCheckerDBAccess_v6.ttypes import CommentData +from codeCheckerDBAccess_v6.ttypes import CommentData, CommentKind from libtest import env from libtest.thrift_client_to_db import get_all_run_results +def separate_comments(comments): + """ + Separate comments and returns a tuple of user comments and system. + """ + user_comments = [c for c in comments + if c.kind == CommentKind.USER] + + system_comments = [c for c in comments + if c.kind == CommentKind.SYSTEM] + + return user_comments, system_comments + + class TestComment(unittest.TestCase): _ccClient = None @@ -152,19 +165,23 @@ def test_comment(self): logging.debug('Comment was edited by john') comments = self._cc_client.getComments(bug.reportId) - self.assertEqual(len(comments), 1) - self.assertEqual(comments[0].message, new_msg) + user_comments, system_comments = separate_comments(comments) + + self.assertEqual(len(user_comments), 1) + self.assertEqual(user_comments[0].message, new_msg) + self.assertEqual(len(system_comments), 1) # Remove the last comment for the first bug - success = self._cc_client.removeComment(comments[0].id) + success = self._cc_client.removeComment(user_comments[0].id) self.assertTrue(success) logging.debug('Comment removed successfully') comments = self._cc_client.getComments(bug.reportId) - self.assertEqual(len(comments), 0) + self.assertEqual(len(comments), 1) + self.assertEqual(comments[0].kind, CommentKind.SYSTEM) num_comment = self._cc_client.getCommentCount(bug.reportId) - self.assertEqual(num_comment, 0) + self.assertEqual(num_comment, 1) # Remove the last comment for the second bug. comments = self._cc_client.getComments(bug2.reportId) @@ -213,10 +230,12 @@ def test_same_bug_hash(self): # There are no comments available for the bug. comments = self._cc_client.getComments(bug_base.reportId) - self.assertEqual(len(comments), 0) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 0) comments = self._cc_client.getComments(bug_new.reportId) - self.assertEqual(len(comments), 0) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 0) # Try to add a new comment for the first bug comment = CommentData(author='Anonymous', message='First msg') @@ -225,10 +244,12 @@ def test_same_bug_hash(self): logging.debug('Bug commented successfully') comments = self._cc_client.getComments(bug_base.reportId) - self.assertEqual(len(comments), 1) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 1) comments = self._cc_client.getComments(bug_new.reportId) - self.assertEqual(len(comments), 1) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 1) # Remove the comment for the bug. success = self._cc_client.removeComment(comments[0].id) @@ -236,7 +257,9 @@ def test_same_bug_hash(self): logging.debug('Comment removed successfully') comments = self._cc_client.getComments(bug_base.reportId) - self.assertEqual(len(comments), 0) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 0) comments = self._cc_client.getComments(bug_new.reportId) - self.assertEqual(len(comments), 0) + user_comments, system_comments = separate_comments(comments) + self.assertEqual(len(user_comments), 0)