Commenting of issues. #742
Conversation
libcodechecker/orm_model.py
Outdated
| @@ -267,6 +267,21 @@ def __init__(self, run_id, path, comment): | |||
| self.run_id = run_id | |||
| self.comment = comment | |||
|
|
|||
| class Comment(Base): | |||
| __tablename__ = 'comment' | |||
There was a problem hiding this comment.
Original object relations follow a convention of singular type name and plural table name. (Though skip_path above already violates this convention. 🙁 )
libcodechecker/orm_model.py
Outdated
| __tablename__ = 'comment' | ||
|
|
||
| id = Column(Integer, autoincrement=True, primary_key=True) | ||
| hash = Column(String, nullable=False) |
There was a problem hiding this comment.
Considering that the bugHash (hash) is used for primarily querying the comments for a bug(group), I think this field should definitely be made an index=True to speed up the database.
There was a problem hiding this comment.
And maybe for greater clarity we could rename this field to be bug_hash or bugHash, according to how other such fields are named.
| from libtest import env | ||
|
|
||
|
|
||
| class TestSkeleton(unittest.TestCase): |
| self.assertEqual(num_comment, 0) | ||
|
|
||
| # Try to add a new comment for the first bug | ||
| comment1 = CommentData(author = 'Anonymous', message = 'First msg') |
There was a problem hiding this comment.
pep8: No spaces between kwargs keys and values.
| @@ -7,6 +7,8 @@ define([ | |||
| 'dojo/store/Memory', | |||
| 'dojo/store/Observable', | |||
| 'dojo/topic', | |||
| 'dojo/fx', | |||
There was a problem hiding this comment.
Imports should be sorted in JS files too.
csordasmarton
force-pushed
the
comments
branch
4 times, most recently
from
fcc2d08
to
13351a6
Compare
| """ | ||
| session = self.__session | ||
| comment = Comment(bug_hash, | ||
| comment_data.author, |
There was a problem hiding this comment.
Security leak!
Author must not be given by the client! It must be queried from the authenticated session. Wire in comment_data.author="Anonymous" for now and add a FIXME and a followup issue in GITHUB to query the proper user name.
There was a problem hiding this comment.
Also add a # TODO: to this line, not just a GitHub issue.
There was a problem hiding this comment.
Very small changes. The feature looks nice.
One small detail: if the BugPath is changed (because I'm jumping back and forth in it), the comments are reloaded. This creates an unneccessary load on the connection, and "blinking" in the window. As long as the user is viewing the same bug, the comment loading should be prevented. But of course, jumping into a different bug while viewing the same source file must work.
api/report_server.thrift
Outdated
| // add new comment for a bug | ||
| bool addComment(1: string bugHash, | ||
| 2: CommentData comment) | ||
| throws(1: shared.RequestFailed requestError), |
| this.hide = false; | ||
| } else { | ||
| toggler.show(); | ||
| this.hide = true |
| //--- Remove comment confirmation dialog ---// | ||
|
|
||
| this._removeDialog = new ConfirmDialog({ | ||
| title : 'CodeChecker - Remove comment', |
There was a problem hiding this comment.
I think putting CodeChecker here is rendundant. The message is really short which already makes the box small.
| var fuzzy; | ||
|
|
||
| if (delta < 30) { | ||
| fuzzy = 'just then.'; |
There was a problem hiding this comment.
just then? When? just now works better. (Also, that's how GitHub and other such sites show it.)
This commit resolves #685 issue.