diff options
author | Joas Schilling <213943+nickvergessen@users.noreply.github.com> | 2022-11-09 14:04:59 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-11-09 14:04:59 +0100 |
commit | 525b42e27f3ef4eee7b0e0d17214f0d0408ef29f (patch) | |
tree | 8ae879ad51a5b4c13175216dcbc14367c4ce4acc | |
parent | f250a67f815d33a95b299f8975e8f3b8f844bb4c (diff) | |
parent | 516d50836a333b146e70c2f8ab1abe6564fa8328 (diff) |
Merge pull request #8291 from nextcloud/bugfix/8289/fix-chat-loading-and-performance-with-many-reactions-or-votes
Fix chat "not loading" and performance issue with many reactions or v…
-rw-r--r-- | src/components/MessagesList/MessagesList.vue | 21 | ||||
-rw-r--r-- | src/constants.js | 4 | ||||
-rw-r--r-- | src/services/messagesService.js | 4 | ||||
-rw-r--r-- | src/services/messagesService.spec.js | 2 | ||||
-rw-r--r-- | src/store/messagesStore.js | 58 | ||||
-rw-r--r-- | src/store/messagesStore.spec.js | 23 |
6 files changed, 64 insertions, 48 deletions
diff --git a/src/components/MessagesList/MessagesList.vue b/src/components/MessagesList/MessagesList.vue index bffb05d92..92c8c58e1 100644 --- a/src/components/MessagesList/MessagesList.vue +++ b/src/components/MessagesList/MessagesList.vue @@ -75,7 +75,10 @@ import MessagesGroup from './MessagesGroup/MessagesGroup.vue' import Axios from '@nextcloud/axios' import { subscribe, unsubscribe } from '@nextcloud/event-bus' import isInLobby from '../../mixins/isInLobby.js' -import { ATTENDEE } from '../../constants.js' +import { + ATTENDEE, + CHAT, +} from '../../constants.js' import debounce from 'debounce' import { EventBus } from '../../services/EventBus.js' import LoadingPlaceholder from '../LoadingPlaceholder.vue' @@ -164,18 +167,7 @@ export default { return this.$store.getters.messagesList(this.token) }, /** - * Gets the messages object, which is structured so that the key of each message element - * corresponds to the id of the message, and makes it easy and efficient to access the - * individual message object. - * - * @return {object} - */ - messages() { - // FIXME: remove if unused ? - return this.$store.getters.messages(this.token) - }, - /** - * Creates an array of messages grouped in nested arrays by same autor. + * Creates an array of messages grouped in nested arrays by same author. * * @return {Array} */ @@ -558,6 +550,7 @@ export default { token: this.token, lastKnownMessageId: this.$store.getters.getFirstKnownMessageId(this.token), includeLastKnown, + minimumVisible: CHAT.MINIMUM_VISIBLE, }) this.loadingOldMessages = false @@ -599,7 +592,7 @@ export default { return } - if (exception.response && exception.response.status === 304) { + if (exception?.response?.status === 304) { // 304 - Not modified // This is not an error, so reset error timeout and poll again this.pollingErrorTimeout = 1 diff --git a/src/constants.js b/src/constants.js index bacf04c7d..b33c5de45 100644 --- a/src/constants.js +++ b/src/constants.js @@ -24,6 +24,10 @@ export const SIGNALING = { CLUSTER_CONVERSATION: 'conversation_cluster', }, } +export const CHAT = { + FETCH_LIMIT: 100, + MINIMUM_VISIBLE: 5, +} export const CONVERSATION = { START_CALL: { EVERYONE: 0, diff --git a/src/services/messagesService.js b/src/services/messagesService.js index c135f71a0..587931957 100644 --- a/src/services/messagesService.js +++ b/src/services/messagesService.js @@ -33,14 +33,16 @@ import Hex from 'crypto-js/enc-hex.js' * @param {string} data.token the conversation token; * @param {string} data.lastKnownMessageId last known message id; * @param {boolean} data.includeLastKnown whether to include the last known message in the response; + * @param {number} data.limit Number of messages to load (default: 100) * @param {object} options options; */ -const fetchMessages = async function({ token, lastKnownMessageId, includeLastKnown }, options) { +const fetchMessages = async function({ token, lastKnownMessageId, includeLastKnown, limit }, options) { return axios.get(generateOcsUrl('apps/spreed/api/v1/chat/{token}', { token }), Object.assign(options, { params: { setReadMarker: 0, lookIntoFuture: 0, lastKnownMessageId, + limit: limit || 100, includeLastKnown: includeLastKnown ? 1 : 0, }, })) diff --git a/src/services/messagesService.spec.js b/src/services/messagesService.spec.js index d27234673..b825b8c1e 100644 --- a/src/services/messagesService.spec.js +++ b/src/services/messagesService.spec.js @@ -32,6 +32,7 @@ describe('messagesService', () => { setReadMarker: 0, lookIntoFuture: 0, lastKnownMessageId: 1234, + limit: 100, includeLastKnown: 0, }, } @@ -55,6 +56,7 @@ describe('messagesService', () => { setReadMarker: 0, lookIntoFuture: 0, lastKnownMessageId: 1234, + limit: 100, includeLastKnown: 1, }, } diff --git a/src/store/messagesStore.js b/src/store/messagesStore.js index 44c9911eb..49e5c3b06 100644 --- a/src/store/messagesStore.js +++ b/src/store/messagesStore.js @@ -37,6 +37,7 @@ import CancelableRequest from '../utils/cancelableRequest.js' import { showError } from '@nextcloud/dialogs' import { ATTENDEE, + CHAT, } from '../constants.js' /** @@ -153,27 +154,10 @@ const getters = { */ messagesList: (state) => (token) => { if (state.messages[token]) { - return Object.values(state.messages[token]).filter(message => { - // Filter out some system messages - if (message.systemMessage === 'reaction' - || message.systemMessage === 'reaction_deleted' - || message.systemMessage === 'reaction_revoked' - || message.systemMessage === 'poll_voted' - ) { - return false - } else { - return true - } - }) + return Object.values(state.messages[token]) } return [] }, - messages: (state) => (token) => { - if (state.messages[token]) { - return state.messages[token] - } - return {} - }, message: (state) => (token, id) => { if (state.messages[token][id]) { return state.messages[token][id] @@ -479,7 +463,9 @@ const actions = { }) } - if (message.systemMessage === 'reaction' || message.systemMessage === 'reaction_revoked') { + if (message.systemMessage === 'reaction' + || message.systemMessage === 'reaction_deleted' + || message.systemMessage === 'reaction_revoked') { context.commit('resetReactions', { token: message.token, messageId: message.parent, @@ -500,7 +486,14 @@ const actions = { }) } - context.commit('addMessage', message) + // Filter out some system messages + if (message.systemMessage !== 'reaction' + && message.systemMessage !== 'reaction_deleted' + && message.systemMessage !== 'reaction_revoked' + && message.systemMessage !== 'poll_voted' + ) { + context.commit('addMessage', message) + } if ((message.messageType === 'comment' && message.message === '{file}' && message.messageParameters?.file) || (message.messageType === 'comment' && message.message === '{object}' && message.messageParameters?.object)) { @@ -740,9 +733,12 @@ const actions = { * @param {string} data.token the conversation token; * @param {object} data.requestOptions request options; * @param {string} data.lastKnownMessageId last known message id; + * @param {number} data.minimumVisible Minimum number of chat messages we want to load * @param {boolean} data.includeLastKnown whether to include the last known message in the response; */ - async fetchMessages(context, { token, lastKnownMessageId, includeLastKnown, requestOptions }) { + async fetchMessages(context, { token, lastKnownMessageId, includeLastKnown, requestOptions, minimumVisible }) { + minimumVisible = (typeof minimumVisible === 'undefined') ? CHAT.MINIMUM_VISIBLE : minimumVisible + context.dispatch('cancelFetchMessages') // Get a new cancelable request function and cancel function pair @@ -754,6 +750,7 @@ const actions = { token, lastKnownMessageId, includeLastKnown, + limit: CHAT.FETCH_LIMIT, }, requestOptions) let newestKnownMessageId = 0 @@ -774,6 +771,14 @@ const actions = { } context.dispatch('processMessage', message) newestKnownMessageId = Math.max(newestKnownMessageId, message.id) + + if (message.systemMessage !== 'reaction' + && message.systemMessage !== 'reaction_deleted' + && message.systemMessage !== 'reaction_revoked' + && message.systemMessage !== 'poll_voted' + ) { + minimumVisible-- + } }) if (response.headers['x-chat-last-given']) { @@ -796,6 +801,17 @@ const actions = { context.commit('loadedMessagesOfConversation', { token }) + if (minimumVisible > 0) { + // There are not yet enough visible messages loaded, so fetch another chunk. + // This can happen when a lot of reactions or poll votings happen + return await context.dispatch('fetchMessages', { + token, + lastKnownMessageId: context.getters.getFirstKnownMessageId(token), + includeLastKnown, + minimumVisible, + }) + } + return response }, diff --git a/src/store/messagesStore.spec.js b/src/store/messagesStore.spec.js index ebabf2d92..8fc605be9 100644 --- a/src/store/messagesStore.spec.js +++ b/src/store/messagesStore.spec.js @@ -137,19 +137,14 @@ describe('messagesStore', () => { expect(store.getters.messagesList(TOKEN)[1]).toStrictEqual(message3) expect(store.getters.messagesList('token-2')[0]).toStrictEqual(message2) - // by id - expect(store.getters.messages(TOKEN)[1]).toStrictEqual(message1) - expect(store.getters.messages(TOKEN)[3]).toStrictEqual(message3) - expect(store.getters.messages('token-2')[2]).toStrictEqual(message2) - // with messages getter - expect(store.getters.messages(TOKEN)).toStrictEqual({ - 1: message1, - 3: message3, - }) - expect(store.getters.messages('token-2')).toStrictEqual({ - 2: message2, - }) + expect(store.getters.messagesList(TOKEN)).toStrictEqual([ + message1, + message3, + ]) + expect(store.getters.messagesList('token-2')).toStrictEqual([ + message2, + ]) }) describe('delete message', () => { @@ -767,12 +762,14 @@ describe('messagesStore', () => { requestOptions: { dummyOption: true, }, + minimumVisible: 0, }) expect(fetchMessages).toHaveBeenCalledWith({ token: TOKEN, lastKnownMessageId: 100, includeLastKnown: true, + limit: 100, }, { dummyOption: true, }) @@ -818,12 +815,14 @@ describe('messagesStore', () => { requestOptions: { dummyOption: true, }, + minimumVisible: 0, }) expect(fetchMessages).toHaveBeenCalledWith({ token: TOKEN, lastKnownMessageId: 100, includeLastKnown: false, + limit: 100, }, { dummyOption: true, }) |