From 9ef7390af690616905232ca9843c0213223c0822 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sun, 17 Dec 2023 11:39:55 +0100 Subject: [PATCH 1/4] fix(participants): don't purge store on each fetch, update only changed participants Signed-off-by: Maksim Sukharev --- src/store/participantsStore.js | 47 +++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/src/store/participantsStore.js b/src/store/participantsStore.js index 93eec5c7466..69161534376 100644 --- a/src/store/participantsStore.js +++ b/src/store/participantsStore.js @@ -600,12 +600,14 @@ const actions = { try { const response = await request(token) - context.dispatch('purgeParticipantsStore', token) - const hasUserStatuses = !!response.headers['x-nextcloud-has-user-statuses'] response.data.ocs.data.forEach(participant => { - context.dispatch('addParticipant', { token, participant }) + if (context.state.attendees[token]?.[participant.attendeeId]) { + context.dispatch('updateParticipantIfHasChanged', { token, participant }) + } else { + context.dispatch('addParticipant', { token, participant }) + } if (participant.participantType === PARTICIPANT.TYPE.GUEST || participant.participantType === PARTICIPANT.TYPE.GUEST_MODERATOR) { @@ -640,6 +642,45 @@ const actions = { } }, + /** + * Update participant in store according to a new participant object + * + * @param {object} context store context + * @param {object} data the wrapping object; + * @param {string} data.token the conversation token; + * @param {object} data.participant the new participant object; + * @return {boolean} whether the participant was changed + */ + updateParticipantIfHasChanged(context, { token, participant }) { + const { attendeeId } = participant + const oldParticipant = context.state.attendees[token][attendeeId] + + // Update, if status has changed + if (oldParticipant.status !== participant.status + || oldParticipant.statusMessage !== participant.statusMessage + || oldParticipant.statusIcon !== participant.statusIcon + || oldParticipant.statusClearAt !== participant.statusClearAt + ) { + context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) + return true + } + + // Check if any property were changed (no properties except status-related supposed to be added or deleted) + for (const key of Object.keys(participant)) { + // "sessionIds" is the only property with non-primitive (array) value and cannot be compared by === + if (key !== 'sessionIds' && oldParticipant[key] !== participant[key]) { + context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) + return true + } else if (key === 'sessionIds' && (oldParticipant.sessionIds.length !== participant.sessionIds.length + || !oldParticipant.sessionIds.every((element, index) => element === participant.sessionIds[index]))) { + context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) + return true + } + } + + return false + }, + /** * Cancels a previously running "fetchParticipants" action if applicable. * From 38185c5acc4d9dba81f86b8ec53700803b2d79a6 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sun, 17 Dec 2023 12:39:29 +0100 Subject: [PATCH 2/4] fix(participants): update user status if there were actual change Signed-off-by: Maksim Sukharev --- .../Participants/ParticipantsTab.vue | 8 +++- src/store/participantsStore.js | 38 +++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/components/RightSidebar/Participants/ParticipantsTab.vue b/src/components/RightSidebar/Participants/ParticipantsTab.vue index 55b9f3266fd..acdece449ea 100644 --- a/src/components/RightSidebar/Participants/ParticipantsTab.vue +++ b/src/components/RightSidebar/Participants/ParticipantsTab.vue @@ -285,7 +285,12 @@ export default { return } - if (this.participants.find(participant => participant.actorId === state.userId)) { + const participant = this.participants.find(participant => participant.actorId === state.userId) + if (participant && (participant.status !== state.status + || participant.statusMessage !== state.message + || participant.statusIcon !== state.icon + || participant.statusClearAt !== state.clearAt + )) { this.$store.dispatch('updateUser', { token: this.token, participantIdentifier: { @@ -296,6 +301,7 @@ export default { status: state.status, statusIcon: state.icon, statusMessage: state.message, + statusClearAt: state.clearAt, }, }) } diff --git a/src/store/participantsStore.js b/src/store/participantsStore.js index 69161534376..12d75753d47 100644 --- a/src/store/participantsStore.js +++ b/src/store/participantsStore.js @@ -53,6 +53,23 @@ import { talkBroadcastChannel } from '../services/talkBroadcastChannel.js' import { useGuestNameStore } from '../stores/guestName.js' import CancelableRequest from '../utils/cancelableRequest.js' +/** + * Emit global event for user status update with the status from a participant + * + * @param {object} participant - a participant object + */ +function emitUserStatusUpdated(participant) { + if (participant.actorType === 'users') { + emit('user_status:status.updated', { + status: participant.status, + message: participant.statusMessage, + icon: participant.statusIcon, + clearAt: participant.statusClearAt, + userId: participant.actorId, + }) + } +} + const state = { attendees: { }, @@ -604,9 +621,12 @@ const actions = { response.data.ocs.data.forEach(participant => { if (context.state.attendees[token]?.[participant.attendeeId]) { - context.dispatch('updateParticipantIfHasChanged', { token, participant }) + context.dispatch('updateParticipantIfHasChanged', { token, participant, hasUserStatuses }) } else { context.dispatch('addParticipant', { token, participant }) + if (hasUserStatuses) { + emitUserStatusUpdated(participant) + } } if (participant.participantType === PARTICIPANT.TYPE.GUEST @@ -616,14 +636,6 @@ const actions = { actorId: Hex.stringify(SHA1(participant.sessionIds[0])), actorDisplayName: participant.displayName, }, { noUpdate: false }) - } else if (participant.actorType === 'users' && hasUserStatuses) { - emit('user_status:status.updated', { - status: participant.status, - message: participant.statusMessage, - icon: participant.statusIcon, - clearAt: participant.statusClearAt, - userId: participant.actorId, - }) } }) @@ -649,19 +661,21 @@ const actions = { * @param {object} data the wrapping object; * @param {string} data.token the conversation token; * @param {object} data.participant the new participant object; + * @param {boolean} data.hasUserStatuses whether user status is enabled or not; * @return {boolean} whether the participant was changed */ - updateParticipantIfHasChanged(context, { token, participant }) { + updateParticipantIfHasChanged(context, { token, participant, hasUserStatuses }) { const { attendeeId } = participant const oldParticipant = context.state.attendees[token][attendeeId] // Update, if status has changed - if (oldParticipant.status !== participant.status + if (hasUserStatuses && (oldParticipant.status !== participant.status || oldParticipant.statusMessage !== participant.statusMessage || oldParticipant.statusIcon !== participant.statusIcon || oldParticipant.statusClearAt !== participant.statusClearAt - ) { + )) { context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) + emitUserStatusUpdated(participant) return true } From 14e025ade5f845169937b947f99dc91aded310a3 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sun, 17 Dec 2023 12:54:21 +0100 Subject: [PATCH 3/4] fix(Participant): remove unused props Signed-off-by: Maksim Sukharev --- .../ParticipantsList/Participant/Participant.vue | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/components/RightSidebar/Participants/ParticipantsList/Participant/Participant.vue b/src/components/RightSidebar/Participants/ParticipantsList/Participant/Participant.vue index b9633fdcd10..9c48077220f 100644 --- a/src/components/RightSidebar/Participants/ParticipantsList/Participant/Participant.vue +++ b/src/components/RightSidebar/Participants/ParticipantsList/Participant/Participant.vue @@ -614,10 +614,6 @@ export default { return this.participant.attendeeId }, - label() { - return this.participant.label - }, - shareWithDisplayNameUnique() { return this.participant.shareWithDisplayNameUnique }, @@ -678,10 +674,6 @@ export default { return this.participantSpeakingInformation?.speaking }, - lastPing() { - return this.participant.lastPing - }, - attendeePin() { return this.canBeModerated && this.participant.attendeePin ? readableNumber(this.participant.attendeePin) : '' }, From 4784f6f424001d135dd08753db198141fd762814 Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Sun, 17 Dec 2023 18:31:49 +0100 Subject: [PATCH 4/4] fix(participants): handle deleting of participants, refactor, add tests Signed-off-by: Maksim Sukharev --- src/store/participantsStore.js | 48 ++++++++++++++++------------- src/store/participantsStore.spec.js | 33 ++++++++++++++++++++ 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/src/store/participantsStore.js b/src/store/participantsStore.js index 12d75753d47..b8e22728ac8 100644 --- a/src/store/participantsStore.js +++ b/src/store/participantsStore.js @@ -619,7 +619,15 @@ const actions = { const response = await request(token) const hasUserStatuses = !!response.headers['x-nextcloud-has-user-statuses'] - response.data.ocs.data.forEach(participant => { + const currentParticipants = context.state.attendees[token] + const newParticipants = response.data.ocs.data + for (const attendeeId of Object.keys(Object(currentParticipants))) { + if (!newParticipants.some(participant => participant.attendeeId === +attendeeId)) { + context.commit('deleteParticipant', { token, attendeeId }) + } + } + + newParticipants.forEach(participant => { if (context.state.attendees[token]?.[participant.attendeeId]) { context.dispatch('updateParticipantIfHasChanged', { token, participant, hasUserStatuses }) } else { @@ -668,31 +676,27 @@ const actions = { const { attendeeId } = participant const oldParticipant = context.state.attendees[token][attendeeId] - // Update, if status has changed - if (hasUserStatuses && (oldParticipant.status !== participant.status - || oldParticipant.statusMessage !== participant.statusMessage - || oldParticipant.statusIcon !== participant.statusIcon - || oldParticipant.statusClearAt !== participant.statusClearAt - )) { - context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) - emitUserStatusUpdated(participant) - return true + // Check if any property has changed + const changedEntries = Object.entries(participant).filter(([key, value]) => { + // "sessionIds" is the only property with non-primitive (array) value and cannot be compared by === + return key === 'sessionIds' + ? JSON.stringify(oldParticipant[key]) !== JSON.stringify(value) + : oldParticipant[key] !== value + }) + + if (changedEntries.length === 0) { + return false } - // Check if any property were changed (no properties except status-related supposed to be added or deleted) - for (const key of Object.keys(participant)) { - // "sessionIds" is the only property with non-primitive (array) value and cannot be compared by === - if (key !== 'sessionIds' && oldParticipant[key] !== participant[key]) { - context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) - return true - } else if (key === 'sessionIds' && (oldParticipant.sessionIds.length !== participant.sessionIds.length - || !oldParticipant.sessionIds.every((element, index) => element === participant.sessionIds[index]))) { - context.commit('updateParticipant', { token, attendeeId, updatedData: participant }) - return true - } + const updatedData = Object.fromEntries(changedEntries) + context.commit('updateParticipant', { token, attendeeId, updatedData }) + + // check if status-related properties have been changed + if (hasUserStatuses && changedEntries.some(([key]) => key.startsWith('status'))) { + emitUserStatusUpdated(participant) } - return false + return true }, /** diff --git a/src/store/participantsStore.spec.js b/src/store/participantsStore.spec.js index 670123d4d4e..08d1a1dd16c 100644 --- a/src/store/participantsStore.spec.js +++ b/src/store/participantsStore.spec.js @@ -399,6 +399,39 @@ describe('participantsStore', () => { expect(store.getters.participantsList(TOKEN)).toMatchObject(payload) }) + test('populates store for the fetched conversation', async () => { + // Arrange + const payloadFirst = [ + { attendeeId: 1, actorType: 'users', sessionIds: ['session-id-1'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, // delete + { attendeeId: 2, actorType: 'users', sessionIds: ['session-id-2-1', 'session-id-2-2'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, + { attendeeId: 3, actorType: 'users', sessionIds: ['session-id-3'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, + { attendeeId: 4, actorType: 'users', sessionIds: ['session-id-4'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, + ] + const payloadSecond = [ + { attendeeId: 2, actorType: 'users', sessionIds: ['session-id-2-2', 'session-id-2-1'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, // change + { attendeeId: 3, actorType: 'users', sessionIds: ['session-id-3'], inCall: PARTICIPANT.CALL_FLAG.IN_CALL }, // change + { attendeeId: 4, actorType: 'users', sessionIds: ['session-id-4'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED, status: 'online' }, // change + { attendeeId: 5, actorType: 'guests', sessionIds: ['session-id-5'], inCall: PARTICIPANT.CALL_FLAG.DISCONNECTED }, // add + ] + fetchParticipants + .mockResolvedValueOnce(generateOCSResponse({ + headers: { 'x-nextcloud-has-user-statuses': true }, + payload: payloadFirst, + })) + .mockResolvedValueOnce(generateOCSResponse({ + headers: { 'x-nextcloud-has-user-statuses': true }, + payload: payloadSecond, + })) + + // Act + await store.dispatch('fetchParticipants', { token: TOKEN }) + await store.dispatch('fetchParticipants', { token: TOKEN }) + + // Assert + expect(emit).toHaveBeenCalledTimes(5) // 4 added users and 1 status changed + expect(store.getters.participantsList(TOKEN)).toMatchObject(payloadSecond) + }) + test('saves a guest name from response', async () => { // Arrange const payload = [{