Merge ~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 878eb77596392fa2add442988d196fd89cba5368
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:comment-editing-ui-codereview
Merge into: launchpad:master
Prerequisite: ~pappacena/launchpad:comment-editing-ui-bugcomment
Diff against target: 145 lines (+29/-8)
6 files modified
lib/lp/code/browser/codereviewcomment.py (+5/-0)
lib/lp/code/browser/tests/test_branchmergeproposal.py (+3/-1)
lib/lp/code/stories/branches/xx-code-review-comments.txt (+4/-4)
lib/lp/code/templates/branchmergeproposal-index.pt (+3/-1)
lib/lp/code/templates/codereviewcomment-header.pt (+12/-1)
lib/lp/services/comments/templates/comment.pt (+2/-1)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+402850@code.launchpad.net

Commit message

Adding editing option to code review comments

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

This should be good to go now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
2index 094cd79..5891c6f 100644
3--- a/lib/lp/code/browser/codereviewcomment.py
4+++ b/lib/lp/code/browser/codereviewcomment.py
5@@ -59,6 +59,7 @@ from lp.services.webapp import (
6 Navigation,
7 stepthrough,
8 )
9+from lp.services.webapp.authorization import check_permission
10 from lp.services.webapp.interfaces import ILaunchBag
11
12
13@@ -215,6 +216,10 @@ class CodeReviewCommentView(LaunchpadView):
14 return download_body(
15 CodeReviewDisplayComment(self.context), self.request)
16
17+ @property
18+ def can_edit(self):
19+ return check_permission('launchpad.Edit', self.context.message)
20+
21 # Should the comment be shown in full?
22 full_comment = True
23 # Show comment expanders?
24diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
25index f32350e..ba2ee85 100644
26--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
27+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
28@@ -2315,7 +2315,9 @@ class TestBranchMergeProposal(BrowserTestCase):
29 comment = self.factory.makeCodeReviewComment(body='x y' * 2000)
30 has_read_more = self.has_read_more(comment)
31 browser = self.getViewBrowser(comment.branch_merge_proposal)
32- self.assertNotIn('x y' * 2000, browser.contents)
33+ txt = extract_text(
34+ find_tags_by_class(browser.contents, "comment-text")[0])
35+ self.assertNotIn('x y' * 2000, txt)
36 self.assertThat(browser.contents, has_read_more)
37
38 def test_short_conversation_comments_no_download(self):
39diff --git a/lib/lp/code/stories/branches/xx-code-review-comments.txt b/lib/lp/code/stories/branches/xx-code-review-comments.txt
40index 1c3738d..30d3a26 100644
41--- a/lib/lp/code/stories/branches/xx-code-review-comments.txt
42+++ b/lib/lp/code/stories/branches/xx-code-review-comments.txt
43@@ -39,7 +39,7 @@ to the main merge proposal page.
44
45 >>> print_comments('boardCommentDetails')
46 Eric the Viking (eric) wrote ...
47- >>> print_comments('boardCommentBody')
48+ >>> print_comments('comment-text')
49 This is a very long comment about what things should be done to the
50 source branch to land it. When this comment is replied to, it should
51 wrap the line properly.
52@@ -82,7 +82,7 @@ since it failed spuriously in buildbot.
53
54 After this, we are taken to the main page for the merge proposal
55
56- >>> print_comments('boardCommentBody', eric_browser, index=1)
57+ >>> print_comments('comment-text', eric_browser, index=1)
58 I like this.
59 I wish I had time to review it properly
60 This is a longer message with several lines
61@@ -91,7 +91,7 @@ After this, we are taken to the main page for the merge proposal
62 Email addresses in code review comments are hidden for users not logged in.
63
64 >>> anon_browser.open(merge_proposal_url)
65- >>> print_comments('boardCommentBody', anon_browser, index=1)
66+ >>> print_comments('comment-text', anon_browser, index=1)
67 I like this.
68 I wish I had time to review it properly
69 This is a longer message with several lines
70@@ -106,7 +106,7 @@ are also displayed in the new proposal.
71 >>> new_merge_proposal_url = canonical_url(new_merge_proposal)
72 >>> logout()
73 >>> anon_browser.open(new_merge_proposal_url)
74- >>> print_comments('boardCommentBody', anon_browser, index=0)
75+ >>> print_comments('comment-text', anon_browser, index=0)
76 This is a very long comment about what things should be done to the
77 source branch to land it. When this comment is replied to, it should
78 wrap the line properly.
79diff --git a/lib/lp/code/templates/branchmergeproposal-index.pt b/lib/lp/code/templates/branchmergeproposal-index.pt
80index 6ba4621..4a01ec5 100644
81--- a/lib/lp/code/templates/branchmergeproposal-index.pt
82+++ b/lib/lp/code/templates/branchmergeproposal-index.pt
83@@ -244,7 +244,8 @@
84 'lp.code.branchmergeproposal.status', 'lp.app.comment',
85 'lp.app.widgets.expander',
86 'lp.code.branch.revisionexpander',
87- 'lp.code.branchmergeproposal.inlinecomments', function(Y) {
88+ 'lp.code.branchmergeproposal.inlinecomments',
89+ 'lp.services.messages.edit', function(Y) {
90
91 Y.on('load', function() {
92 var logged_in = LP.links['me'] !== undefined;
93@@ -285,6 +286,7 @@
94 '.expander-content',
95 false,
96 Y.lp.code.branch.revisionexpander.bmp_diff_loader);
97+ Y.lp.services.messages.edit.setup();
98 });
99 });
100 <tal:script replace="structure string:&lt;/script&gt;" />
101diff --git a/lib/lp/code/templates/codereviewcomment-header.pt b/lib/lp/code/templates/codereviewcomment-header.pt
102index efd37eb..77c4650 100644
103--- a/lib/lp/code/templates/codereviewcomment-header.pt
104+++ b/lib/lp/code/templates/codereviewcomment-header.pt
105@@ -18,7 +18,13 @@
106 datetime context/comment_date/fmt:isodate"
107 tal:content="context/comment_date/fmt:displaydate">
108 7 minutes ago
109- </time>:
110+ </time><span class="editable-message-last-edit-date"><tal:last-edit condition="context/date_last_edited">
111+ (last edit <time
112+ itemprop="editTime"
113+ tal:attributes="title context/date_last_edited/fmt:datetime;
114+ datetime context/date_last_edited/fmt:isodate"
115+ tal:content="context/date_last_edited/fmt:displaydate" />)</tal:last-edit>:
116+ </span>
117 <span
118 tal:condition="context/from_superseded"
119 class="sprite warning-icon"
120@@ -29,6 +35,11 @@
121 of this proposal</span>
122 </td>
123
124+ <td>
125+ <img class="sprite edit action-icon editable-message-edit-btn"
126+ tal:condition="view/can_edit"/>
127+ </td>
128+
129 <td class="bug-comment-index">
130 <a itemprop="url"
131 tal:attributes="href context/fmt:url">#</a>
132diff --git a/lib/lp/services/comments/templates/comment.pt b/lib/lp/services/comments/templates/comment.pt
133index 19a4d1f..5061471 100644
134--- a/lib/lp/services/comments/templates/comment.pt
135+++ b/lib/lp/services/comments/templates/comment.pt
136@@ -6,7 +6,8 @@
137 <div
138 itemscope=""
139 itemtype="http://schema.org/UserComments"
140- tal:attributes="class string:boardComment ${context/extra_css_class|nothing}">
141+ tal:attributes="class string:boardComment editable-message ${context/extra_css_class|nothing};
142+ data-baseurl context/fmt:url|nothing">
143 <div class="boardCommentDetails"
144 tal:content="structure context/@@+comment-header">
145 Details - everyone has details.