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 (community) 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
diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index 1e33ca7..086e64d 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -625,6 +625,13 @@ body {
625 padding-left: 20px;625 padding-left: 20px;
626 padding-bottom: 10px;626 padding-bottom: 10px;
627 }627 }
628
629 // If the content was deleted, we show a default message with
630 // this CSS class.
631 .deleted-content {
632 padding: 5px;
633 opacity: 50%;
634 }
628 }635 }
629636
630 .active {637 .active {
diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index fa39275..8b58409 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -8,7 +8,8 @@
8 tal:define="css_classes view/getBoardCommentCSSClass"8 tal:define="css_classes view/getBoardCommentCSSClass"
9 tal:attributes="class string:${css_classes};9 tal:attributes="class string:${css_classes};
10 id string:comment-${context/index};10 id string:comment-${context/index};
11 data-baseurl context/fmt:url">11 data-baseurl context/fmt:url;
12 data-i-can-edit view/can_edit">
12 <div class="boardCommentDetails">13 <div class="boardCommentDetails">
13 <table>14 <table>
14 <tbody>15 <tbody>
@@ -23,6 +24,9 @@
23 <script type="text/template">24 <script type="text/template">
24 <div class='message-revision-item'>25 <div class='message-revision-item'>
25 <div class='message-revision-title'>26 <div class='message-revision-title'>
27 <a class="sprite remove action-icon message-revision-del-btn">
28 Remove
29 </a>
26 <a class="js-action">30 <a class="js-action">
27 Revision #{revision}, created at {date_created_display}31 Revision #{revision}, created at {date_created_display}
28 </a>32 </a>
diff --git a/lib/lp/bugs/templates/bugcomment-box.pt b/lib/lp/bugs/templates/bugcomment-box.pt
index c9f1d60..b25c482 100644
--- a/lib/lp/bugs/templates/bugcomment-box.pt
+++ b/lib/lp/bugs/templates/bugcomment-box.pt
@@ -11,8 +11,8 @@
11 python: comment.show_for_admin and 'adminHiddenComment' or ''"11 python: comment.show_for_admin and 'adminHiddenComment' or ''"
12 tal:attributes="class string:boardComment editable-message12 tal:attributes="class string:boardComment editable-message
13 ${remote_bug_comment_class} ${admin_comment_hidden_class};13 ${remote_bug_comment_class} ${admin_comment_hidden_class};
14 data-baseurl comment/fmt:url">14 data-baseurl comment/fmt:url;
1515 data-i-can-edit view/can_edit">
16 <div class="boardCommentDetails">16 <div class="boardCommentDetails">
17 <div class="message-revision-container">17 <div class="message-revision-container">
18 <div class="message-revision-container-header">18 <div class="message-revision-container-header">
@@ -23,6 +23,9 @@
23 <script type="text/template">23 <script type="text/template">
24 <div class='message-revision-item'>24 <div class='message-revision-item'>
25 <div class='message-revision-title'>25 <div class='message-revision-title'>
26 <a class="sprite remove action-icon message-revision-del-btn">
27 Remove
28 </a>
26 <a class="js-action">29 <a class="js-action">
27 Revision #{revision}, created at {date_created_display}30 Revision #{revision}, created at {date_created_display}
28 </a>31 </a>
diff --git a/lib/lp/code/templates/codereviewcomment-header.pt b/lib/lp/code/templates/codereviewcomment-header.pt
index 8ddea58..1f597c3 100644
--- a/lib/lp/code/templates/codereviewcomment-header.pt
+++ b/lib/lp/code/templates/codereviewcomment-header.pt
@@ -12,6 +12,9 @@
12 <script type="text/template">12 <script type="text/template">
13 <div class='message-revision-item'>13 <div class='message-revision-item'>
14 <div class='message-revision-title'>14 <div class='message-revision-title'>
15 <a class="sprite remove action-icon message-revision-del-btn">
16 Remove
17 </a>
15 <a class="js-action">18 <a class="js-action">
16 Revision #{revision}, created at {date_created_display}19 Revision #{revision}, created at {date_created_display}
17 </a>20 </a>
diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
index 48c9b8a..f148a45 100644
--- a/lib/lp/services/messages/javascript/messages.edit.js
+++ b/lib/lp/services/messages/javascript/messages.edit.js
@@ -7,6 +7,7 @@
7 * - A div container with the class .editable-message containing everything7 * - A div container with the class .editable-message containing everything
8 * else related to the message8 * else related to the message
9 * - A data-baseurl="/path/to/msg" on the .editable-message container9 * - A data-baseurl="/path/to/msg" on the .editable-message container
10 * - A data-i-can-edit="True|False" on the .editable-message container
10 * - A .editable-message-body container with the original msg content11 * - A .editable-message-body container with the original msg content
11 * - A .editable-message-edit-btn element inside the main container, that will12 * - A .editable-message-edit-btn element inside the main container, that will
12 * switch the view to edit form when clicked.13 * switch the view to edit form when clicked.
@@ -39,6 +40,16 @@ YUI.add('lp.services.messages.edit', function(Y) {
39 "Please try again in a few minutes."40 "Please try again in a few minutes."
40 );41 );
4142
43 module.confirm_delete_revision_msg = (
44 "Are you sure you want to delete this "+
45 "revision content?");
46
47 module.deleted_content_msg = (
48 "<span class='deleted-content'>" +
49 "Content deleted by the user." +
50 "</span>"
51 );
52
42 module.confirm_delete_message = (53 module.confirm_delete_message = (
43 "Are you sure you want to delete this "+54 "Are you sure you want to delete this "+
44 "comment and all its revisions?");55 "comment and all its revisions?");
@@ -255,6 +266,7 @@ YUI.add('lp.services.messages.edit', function(Y) {
255 module.fillMessageRevisions = function(elements, revisions) {266 module.fillMessageRevisions = function(elements, revisions) {
256 // Position the message revision list element.267 // Position the message revision list element.
257 revisions = revisions.reverse();268 revisions = revisions.reverse();
269 var i_can_edit = elements.container.getData('i-can-edit') === "True";
258 var revisions_container = elements.container.one(270 var revisions_container = elements.container.one(
259 ".message-revision-container");271 ".message-revision-container");
260 var last_edit_el = elements.last_edit.getDOMNode();272 var last_edit_el = elements.last_edit.getDOMNode();
@@ -271,14 +283,26 @@ YUI.add('lp.services.messages.edit', function(Y) {
271 revisions_container.setStyle('display', 'none');283 revisions_container.setStyle('display', 'none');
272 });284 });
273285
274 var content = "";286 nodes_holder.getDOMNode().innerHTML = "";
275 revisions.forEach(function(rev) {287 revisions.forEach(function(rev) {
276 var attrs = rev.getAttrs();288 var attrs = rev.getAttrs();
277 attrs.content = module.htmlify_msg(attrs.content);289 if (!attrs.date_deleted) {
278 content += Y.Lang.sub(template, attrs);290 attrs.content = module.htmlify_msg(attrs.content);
291 }
292 else {
293 attrs.content = module.deleted_content_msg;
294 }
295 var node = Y.DOM.create(Y.Lang.sub(template, attrs));
296 node.dataset['revision_url'] = attrs.self_link;
297 nodes_holder.appendChild(node);
298
299 if(attrs.date_deleted || !i_can_edit) {
300 // If it was already deleted or I don't have permission to
301 // delete it, remove the "delete button".
302 module.removeDeleteRevisionButton(Y.Node(node));
303 }
279 });304 });
280305
281 nodes_holder.getDOMNode().innerHTML = content;
282 nodes_holder.all(".message-revision-item").each(function(rev_item) {306 nodes_holder.all(".message-revision-item").each(function(rev_item) {
283 rev_item.one(".message-revision-title").on('click', function() {307 rev_item.one(".message-revision-title").on('click', function() {
284 nodes_holder.all('.message-revision-body').setStyle(308 nodes_holder.all('.message-revision-body').setStyle(
@@ -291,9 +315,44 @@ YUI.add('lp.services.messages.edit', function(Y) {
291 'active');315 'active');
292 rev_item.addClass('active');316 rev_item.addClass('active');
293 });317 });
318
319 var delete_btn = rev_item.one(".message-revision-del-btn");
320 if (delete_btn) {
321 delete_btn.on('click', function() {
322 module.deleteMessageRevisionContent(rev_item);
323 });
324 }
294 });325 });
295 };326 };
296327
328 module.removeDeleteRevisionButton = function(rev_item_node) {
329 var delete_btn = rev_item_node.one('.message-revision-del-btn');
330 if (delete_btn) {
331 var node_to_remove = delete_btn.getDOMNode();
332 node_to_remove.parentNode.removeChild(node_to_remove);
333 }
334 };
335
336 module.deleteMessageRevisionContent = function(rev_item) {
337 var revision_url = rev_item.getData('revision_url');
338 if (module.confirm(module.confirm_delete_revision_msg)) {
339 var config = {
340 on: {
341 success: function() {
342 var body_dom = rev_item.one(
343 '.message-revision-body').getDOMNode();
344 body_dom.innerHTML = module.deleted_content_msg;
345 module.removeDeleteRevisionButton(rev_item);
346 },
347 failure: function(err) {
348 alert("There was an error. Please try again.");
349 }
350 }
351 };
352 this.lp_client.named_post(revision_url, 'deleteContent', config);
353 }
354 };
355
297 module.onLastEditClick = function(elements, baseurl) {356 module.onLastEditClick = function(elements, baseurl) {
298 // Hide all open revision containers.357 // Hide all open revision containers.
299 Y.all('.message-revision-container').each(function(container) {358 Y.all('.message-revision-container').each(function(container) {
diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.html b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
index ce4d0df..f035857 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
@@ -54,7 +54,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
5454
55 <!-- First editable message -->55 <!-- First editable message -->
56 <div class="editable-message" id="first-message"56 <div class="editable-message" id="first-message"
57 data-baseurl="/message/1">57 data-baseurl="/message/1" data-i-can-edit="True">
58 <!-- Revision list container and its template -->58 <!-- Revision list container and its template -->
59 <div class="message-revision-container">59 <div class="message-revision-container">
60 <!-- The revision list pop-up header, with the image that closes60 <!-- The revision list pop-up header, with the image that closes
@@ -74,6 +74,9 @@ GNU Affero General Public License version 3 (see the file LICENSE).
74 <!-- The description of the revision, that will expand the74 <!-- The description of the revision, that will expand the
75 revision content once clicked. -->75 revision content once clicked. -->
76 <div class='message-revision-title'>76 <div class='message-revision-title'>
77 <a class="sprite remove action-icon message-revision-del-btn">
78 Remove
79 </a>
77 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>80 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>
78 </div>81 </div>
79 <!-- The revision content itself -->82 <!-- The revision content itself -->
@@ -109,7 +112,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
109112
110 <!-- Second editable message -->113 <!-- Second editable message -->
111 <div class="editable-message" id="second-message"114 <div class="editable-message" id="second-message"
112 data-baseurl="/message/2">115 data-baseurl="/message/2" data-i-can-edit="True">
113 <!-- Revision list container and its template -->116 <!-- Revision list container and its template -->
114 <div class="message-revision-container">117 <div class="message-revision-container">
115 <div class="message-revision-container-header">118 <div class="message-revision-container-header">
@@ -120,6 +123,9 @@ GNU Affero General Public License version 3 (see the file LICENSE).
120 <script type="text/template">123 <script type="text/template">
121 <div class='message-revision-item'>124 <div class='message-revision-item'>
122 <div class='message-revision-title'>125 <div class='message-revision-title'>
126 <a class="sprite remove action-icon message-revision-del-btn">
127 Remove
128 </a>
123 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>129 <a class="js-action">Revision #{revision}, created at {date_created_display}</a>
124 </div>130 </div>
125 <div class='message-revision-body'>{content}</div>131 <div class='message-revision-body'>{content}</div>
diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.js b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
index 0bcccb3..dfc2a07 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.js
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
@@ -82,11 +82,11 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
82 this.msg_bodies[i].setStyle('display', '');82 this.msg_bodies[i].setStyle('display', '');
83 this.msg_forms[i].setStyle('display', '');83 this.msg_forms[i].setStyle('display', '');
84 this.textareas[i].getDOMNode().value = '';84 this.textareas[i].getDOMNode().value = '';
85 this.last_edit[0].getDOMNode().innerHTML = ':';
86 this.last_edit[1].getDOMNode().innerHTML = (
87 '(last edit 5 minutes ago):');
88 this.revision_history_lists[i].getDOMNode().innerHTML = '';85 this.revision_history_lists[i].getDOMNode().innerHTML = '';
89 }86 }
87 this.last_edit[0].getDOMNode().innerHTML = ':';
88 this.last_edit[1].getDOMNode().innerHTML = (
89 '(last edit 5 minutes ago):');
90 },90 },
9191
92 test_instantiation_hides_forms: function() {92 test_instantiation_hides_forms: function() {
@@ -223,17 +223,65 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
223 var title = rev.one('.message-revision-title');223 var title = rev.one('.message-revision-title');
224 var body = rev.one('.message-revision-body');224 var body = rev.one('.message-revision-body');
225 var expected_title = Y.Lang.sub(225 var expected_title = Y.Lang.sub(
226 '<a class="js-action">' +226 'Remove Revision #{revision},'+
227 'Revision #{revision}, created at {date_created_display}' +227 ' created at {date_created_display}',
228 '</a>',
229 entry);228 entry);
230 Y.Assert.areSame(229 Y.Assert.areEqual(
231 expected_title, title.getDOMNode().innerHTML.trim());230 expected_title, title.getDOMNode().innerText);
231
232 Y.Assert.areSame(232 Y.Assert.areSame(
233 module.htmlify_msg(entry.content),233 module.htmlify_msg(entry.content),
234 body.getDOMNode().innerHTML);234 body.getDOMNode().innerHTML);
235 });235 });
236236
237 revisions = this.revision_history_lists[1].all(
238 ".message-revision-item");
239 Y.Assert.areSame(2, revisions.size());
240
241 // Delete one revision from the revision list of
242 // the second message
243 revisions.each(function(rev, j) {
244 var entry = response.entries[response.entries.length - j -1];
245 var title = rev.one('.message-revision-title');
246 var body = rev.one('.message-revision-body');
247
248 if (j===0){
249 // mock user's response to modal widonw that is
250 // asking the confirmation to delete revision
251 module.confirm = function(){
252 return 'True';
253 };
254 // user clicks on the Remove icon for the revision
255 rev.one('.message-revision-del-btn').simulate('click');
256 module.lp_client.io_provider.success({
257 responseText: 'null',
258 responseHeaders: {'Content-Type': 'application/json'}
259 });
260
261 // the title displayed for the revision stays the same
262 var expected_title = Y.Lang.sub(
263 'Revision #{revision},' +
264 ' created at {date_created_display}',
265 entry);
266 Y.Assert.areSame(
267 expected_title,
268 title.getDOMNode().innerText);
269
270 // the content has changed to reflect deletion
271 var expected_body = Y.Lang.sub(
272 'Content deleted by the user.',
273 entry);
274 Y.Assert.areEqual(
275 expected_body, body.getDOMNode().innerText);
276 }
277 });
278 // The number of revisions in the list stays the same
279 // and we asserted above the content displayed has changed
280 // to reflect deletion for revision 2
281 var after_delete_revis = this.revision_history_lists[1].all(
282 ".message-revision-item");
283 Y.Assert.areSame(2, after_delete_revis.size());
284
237 // Lets make sure that a click on the "close" button hides the285 // Lets make sure that a click on the "close" button hides the
238 // revisions list.286 // revisions list.
239 revisions_container.one(287 revisions_container.one(

Subscribers

People subscribed via source and target branches

to status/vote changes: