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

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: 9e068e5c903b1409bc54fb6c58f3b88e7b8ecb97
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/launchpad:comment-editing-ui-list-revisions
Merge into: launchpad:master
Diff against target: 540 lines (+318/-27)
8 files modified
lib/canonical/launchpad/icing/css/base.scss (+55/-0)
lib/lp/answers/browser/question.py (+1/-3)
lib/lp/answers/stories/question-workflow.txt (+1/-1)
lib/lp/answers/templates/questionmessage-display.pt (+22/-2)
lib/lp/services/messages/interfaces/messagerevision.py (+2/-1)
lib/lp/services/messages/javascript/messages.edit.js (+103/-18)
lib/lp/services/messages/javascript/tests/test_messages.edit.html (+58/-1)
lib/lp/services/messages/javascript/tests/test_messages.edit.js (+76/-1)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+403213@code.launchpad.net

Commit message

Showing message revision list in the UI

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 :

Pushed the requested changes. This should be good to go now.

Revision history for this message
Colin Watson (cjwatson) :

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 6ee2779..1e33ca7 100644
3--- a/lib/canonical/launchpad/icing/css/base.scss
4+++ b/lib/canonical/launchpad/icing/css/base.scss
5@@ -579,6 +579,61 @@ body {
6 margin: 5px;
7 }
8 }
9+
10+ .message-revision-container {
11+ position: absolute;
12+ background-color: white;
13+ margin-top: 20px;
14+ display: none;
15+ border: 1px solid #ddd;
16+ width: 45em;
17+ z-index: 100;
18+ max-height: 200px;
19+ overflow-y: auto;
20+ overflow-x: hidden;
21+
22+ .message-revision-container-header {
23+ background-color: #eee;
24+ border-bottom: 1px solid #ddd;
25+ padding: 10px;
26+
27+ span {
28+ text-align: left;
29+ display: inline-block;
30+ font-size: 110%;
31+ font-weight: bold;
32+ }
33+
34+ img {
35+ float: right;
36+ cursor: pointer;
37+ }
38+ }
39+
40+ .message-revision-item {
41+ border-bottom: 1px solid #ddd;
42+ padding: 2px;
43+
44+ .message-revision-title {
45+ padding: 5px;
46+ cursor: pointer;
47+ font-weight: 300;
48+ }
49+
50+ .message-revision-body {
51+ display: none;
52+ padding-left: 20px;
53+ padding-bottom: 10px;
54+ }
55+ }
56+
57+ .active {
58+ background-color: #eee;
59+ .message-revision-title {
60+ font-weight: bold;
61+ }
62+ }
63+ }
64 }
65
66 @import 'typography',
67diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
68index 9d4986d..52cff40 100644
69--- a/lib/lp/answers/browser/question.py
70+++ b/lib/lp/answers/browser/question.py
71@@ -1195,13 +1195,11 @@ class QuestionMessageDisplayView(LaunchpadView):
72 return check_permission('launchpad.Moderate', self.context)
73
74 def getBoardCommentCSSClass(self):
75- css_classes = ["boardComment"]
76+ css_classes = ["boardComment", "editable-message"]
77 if not self.context.visible:
78 # If a comment that isn't visible is being rendered, it's being
79 # rendered for an admin or registry_expert.
80 css_classes.append("adminHiddenComment")
81- if self.can_edit:
82- css_classes.append("editable-message")
83 return " ".join(css_classes)
84
85 @property
86diff --git a/lib/lp/answers/stories/question-workflow.txt b/lib/lp/answers/stories/question-workflow.txt
87index 5e39acd..1a0b67a 100755
88--- a/lib/lp/answers/stories/question-workflow.txt
89+++ b/lib/lp/answers/stories/question-workflow.txt
90@@ -215,7 +215,7 @@ The confirmed answer is also highlighted.
91
92 >>> soup = find_main_content(owner_browser.contents)
93 >>> bestAnswer = soup.find_all('div', 'boardComment')[-2]
94- >>> print(bestAnswer.find('img'))
95+ >>> print(bestAnswer.find_all('img')[1])
96 <img ... src="/@@/favourite-yes" ... title="Marked as best answer"/>
97
98 >>> print(soup.find(
99diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
100index fc3651c..c1ed29a 100644
101--- a/lib/lp/answers/templates/questionmessage-display.pt
102+++ b/lib/lp/answers/templates/questionmessage-display.pt
103@@ -14,6 +14,25 @@
104 <tbody>
105 <tr>
106 <td>
107+ <div class="message-revision-container">
108+ <div class="message-revision-container-header">
109+ <span>Revision history for this message</span>
110+ <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
111+ class="message-revision-close"/>
112+ </div>
113+ <script type="text/template">
114+ <div class='message-revision-item'>
115+ <div class='message-revision-title'>
116+ <a class="js-action">
117+ Revision #{revision}, created at {date_created}
118+ </a>
119+ </div>
120+ <div class='message-revision-body'>{content}</div>
121+ </div>
122+ </script>
123+
124+ <div class="message-revision-list"></div>
125+ </div>
126 <tal:bestanswer condition="view/isBestAnswer">
127 <img src="/@@/favourite-yes" style="float:right;" alt="Best"
128 title="Marked as best answer" />
129@@ -28,11 +47,12 @@
130 datetime context/datecreated/fmt:isodate"
131 tal:content="context/datecreated/fmt:displaydate">Thursday 13:21
132 </time><span class="editable-message-last-edit-date"><tal:last-edit condition="context/date_last_edited">
133- (last edit <time
134+ <a href="#" class="editable-message-last-edit-link"
135+ tal:condition="context/date_last_edited">(last edit <time
136 itemprop="editTime"
137 tal:attributes="title context/date_last_edited/fmt:datetime;
138 datetime context/date_last_edited/fmt:isodate"
139- tal:content="context/date_last_edited/fmt:displaydate" />)</tal:last-edit>:
140+ tal:content="context/date_last_edited/fmt:displaydate" />)</a></tal:last-edit>:
141 </span>
142 </td>
143 <td>
144diff --git a/lib/lp/services/messages/interfaces/messagerevision.py b/lib/lp/services/messages/interfaces/messagerevision.py
145index 3ee5a90..aa0ff0a 100644
146--- a/lib/lp/services/messages/interfaces/messagerevision.py
147+++ b/lib/lp/services/messages/interfaces/messagerevision.py
148@@ -35,7 +35,8 @@ class IMessageRevisionView(Interface):
149 """IMessageRevision readable attributes."""
150 id = Int(title=_("ID"), required=True, readonly=True)
151
152- revision = Int(title=_("Revision number"), required=True, readonly=True)
153+ revision = exported(
154+ Int(title=_("Revision number"), required=True, readonly=True))
155
156 content = exported(Text(
157 title=_("The message at the given revision"),
158diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
159index a1e6806..f84bf72 100644
160--- a/lib/lp/services/messages/javascript/messages.edit.js
161+++ b/lib/lp/services/messages/javascript/messages.edit.js
162@@ -2,7 +2,8 @@
163 * GNU Affero General Public License version 3 (see the file LICENSE).
164 *
165 * This modules controls HTML comments in order to make them editable. To do
166- * so, it requires:
167+ * so, it requires some definitions (see test_messages.edit.html file for the
168+ * complete structure reference):
169 * - A div container with the class .editable-message containing everything
170 * else related to the message
171 * - A data-baseurl="/path/to/msg" on the .editable-message container
172@@ -14,6 +15,16 @@
173 * - A .editable-message-last-edit-date span, where we update the date of the
174 * last message editing.
175 *
176+ * For the message revision history, you should define:
177+ * - A .editable-message-last-edit-link, that will show the revisions pop-up
178+ * once clicked.
179+ * - A .message-revision-container, holding the header and the body of the
180+ * revision list, and the template for each revision item.
181+ * - A .message-revision-container-header.
182+ * - A .message-revision-list, where the revisions will be placed.
183+ * - A <script type="text/template">, with the definition of how each item
184+ * should look like.
185+ *
186 * Once those HTML elements are available in the page, this module should be
187 * initialized with `lp.services.messages.edit.setup()`.
188 *
189@@ -23,15 +34,6 @@
190 YUI.add('lp.services.messages.edit', function(Y) {
191 var module = Y.namespace('lp.services.messages.edit');
192
193- // XXX pappacena 2021-05-21: We should drop this message once we have a
194- // way to list the old message revisions in the web UI.
195- module.msg_edit_success_notification = (
196- "Message edited, but the original content may still be publicly " +
197- "visible using the API.<br />Please " +
198- "<a href='https://launchpad.net/+apidoc/devel.html#message'>" +
199- "check the API documentation</a> in case you " +
200- "need to remove old message revisions."
201- );
202 module.msg_edit_error_notification = (
203 "There was an error updating the comment. " +
204 "Please try again in a few minutes."
205@@ -115,7 +117,7 @@ YUI.add('lp.services.messages.edit', function(Y) {
206 // text area.
207 module.showEditMessageField(elements.msg_body, elements.msg_form);
208 elements.msg_form.one('textarea').getDOMNode().focus();
209- }
210+ };
211
212 // What to do when a user clicks "cancel edit" button.
213 module.onEditCancelClick = function(elements) {
214@@ -134,13 +136,15 @@ YUI.add('lp.services.messages.edit', function(Y) {
215
216 module.saveMessageContent(
217 baseurl, new_content,
218- function(err) { module.onMessageSaved(elements, new_content); },
219+ function(err) {
220+ module.onMessageSaved(elements, new_content, baseurl);
221+ },
222 function(err) { module.onMessageSaveError(elements, err); }
223 );
224 };
225
226 // What to do when a message is saved in the backend.
227- module.onMessageSaved = function(elements, new_content) {
228+ module.onMessageSaved = function(elements, new_content, baseurl) {
229 // When finished updating at the backend, re-enable UI
230 // elements and display the new message.
231 var html_msg = module.htmlify_msg(new_content);
232@@ -150,11 +154,18 @@ YUI.add('lp.services.messages.edit', function(Y) {
233 elements.textarea.getDOMNode().disabled = false;
234 elements.update_btn.getDOMNode().disabled = false;
235 module.hideLoading(elements.container);
236- module.showNotification(
237- elements.container,
238- module.msg_edit_success_notification, true);
239 elements.last_edit.getDOMNode().innerHTML = (
240- ' (last edit a moment ago): ');
241+ '<a href="#" class="editable-message-last-edit-link">' +
242+ '(last edit a moment ago):' +
243+ '</a>');
244+
245+ // Wire click handler to the newly created "last edit" button.
246+ var last_edit_btn = elements.container.one(
247+ '.editable-message-last-edit-link');
248+ last_edit_btn.on('click', function(e) {
249+ e.preventDefault();
250+ module.onLastEditClick(elements, baseurl);
251+ });
252 };
253
254 // What to do when a message fails to update on the backend.
255@@ -168,6 +179,72 @@ YUI.add('lp.services.messages.edit', function(Y) {
256 elements.update_btn.getDOMNode().disabled = false;
257 };
258
259+ module.fillMessageRevisions = function(elements, revisions) {
260+ // Position the message revision list element.
261+ revisions = revisions.reverse();
262+ var revisions_container = elements.container.one(
263+ ".message-revision-container");
264+ var last_edit_el = elements.last_edit.getDOMNode();
265+ var target_position = last_edit_el.getBoundingClientRect();
266+ var nodes_holder = revisions_container.one(".message-revision-list");
267+ var template = revisions_container.one(
268+ "script[type='text/template']").getDOMNode().innerHTML;
269+
270+ revisions_container.setStyle('left', target_position.left);
271+ revisions_container.setStyle('display', 'block');
272+ revisions_container.one(".message-revision-close").on(
273+ "click", function() {
274+ nodes_holder.getDOMNode().innerHTML = '';
275+ revisions_container.setStyle('display', 'none');
276+ });
277+
278+ var content = "";
279+ revisions.forEach(function(rev) {
280+ var attrs = rev.getAttrs();
281+ var date_created = new Date(attrs.date_created);
282+ attrs.date_created = date_created.toLocaleString();
283+ attrs.content = module.htmlify_msg(attrs.content);
284+ content += Y.Lang.sub(template, attrs);
285+ });
286+
287+ nodes_holder.getDOMNode().innerHTML = content;
288+ nodes_holder.all(".message-revision-item").each(function(rev_item) {
289+ rev_item.one(".message-revision-title").on('click', function() {
290+ nodes_holder.all('.message-revision-body').setStyle(
291+ 'display', 'none');
292+ var body = rev_item.one('.message-revision-body');
293+ var current_display = body.getStyle('display');
294+ body.setStyle(
295+ 'display', current_display === 'block'? 'none' : 'block');
296+ nodes_holder.all(".message-revision-item").removeClass(
297+ 'active');
298+ rev_item.addClass('active');
299+ });
300+ });
301+ };
302+
303+ module.onLastEditClick = function(elements, baseurl) {
304+ // Hide all open revision containers.
305+ Y.all('.message-revision-container').each(function(container) {
306+ container.setStyle('display', 'none');
307+ });
308+
309+ var url = "/api/devel" + baseurl + "/revisions";
310+ var config = {
311+ on: {
312+ success: function(response) {
313+ module.fillMessageRevisions(elements, response.entries);
314+ },
315+ failure: function(err) {
316+ alert("Error fetching revisions.");
317+ }
318+ },
319+ // XXX pappacena 2021-05-19: Pagination will be needed here.
320+ size: 100
321+ };
322+ this.lp_client.get(url, config);
323+ };
324+
325 module.wireEventHandlers = function(container) {
326 var node = container.getDOMNode();
327 var baseurl = node.dataset.baseurl;
328@@ -179,7 +256,8 @@ YUI.add('lp.services.messages.edit', function(Y) {
329 "edit_btn": container.one('.editable-message-edit-btn'),
330 "update_btn": container.one('.editable-message-update-btn'),
331 "cancel_btn": container.one('.editable-message-cancel-btn'),
332- "last_edit": container.one('.editable-message-last-edit-date')
333+ "last_edit": container.one('.editable-message-last-edit-date'),
334+ "last_edit_btn": container.one('.editable-message-last-edit-link')
335 };
336 // If the msg body or the msg form are not defined, don't try to do
337 // anything else.
338@@ -190,6 +268,13 @@ YUI.add('lp.services.messages.edit', function(Y) {
339
340 module.hideEditMessageField(elements.msg_body, elements.msg_form);
341
342+ if (elements.last_edit_btn) {
343+ elements.last_edit_btn.on('click', function(e) {
344+ e.preventDefault();
345+ module.onLastEditClick(elements, baseurl);
346+ });
347+ }
348+
349 // If the edit button is not present, do not try to bind the
350 // handlers.
351 if (!elements.edit_btn || !baseurl) {
352diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.html b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
353index 3036d88..e6a27c2 100644
354--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
355+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
356@@ -52,10 +52,45 @@ GNU Affero General Public License version 3 (see the file LICENSE).
357 <li>lp.services.messages.edit.test</li>
358 </ul>
359
360+ <!-- First editable message -->
361 <div class="editable-message" id="first-message"
362 data-baseurl="/message/1">
363+ <!-- Revision list container and its template -->
364+ <div class="message-revision-container">
365+ <!-- The revision list pop-up header, with the image that closes
366+ the pop-up when clicked. -->
367+ <div class="message-revision-container-header">
368+ <span>Revision history for this message</span>
369+ <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
370+ class="message-revision-close"/>
371+ </div>
372+ <!-- The template for each revision item in the list. The
373+ content of this tag will be repeated multiple times, replacing
374+ the {variable} with each MessageRevision entry returned by
375+ the API.
376+ -->
377+ <script type="text/template">
378+ <div class='message-revision-item'>
379+ <!-- The description of the revision, that will expand the
380+ revision content once clicked. -->
381+ <div class='message-revision-title'>
382+ <a class="js-action">Revision #{revision}, created at {date_created}</a>
383+ </div>
384+ <!-- The revision content itself -->
385+ <div class='message-revision-body'>{content}</div>
386+ </div>
387+ </script>
388+ <!-- This is the place where all the revisions will be placed,
389+ by applying the above template to each MessageRevision entry
390+ returned by the API.
391+ -->
392+ <div class="message-revision-list"></div>
393+ </div>
394+
395+ <!-- Message body -->
396 <div>
397 Comment from @some-user a while ago
398+ Comment from @some-user a while ago
399 <span class="editable-message-last-edit-date">:</span>
400 </div>
401 <div class="editable-message-body">
402@@ -71,12 +106,34 @@ GNU Affero General Public License version 3 (see the file LICENSE).
403 </div>
404 </div>
405
406+ <!-- Second editable message -->
407 <div class="editable-message" id="second-message"
408 data-baseurl="/message/2">
409+ <!-- Revision list container and its template -->
410+ <div class="message-revision-container">
411+ <div class="message-revision-container-header">
412+ <span>Revision history for this message</span>
413+ <img src="/+icing/build/overlay/assets/skins/sam/images/close.gif"
414+ class="message-revision-close"/>
415+ </div>
416+ <script type="text/template">
417+ <div class='message-revision-item'>
418+ <div class='message-revision-title'>
419+ <a class="js-action">Revision #{revision}, created at {date_created}</a>
420+ </div>
421+ <div class='message-revision-body'>{content}</div>
422+ </div>
423+ </script>
424+ <div class="message-revision-list"></div>
425+ </div>
426+
427+ <!-- Message body -->
428 <div>
429 Comment from @some-user a while ago
430 <span class="editable-message-last-edit-date">
431- (last edit 5 minutes ago):
432+ <a href="#" class="editable-message-last-edit-link">
433+ (last edit 5 minutes ago):
434+ </a>
435 </span>
436 </div>
437 <div class="editable-message-body">
438diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.js b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
439index a8a982c..5d12dab 100644
440--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.js
441+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.js
442@@ -35,6 +35,14 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
443 this.containers[0].one(".editable-message-last-edit-date"),
444 this.containers[1].one(".editable-message-last-edit-date")
445 ];
446+ this.revision_history_link = [
447+ this.containers[0].one(".editable-message-last-edit-link"),
448+ this.containers[1].one(".editable-message-last-edit-link")
449+ ];
450+ this.revision_history_lists = [
451+ this.containers[0].one(".message-revision-list"),
452+ this.containers[1].one(".message-revision-list")
453+ ];
454 this.msg_bodies = [
455 this.containers[0].one(".editable-message-body"),
456 this.containers[1].one(".editable-message-body")
457@@ -73,6 +81,7 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
458 this.last_edit[0].getDOMNode().innerHTML = ':';
459 this.last_edit[1].getDOMNode().innerHTML = (
460 '(last edit 5 minutes ago):');
461+ this.revision_history_lists[i].getDOMNode().innerHTML = '';
462 }
463 },
464
465@@ -160,8 +169,74 @@ YUI.add('lp.services.messages.edit.test', function(Y) {
466 // Check that the "last edit" header changed.
467 Y.Assert.areSame(":", this.last_edit[0].getDOMNode().innerHTML);
468 Y.Assert.areSame(
469- " (last edit a moment ago): ",
470+ '<a href="#" class="editable-message-last-edit-link">' +
471+ '(last edit a moment ago):</a>',
472 this.last_edit[1].getDOMNode().innerHTML);
473+ },
474+
475+ test_shows_revision_history: function() {
476+ module.setup();
477+ module.lp_client.io_provider = new Y.lp.testing.mockio.MockIo();
478+ var revisions_container = this.containers[1].one(
479+ '.message-revision-container');
480+ // Revisions container should not be shown before it's clicked.
481+ Y.Assert.areSame('none', revisions_container.getStyle('display'));
482+
483+ // Simulates the click and the request.
484+ this.revision_history_link[1].simulate('click');
485+
486+ var response = {
487+ "total_size": 1, "start": 0,
488+ "entries": [{
489+ "revision": 1,
490+ "content": "content 1",
491+ "date_created": "2021-05-10T19:41:36.102749+00:00"
492+ }, {
493+ "revision": 2,
494+ "content": "content 2",
495+ "date_created": "2021-05-11T22:17:11.000123+00:00"
496+ }]
497+ };
498+ module.lp_client.io_provider.success({
499+ responseText: Y.JSON.stringify(response),
500+ responseHeaders: {'Content-Type': 'application/json'}
501+ });
502+
503+ // Make sure it didn't fill the first container.
504+ Y.Assert.areSame(
505+ "", this.revision_history_lists[0].getDOMNode().innerHTML);
506+
507+ // Check that revision list pop-up is shown
508+ Y.Assert.areSame('block', revisions_container.getStyle('display'));
509+
510+ // Check the items in the pop-up revisions list.
511+ var revisions = this.revision_history_lists[1].all(
512+ ".message-revision-item");
513+ Y.Assert.areSame(2, revisions.size());
514+ revisions.each(function(rev, i) {
515+ // Entries are shown in reverse order.
516+ var entry = response.entries[response.entries.length - i -1];
517+ var date_created = new Date(entry.date_created);
518+ entry['formatted_date'] = date_created.toLocaleString();
519+ var title = rev.one('.message-revision-title');
520+ var body = rev.one('.message-revision-body');
521+ var expected_title = Y.Lang.sub(
522+ '<a class="js-action">' +
523+ 'Revision #{revision}, created at {formatted_date}' +
524+ '</a>',
525+ entry);
526+ Y.Assert.areSame(
527+ expected_title, title.getDOMNode().innerHTML.trim());
528+ Y.Assert.areSame(
529+ module.htmlify_msg(entry.content),
530+ body.getDOMNode().innerHTML);
531+ });
532+
533+ // Lets make sure that a click on the "close" button hides the
534+ // revisions list.
535+ revisions_container.one(
536+ '.message-revision-close').simulate('click');
537+ Y.Assert.areSame('none', revisions_container.getStyle('display'));
538 }
539
540 };