Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(642)

Issue 256840043: ticket comments

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 8 months ago by vipul
Modified:
8 years, 7 months ago
Reviewers:
thomas.j.waldmann
Visibility:
Public.

Description

Supports markdown syntax, and automatically posts a comment if changes are made in the metadata of ticket with a proper message. Re-build the index before running this patch as new fields have been added for whoosh indexing.

Patch Set 1 : threaded comments #

Total comments: 9

Patch Set 2 : reduced number of queries made for the comments/replies #

Patch Set 3 : comments #

Total comments: 4

Patch Set 4 : fixed rendering of datetime #

Patch Set 5 : fixed UI for comments #

Total comments: 9

Patch Set 6 : created indented comments/replies threads #

Total comments: 10

Patch Set 7 : created indented comments/replies threads #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -15 lines) Patch
M MoinMoin/apps/frontend/views.py View 1 2 3 4 5 6 2 chunks +23 lines, -0 lines 0 comments Download
M MoinMoin/items/ticket.py View 1 2 3 4 5 6 7 chunks +72 lines, -8 lines 0 comments Download
M MoinMoin/static/css/ticket.css View 1 2 3 4 5 2 chunks +48 lines, -5 lines 0 comments Download
M MoinMoin/static/js/tickets.js View 1 2 3 4 5 1 chunk +38 lines, -0 lines 0 comments Download
A MoinMoin/templates/comments.html View 1 chunk +6 lines, -0 lines 0 comments Download
M MoinMoin/templates/ticket/base.html View 1 2 3 4 5 6 2 chunks +37 lines, -0 lines 0 comments Download
M MoinMoin/templates/ticket/modify.html View 1 2 3 4 5 6 1 chunk +7 lines, -1 line 0 comments Download
M MoinMoin/templates/ticket/submit.html View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 12
Thomas.J.Waldmann
some hints, esp. about how to build the comments tree with just 1 whoosh query ...
8 years, 8 months ago (2015-07-14 18:09:20 UTC) #1
vipul
https://codereview.appspot.com/256840043/diff/20001/MoinMoin/apps/frontend/views.py File MoinMoin/apps/frontend/views.py (right): https://codereview.appspot.com/256840043/diff/20001/MoinMoin/apps/frontend/views.py#newcode2393 MoinMoin/apps/frontend/views.py:2393: @frontend.route('/+comment', defaults=dict(item_name=u''), methods=['GET', 'POST']) On 2015/07/14 18:09:20, Thomas.J.Waldmann wrote: ...
8 years, 8 months ago (2015-07-17 12:28:56 UTC) #2
sksaurabhkathpalia
On 2015/07/17 12:28:56, vipul wrote: > https://codereview.appspot.com/256840043/diff/20001/MoinMoin/apps/frontend/views.py > File MoinMoin/apps/frontend/views.py (right): > > https://codereview.appspot.com/256840043/diff/20001/MoinMoin/apps/frontend/views.py#newcode2393 > ...
8 years, 8 months ago (2015-08-01 21:31:18 UTC) #3
vipul
https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js File MoinMoin/static/js/tickets.js (right): https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js#newcode46 MoinMoin/static/js/tickets.js:46: type: "GET", I need to do it using a ...
8 years, 8 months ago (2015-08-02 20:17:38 UTC) #4
Thomas.J.Waldmann
https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js File MoinMoin/static/js/tickets.js (right): https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js#newcode46 MoinMoin/static/js/tickets.js:46: type: "GET", On 2015/08/02 20:17:37, vipul wrote: > I ...
8 years, 7 months ago (2015-08-11 15:41:18 UTC) #5
vipul
On 2015/08/11 15:41:18, Thomas.J.Waldmann wrote: > https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js > File MoinMoin/static/js/tickets.js (right): > > https://codereview.appspot.com/256840043/diff/80001/MoinMoin/static/js/tickets.js#newcode46 > ...
8 years, 7 months ago (2015-08-11 15:52:31 UTC) #6
Thomas.J.Waldmann
https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py File MoinMoin/apps/frontend/views.py (right): https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py#newcode23 MoinMoin/apps/frontend/views.py:23: import markdown did you look at our own parsers ...
8 years, 7 months ago (2015-08-11 16:23:09 UTC) #7
vipul
https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py File MoinMoin/apps/frontend/views.py (right): https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py#newcode23 MoinMoin/apps/frontend/views.py:23: import markdown On 2015/08/11 16:23:08, Thomas.J.Waldmann wrote: > did ...
8 years, 7 months ago (2015-08-12 06:59:05 UTC) #8
vipul
8 years, 7 months ago (2015-08-16 19:34:17 UTC) #9
vipul
8 years, 7 months ago (2015-08-17 06:06:05 UTC) #10
Thomas.J.Waldmann
https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py File MoinMoin/apps/frontend/views.py (right): https://codereview.appspot.com/256840043/diff/120001/MoinMoin/apps/frontend/views.py#newcode2403 MoinMoin/apps/frontend/views.py:2403: item.modify({}, data=markdown.markdown(data), element=u'comment', contenttype_guessed=u'text/x.moin.wiki;charset=utf-8', \ if you just create ...
8 years, 7 months ago (2015-08-18 16:07:19 UTC) #11
vipul
8 years, 7 months ago (2015-08-19 06:18:33 UTC) #12
https://codereview.appspot.com/256840043/diff/260001/MoinMoin/items/ticket.py
File MoinMoin/items/ticket.py (right):

