Merge ~ilasc/launchpad:add-delete-comment-revision-ui into launchpad:master

Proposed by Ioana Lasc
Status: Merged
Approved by: Ioana Lasc
Approved revision: 154ff1ec9f88b5dab54d42bf40a27bd7db4337b7
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ilasc/launchpad:add-delete-comment-revision-ui
Merge into: launchpad:master
Diff against target: 326 lines (+147/-17)
7 files modified
lib/canonical/launchpad/icing/css/base.scss (+7/-0)
lib/lp/answers/templates/questionmessage-display.pt (+5/-1)
lib/lp/bugs/templates/bugcomment-box.pt (+5/-2)
lib/lp/code/templates/codereviewcomment-header.pt (+3/-0)
lib/lp/services/messages/javascript/messages.edit.js (+63/-4)
lib/lp/services/messages/javascript/tests/test_messages.edit.html (+8/-2)
lib/lp/services/messages/javascript/tests/test_messages.edit.js (+56/-8)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+406151@code.launchpad.net

Commit message

Add delete comment revision capability

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) wrote :

Screenshots available here with the respective context available above each pic in MM:

https://chat.canonical.com/canonical/pl/gdc3uozzepyq9rs8qd1m3qctiw
https://chat.canonical.com/canonical/pl/8whytdy1ajyu7g6z856jjj5s7e

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
Revision history for this message
Ioana Lasc (ilasc) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
2index 1e33ca7..086e64d 100644
3--- a/lib/canonical/launchpad/icing/css/base.scss
4+++ b/lib/canonical/launchpad/icing/css/base.scss
5@@ -625,6 +625,13 @@ body {
6 padding-left: 20px;
7 padding-bottom: 10px;
8 }
9+
10+ // If the content was deleted, we show a default message with
11+ // this CSS class.
12+ .deleted-content {
13+ padding: 5px;
14+ opacity: 50%;
15+ }
16 }
17
18 .active {
19diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
20index fa39275..8b58409 100644
21--- a/lib/lp/answers/templates/questionmessage-display.pt
22+++ b/lib/lp/answers/templates/questionmessage-display.pt
23@@ -8,7 +8,8 @@
24 tal:define="css_classes view/getBoardCommentCSSClass"
25 tal:attributes="class string:${css_classes};
26 id string:comment-${context/index};
27- data-baseurl context/fmt:url">
28+ data-baseurl context/fmt:url;
29+ data-i-can-edit view/can_edit">
30 <div class="boardCommentDetails">
31 <table>
32 <tbody>
33@@ -23,6 +24,9 @@
34 <script type="text/template">
35 <div class='message-revision-item'>
36 <div class='message-revision-title'>
37+ <a class="sprite remove action-icon message-revision-del-btn">
38+ Remove
39+ </a>
40 <a class="js-action">
41 Revision #{revision}, created at {date_created_display}
42 </a>
43diff --git a/lib/lp/bugs/templates/bugcomment-box.pt b/lib/lp/bugs/templates/bugcomment-box.pt
44index c9f1d60..b25c482 100644
45--- a/lib/lp/bugs/templates/bugcomment-box.pt
46+++ b/lib/lp/bugs/templates/bugcomment-box.pt
47@@ -11,8 +11,8 @@
48 python: comment.show_for_admin and 'adminHiddenComment' or ''"
49 tal:attributes="class string:boardComment editable-message
50 ${remote_bug_comment_class} ${admin_comment_hidden_class};
51- data-baseurl comment/fmt:url">
52-
53+ data-baseurl comment/fmt:url;
54+ data-i-can-edit view/can_edit">
55 <div class="boardCommentDetails">
56 <div class="message-revision-container">
57 <div class="message-revision-container-header">
58@@ -23,6 +23,9 @@
59 <script type="text/template">
60 <div class='message-revision-item'>
61 <div class='message-revision-title'>
62+ <a class="sprite remove action-icon message-revision-del-btn">
63+ Remove
64+ </a>
65 <a class="js-action">
66 Revision #{revision}, created at {date_created_display}
67 </a>
68diff --git a/lib/lp/code/templates/codereviewcomment-header.pt b/lib/lp/code/templates/codereviewcomment-header.pt
69index 8ddea58..1f597c3 100644
70--- a/lib/lp/code/templates/codereviewcomment-header.pt
71+++ b/lib/lp/code/templates/codereviewcomment-header.pt
72@@ -12,6 +12,9 @@
73 <script type="text/template">
74 <div class='message-revision-item'>
75 <div class='message-revision-title'>
76+ <a class="sprite remove action-icon message-revision-del-btn">
77+ Remove
78+ </a>
79 <a class="js-action">
80 Revision #{revision}, created at {date_created_display}
81 </a>
82diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
83index 48c9b8a..f148a45 100644
84--- a/lib/lp/services/messages/javascript/messages.edit.js
85+++ b/lib/lp/services/messages/javascript/messages.edit.js
86@@ -7,6 +7,7 @@
87 * - A div container with the class .editable-message containing everything
88 * else related to the message
89 * - A data-baseurl="/path/to/msg" on the .editable-message container
90+ * - A data-i-can-edit="True|False" on the .editable-message container
91 * - A .editable-message-body container with the original msg content
92 * - A .editable-message-edit-btn element inside the main container, that will
93 * switch the view to edit form when clicked.
94@@ -39,6 +40,16 @@ YUI.add('lp.services.messages.edit', function(Y) {
95 "Please try again in a few minutes."
96 );
97
98+ module.confirm_delete_revision_msg = (
99+ "Are you sure you want to delete this "+
100+ "revision content?");
101+
102+ module.deleted_content_msg = (
103+ "<span class='deleted-content'>" +
104+ "Content deleted by the user." +
105+ "</span>"
106+ );
107+
108 module.confirm_delete_message = (
109 "Are you sure you want to delete this "+
110 "comment and all its revisions?");
111@@ -255,6 +266,7 @@ YUI.add('lp.services.messages.edit', function(Y) {
112 module.fillMessageRevisions = function(elements, revisions) {
113 // Position the message revision list element.
114 revisions = revisions.reverse();
115+ var i_can_edit = elements.container.getData('i-can-edit') === "True";
116 var revisions_container = elements.container.one(
117 ".message-revision-container");
118 var last_edit_el = elements.last_edit.getDOMNode();
119@@ -271,14 +283,26 @@ YUI.add('lp.services.messages.edit', function(Y) {
120 revisions_container.setStyle('display', 'none');
121 });
122
123- var content = "";
124+ nodes_holder.getDOMNode().innerHTML = "";
125 revisions.forEach(function(rev) {
126 var attrs = rev.getAttrs();
127- attrs.content = module.htmlify_msg(attrs.content);
128- content += Y.Lang.sub(template, attrs);
129+ if (!attrs.date_deleted) {
130+ attrs.content = module.htmlify_msg(attrs.content);
131+ }
132+ else {
133+ attrs.content = module.deleted_content_msg;
134+ }
135+ var node = Y.DOM.create(Y.Lang.sub(template, attrs));
136+ node.dataset['revision_url'] = attrs.self_link;
137+ nodes_holder.appendChild(node);
138+
139+ if(attrs.date_deleted || !i_can_edit) {
140+ // If it was already deleted or I don't have permission to
141+ // delete it, remove the "delete button".
142+ module.removeDeleteRevisionButton(Y.Node(node));
143+ }
144 });
145
146- nodes_holder.getDOMNode().innerHTML = content;
147 nodes_holder.all(".message-revision-item").each(function(rev_item) {
148 rev_item.one(".message-revision-title").on('click', function() {
149 nodes_holder.all('.message-revision-body').setStyle(
150@@ -291,9 +315,44 @@ YUI.add('lp.services.messages.edit', function(Y) {
151 'active');
152 rev_item.addClass('active');
153 });
154+
155+ var delete_btn = rev_item.one(".message-revision-del-btn");
156+ if (delete_btn) {
157+ delete_btn.on('click', function() {
158+ module.deleteMessageRevisionContent(rev_item);
159+ });
160+ }
161 });
162 };
163
164+ module.removeDeleteRevisionButton = function(rev_item_node) {
165+ var delete_btn = rev_item_node.one('.message-revision-del-btn');
166+ if (delete_btn) {
167+ var node_to_remove = delete_btn.getDOMNode();
168+ node_to_remove.parentNode.removeChild(node_to_remove);
169+ }
170+ };
171+
172+ module.deleteMessageRevisionContent = function(rev_item) {
173+ var revision_url = rev_item.getData('revision_url');
174+ if (module.confirm(module.confirm_delete_revision_msg)) {
175+ var config = {
176+ on: {
177+ success: function() {
178+ var body_dom = rev_item.one(
179+ '.message-revision-body').getDOMNode();
180+ body_dom.innerHTML = module.deleted_content_msg;
181+ module.removeDeleteRevisionButton(rev_item);
182+ },
183+ failure: function(err) {
184+ alert("There was an error. Please try again.");
185+ }
186+ }
187+ };
188+ this.lp_client.named_post(revision_url, 'deleteContent', config);
189+ }
190+ };
191+
192 module.onLastEditClick = function(elements, baseurl) {
193 // Hide all open revision containers.
194 Y.all('.message-revision-container').each(function(container) {
195diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.html b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
196index ce4d0df..f035857 100644
197--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
198+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
199@@ -54,7 +54,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
200
201 <!-- First editable message -->
202 <div class="editable-message" id="first-message"
203- data-baseurl="/message/1">
204+ data-baseurl="/message/1" data-i-can-edit="True">
205 <!-- Revision list container and its template -->
206 <div class="message-revision-container">
207 <!-- The revision list pop-up header, with the image that closes
208@@ -74,6 +74,9 @@ GNU Affero General Public License version 3 (see the file LICENSE).
209 <!-- The description of the revision, that will expand the
210 revision content once clicked. -->
211 <div class='message-revision-title'>
212+ <a class="sprite remove action-icon message-revision-del-btn">
213+ Remove
214+ </a>
215 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>
216 </div>
217 <!-- The revision content itself -->
218@@ -109,7 +112,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
219
220 <!-- Second editable message -->
221 <div class="editable-message" id="second-message"
222- data-baseurl="/message/2">
223+ data-baseurl="/message/2" data-i-can-edit="True">
224 <!-- Revision list container and its template -->
225 <div class="message-revision-container">
226 <div class="message-revision-container-header">
227@@ -120,6 +123,9 @@ GNU Affero General Public License version 3 (see the file LICENSE).
228 <script type="text/template">
229 <div class='message-revision-item'>
230 <div class='message-revision-title'>
231+ <a class="sprite remove action-icon message-revision-del-btn">
232+ Remove
233+ </a>
234 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>
235 </div>
236 <div class='message-revision-body'>{content}</div>
237diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.js b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
238index 0bcccb3..dfc2a07 100644
239--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.js
240+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
241@@ -82,11 +82,11 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
242 this.msg_bodies[i].setStyle('display', '');
243 this.msg_forms[i].setStyle('display', '');
244 this.textareas[i].getDOMNode().value = '';
245- this.last_edit[0].getDOMNode().innerHTML = ':';
246- this.last_edit[1].getDOMNode().innerHTML = (
247- '(last edit 5 minutes ago):');
248 this.revision_history_lists[i].getDOMNode().innerHTML = '';
249 }
250+ this.last_edit[0].getDOMNode().innerHTML = ':';
251+ this.last_edit[1].getDOMNode().innerHTML = (
252+ '(last edit 5 minutes ago):');
253 },
254
255 test_instantiation_hides_forms: function() {
256@@ -223,17 +223,65 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
257 var title = rev.one('.message-revision-title');
258 var body = rev.one('.message-revision-body');
259 var expected_title = Y.Lang.sub(
260- '<a class="js-action">' +
261- 'Revision #{revision}, created at {date_created_display}' +
262- '</a>',
263+ 'Remove Revision #{revision},'+
264+ ' created at {date_created_display}',
265 entry);
266- Y.Assert.areSame(
267- expected_title, title.getDOMNode().innerHTML.trim());
268+ Y.Assert.areEqual(
269+ expected_title, title.getDOMNode().innerText);
270+
271 Y.Assert.areSame(
272 module.htmlify_msg(entry.content),
273 body.getDOMNode().innerHTML);
274 });
275
276+ revisions = this.revision_history_lists[1].all(
277+ ".message-revision-item");
278+ Y.Assert.areSame(2, revisions.size());
279+
280+ // Delete one revision from the revision list of
281+ // the second message
282+ revisions.each(function(rev, j) {
283+ var entry = response.entries[response.entries.length - j -1];
284+ var title = rev.one('.message-revision-title');
285+ var body = rev.one('.message-revision-body');
286+
287+ if (j===0){
288+ // mock user's response to modal widonw that is
289+ // asking the confirmation to delete revision
290+ module.confirm = function(){
291+ return 'True';
292+ };
293+ // user clicks on the Remove icon for the revision
294+ rev.one('.message-revision-del-btn').simulate('click');
295+ module.lp_client.io_provider.success({
296+ responseText: 'null',
297+ responseHeaders: {'Content-Type': 'application/json'}
298+ });
299+
300+ // the title displayed for the revision stays the same
301+ var expected_title = Y.Lang.sub(
302+ 'Revision #{revision},' +
303+ ' created at {date_created_display}',
304+ entry);
305+ Y.Assert.areSame(
306+ expected_title,
307+ title.getDOMNode().innerText);
308+
309+ // the content has changed to reflect deletion
310+ var expected_body = Y.Lang.sub(
311+ 'Content deleted by the user.',
312+ entry);
313+ Y.Assert.areEqual(
314+ expected_body, body.getDOMNode().innerText);
315+ }
316+ });
317+ // The number of revisions in the list stays the same
318+ // and we asserted above the content displayed has changed
319+ // to reflect deletion for revision 2
320+ var after_delete_revis = this.revision_history_lists[1].all(
321+ ".message-revision-item");
322+ Y.Assert.areSame(2, after_delete_revis.size());
323+
324 // Lets make sure that a click on the "close" button hides the
325 // revisions list.
326 revisions_container.one(

Subscribers

People subscribed via source and target branches

to status/vote changes: