Merge pull request #7142 from nextcloud/bugfix/7024/dont-update-last-message

Don't update last message of conversation with invisible message

Author: marcoambrosini

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
 		);
 	}
 

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;

src/components/LeftSidebar/ConversationsList/Conversation.spec.js

@@ -130,42 +130,36 @@ 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', () => {
 				item.lastMessage.message = 'Alice has joined the call'
 				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', () => {
 				item.type = CONVERSATION.TYPE.ONE_TO_ONE
 				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)
-			})
 		})
 	})
 

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
 		},
 

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 })
 		}
 	},