https://codereview.appspot.com/256840043/diff/260001/MoinMoin/items/ticket.py...
MoinMoin/items/ticket.py:191: comments[rev] = []
On 2015/08/18 16:07:19, Thomas.J.Waldmann wrote:
> does that even work?
> using a whole rev as dict key seems rather brave...
> likely you could also just use an itemid as key here.
> you could also name it childs (you already use "parent"), right?

yes this works. We store dictionary "values" as revs and while building comment
tree we use those values i.e. the revs as "keys" to find its child comments
therefore the keys and values must be of same type hence, they need to be revs

https://codereview.appspot.com/256840043/diff/260001/MoinMoin/items/ticket.py...
MoinMoin/items/ticket.py:194: roots.append(rev)
On 2015/08/18 16:07:19, Thomas.J.Waldmann wrote:
> how many roots will you get?
> doesn't every comment have a reply_to entry (and if it only point to the
ticket
> item itself as every "toplevel" comment is a reply to the ticket text)?

"roots" contains the first level comments which are the comments which are NOT
the replies of any other comment. There can be more than one 1st level comments
which may contain a series of comment/reply thread for that comment. Therefore,
even if we assign the "reply_to" field of 1st level comment as the "itemid" of
the ticket we will still have multiple roots i.e. multiple 1st level comments.
Therefore, keeping "reply_to" field for 1st level comments as itemid or empty
will lead to same result.

https://codereview.appspot.com/256840043/diff/260001/MoinMoin/items/ticket.py...
MoinMoin/items/ticket.py:196: parent = lookup[rev.meta['reply_to']]
On 2015/08/18 16:07:19, Thomas.J.Waldmann wrote:
> maybe reduce duplication by putting
> reply_to = rev.meta['reply_to']
> before the if/else block.

ok will update this

https://codereview.appspot.com/256840043/diff/260001/MoinMoin/templates/ticke...
File MoinMoin/templates/ticket/base.html (right):

https://codereview.appspot.com/256840043/diff/260001/MoinMoin/templates/ticke...
MoinMoin/templates/ticket/base.html:42: {% macro print_comment(comment) %}
On 2015/08/18 16:07:19, Thomas.J.Waldmann wrote:
> maybe use render_comment rather.
> so in case someone greps for stray debug-prints, he does not find your stuff.

yes you are right. I will update this
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b