Fix message reorder issue after edit (#2300)

* keep message in place after edition by assigning previous message original timestamp
* update time displayed to keep current behaviour
* add test to check a message remains in the same position of the history after being edited
* briefly describe pr changes in changelog

CHANGES.md

@@ -36,6 +36,7 @@ Soon we'll deprecate the latter, so prepare now.
 - #2220: fix rendering of emojis in case `use_system_emojis == false` (again).
 - #2092: fixes room list update loop when having the `locked_muc_domain` truthy or `'hidden'`
 - #2285: Rename config option `muc_hats_from_vcard` to [muc_hats](https://conversejs.org/docs/html/configuration.html#muc-hats). Now accepts a list instead of a boolean and allows for more flexible choices regarding user badges.
+- #2300: Fix message reorder issue after message correction.
 - #2304: Custom emojis (stickers) images not shown 
 - #2307: BootstrapModal is not accessible to plugins
 - #2308: Allow getHats method to be overriden in the `overrides` object in plugins.

spec/muc_messages.js

@@ -592,6 +592,102 @@ describe("A Groupchat Message", function () {
         done();
     }));
 
+    it("keeps the same position in history after a correction",
+        mock.initConverse(
+            ['rosterGroupsFetched'], {},
+            async function (done, _converse) {
+
+        const muc_jid = 'lounge@montague.lit';
+        await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
+        const view = _converse.api.chatviews.get(muc_jid);
+        const stanza = $pres({
+                to: 'romeo@montague.lit/_converse.js-29092160',
+                from: 'coven@chat.shakespeare.lit/newguy'
+            })
+            .c('x', {xmlns: Strophe.NS.MUC_USER})
+            .c('item', {
+                'affiliation': 'none',
+                'jid': 'newguy@montague.lit/_converse.js-290929789',
+                'role': 'participant'
+            }).tree();
+        _converse.connection._dataRecv(mock.createRequest(stanza));
+        const msg_id = u.getUniqueId();
+
+        // Receiving the first message
+        await view.model.handleMessageStanza($msg({
+                'from': 'lounge@montague.lit/newguy',
+                'to': _converse.connection.jid,
+                'type': 'groupchat',
+                'id': msg_id,
+            }).c('body').t('But soft, what light through yonder airlock breaks?').tree());
+
+        // Receiving own message to check order against
+        await view.model.handleMessageStanza($msg({
+            'from': 'lounge@montague.lit/romeo',
+            'to': _converse.connection.jid,
+            'type': 'groupchat',
+            'id': msg_id,
+        }).c('body').t('But soft, what light through yonder airlock breaks?').tree());
+
+        await u.waitUntil(() => view.el.querySelectorAll('.chat-msg').length === 2);
+        expect(view.el.querySelectorAll('.chat-msg').length).toBe(2);
+        expect(view.el.querySelectorAll('.chat-msg__text')[0].textContent)
+            .toBe('But soft, what light through yonder airlock breaks?');
+        expect(view.el.querySelectorAll('.chat-msg__text')[1].textContent)
+        .toBe('But soft, what light through yonder airlock breaks?');
+
+        // First message correction
+        await view.model.handleMessageStanza($msg({
+                'from': 'lounge@montague.lit/newguy',
+                'to': _converse.connection.jid,
+                'type': 'groupchat',
+                'id': u.getUniqueId(),
+            }).c('body').t('But soft, what light through yonder chimney breaks?').up()
+                .c('replace', {'id': msg_id, 'xmlns': 'urn:xmpp:message-correct:0'}).tree());
+
+        await u.waitUntil(() => view.el.querySelector('.chat-msg__text').textContent ===
+            'But soft, what light through yonder chimney breaks?', 500);
+        expect(view.el.querySelectorAll('.chat-msg').length).toBe(2);
+        await u.waitUntil(() => view.el.querySelector('.chat-msg__content .fa-edit'));
+
+        // Second message correction
+        await view.model.handleMessageStanza($msg({
+                'from': 'lounge@montague.lit/newguy',
+                'to': _converse.connection.jid,
+                'type': 'groupchat',
+                'id': u.getUniqueId(),
+            }).c('body').t('But soft, what light through yonder window breaks?').up()
+                .c('replace', {'id': msg_id, 'xmlns': 'urn:xmpp:message-correct:0'}).tree());
+
+        // Second own message
+        await view.model.handleMessageStanza($msg({
+            'from': 'lounge@montague.lit/romeo',
+            'to': _converse.connection.jid,
+            'type': 'groupchat',
+            'id': u.getUniqueId(),
+        }).c('body').t('But soft, what light through yonder window breaks?').tree());
+
+        await u.waitUntil(() => view.el.querySelectorAll('.chat-msg__text')[0].textContent ===
+            'But soft, what light through yonder window breaks?', 500);
+        await u.waitUntil(() => view.el.querySelectorAll('.chat-msg__text').length === 3);
+        await u.waitUntil(() => view.el.querySelectorAll('.chat-msg__text')[2].textContent ===
+            'But soft, what light through yonder window breaks?', 500);
+
+        expect(view.el.querySelectorAll('.chat-msg').length).toBe(3);
+        expect(view.el.querySelectorAll('.chat-msg__content .fa-edit').length).toBe(1);
+        const edit = await u.waitUntil(() => view.el.querySelector('.chat-msg__content .fa-edit'));
+        edit.click();
+        const modal = await u.waitUntil(() => view.el.querySelectorAll('converse-chat-message')[0].message_versions_modal);
+        await u.waitUntil(() => u.isVisible(modal.el), 1000);
+        const older_msgs = modal.el.querySelectorAll('.older-msg');
+        expect(older_msgs.length).toBe(2);
+        expect(older_msgs[0].childNodes[2].textContent).toBe('But soft, what light through yonder airlock breaks?');
+        expect(older_msgs[0].childNodes[0].nodeName).toBe('TIME');
+        expect(older_msgs[1].childNodes[0].nodeName).toBe('TIME');
+        expect(older_msgs[1].childNodes[2].textContent).toBe('But soft, what light through yonder chimney breaks?');
+        done();
+    }));
+
     it("can be sent as a correction by using the up arrow",
         mock.initConverse(
             ['rosterGroupsFetched'], {},

src/components/message.js

@@ -63,7 +63,7 @@ export default class Message extends CustomElement {
 
     render () {
         const format = api.settings.get('time_format');
-        this.pretty_time = dayjs(this.time).format(format);
+        this.pretty_time = dayjs(this.edited || this.time).format(format);
         if (this.show_spinner) {
             return tpl_spinner();
         } else if (this.model.get('file') && !this.model.get('oob_url')) {
@@ -264,7 +264,7 @@ export default class Message extends CustomElement {
     renderAvatarByline () {
         return html`
             ${ this.hats.map(h => html`<span class="badge badge-secondary">${h.title}</span>`) }
-            <time timestamp="${this.time}" class="chat-msg__time">${this.pretty_time}</time>
+            <time timestamp="${this.edited || this.time}" class="chat-msg__time">${this.pretty_time}</time>
         `;
     }
 

src/headless/converse-chat.js

@@ -744,9 +744,14 @@ converse.plugins.add('converse-chat', {
                     message.save({'older_versions': older_versions});
                 } else {
                     // This is a correction of an earlier message we already received
-                    older_versions[message.get('time')] = message.get('message');
+                    if(Object.keys(older_versions).length) {
+                        older_versions[message.get('edited')] = message.get('message');
+                    }else {
+                        older_versions[message.get('time')] = message.get('message');
+                    }
                     attrs = Object.assign(attrs, {'older_versions': older_versions});
                     delete attrs['id']; // Delete id, otherwise a new cache entry gets created
+                    attrs['time'] = message.get('time');
                     message.save(attrs);
                 }
                 return message;

src/templates/chat_message.js

@@ -28,7 +28,7 @@ export default (o) => {
                 <div class="chat-msg__body chat-msg__body--${o.message_type} ${o.received ? 'chat-msg__body--received' : '' } ${o.is_delayed ? 'chat-msg__body--delayed' : '' }">
                     <div class="chat-msg__message">
                         ${ (o.is_me_message) ? html`
-                            <time timestamp="${o.time}" class="chat-msg__time">${o.pretty_time}</time>&nbsp;
+                            <time timestamp="${o.edited || o.time}" class="chat-msg__time">${o.pretty_time}</time>&nbsp;
                             <span class="chat-msg__author">${ o.is_me_message ? '**' : ''}${o.username}</span>&nbsp;` : '' }
                         ${ o.is_retracted ? o.renderRetraction() : o.renderMessageText() }
                     </div>