Merge pull request #14205 from nextcloud/fix/noid/ease-message-list

nextcloud · Jan 27, 2025 · 6b93ad4 · 6b93ad4
2 parents 932ac14 + 628071d
commit 6b93ad4
Show file tree

Hide file tree

Showing 3 changed files with 95 additions and 33 deletions.
diff --git a/src/components/MessagesList/MessagesList.vue b/src/components/MessagesList/MessagesList.vue
@@ -621,7 +621,7 @@ export default {
 			let isFocused = null
 			if (focusMessageId) {
 				// scroll to message in URL anchor
-				this.focusMessage(focusMessageId)
+				this.focusMessage(focusMessageId, false)
 				return
 			}
 

diff --git a/src/store/messagesStore.js b/src/store/messagesStore.js
@@ -490,26 +490,42 @@ const mutations = {
 		})
 	},
 
-	easeMessageList(state, { token }) {
+	easeMessageList(state, { token, lastReadMessage }) {
 		if (!state.messages[token]) {
 			return
 		}
 
-		const messageIds = Object.keys(state.messages[token])
+		const messageIds = Object.keys(state.messages[token]).sort((a, b) => b - a)
 		if (messageIds.length < 300) {
 			return
 		}
 
-		const messagesToRemove = messageIds.sort((a, b) => b - a).slice(199)
+		// If lastReadMessage is rendered, keep it and +- 100 messages, otherwise only newest 200 messages
+		const lastReadMessageIndex = messageIds.findIndex(id => +id === lastReadMessage)
+
+		const messagesToRemove = lastReadMessageIndex !== -1
+			? messageIds.slice(lastReadMessageIndex + 99)
+			: messageIds.slice(199)
 		const newFirstKnown = messagesToRemove.shift()
 
+		const newMessagesToRemove = (lastReadMessageIndex !== -1 && lastReadMessageIndex > 100)
+			? messageIds.slice(0, lastReadMessageIndex - 99)
+			: []
+		const newLastKnown = newMessagesToRemove.pop()
+
 		messagesToRemove.forEach((messageId) => {
 			Vue.delete(state.messages[token], messageId)
 		})
+		newMessagesToRemove.forEach((messageId) => {
+			Vue.delete(state.messages[token], messageId)
+		})
 
 		if (state.firstKnown[token] && messagesToRemove.includes(state.firstKnown[token].toString())) {
 			Vue.set(state.firstKnown, token, +newFirstKnown)
 		}
+		if (state.lastKnown[token] && newMessagesToRemove.includes(state.lastKnown[token].toString())) {
+			Vue.set(state.lastKnown, token, +newLastKnown)
+		}
 	},
 }
 
@@ -1378,7 +1394,8 @@ const actions = {
 	},
 
 	async easeMessageList(context, { token }) {
-		context.commit('easeMessageList', { token })
+		const lastReadMessage = context.getters.conversation(token)?.lastReadMessage
+		context.commit('easeMessageList', { token, lastReadMessage })
 	},
 
 	loadedMessagesOfConversation(context, { token }) {

diff --git a/src/store/messagesStore.spec.js b/src/store/messagesStore.spec.js
@@ -258,35 +258,80 @@ describe('messagesStore', () => {
 		})
 	})
 
-	test('message list', () => {
-		const message1 = {
-			id: 1,
-			token: TOKEN,
-		}
-		const message2 = {
-			id: 2,
-			token: 'token-2',
-		}
-		const message3 = {
-			id: 3,
-			token: TOKEN,
-		}
+	describe('messages list', () => {
+		test('returns messages list', () => {
+			const message1 = {
+				id: 1,
+				token: TOKEN,
+			}
+			const message2 = {
+				id: 2,
+				token: 'token-2',
+			}
+			const message3 = {
+				id: 3,
+				token: TOKEN,
+			}
+
+			store.dispatch('processMessage', { token: message1.token, message: message1 })
+			store.dispatch('processMessage', { token: message2.token, message: message2 })
+			store.dispatch('processMessage', { token: message3.token, message: message3 })
+			expect(store.getters.messagesList(TOKEN)[0]).toStrictEqual(message1)
+			expect(store.getters.messagesList(TOKEN)[1]).toStrictEqual(message3)
+			expect(store.getters.messagesList('token-2')[0]).toStrictEqual(message2)
+
+			// with messages getter
+			expect(store.getters.messagesList(TOKEN)).toStrictEqual([
+				message1,
+				message3,
+			])
+			expect(store.getters.messagesList('token-2')).toStrictEqual([
+				message2,
+			])
+		})
 
-		store.dispatch('processMessage', { token: message1.token, message: message1 })
-		store.dispatch('processMessage', { token: message2.token, message: message2 })
-		store.dispatch('processMessage', { token: message3.token, message: message3 })
-		expect(store.getters.messagesList(TOKEN)[0]).toStrictEqual(message1)
-		expect(store.getters.messagesList(TOKEN)[1]).toStrictEqual(message3)
-		expect(store.getters.messagesList('token-2')[0]).toStrictEqual(message2)
-
-		// with messages getter
-		expect(store.getters.messagesList(TOKEN)).toStrictEqual([
-			message1,
-			message3,
-		])
-		expect(store.getters.messagesList('token-2')).toStrictEqual([
-			message2,
-		])
+		const testCases = [
+			// Default
+			[1, 200, 1, 200, 200, undefined],
+			[1, 400, 201, 400, 200, undefined],
+			// with lastReadMessage
+			[201, 600, 401, 600, 200, 200],
+			[1, 400, 101, 300, 200, 200],
+			[1, 400, 201, 400, 200, 300],
+			// Border values
+			[1, 400, 1, 101, 101, 1],
+			[1, 400, 301, 400, 100, 400],
+			[1, 400, 1, 199, 199, 99],
+			[1, 400, 1, 200, 200, 100],
+			[1, 400, 2, 201, 200, 101],
+			[1, 400, 202, 400, 199, 301],
+			[1, 400, 201, 400, 200, 300],
+			[1, 400, 200, 399, 200, 299],
+		]
+
+		it.each(testCases)('eases list from [%s - %s] to [%s - %s] (length: %s) with lastReadMessage %s',
+			(oldFirst, oldLast, newFirst, newLast, length, lastReadMessage) => {
+			// Arrange
+				conversationMock.mockReturnValue({ lastReadMessage })
+				for (let id = oldFirst; id <= oldLast; id++) {
+					store.dispatch('processMessage', { token: TOKEN, message: { token: TOKEN, id } })
+				}
+				store.dispatch('setFirstKnownMessageId', { token: TOKEN, id: oldFirst })
+				store.dispatch('setLastKnownMessageId', { token: TOKEN, id: oldLast })
+
+				// Act
+				store.dispatch('easeMessageList', { token: TOKEN })
+
+				// Assert
+				expect(store.getters.messagesList(TOKEN)).toHaveLength(length)
+				expect(store.getters.messagesList(TOKEN).at(0)).toStrictEqual({ token: TOKEN, id: newFirst })
+				expect(store.getters.messagesList(TOKEN).at(-1)).toStrictEqual({ token: TOKEN, id: newLast })
+				expect(store.getters.getFirstKnownMessageId(TOKEN)).toStrictEqual(newFirst)
+				expect(store.getters.getLastKnownMessageId(TOKEN)).toStrictEqual(newLast)
+				if (oldFirst < lastReadMessage && lastReadMessage < oldLast) {
+					expect(store.getters.message(TOKEN, lastReadMessage)).toBeDefined()
+				}
+			})
 	})
 
 	describe('delete message', () => {