diff --git a/lib/Chat/ChatManager.php b/lib/Chat/ChatManager.php index 42fe56a1087..398a5f6cfc3 100644 --- a/lib/Chat/ChatManager.php +++ b/lib/Chat/ChatManager.php @@ -363,7 +363,8 @@ public function deleteMessage(Room $chat, IComment $comment, Participant $partic $this->timeFactory->getDateTime(), false, null, - (int) $comment->getId() + (int) $comment->getId(), + true ); } diff --git a/lib/Chat/ReactionManager.php b/lib/Chat/ReactionManager.php index de346fa7b22..ddee0fb1e69 100644 --- a/lib/Chat/ReactionManager.php +++ b/lib/Chat/ReactionManager.php @@ -141,7 +141,8 @@ public function deleteReactionMessage(Room $chat, Participant $participant, int $this->timeFactory->getDateTime(), false, null, - $messageId + $messageId, + true ); return $comment; diff --git a/src/components/LeftSidebar/ConversationsList/Conversation.spec.js b/src/components/LeftSidebar/ConversationsList/Conversation.spec.js index ff82bbdd282..75e046d4a8e 100644 --- a/src/components/LeftSidebar/ConversationsList/Conversation.spec.js +++ b/src/components/LeftSidebar/ConversationsList/Conversation.spec.js @@ -130,20 +130,17 @@ describe('Conversation.vue', () => { describe('author name', () => { test('displays last chat message with shortened author name', () => { testConversationLabel(item, 'Alice: hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays last chat message with author name if no space in name', () => { item.lastMessage.actorDisplayName = 'Bob' testConversationLabel(item, 'Bob: hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays own last chat message with "You" as author', () => { item.lastMessage.actorId = 'user-id-self' testConversationLabel(item, 'You: hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays last system message without author', () => { @@ -151,13 +148,11 @@ describe('Conversation.vue', () => { item.lastMessage.systemMessage = 'call_joined' testConversationLabel(item, 'Alice has joined the call') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays last message without author in one to one conversations', () => { item.type = CONVERSATION.TYPE.ONE_TO_ONE testConversationLabel(item, 'hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays own last message with "You" author in one to one conversations', () => { @@ -165,7 +160,6 @@ describe('Conversation.vue', () => { item.lastMessage.actorId = 'user-id-self' testConversationLabel(item, 'You: hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays last guest message with default author when none set', () => { @@ -174,14 +168,12 @@ describe('Conversation.vue', () => { item.lastMessage.actorType = ATTENDEE.ACTOR_TYPE.GUESTS testConversationLabel(item, 'Guest: hello') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) test('displays last message for search results', () => { // search results have no actor id item.actorId = null testConversationLabel(item, 'Alice: hello', true) - expect(messagesMock).toHaveBeenCalledWith(TOKEN) }) }) @@ -194,80 +186,6 @@ describe('Conversation.vue', () => { } testConversationLabel(item, 'Alice: filename.jpg') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) - }) - - describe('last message from messages store', () => { - // see Conversation.lastChatMessage() description for the reasoning - let displayedLastStoreMessage - let lastMessageFromConversation - - beforeEach(() => { - displayedLastStoreMessage = { - id: 100, - actorId: 'user-id-alice', - actorDisplayName: 'Alice Wonderland', - actorType: ATTENDEE.ACTOR_TYPE.USERS, - message: 'hello from store', - messageParameters: {}, - systemMessage: '', - timestamp: 100, - } - - lastMessageFromConversation = { - id: 110, - actorId: 'user-id-alice', - actorDisplayName: 'Alice Wonderland', - actorType: ATTENDEE.ACTOR_TYPE.USERS, - message: 'hello from conversation', - messageParameters: {}, - systemMessage: '', - timestamp: 100, - } - - item.lastMessage = lastMessageFromConversation - - messagesMock.mockClear().mockReturnValue({ - 100: displayedLastStoreMessage, - }) - }) - - test('displays store message when more recent', () => { - displayedLastStoreMessage.timestamp = 2000 - - testConversationLabel(item, 'Alice: hello from store') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) - }) - - test('displays conversation message when last one is a temporary command message', () => { - messagesMock.mockClear().mockReturnValue({ - 'temp-100': displayedLastStoreMessage, - }) - - displayedLastStoreMessage.timestamp = 2000 - displayedLastStoreMessage.id = 'temp-100' - displayedLastStoreMessage.message = '/me doing things' - - testConversationLabel(item, 'Alice: hello from conversation') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) - }) - - test('displays conversation message when last one is a bot message', () => { - displayedLastStoreMessage.timestamp = 2000 - displayedLastStoreMessage.actorType = ATTENDEE.ACTOR_TYPE.BOTS - - testConversationLabel(item, 'Alice: hello from conversation') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) - }) - - test('displays store message when last one is a changelog bot message', () => { - displayedLastStoreMessage.timestamp = 2000 - displayedLastStoreMessage.actorType = ATTENDEE.ACTOR_TYPE.BOTS - displayedLastStoreMessage.actorId = ATTENDEE.CHANGELOG_BOT_ID - - testConversationLabel(item, 'Alice: hello from store') - expect(messagesMock).toHaveBeenCalledWith(TOKEN) - }) }) }) diff --git a/src/components/LeftSidebar/ConversationsList/Conversation.vue b/src/components/LeftSidebar/ConversationsList/Conversation.vue index 66323d5d6eb..efffdfd83ed 100644 --- a/src/components/LeftSidebar/ConversationsList/Conversation.vue +++ b/src/components/LeftSidebar/ConversationsList/Conversation.vue @@ -219,39 +219,12 @@ export default { }) }, - // The messages array for this conversation - messages() { - return this.$store.getters.messages(this.item.token) - }, - // Get the last message for this conversation from the message store instead // of the conversations store. The message store is updated immediately, // while the conversations store is refreshed every 30 seconds. This allows // to display message previews in this component as soon as new messages are // received by the server. lastChatMessage() { - const lastMessageTimestamp = this.item.lastMessage ? this.item.lastMessage.timestamp : 0 - - if (Object.keys(this.messages).length > 0) { - // FIXME: risky way to get last message that assumes that keys are always sorted - // should use this.$store.getters.messagesList instead ? - const messagesKeys = Object.keys(this.messages) - const lastMessageId = messagesKeys[messagesKeys.length - 1] - - /** - * Only use the last message as lastmessage when: - * 1. It's newer than the conversations last message - * 2. It's not a command reply - * 3. It's not a temporary message starting with "/" which is a user posting a command - */ - if (this.messages[lastMessageId].timestamp > lastMessageTimestamp - && (this.messages[lastMessageId].actorType !== ATTENDEE.ACTOR_TYPE.BOTS - || this.messages[lastMessageId].actorId === ATTENDEE.CHANGELOG_BOT_ID) - && (!lastMessageId.startsWith('temp-') - || !this.messages[lastMessageId].message.startsWith('/'))) { - return this.messages[lastMessageId] - } - } return this.item.lastMessage }, diff --git a/src/store/conversationsStore.js b/src/store/conversationsStore.js index 27dd0ecbd08..d5daa289c7e 100644 --- a/src/store/conversationsStore.js +++ b/src/store/conversationsStore.js @@ -410,11 +410,18 @@ const actions = { * Only use the last message as lastmessage when: * 1. It's not a command reply * 2. It's not a temporary message starting with "/" which is a user posting a command + * 3. It's not a reaction or deletion of a reaction + * 3. It's not a deletion of a message */ if ((lastMessage.actorType !== 'bots' || lastMessage.actorId === 'changelog') - && ((typeof lastMessage.id.startsWith === 'function' && !lastMessage.id.startsWith('temp-')) - || !lastMessage.message.startsWith('/'))) { + && lastMessage.systemMessage !== 'reaction' + && lastMessage.systemMessage !== 'reaction_deleted' + && lastMessage.systemMessage !== 'reaction_revoked' + && lastMessage.systemMessage !== 'message_deleted' + && !(typeof lastMessage.id.startsWith === 'function' + && lastMessage.id.startsWith('temp-') + && lastMessage.message.startsWith('/'))) { commit('updateConversationLastMessage', { token, lastMessage }) } }, diff --git a/src/store/conversationsStore.spec.js b/src/store/conversationsStore.spec.js index 83f38785ba1..739df851e38 100644 --- a/src/store/conversationsStore.spec.js +++ b/src/store/conversationsStore.spec.js @@ -50,6 +50,13 @@ jest.mock('../services/conversationsService', () => ({ describe('conversationsStore', () => { const testToken = 'XXTOKENXX' + const previousLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: '', + id: 31, + message: 'Message 1', + } let testStoreConfig = null let testConversation let localVue = null @@ -592,6 +599,232 @@ describe('conversationsStore', () => { }) }) + describe('update last message', () => { + beforeEach(() => { + store = new Vuex.Store(testStoreConfig) + }) + + test('successful update from user', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: '', + id: 42, + message: 'Message 2', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(testLastMessage) + }) + + test('ignore update from bot', () => { + const testLastMessage = { + actorType: 'bots', + actorId: 'selfmade', + systemMessage: '', + id: 42, + message: 'Message 2', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + + test('ignore update from bot but not from changelog', () => { + const testLastMessage = { + actorType: 'bots', + actorId: 'changelog', + systemMessage: '', + id: 42, + message: 'Message 2', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(testLastMessage) + }) + + test('ignore update reactions', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: 'reaction', + id: 42, + message: '👍', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + + test('ignore update from the action of deleting reactions', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: 'reaction_revoked', + id: 42, + message: 'Admin deleted a reaction', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + + test('ignore update deleted reactions (only theory as the action of deleting would come after it anyway)', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: 'reaction_deleted', + id: 42, + message: 'Reaction deleted by author', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + + test('ignore update from deleting a message', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: 'message_deleted', + id: 42, + message: 'Admin deleted a message', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + + test('successfully update temporary messages', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: '', + id: 'temp-42', + message: 'quit', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(testLastMessage) + }) + + test('successfully update also posted messages which start with a slash', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: '', + id: 42, + message: '/quit', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(testLastMessage) + }) + + test('ignore update from temporary if posting a command', () => { + const testLastMessage = { + actorType: 'users', + actorId: 'admin', + systemMessage: '', + id: 'temp-42', + message: '/quit', + } + + testConversation.lastMessage = previousLastMessage + + store.dispatch('addConversation', testConversation) + + store.dispatch('updateConversationLastMessage', { + token: testToken, + lastMessage: testLastMessage, + }) + + const changedConversation = store.getters.conversation(testToken) + expect(changedConversation.lastMessage).toBe(previousLastMessage) + }) + }) + describe('creating conversations', () => { test('creates one to one conversation', async () => { const newConversation = {