From 0cdf9d86f4320ec70c9b84813dc6b5cc2babcebf Mon Sep 17 00:00:00 2001 From: Maksim Sukharev Date: Mon, 4 Mar 2024 16:51:09 +0100 Subject: [PATCH] fix(conversation): reuse read/unread server response for store update Signed-off-by: Maksim Sukharev --- .../MessageButtonsBar.spec.js | 4 - .../MessageButtonsBar/MessageButtonsBar.vue | 3 - src/services/messagesService.js | 2 +- src/store/conversationsStore.js | 13 +-- src/store/conversationsStore.spec.js | 34 ++++++-- src/store/messagesStore.js | 12 +-- src/store/messagesStore.spec.js | 79 +++++++++++-------- 7 files changed, 82 insertions(+), 65 deletions(-) diff --git a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.spec.js b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.spec.js index 279f414d0335..b5b20e63ba39 100644 --- a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.spec.js +++ b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.spec.js @@ -378,10 +378,6 @@ describe('MessageButtonsBar.vue', () => { id: 100, updateVisually: true, }) - - expect(fetchConversationAction).toHaveBeenCalledWith(expect.anything(), { - token: TOKEN, - }) }) test('copies message link', async () => { diff --git a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue index bb3a1fb6d56d..2cc4744ae091 100644 --- a/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue +++ b/src/components/MessagesList/MessagesGroup/Message/MessageButtonsBar/MessageButtonsBar.vue @@ -722,9 +722,6 @@ export default { id: this.previousMessageId, updateVisually: true, }) - - // reload conversation to update additional attributes that have computed values - await this.$store.dispatch('fetchConversation', { token: this.token }) }, handleReactionClick(selectedEmoji) { diff --git a/src/services/messagesService.js b/src/services/messagesService.js index c86fadaf720e..93d7cfab3084 100644 --- a/src/services/messagesService.js +++ b/src/services/messagesService.js @@ -170,7 +170,7 @@ const postRichObjectToConversation = async function(token, { objectType, objectI /** * Updates the last read message id * - * @param {string} token The token of the conversation to be removed from favorites + * @param {string} token The token of the conversation * @param {number} lastReadMessage id of the last read message to set * @param {object} options request options */ diff --git a/src/store/conversationsStore.js b/src/store/conversationsStore.js index 2449da768c10..6c0984cda800 100644 --- a/src/store/conversationsStore.js +++ b/src/store/conversationsStore.js @@ -647,23 +647,14 @@ const actions = { commit('addConversation', conversation) }, - async markConversationRead({ commit, getters }, token) { - if (!getters.conversations[token]) { - return - } - - commit('updateUnreadMessages', { token, unreadMessages: 0, unreadMention: false, unreadMentionDirect: false }) - }, - async markConversationUnread({ commit, dispatch, getters }, { token }) { if (!getters.conversations[token]) { return } try { - await setConversationUnread(token) - commit('updateUnreadMessages', { token, unreadMessages: 1 }) - await dispatch('fetchConversation', { token }) + const response = await setConversationUnread(token) + dispatch('addConversation', response.data.ocs.data) } catch (error) { console.error('Error while setting the conversation as unread: ', error) } diff --git a/src/store/conversationsStore.spec.js b/src/store/conversationsStore.spec.js index 3b933bd2dff9..5e8d291661c8 100644 --- a/src/store/conversationsStore.spec.js +++ b/src/store/conversationsStore.spec.js @@ -34,6 +34,7 @@ import { setCallPermissions, setConversationUnread, } from '../services/conversationsService.js' +import { updateLastReadMessage } from '../services/messagesService.js' import { useTalkHashStore } from '../stores/talkHash.js' import { generateOCSErrorResponse, generateOCSResponse } from '../test-helpers.js' @@ -58,6 +59,10 @@ jest.mock('../services/conversationsService', () => ({ setConversationUnread: jest.fn(), })) +jest.mock('../services/messagesService', () => ({ + updateLastReadMessage: jest.fn(), +})) + jest.mock('@nextcloud/event-bus') jest.mock('../services/BrowserStorage.js', () => ({ @@ -99,6 +104,7 @@ describe('conversationsStore', () => { actorId: 'actor-id', defaultPermissions: PARTICIPANT.PERMISSIONS.CUSTOM, callPermissions: PARTICIPANT.PERMISSIONS.CUSTOM, + lastMessage: { ...previousLastMessage }, } testStoreConfig = cloneDeep(storeConfig) @@ -914,34 +920,48 @@ describe('conversationsStore', () => { describe('read marker', () => { beforeEach(() => { store = new Vuex.Store(testStoreConfig) + store.commit('setUserId', 'current-user') }) - test('marks conversation as read by clearing unread counters', () => { + test('marks conversation as read by clearing unread counters', async () => { + // Arrange testConversation.unreadMessages = 1024 testConversation.unreadMention = true - store.dispatch('addConversation', testConversation) - store.dispatch('markConversationRead', testToken) + const response = generateOCSResponse({ + payload: { + ...testConversation, + unreadMessages: 0, + unreadMention: false, + } + }) + updateLastReadMessage.mockResolvedValue(response) + // Act + store.dispatch('clearLastReadMessage', { token: testToken }) + await flushPromises() + + // Assert const changedConversation = store.getters.conversation(testToken) expect(changedConversation.unreadMessages).toBe(0) expect(changedConversation.unreadMention).toBe(false) }) test('marks conversation as unread', async () => { + // Arrange testConversation.unreadMessages = 0 store.dispatch('addConversation', testConversation) const response = generateOCSResponse({ payload: { ...testConversation, unreadMessages: 1 } }) - fetchConversation.mockResolvedValue(response) - store.dispatch('markConversationUnread', { token: testToken }) + setConversationUnread.mockResolvedValue(response) + // Act + store.dispatch('markConversationUnread', { token: testToken }) await flushPromises() + // Assert expect(setConversationUnread).toHaveBeenCalledWith(testConversation.token) - expect(fetchConversation).toHaveBeenCalledWith(testConversation.token) - const changedConversation = store.getters.conversation(testToken) expect(changedConversation.unreadMessages).toBe(1) }) diff --git a/src/store/messagesStore.js b/src/store/messagesStore.js index 06c810a161f3..f4739394e3e4 100644 --- a/src/store/messagesStore.js +++ b/src/store/messagesStore.js @@ -821,13 +821,12 @@ const actions = { * @param {boolean} data.updateVisually whether to also clear the marker visually in the UI; */ async clearLastReadMessage(context, { token, updateVisually = false }) { - const conversation = context.getters.conversations[token] - if (!conversation || !conversation.lastMessage) { + const conversation = context.getters.conversation(token) + if (!conversation?.lastMessage) { return } // set the id to the last message context.dispatch('updateLastReadMessage', { token, id: conversation.lastMessage.id, updateVisually }) - context.dispatch('markConversationRead', token) }, /** @@ -836,12 +835,12 @@ const actions = { * * @param {object} context default store context; * @param {object} data the wrapping object; - * @param {object} data.token the token of the conversation to be updated; + * @param {string} data.token the token of the conversation to be updated; * @param {number} data.id the id of the message on which to set the read marker; * @param {boolean} data.updateVisually whether to also update the marker visually in the UI; */ async updateLastReadMessage(context, { token, id = 0, updateVisually = false }) { - const conversation = context.getters.conversations[token] + const conversation = context.getters.conversation(token) if (!conversation || conversation.lastReadMessage === id) { return } @@ -858,7 +857,8 @@ const actions = { if (context.getters.getUserId()) { // only update on server side if there's an actual user, not guest - await updateLastReadMessage(token, id) + const response = await updateLastReadMessage(token, id) + context.dispatch('addConversation', response.data.ocs.data) } }, diff --git a/src/store/messagesStore.spec.js b/src/store/messagesStore.spec.js index 644dd500ef68..75f593f4f54c 100644 --- a/src/store/messagesStore.spec.js +++ b/src/store/messagesStore.spec.js @@ -1,6 +1,5 @@ import { createLocalVue } from '@vue/test-utils' import flushPromises from 'flush-promises' -import mockConsole from 'jest-mock-console' import { cloneDeep } from 'lodash' import { createPinia, setActivePinia } from 'pinia' import Vuex from 'vuex' @@ -611,14 +610,12 @@ describe('messagesStore', () => { }) describe('last read message markers', () => { - let conversationsMock - let markConversationReadAction + let conversationMock let getUserIdMock let updateConversationLastReadMessageMock beforeEach(() => { - const conversations = {} - conversations[TOKEN] = { + const conversation = { lastMessage: { id: 123, }, @@ -627,15 +624,15 @@ describe('messagesStore', () => { testStoreConfig = cloneDeep(messagesStore) getUserIdMock = jest.fn() - conversationsMock = jest.fn().mockReturnValue(conversations) - markConversationReadAction = jest.fn() + conversationMock = jest.fn().mockReturnValue(conversation) updateConversationLastReadMessageMock = jest.fn() - testStoreConfig.getters.conversations = conversationsMock + testStoreConfig.getters.conversation = jest.fn().mockReturnValue(conversationMock) testStoreConfig.getters.getUserId = jest.fn().mockReturnValue(getUserIdMock) - testStoreConfig.actions.markConversationRead = markConversationReadAction testStoreConfig.actions.updateConversationLastReadMessage = updateConversationLastReadMessageMock + testStoreConfig.actions.addConversation = jest.fn() - updateLastReadMessage.mockResolvedValueOnce() + const response = generateOCSResponse({ payload: conversation }) + updateLastReadMessage.mockResolvedValue(response) store = new Vuex.Store(testStoreConfig) }) @@ -657,8 +654,7 @@ describe('messagesStore', () => { updateVisually: false, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).toHaveBeenCalledWith(expect.anything(), TOKEN) + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -678,8 +674,7 @@ describe('messagesStore', () => { updateVisually: true, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).toHaveBeenCalledWith(expect.anything(), TOKEN) + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -699,8 +694,7 @@ describe('messagesStore', () => { updateVisually: true, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).toHaveBeenCalledWith(expect.anything(), TOKEN) + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -713,6 +707,13 @@ describe('messagesStore', () => { test('updates last read message', async () => { getUserIdMock.mockReturnValue('user-1') + const response = generateOCSResponse({ + payload: { + unreadMessages: 0, + unreadMention: false, + } + }) + updateLastReadMessage.mockResolvedValue(response) store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 }) await store.dispatch('updateLastReadMessage', { @@ -721,8 +722,7 @@ describe('messagesStore', () => { updateVisually: false, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).not.toHaveBeenCalled() + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -735,6 +735,13 @@ describe('messagesStore', () => { test('updates last read message and update visually', async () => { getUserIdMock.mockReturnValue('user-1') + const response = generateOCSResponse({ + payload: { + unreadMessages: 0, + unreadMention: false, + } + }) + updateLastReadMessage.mockResolvedValue(response) store.dispatch('setVisualLastReadMessageId', { token: TOKEN, id: 100 }) await store.dispatch('updateLastReadMessage', { @@ -743,8 +750,7 @@ describe('messagesStore', () => { updateVisually: true, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).not.toHaveBeenCalled() + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -765,8 +771,7 @@ describe('messagesStore', () => { updateVisually: true, }) - expect(conversationsMock).toHaveBeenCalled() - expect(markConversationReadAction).not.toHaveBeenCalled() + expect(conversationMock).toHaveBeenCalled() expect(getUserIdMock).toHaveBeenCalled() expect(updateConversationLastReadMessageMock).toHaveBeenCalledWith(expect.anything(), { token: TOKEN, @@ -1538,28 +1543,30 @@ describe('messagesStore', () => { describe('posting new message', () => { let message1 let conversationMock + let getUserIdMock let updateLastCommonReadMessageAction - let updateLastReadMessageAction let updateConversationLastMessageAction let cancelFunctionMocks - let restoreConsole beforeEach(() => { testStoreConfig = cloneDeep(messagesStore) jest.useFakeTimers() - restoreConsole = mockConsole(['error']) + console.error = jest.fn() + conversationMock = jest.fn() + getUserIdMock = jest.fn() updateConversationLastMessageAction = jest.fn() updateLastCommonReadMessageAction = jest.fn() - updateLastReadMessageAction = jest.fn() testStoreConfig.getters.conversation = jest.fn().mockReturnValue(conversationMock) + testStoreConfig.getters.getUserId = jest.fn().mockReturnValue(getUserIdMock) testStoreConfig.actions.updateConversationLastMessage = updateConversationLastMessageAction testStoreConfig.actions.updateLastCommonReadMessage = updateLastCommonReadMessageAction // mock this complex local action as we already tested it elsewhere - testStoreConfig.actions.updateLastReadMessage = updateLastReadMessageAction testStoreConfig.actions.updateConversationLastActive = updateConversationLastActiveAction + testStoreConfig.actions.updateConversationLastReadMessage = jest.fn() + testStoreConfig.actions.addConversation = jest.fn() cancelFunctionMocks = [] CancelableRequest.mockImplementation((request) => { @@ -1582,7 +1589,7 @@ describe('messagesStore', () => { }) afterEach(() => { - restoreConsole() + jest.clearAllMocks() }) test('posts new message', async () => { @@ -1591,6 +1598,7 @@ describe('messagesStore', () => { lastMessage: { id: 100 }, lastReadMessage: 50, }) + getUserIdMock.mockReturnValue('current-user') const temporaryMessage = { id: 'temp-123', @@ -1613,6 +1621,13 @@ describe('messagesStore', () => { }) postNewMessage.mockResolvedValueOnce(response) + const response2 = generateOCSResponse({ + payload: { + unreadMessages: 0, + unreadMention: false, + } + }) + updateLastReadMessage.mockResolvedValue(response2) store.dispatch('postNewMessage', { token: TOKEN, temporaryMessage, options: { silent: false } }).catch(() => { }) expect(postNewMessage).toHaveBeenCalledWith(temporaryMessage, { silent: false }) @@ -1631,11 +1646,7 @@ describe('messagesStore', () => { expect(updateConversationLastMessageAction) .toHaveBeenCalledWith(expect.anything(), { token: TOKEN, lastMessage: messageResponse }) - expect(updateLastReadMessageAction).toHaveBeenCalledWith(expect.anything(), { - token: TOKEN, - id: 200, - updateVisually: true, - }) + expect(updateLastReadMessage).toHaveBeenCalledWith(TOKEN, 200) }) test('cancels posting new messages individually', () => { @@ -1690,6 +1701,8 @@ describe('messagesStore', () => { status: statusCode, } + console.error = jest.fn() + postNewMessage.mockRejectedValueOnce({ isAxiosError: true, response }) await expect( store.dispatch('postNewMessage', { token: TOKEN, temporaryMessage, options: { silent: false } })