From 82a95a03beda6a9126d9f382e2f2394695bf15f7 Mon Sep 17 00:00:00 2001 From: Julian Lam Date: Mon, 18 Dec 2017 14:45:06 -0500 Subject: [PATCH] Chat message soft deletion -- closes #6181 Squashed commit of the following: commit f84c06bdcc45f24ef7ffde6a8f33b48d8f97fc36 Author: Julian Lam Date: Mon Dec 18 14:42:47 2017 -0500 added restore handler for chat messages commit 725cd370c6ea1e8f4a28298350f3dc024d4e668e Author: Julian Lam Date: Mon Dec 18 14:23:52 2017 -0500 backend logic and testing complete for deletion and restoration of chat messages commit 072da758319cc93fa4c6f8bc0d672a1b716dc06e Author: Julian Lam Date: Mon Dec 18 13:52:35 2017 -0500 changing message delete logic to not remove mids, but to filter when retrieving commit 68bf373305ab82737658a7c31dc5549af4d6d69f Author: Julian Lam Date: Mon Dec 18 12:37:58 2017 -0500 logic to handle deletion of a deleted chat message -- added some failing tests commit 6899d0d234fa752e227188aa69cfcabd0d0500cc Author: Julian Lam Date: Mon Dec 18 11:35:36 2017 -0500 chat message deletion logic --- public/language/en-GB/error.json | 2 + public/src/client/chats.js | 27 +++++++---- public/src/client/chats/messages.js | 17 +++++-- src/messaging.js | 10 ++++ src/messaging/create.js | 1 + src/messaging/data.js | 2 + src/messaging/delete.js | 31 +++++++------ src/socket.io/modules.js | 15 ++++++ test/messaging.js | 72 ++++++++++++++++++++++++----- 9 files changed, 142 insertions(+), 35 deletions(-) diff --git a/public/language/en-GB/error.json b/public/language/en-GB/error.json index 7700006520..0cb282a8d4 100644 --- a/public/language/en-GB/error.json +++ b/public/language/en-GB/error.json @@ -140,6 +140,8 @@ "cant-delete-chat-message": "You are not allowed to delete this message", "chat-edit-duration-expired": "You are only allowed to edit chat messages for %1 second(s) after posting", "chat-delete-duration-expired": "You are only allowed to delete chat messages for %1 second(s) after posting", + "chat-deleted-already": "This chat message has already been deleted.", + "chat-restored'already": "This chat message has already been restored.", "already-voting-for-this-post": "You have already voted for this post.", "reputation-system-disabled": "Reputation system is disabled.", diff --git a/public/src/client/chats.js b/public/src/client/chats.js index f82623fd5d..59e923fb8a 100644 --- a/public/src/client/chats.js +++ b/public/src/client/chats.js @@ -71,7 +71,7 @@ define('forum/chats', [ }); }); - Chats.addEditDeleteHandler(components.get('chat/messages'), ajaxify.data.roomId); + Chats.addActionHandlers(components.get('chat/messages'), ajaxify.data.roomId); Chats.addRenameHandler(ajaxify.data.roomId, $('[component="chat/room/name"]')); Chats.addScrollHandler(ajaxify.data.roomId, ajaxify.data.uid, $('.chat-content')); @@ -123,14 +123,25 @@ define('forum/chats', [ }); }; - Chats.addEditDeleteHandler = function (element, roomId) { - element.on('click', '[data-action="edit"]', function () { + Chats.addActionHandlers = function (element, roomId) { + element.on('click', '[data-action]', function () { var messageId = $(this).parents('[data-mid]').attr('data-mid'); - var inputEl = $('[data-roomid="' + roomId + '"] [component="chat/input"]'); - messages.prepEdit(inputEl, messageId, roomId); - }).on('click', '[data-action="delete"]', function () { - var messageId = $(this).parents('[data-mid]').attr('data-mid'); - messages.delete(messageId, roomId); + var action = this.getAttribute('data-action'); + + switch (action) { + case 'edit': + var inputEl = $('[data-roomid="' + roomId + '"] [component="chat/input"]'); + messages.prepEdit(inputEl, messageId, roomId); + break; + + case 'delete': + messages.delete(messageId, roomId); + break; + + case 'restore': + messages.restore(messageId, roomId); + break; + } }); }; diff --git a/public/src/client/chats/messages.js b/public/src/client/chats/messages.js index fc4a327021..cfbcacf8f8 100644 --- a/public/src/client/chats/messages.js +++ b/public/src/client/chats/messages.js @@ -146,13 +146,24 @@ define('forum/chats/messages', ['components', 'sounds', 'translator', 'benchpres return app.alertError(err.message); } - components.get('chat/message', messageId).slideUp('slow', function () { - $(this).remove(); - }); + components.get('chat/message', messageId).toggleClass('deleted', true); }); }); }); }; + messages.restore = function (messageId, roomId) { + socket.emit('modules.chats.restore', { + messageId: messageId, + roomId: roomId, + }, function (err) { + if (err) { + return app.alertError(err.message); + } + + components.get('chat/message', messageId).toggleClass('deleted', false); + }); + }; + return messages; }); diff --git a/src/messaging.js b/src/messaging.js index 9ceda65b7d..ab22b3c773 100644 --- a/src/messaging.js +++ b/src/messaging.js @@ -56,6 +56,16 @@ Messaging.getMessages = function (params, callback) { messageData.forEach(function (messageData) { messageData.index = indices[messageData.messageId.toString()]; }); + + // Filter out deleted messages unless you're the sender of said message + messageData = messageData.filter(function (messageData) { + if (messageData.deleted && parseInt(messageData.fromuid, 10) !== parseInt(params.uid, 10)) { + return false; + } + + return true; + }); + next(null, messageData); }, ], callback); diff --git a/src/messaging/create.js b/src/messaging/create.js index f4fe421eac..6f355a9ff8 100644 --- a/src/messaging/create.js +++ b/src/messaging/create.js @@ -58,6 +58,7 @@ module.exports = function (Messaging) { timestamp: timestamp, fromuid: fromuid, roomId: roomId, + deleted: 0, }; plugins.fireHook('filter:messaging.save', message, next); diff --git a/src/messaging/data.js b/src/messaging/data.js index e18d71113f..bae9c6deac 100644 --- a/src/messaging/data.js +++ b/src/messaging/data.js @@ -67,6 +67,8 @@ module.exports = function (Messaging) { if (message.hasOwnProperty('edited')) { message.editedISO = new Date(parseInt(message.edited, 10)).toISOString(); } + + message.deleted = !!parseInt(message.deleted, 10); }); async.map(messages, function (message, next) { diff --git a/src/messaging/delete.js b/src/messaging/delete.js index e3058c0ba9..55907491f2 100644 --- a/src/messaging/delete.js +++ b/src/messaging/delete.js @@ -1,25 +1,30 @@ 'use strict'; var async = require('async'); -var db = require('../database'); module.exports = function (Messaging) { Messaging.deleteMessage = function (mid, roomId, callback) { async.waterfall([ - function (next) { - Messaging.getUidsInRoom(roomId, 0, -1, next); - }, - function (uids, next) { - if (!uids.length) { - return next(); + async.apply(Messaging.getMessageField, mid, 'deleted'), + function (deleted, next) { + if (parseInt(deleted, 10)) { + return next(new Error('[[error:chat-deleted-already]]')); } - var keys = uids.map(function (uid) { - return 'uid:' + uid + ':chat:room:' + roomId + ':mids'; - }); - db.sortedSetsRemove(keys, mid, next); + + Messaging.setMessageField(mid, 'deleted', 1, next); }, - function (next) { - db.delete('message:' + mid, next); + ], callback); + }; + + Messaging.restoreMessage = function (mid, roomId, callback) { + async.waterfall([ + async.apply(Messaging.getMessageField, mid, 'deleted'), + function (deleted, next) { + if (!parseInt(deleted, 10)) { + return next(new Error('[[error:chat-restored-already]]')); + } + + Messaging.setMessageField(mid, 'deleted', 0, next); }, ], callback); }; diff --git a/src/socket.io/modules.js b/src/socket.io/modules.js index d58fb7fa59..7ff5accf40 100644 --- a/src/socket.io/modules.js +++ b/src/socket.io/modules.js @@ -267,6 +267,21 @@ SocketModules.chats.delete = function (socket, data, callback) { ], callback); }; +SocketModules.chats.restore = function (socket, data, callback) { + if (!data || !data.roomId || !data.messageId) { + return callback(new Error('[[error:invalid-data]]')); + } + + async.waterfall([ + function (next) { + Messaging.canDelete(data.messageId, socket.uid, next); + }, + function (next) { + Messaging.restoreMessage(data.messageId, data.roomId, next); + }, + ], callback); +}; + SocketModules.chats.canMessage = function (socket, roomId, callback) { Messaging.canMessageRoom(socket.uid, roomId, callback); }; diff --git a/test/messaging.js b/test/messaging.js index 0253fbf6b0..024c204ff3 100644 --- a/test/messaging.js +++ b/test/messaging.js @@ -14,8 +14,8 @@ var helpers = require('./helpers'); var socketModules = require('../src/socket.io/modules'); describe('Messaging Library', function () { - var fooUid; - var bazUid; + var fooUid; // the admin + var bazUid; // the user with chat restriction enabled var herpUid; var roomId; @@ -552,21 +552,71 @@ describe('Messaging Library', function () { }); }); - - it('should delete message', function (done) { + it('should mark the message as deleted', function (done) { socketModules.chats.delete({ uid: fooUid }, { messageId: mid, roomId: roomId }, function (err) { assert.ifError(err); - db.exists('message:' + mid, function (err, exists) { + db.getObjectField('message:' + mid, 'deleted', function (err, value) { assert.ifError(err); - assert(!exists); - db.isSortedSetMember('uid:' + fooUid + ':chat:room:' + roomId + ':mids', mid, function (err, isMember) { - assert.ifError(err); - assert(!isMember); - done(); - }); + assert.strictEqual(1, parseInt(value, 10)); + done(); }); }); }); + + it('should show deleted message to original users', function (done) { + socketModules.chats.getMessages({ uid: fooUid }, { uid: fooUid, roomId: roomId, start: 0 }, function (err, messages) { + assert.ifError(err); + + // Reduce messages to their mids + var mids = messages.reduce(function (mids, cur) { + mids.push(cur.messageId); + return mids; + }, []); + + assert(mids.includes(mid)); + done(); + }); + }); + + it('should not show deleted message to other users', function (done) { + socketModules.chats.getMessages({ uid: herpUid }, { uid: herpUid, roomId: roomId, start: 0 }, function (err, messages) { + assert.ifError(err); + + // Reduce messages to their mids + var mids = messages.reduce(function (mids, cur) { + mids.push(cur.messageId); + return mids; + }, []); + + assert(!mids.includes(mid)); + done(); + }); + }); + + it('should error out if a message is deleted again', function (done) { + socketModules.chats.delete({ uid: fooUid }, { messageId: mid, roomId: roomId }, function (err) { + assert.strictEqual('[[error:chat-deleted-already]]', err.message); + done(); + }); + }); + + it('should restore the message', function (done) { + socketModules.chats.restore({ uid: fooUid }, { messageId: mid, roomId: roomId }, function (err) { + assert.ifError(err); + db.getObjectField('message:' + mid, 'deleted', function (err, value) { + assert.ifError(err); + assert.strictEqual(0, parseInt(value, 10)); + done(); + }); + }); + }); + + it('should error out if a message is restored again', function (done) { + socketModules.chats.restore({ uid: fooUid }, { messageId: mid, roomId: roomId }, function (err) { + assert.strictEqual('[[error:chat-restored-already]]', err.message); + done(); + }); + }); }); describe('controller', function () {