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

[Ioana Lasc]

Add delete comment UI

[Colin Watson (cjwatson)]

This basically all looks good as far as it goes, thanks! As discussed on Mattermost, though, I think we need to wire up some kind of confirmation here before landing this: it feels a little too easy to accidentally delete messages right now, either due to not having very good pointer coordination or by not understanding what the icon means.

review: Needs Fixing

82c1799... by Ioana Lasc on 2021-07-26

Shorten revision list heading

9e7b120... by Ioana Lasc on 2021-07-27

Add confirmation dialog to delete comment

[Ioana Lasc (ilasc)]

Added a dialog popup for confirmation prior to actual deletion, screenshots here:

https://chat.canonical.com/canonical/pl/8xh5eduda7839pd9juhubd9bze
https://chat.canonical.com/canonical/pl/643y4suakpgfmyn7d4o9pbjcrc

MP now ready for another look.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index 49973ab..fa39275 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -59,6 +59,10 @@
       <img class="sprite edit action-icon editable-message-edit-btn"
            tal:condition="view/can_edit"/>
     </td>
+    <td>
+      <img class="sprite remove action-icon editable-message-delete-btn"
+           tal:condition="view/can_edit"/>
+    </td>
     <td class="bug-comment-index">
       <a
 	tal:content="string: #${context/display_index}" />
diff --git a/lib/lp/bugs/templates/bugcomment-box.pt b/lib/lp/bugs/templates/bugcomment-box.pt
index 1e1882a..c9f1d60 100644
--- a/lib/lp/bugs/templates/bugcomment-box.pt
+++ b/lib/lp/bugs/templates/bugcomment-box.pt
@@ -92,7 +92,10 @@
             <img class="sprite edit action-icon editable-message-edit-btn"
                  tal:condition="view/can_edit"/>
           </td>
-
+          <td>
+            <img class="sprite remove action-icon editable-message-delete-btn"
+                 tal:condition="view/can_edit"/>
+          </td>
           <td class="bug-comment-index">
           <a itemprop="url"
              tal:attributes="href comment/fmt:url"
diff --git a/lib/lp/code/templates/codereviewcomment-header.pt b/lib/lp/code/templates/codereviewcomment-header.pt
index 3511b9f..8ddea58 100644
--- a/lib/lp/code/templates/codereviewcomment-header.pt
+++ b/lib/lp/code/templates/codereviewcomment-header.pt
@@ -59,7 +59,10 @@
         <img class="sprite edit action-icon editable-message-edit-btn"
              tal:condition="view/can_edit"/>
       </td>
-
+      <td>
+        <img class="sprite remove action-icon editable-message-delete-btn"
+             tal:condition="view/can_edit"/>
+      </td>
       <td class="bug-comment-index">
         <a itemprop="url"
           tal:attributes="href context/fmt:url">#</a>
diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
index 56762e8..48c9b8a 100644
--- a/lib/lp/services/messages/javascript/messages.edit.js
+++ b/lib/lp/services/messages/javascript/messages.edit.js
@@ -39,6 +39,15 @@ YUI.add('lp.services.messages.edit', function(Y) {
         "Please try again in a few minutes."
     );
 
+    module.confirm_delete_message = (
+        "Are you sure you want to delete this "+
+        "comment and all its revisions?");
+
+    // Making it easier to mock on tests.
+    module.confirm = function(msg) {
+        return confirm(msg);
+    };
+
     module.htmlify_msg = function(text) {
         text = text.replace(/&/g, "&amp;");
         text = text.replace(/</g, "&lt;");
@@ -70,6 +79,20 @@ YUI.add('lp.services.messages.edit', function(Y) {
         this.lp_client.named_post(msg_url, 'editContent', config);
     };
 
+    module.deleteMessageContent = function(
+            msg_path, on_success, on_failure) {
+        var msg_url = "/api/devel" + msg_path;
+        if (module.confirm(module.confirm_delete_message)) {
+            var config = {
+                on: {
+                     success: on_success,
+                     failure: on_failure
+                 }
+            };
+            this.lp_client.named_post(msg_url, 'deleteContent', config);
+        }
+    };
+
     module.showNotification = function(container, msg, can_dismiss) {
         can_dismiss = can_dismiss || false;
         // Clean up previous notification.
@@ -168,6 +191,41 @@ YUI.add('lp.services.messages.edit', function(Y) {
         });
     };
 
+    // What to do when a message is deleted in the backend.
+    module.onMessageDeleted = function(elements, baseurl) {
+        elements.msg_body_text.getDOMNode().innerHTML = '';
+        // if after a delete the user wants to edit the empty
+        // message placeholder, they won't be able to
+        // unless we enable the text area and update button
+        elements.textarea.getDOMNode().disabled = false;
+        elements.update_btn.getDOMNode().disabled = false;
+        module.hideLoading(elements.container);
+        elements.last_edit.getDOMNode().innerHTML = (
+            ' <a href="#" class="editable-message-last-edit-link">' +
+            '(message and all revisions deleted a moment ago):' +
+            '</a>');
+
+        // Wire click handler to the newly created
+        // "message deleted / last edit" button.
+        var last_edit_btn = elements.container.one(
+            '.editable-message-last-edit-link');
+        last_edit_btn.on('click', function(e) {
+            e.preventDefault();
+            module.onLastEditClick(elements, baseurl);
+        });
+    };
+
+    // What to do when a message fails to delete on the backend.
+    module.onMessageDeleteError = function(elements, err) {
+        // When something goes wrong at the backend, re-enable
+        // UI elements and display an error.
+        module.showNotification(
+            elements.container,
+            module.msg_edit_error_notification,  true);
+        elements.textarea.getDOMNode().disabled = false;
+        elements.update_btn.getDOMNode().disabled = false;
+    };
+
     // What to do when a message fails to update on the backend.
     module.onMessageSaveError = function(elements, err) {
         // When something goes wrong at the backend, re-enable
@@ -179,6 +237,21 @@ YUI.add('lp.services.messages.edit', function(Y) {
         elements.update_btn.getDOMNode().disabled = false;
     };
 
+    // What to do when a user clicks the delete button on a msg.
+    module.onDeleteClick = function(elements, baseurl) {
+        module.showLoading(elements.container);
+        var textarea = elements.textarea.getDOMNode();
+        textarea.disabled = true;
+        elements.delete_btn.getDOMNode().disabled = true;
+
+        module.deleteMessageContent(
+            baseurl,
+            function(err) {
+                module.onMessageDeleted(elements, baseurl);
+            },
+            function(err) { module.onMessageDeleteError(elements, err); }
+        );
+    };
     module.fillMessageRevisions = function(elements, revisions) {
         // Position the message revision list element.
         revisions = revisions.reverse();
@@ -255,7 +328,8 @@ YUI.add('lp.services.messages.edit', function(Y) {
             "update_btn": container.one('.editable-message-update-btn'),
             "cancel_btn": container.one('.editable-message-cancel-btn'),
             "last_edit": container.one('.editable-message-last-edit-date'),
-            "last_edit_btn": container.one('.editable-message-last-edit-link')
+            "last_edit_btn": container.one('.editable-message-last-edit-link'),
+            "delete_btn": container.one('.editable-message-delete-btn')
         };
         // If the msg body or the msg form are not defined, don't try to do
         // anything else.
@@ -290,6 +364,10 @@ YUI.add('lp.services.messages.edit', function(Y) {
         elements.cancel_btn.on('click', function(e) {
             module.onEditCancelClick(elements);
         });
+
+        elements.delete_btn.on('click', function(e) {
+            module.onDeleteClick(elements, baseurl);
+        });
     };
 
     module.setup = function() {
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 9d394c2..ce4d0df 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
@@ -98,6 +98,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
                 The message is above :)
             </div>
             <img class="sprite edit action-icon editable-message-edit-btn" />
+            <img class="sprite remove action-icon editable-message-delete-btn" />
 
             <div class="editable-message-form">
                 <textarea></textarea>
@@ -141,6 +142,7 @@ GNU Affero General Public License version 3 (see the file LICENSE).
                 The message is above :)
             </div>
             <img class="sprite edit action-icon editable-message-edit-btn" />
+            <img class="sprite remove action-icon editable-message-delete-btn" />
 
             <div class="editable-message-form">
                 <textarea></textarea>
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 d9644f0..0bcccb3 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.js
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
@@ -71,6 +71,10 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
                 this.containers[0].one(".editable-message-update-btn"),
                 this.containers[1].one(".editable-message-update-btn")
             ];
+            this.delete_btns = [
+                this.containers[0].one(".editable-message-delete-btn"),
+                this.containers[1].one(".editable-message-delete-btn")
+            ];
 
             for(var i=0 ; i<this.containers.length ; i++) {
                 this.msg_texts[i].getDOMNode().innerHTML = (
@@ -235,8 +239,58 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
             revisions_container.one(
                 '.message-revision-close').simulate('click');
             Y.Assert.areSame('none', revisions_container.getStyle('display'));
-        }
+        },
+
+        test_delete_comment: function() {
+            module.setup();
+            module.lp_client.io_provider = new Y.lp.testing.mockio.MockIo();
+            module.confirm = function(){
+                return 'True';
+            };
+            // Delete the comment
+            this.delete_btns[1].simulate('click');
+            module.lp_client.io_provider.success({
+                responseText:'null',
+                responseHeaders: {'Content-Type': 'application/json'}
+            });
+
+            // Check message was deleted
+            Y.Assert.areSame(
+                "",
+                this.msg_texts[1].getDOMNode().innerHTML);
+
+            // All forms should be released.
+            Y.Assert.isFalse(this.textareas[1].getDOMNode().disabled);
+            Y.Assert.isFalse(this.update_btns[1].getDOMNode().disabled);
+            Y.Assert.isFalse(this.textareas[0].getDOMNode().disabled);
+            Y.Assert.isFalse(this.update_btns[0].getDOMNode().disabled);
+
+            // Check forms and msg bodies visibility are back to normal.
+            assertDisplayStyle(this.msg_bodies[0], 'block');
+            assertDisplayStyle(this.msg_forms[0], 'none');
+            assertDisplayStyle(this.msg_bodies[1], 'block');
+            assertDisplayStyle(this.msg_forms[1], 'none');
 
+            // Check that the request was made correctly.
+            var last_request = module.lp_client.io_provider.last_request;
+            Y.Assert.areSame("/api/devel/message/2", last_request.url);
+            Y.Assert.areSame("POST", last_request.config.method);
+            Y.Assert.areSame(
+                "ws.op=deleteContent",
+                last_request.config.data);
+
+            // Check that the "last edit" header changed.
+            Y.Assert.areSame(":", this.last_edit[0].getDOMNode().innerHTML);
+            Y.Assert.areSame(
+                ' <a href="#" class="editable-message-last-edit-link">' +
+                '(message and all revisions deleted a moment ago):</a>',
+                this.last_edit[1].getDOMNode().innerHTML);
+
+            // Check the pop-up revisions list is empty after deleting
+            var revisions = this.revision_history_lists[1].all(
+                ".message-revision-item");
+            Y.Assert.areSame(0, revisions.size());
+        }
     };
 
     suite.add(new Y.Test.Case(TestMessageEdit));