launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master.

Commit message:
Showing message revision list in the UI

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403213
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master.

diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index 6ee2779..1e33ca7 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -579,6 +579,61 @@ body {
         margin: 5px;
       }
     }
+
+    .message-revision-container {
+        position: absolute;
+        background-color: white;
+        margin-top: 20px;
+        display: none;
+        border: 1px solid #ddd;
+        width: 45em;
+        z-index: 100;
+        max-height: 200px;
+        overflow-y: auto;
+        overflow-x: hidden;
+
+        .message-revision-container-header {
+            background-color: #eee;
+            border-bottom: 1px solid #ddd;
+            padding: 10px;
+
+            span {
+                text-align: left;
+                display: inline-block;
+                font-size: 110%;
+                font-weight: bold;
+            }
+
+            img {
+                float: right;
+                cursor: pointer;
+            }
+        }
+
+        .message-revision-item {
+            border-bottom: 1px solid #ddd;
+            padding: 2px;
+
+            .message-revision-title {
+                padding: 5px;
+                cursor: pointer;
+                font-weight: 300;
+            }
+
+            .message-revision-body {
+                display: none;
+                padding-left: 20px;
+                padding-bottom: 10px;
+            }
+        }
+
+        .active {
+            background-color: #eee;
+            .message-revision-title {
+                font-weight: bold;
+            }
+        }
+    }
   }
 
 @import 'typography',
diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
index 9d4986d..52cff40 100644
--- a/lib/lp/answers/browser/question.py
+++ b/lib/lp/answers/browser/question.py
@@ -1195,13 +1195,11 @@ class QuestionMessageDisplayView(LaunchpadView):
         return check_permission('launchpad.Moderate', self.context)
 
     def getBoardCommentCSSClass(self):
-        css_classes = ["boardComment"]
+        css_classes = ["boardComment", "editable-message"]
         if not self.context.visible:
             # If a comment that isn't visible is being rendered, it's being
             # rendered for an admin or registry_expert.
             css_classes.append("adminHiddenComment")
-        if self.can_edit:
-            css_classes.append("editable-message")
         return " ".join(css_classes)
 
     @property
diff --git a/lib/lp/answers/stories/question-workflow.txt b/lib/lp/answers/stories/question-workflow.txt
index 5e39acd..1a0b67a 100755
--- a/lib/lp/answers/stories/question-workflow.txt
+++ b/lib/lp/answers/stories/question-workflow.txt
@@ -215,7 +215,7 @@ The confirmed answer is also highlighted.
 
     >>> soup = find_main_content(owner_browser.contents)
     >>> bestAnswer = soup.find_all('div', 'boardComment')[-2]
-    >>> print(bestAnswer.find('img'))
+    >>> print(bestAnswer.find_all('img')[1])
     <img ... src="/@@/favourite-yes" ... title="Marked as best answer"/>
 
     >>> print(soup.find(
diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index fc3651c..c1ed29a 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -14,6 +14,25 @@
       <tbody>
         <tr>
     <td>
+      <div class="message-revision-container">
+        <div class="message-revision-container-header">
+          <span>Revision history for this message</span>
+          <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
+               class="message-revision-close"/>
+        </div>
+        <script type="text/template">
+          <div class='message-revision-item'>
+            <div class='message-revision-title'>
+                <a class="js-action">
+                    Revision #{revision}, created at  {date_created}
+                </a>
+            </div>
+            <div class='message-revision-body'>{content}</div>
+          </div>
+        </script>
+
+        <div class="message-revision-list"></div>
+      </div>
     <tal:bestanswer condition="view/isBestAnswer">
       <img src="/@@/favourite-yes" style="float:right;" alt="Best"
            title="Marked as best answer" />
@@ -28,11 +47,12 @@
         datetime context/datecreated/fmt:isodate"
       tal:content="context/datecreated/fmt:displaydate">Thursday 13:21
     </time><span class="editable-message-last-edit-date"><tal:last-edit condition="context/date_last_edited">
-        (last edit <time
+        <a href="#" class="editable-message-last-edit-link"
+            tal:condition="context/date_last_edited">(last edit <time
             itemprop="editTime"
             tal:attributes="title context/date_last_edited/fmt:datetime;
               datetime context/date_last_edited/fmt:isodate"
-            tal:content="context/date_last_edited/fmt:displaydate" />)</tal:last-edit>:
+            tal:content="context/date_last_edited/fmt:displaydate" />)</a></tal:last-edit>:
       </span>
     </td>
     <td>
diff --git a/lib/lp/services/messages/interfaces/messagerevision.py b/lib/lp/services/messages/interfaces/messagerevision.py
index 3ee5a90..aa0ff0a 100644
--- a/lib/lp/services/messages/interfaces/messagerevision.py
+++ b/lib/lp/services/messages/interfaces/messagerevision.py
@@ -35,7 +35,8 @@ class IMessageRevisionView(Interface):
     """IMessageRevision readable attributes."""
     id = Int(title=_("ID"), required=True, readonly=True)
 
-    revision = Int(title=_("Revision number"), required=True, readonly=True)
+    revision = exported(
+        Int(title=_("Revision number"), required=True, readonly=True))
 
     content = exported(Text(
         title=_("The message at the given revision"),
diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
index 3c4c0e5..912a24c 100644
--- a/lib/lp/services/messages/javascript/messages.edit.js
+++ b/lib/lp/services/messages/javascript/messages.edit.js
@@ -2,7 +2,8 @@
  * GNU Affero General Public License version 3 (see the file LICENSE).
  *
  * This modules controls HTML comments in order to make them editable. To do
- * so, it requires:
+ * so, it requires some definitions (see test_messages.edit.html file for the
+ * complete structure reference):
  *  - A div container with the class .editable-message containing everything
  *      else related to the message
  *  - A data-baseurl="/path/to/msg" on the .editable-message container
@@ -14,6 +15,16 @@
  *  - A .editable-message-last-edit-date span, where we update the date of the
  *       last message editing.
  *
+ * For the message revision history, you should define:
+ *  - A .editable-message-last-edit-link, that will show the revisions pop-up
+ *      once clicked.
+ *  - A .message-revision-container, holding the header and the body of the
+ *      revision list, and the template for each revision item.
+ *  - A .message-revision-container-header.
+ *  - A .message-revision-list, where the revisions will be placed.
+ *  - A <script type="text/template">, with the definition of how each item
+ *      should look like.
+ *
  * Once those HTML elements are available in the page, this module should be
  * initialized with `lp.services.messages.edit.setup()`.
  *
@@ -23,15 +34,6 @@
 YUI.add('lp.services.messages.edit', function(Y) {
     var module = Y.namespace('lp.services.messages.edit');
 
-    // XXX pappacena 2021-05-21: We should drop this message once we have a
-    // way to list the old message revisions in the web UI.
-    module.msg_edit_success_notification = (
-        "Message edited, but the original content may still be publicly " +
-        "visible using the API.<br />Please " +
-        "<a href='https://launchpad.net/+apidoc/devel.html#message'>" +
-        "check the API documentation</a> in case you " +
-        "need to remove old message revisions."
-    );
     module.msg_edit_error_notification = (
         "There was an error updating the comment. " +
         "Please try again in a few minutes."
@@ -115,7 +117,7 @@ YUI.add('lp.services.messages.edit', function(Y) {
         // text area.
         module.showEditMessageField(elements.msg_body, elements.msg_form);
         elements.msg_form.one('textarea').getDOMNode().focus();
-    }
+    };
 
     // What to do when a user clicks "cancel edit" button.
     module.onEditCancelClick = function(elements) {
@@ -134,13 +136,15 @@ YUI.add('lp.services.messages.edit', function(Y) {
 
         module.saveMessageContent(
             baseurl, new_content,
-            function(err) { module.onMessageSaved(elements, new_content); },
+            function(err) {
+                module.onMessageSaved(elements, new_content, baseurl);
+            },
             function(err) { module.onMessageSaveError(elements, err); }
         );
     };
 
     // What to do when a message is saved in the backend.
-    module.onMessageSaved = function(elements, new_content) {
+    module.onMessageSaved = function(elements, new_content, baseurl) {
         // When finished updating at the backend, re-enable UI
         // elements and display the new message.
         var html_msg = module.htmlify_msg(new_content);
@@ -150,11 +154,18 @@ YUI.add('lp.services.messages.edit', function(Y) {
         elements.textarea.getDOMNode().disabled = false;
         elements.update_btn.getDOMNode().disabled = false;
         module.hideLoading(elements.container);
-        module.showNotification(
-            elements.container,
-            module.msg_edit_success_notification, true);
         elements.last_edit.getDOMNode().innerHTML = (
-            ' (last edit a moment ago): ');
+            '<a href="#" class="editable-message-last-edit-link">' +
+            '(last edit a moment ago):' +
+            '</a>');
+
+        // Wire click handler to the newly created "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 update on the backend.
@@ -168,6 +179,74 @@ YUI.add('lp.services.messages.edit', function(Y) {
         elements.update_btn.getDOMNode().disabled = false;
     };
 
+    module.fillMessageRevisions = function(elements, revisions) {
+        // Position the message revision list element.
+        revisions = revisions.reverse();
+        var revisions_container = elements.container.one(
+            ".message-revision-container");
+        var last_edit_el = elements.last_edit.getDOMNode();
+        var target_position = last_edit_el.getBoundingClientRect();
+        var nodes_holder = revisions_container.one(".message-revision-list");
+        var template = revisions_container.one(
+            "script[type='text/template']").getDOMNode().innerHTML;
+
+        revisions_container.setStyle('left', target_position.left);
+        revisions_container.setStyle('display', 'block');
+        revisions_container.one(".message-revision-close").on(
+            "click", function() {
+                nodes_holder.getDOMNode().innerHTML = '';
+                revisions_container.setStyle('display', 'none');
+            });
+
+        var content = "";
+        revisions.forEach(function(rev) {
+            var attrs = rev.getAttrs();
+            var date_created = new Date(attrs.date_created);
+            attrs.date_created = (
+                date_created.toISOString().substr(0,10) + ", " +
+                date_created.toLocaleTimeString());
+            attrs.content = module.htmlify_msg(attrs.content);
+            content += Y.Lang.sub(template, attrs);
+        });
+
+        nodes_holder.getDOMNode().innerHTML = content;
+        nodes_holder.all(".message-revision-item").each(function(rev_item) {
+            rev_item.one(".message-revision-title").on('click', function() {
+                nodes_holder.all('.message-revision-body').setStyle(
+                    'display', 'none');
+                var body = rev_item.one('.message-revision-body');
+                var current_display = body.getStyle('display');
+                body.setStyle(
+                    'display', current_display === 'block'? 'none' : 'block');
+                nodes_holder.all(".message-revision-item").removeClass(
+                    'active');
+                rev_item.addClass('active');
+            });
+        });
+    };
+
+    module.onLastEditClick = function(elements, baseurl) {
+        // Hide all open revision containers.
+        Y.all('.message-revision-container').each(function(container) {
+            container.setStyle('display', 'none');
+        });
+
+        var url = "/api/devel" + baseurl + "/revisions";
+        var config = {
+            on: {
+                 success: function(response) {
+                    module.fillMessageRevisions(elements, response.entries);
+                 },
+                 failure: function(err) {
+                    alert("Error fetching revisions.");
+                 }
+             },
+             // XXX pappacena 2021-05-19: Pagination will be needed here.
+             size: 100
+        };
+        this.lp_client.get(url, config);
+    };
+
     module.wireEventHandlers = function(container) {
         var node = container.getDOMNode();
         var baseurl = node.dataset.baseurl;
@@ -179,12 +258,20 @@ YUI.add('lp.services.messages.edit', function(Y) {
             "edit_btn": container.one('.editable-message-edit-btn'),
             "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": container.one('.editable-message-last-edit-date'),
+            "last_edit_btn": container.one('.editable-message-last-edit-link')
         };
         elements.textarea = elements.msg_form.one('textarea');
 
         module.hideEditMessageField(elements.msg_body, elements.msg_form);
 
+        if (elements.last_edit_btn) {
+            elements.last_edit_btn.on('click', function(e) {
+                e.preventDefault();
+                module.onLastEditClick(elements, baseurl);
+            });
+        }
+
         // If the edit button is not present, do not try to bind the
         // handlers.
         if (!elements.edit_btn || !baseurl) {
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 94c60ea..21667b5 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
@@ -52,10 +52,45 @@ GNU Affero General Public License version 3 (see the file LICENSE).
             <li>lp.services.messages.edit.test</li>
         </ul>
 
+        <!-- First editable message -->
         <div class="editable-message" id="first-message"
              data-baseurl="/message/1">
+            <!-- Revision list container and its template -->
+            <div class="message-revision-container">
+              <!-- The revision list pop-up header, with the image that closes
+              the pop-up when clicked. -->
+              <div class="message-revision-container-header">
+                <span>Revision history for this message</span>
+                <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
+                     class="message-revision-close"/>
+              </div>
+              <!-- The template for each revision item in the list. The
+              content of this tag will be repeated multiple times, replacing
+               the {variable} with each MessageRevision entry returned by
+               the API.
+               -->
+              <script type="text/template">
+                <div class='message-revision-item'>
+                  <!-- The description of the revision, that will expand the
+                   revision content once clicked. -->
+                  <div class='message-revision-title'>
+                      <a class="js-action">Revision #{revision}, created at  {date_created}</a>
+                  </div>
+                  <!-- The revision content itself -->
+                  <div class='message-revision-body'>{content}</div>
+                </div>
+              </script>
+              <!-- This is the place where all the revisions will be placed,
+               by applying the above template to each MessageRevision entry
+               returned by the API.
+               -->
+              <div class="message-revision-list"></div>
+            </div>
+
+            <!-- Message body -->
             <div>
                 Comment from @some-user a while ago
+                                Comment from @some-user a while ago
                 <span class="editable-message-last-edit-date">:</span>
             </div>
             <div class="editable-message-body">
@@ -71,12 +106,34 @@ GNU Affero General Public License version 3 (see the file LICENSE).
             </div>
         </div>
 
+        <!-- Second editable message -->
         <div class="editable-message" id="second-message"
              data-baseurl="/message/2">
+            <!-- Revision list container and its template -->
+            <div class="message-revision-container">
+              <div class="message-revision-container-header">
+                <span>Revision history for this message</span>
+                <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
+                     class="message-revision-close"/>
+              </div>
+              <script type="text/template">
+                <div class='message-revision-item'>
+                  <div class='message-revision-title'>
+                      <a class="js-action">Revision #{revision}, created at {date_created}</a>
+                  </div>
+                  <div class='message-revision-body'>{content}</div>
+                </div>
+              </script>
+              <div class="message-revision-list"></div>
+            </div>
+
+            <!-- Message body -->
             <div>
                 Comment from @some-user a while ago
                 <span class="editable-message-last-edit-date">
-                    (last edit 5 minutes ago):
+                    <a href="#" class="editable-message-last-edit-link">
+                        (last edit 5 minutes ago):
+                    </a>
                 </span>
             </div>
             <div class="editable-message-body">
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 a8a982c..abdefd5 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.js
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
@@ -35,6 +35,14 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
                 this.containers[0].one(".editable-message-last-edit-date"),
                 this.containers[1].one(".editable-message-last-edit-date")
             ];
+            this.revision_history_link = [
+                this.containers[0].one(".editable-message-last-edit-link"),
+                this.containers[1].one(".editable-message-last-edit-link")
+            ];
+            this.revision_history_lists = [
+                this.containers[0].one(".message-revision-list"),
+                this.containers[1].one(".message-revision-list")
+            ];
             this.msg_bodies = [
                 this.containers[0].one(".editable-message-body"),
                 this.containers[1].one(".editable-message-body")
@@ -73,6 +81,7 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
                 this.last_edit[0].getDOMNode().innerHTML = ':';
                 this.last_edit[1].getDOMNode().innerHTML = (
                     '(last edit 5 minutes ago):');
+                this.revision_history_lists[i].getDOMNode().innerHTML = '';
             }
         },
 
@@ -160,8 +169,76 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
             // Check that the "last edit" header changed.
             Y.Assert.areSame(":", this.last_edit[0].getDOMNode().innerHTML);
             Y.Assert.areSame(
-                " (last edit a moment ago): ",
+                '<a href="#" class="editable-message-last-edit-link">' +
+                '(last edit a moment ago):</a>',
                 this.last_edit[1].getDOMNode().innerHTML);
+        },
+
+        test_shows_revision_history: function() {
+            module.setup();
+            module.lp_client.io_provider = new Y.lp.testing.mockio.MockIo();
+            var revisions_container = this.containers[1].one(
+                '.message-revision-container');
+            // Revisions container should not be shown before it's clicked.
+            Y.Assert.areSame('none', revisions_container.getStyle('display'));
+
+            // Simulates the click and the request.
+            this.revision_history_link[1].simulate('click');
+
+            var response = {
+                "total_size": 1, "start": 0,
+                "entries": [{
+                    "revision": 1,
+                    "content": "content 1",
+                    "date_created": "2021-05-10T19:41:36.102749+00:00"
+                }, {
+                    "revision": 2,
+                    "content": "content 2",
+                    "date_created": "2021-05-11T22:17:11.000123+00:00"
+                }]
+            };
+            module.lp_client.io_provider.success({
+                responseText: Y.JSON.stringify(response),
+                responseHeaders: {'Content-Type': 'application/json'}
+            });
+
+            // Make sure it didn't fill the first container.
+            Y.Assert.areSame(
+                "", this.revision_history_lists[0].getDOMNode().innerHTML);
+
+            // Check that revision list pop-up is shown
+            Y.Assert.areSame('block', revisions_container.getStyle('display'));
+
+            // Check the items in the pop-up revisions list.
+            var revisions = this.revision_history_lists[1].all(
+                ".message-revision-item");
+            Y.Assert.areSame(2, revisions.size());
+            revisions.each(function(rev, i) {
+                // Entries are shown in reverse order.
+                var entry = response.entries[response.entries.length - i -1];
+                var date_created = new Date(entry.date_created);
+                entry['formatted_date'] = (
+                    date_created.toISOString().substr(0,10) + ", " +
+                    date_created.toLocaleTimeString());
+                var title = rev.one('.message-revision-title');
+                var body = rev.one('.message-revision-body');
+                var expected_title = Y.Lang.sub(
+                    '<a class="js-action">' +
+                    'Revision #{revision}, created at {formatted_date}' +
+                    '</a>',
+                    entry);
+                Y.Assert.areSame(
+                    expected_title, title.getDOMNode().innerHTML.trim());
+                Y.Assert.areSame(
+                    module.htmlify_msg(entry.content),
+                    body.getDOMNode().innerHTML);
+            });
+
+            // Lets make sure that a click on the "close" button hides the
+            // revisions list.
+            revisions_container.one(
+                '.message-revision-close').simulate('click');
+            Y.Assert.areSame('none', revisions_container.getStyle('display'));
         }
 
     };