From 28fc95ea23cce8f55144827d33d089037b3824be Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 24 Dec 2024 11:26:38 +0530 Subject: [PATCH] fix: attachment initalization issues --- .../store/modules/conversations/getters.js | 5 +- .../store/modules/conversations/index.js | 69 +++++++++---------- .../specs/conversations/getters.spec.js | 28 +++----- .../specs/conversations/mutations.spec.js | 54 ++++++++------- 4 files changed, 73 insertions(+), 83 deletions(-) diff --git a/app/javascript/dashboard/store/modules/conversations/getters.js b/app/javascript/dashboard/store/modules/conversations/getters.js index 8c2070963..1a905c3c5 100644 --- a/app/javascript/dashboard/store/modules/conversations/getters.js +++ b/app/javascript/dashboard/store/modules/conversations/getters.js @@ -18,9 +18,8 @@ const getters = { ); return selectedChat || {}; }, - getSelectedChatAttachments: (_state, _getters) => { - const selectedChat = _getters.getSelectedChat; - return selectedChat.attachments || []; + getSelectedChatAttachments: ({ selectedChatId, attachments }) => { + return attachments[selectedChatId] || []; }, getChatListFilters: ({ conversationFilters }) => conversationFilters, getLastEmailInSelectedChat: (stage, _getters) => { diff --git a/app/javascript/dashboard/store/modules/conversations/index.js b/app/javascript/dashboard/store/modules/conversations/index.js index 769354ed2..70dc18783 100644 --- a/app/javascript/dashboard/store/modules/conversations/index.js +++ b/app/javascript/dashboard/store/modules/conversations/index.js @@ -10,6 +10,7 @@ import { emitter } from 'shared/helpers/mitt'; const state = { allConversations: [], + attachments: {}, listLoadingStatus: true, chatStatusFilter: wootConstants.STATUS_TYPE.OPEN, chatSortFilter: wootConstants.SORT_BY_TYPE.LATEST, @@ -78,10 +79,10 @@ export const mutations = { } }, [types.SET_ALL_ATTACHMENTS](_state, { id, data }) { - const [chat] = _state.allConversations.filter(c => c.id === id); - if (!chat) return; - Vue.set(chat, 'attachments', []); - chat.attachments.push(...data); + const attachments = _state.attachments[id] || []; + + attachments.push(...data); + _state.attachments[id] = [...attachments]; }, [types.SET_MISSING_MESSAGES](_state, { id, data }) { const [chat] = _state.allConversations.filter(c => c.id === id); @@ -144,42 +145,40 @@ export const mutations = { Vue.set(chat, 'muted', false); }, - [types.ADD_CONVERSATION_ATTACHMENTS]({ allConversations }, message) { - const { conversation_id: conversationId } = message; - const [chat] = getSelectedChatConversation({ - allConversations, - selectedChatId: conversationId, + [types.ADD_CONVERSATION_ATTACHMENTS](_state, message) { + // early return if the message has not been sent, or has no attachments + if ( + message.status !== MESSAGE_STATUS.SENT || + !message.attachments?.length + ) { + return; + } + + const id = message.conversation_id; + const existingAttachments = _state.attachments[id] || []; + + const attachmentsToAdd = message.attachments.filter(attachment => { + // if the attachment is not already in the store, add it + // this is to prevent duplicates + return !existingAttachments.some( + existingAttachment => existingAttachment.id === attachment.id + ); }); - if (!chat) return; - - const isMessageSent = - message.status === MESSAGE_STATUS.SENT && message.attachments; - if (isMessageSent) { - message.attachments.forEach(attachment => { - if (!chat.attachments.some(a => a.id === attachment.id)) { - chat.attachments.push(attachment); - } - }); - } + // replace the attachments in the store + _state.attachments[id] = [...existingAttachments, ...attachmentsToAdd]; }, - [types.DELETE_CONVERSATION_ATTACHMENTS]({ allConversations }, message) { - const { conversation_id: conversationId } = message; - const [chat] = getSelectedChatConversation({ - allConversations, - selectedChatId: conversationId, + [types.DELETE_CONVERSATION_ATTACHMENTS](_state, message) { + if (message.status !== MESSAGE_STATUS.SENT) return; + + const { conversation_id: id } = message; + const existingAttachments = _state.attachments[id] || []; + if (!existingAttachments.length) return; + + _state.attachments[id] = existingAttachments.filter(attachment => { + return attachment.message_id !== message.id; }); - - if (!chat) return; - - const isMessageSent = message.status === MESSAGE_STATUS.SENT; - if (isMessageSent) { - const attachmentIndex = chat.attachments.findIndex( - a => a.message_id === message.id - ); - if (attachmentIndex !== -1) chat.attachments.splice(attachmentIndex, 1); - } }, [types.ADD_MESSAGE]({ allConversations, selectedChatId }, message) { diff --git a/app/javascript/dashboard/store/modules/specs/conversations/getters.spec.js b/app/javascript/dashboard/store/modules/specs/conversations/getters.spec.js index 3c3bf79de..cded29329 100644 --- a/app/javascript/dashboard/store/modules/specs/conversations/getters.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversations/getters.spec.js @@ -245,30 +245,18 @@ describe('#getters', () => { describe('#getSelectedChatAttachments', () => { it('Returns attachments in selected chat', () => { - const state = {}; - const getSelectedChat = { - attachments: [ - { - id: 1, - file_name: 'test1', - }, - { - id: 2, - file_name: 'test2', - }, + const attachments = { + 1: [ + { id: 1, file_name: 'test1' }, + { id: 2, file_name: 'test2' }, ], }; + const selectedChatId = 1; expect( - getters.getSelectedChatAttachments(state, { getSelectedChat }) + getters.getSelectedChatAttachments({ selectedChatId, attachments }) ).toEqual([ - { - id: 1, - file_name: 'test1', - }, - { - id: 2, - file_name: 'test2', - }, + { id: 1, file_name: 'test1' }, + { id: 2, file_name: 'test2' }, ]); }); }); diff --git a/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js b/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js index de596534c..6340e4de9 100644 --- a/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js +++ b/app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js @@ -313,7 +313,6 @@ describe('#mutations', () => { { id: 1, messages: [{ id: 1, content: 'test' }], - attachments: [{ id: 1, name: 'test1.png' }], dataFetched: true, allMessagesLoaded: true, }, @@ -325,7 +324,6 @@ describe('#mutations', () => { id: 1, name: 'test', messages: [{ id: 1, content: 'updated message' }], - attachments: [{ id: 1, name: 'test.png' }], dataFetched: true, allMessagesLoaded: true, }, @@ -335,7 +333,6 @@ describe('#mutations', () => { id: 1, name: 'test', messages: [{ id: 1, content: 'test' }], - attachments: [{ id: 1, name: 'test1.png' }], dataFetched: true, allMessagesLoaded: true, }, @@ -371,25 +368,28 @@ describe('#mutations', () => { it('set all attachments', () => { const state = { allConversations: [{ id: 1 }], + attachments: {}, }; const data = [{ id: 1, name: 'test' }]; mutations[types.SET_ALL_ATTACHMENTS](state, { id: 1, data }); - expect(state.allConversations[0].attachments).toEqual(data); + expect(state.attachments[1]).toEqual(data); }); it('set attachments key even if the attachments are empty', () => { const state = { allConversations: [{ id: 1 }], + attachments: {}, }; const data = []; mutations[types.SET_ALL_ATTACHMENTS](state, { id: 1, data }); - expect(state.allConversations[0].attachments).toEqual([]); + expect(state.attachments[1]).toEqual([]); }); }); describe('#ADD_CONVERSATION_ATTACHMENTS', () => { it('add conversation attachments', () => { const state = { - allConversations: [{ id: 1, attachments: [] }], + allConversations: [{ id: 1 }], + attachments: {}, }; const message = { conversation_id: 1, @@ -398,19 +398,13 @@ describe('#mutations', () => { }; mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toEqual( - message.attachments - ); + expect(state.attachments[1]).toEqual(message.attachments); }); it('should not add duplicate attachments', () => { const state = { - allConversations: [ - { - id: 1, - attachments: [{ id: 1, name: 'existing' }], - }, - ], + allConversations: [{ id: 1 }], + attachments: { 1: [{ id: 1, name: 'existing' }] }, }; const message = { conversation_id: 1, @@ -422,12 +416,12 @@ describe('#mutations', () => { }; mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toHaveLength(2); - expect(state.allConversations[0].attachments).toContainEqual({ + expect(state.attachments[1]).toHaveLength(2); + expect(state.attachments[1]).toContainEqual({ id: 1, name: 'existing', }); - expect(state.allConversations[0].attachments).toContainEqual({ + expect(state.attachments[1]).toContainEqual({ id: 2, name: 'new', }); @@ -436,6 +430,9 @@ describe('#mutations', () => { it('should not add attachments if chat not found', () => { const state = { allConversations: [{ id: 1, attachments: [] }], + attachments: { + 1: [], + }, }; const message = { conversation_id: 2, @@ -444,14 +441,17 @@ describe('#mutations', () => { }; mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toHaveLength(0); + expect(state.attachments[1]).toHaveLength(0); }); }); describe('#DELETE_CONVERSATION_ATTACHMENTS', () => { it('delete conversation attachments', () => { const state = { - allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], + allConversations: [{ id: 1 }], + attachments: { + 1: [{ id: 1, message_id: 1 }], + }, }; const message = { conversation_id: 1, @@ -460,12 +460,15 @@ describe('#mutations', () => { }; mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toHaveLength(0); + expect(state.attachments[1]).toHaveLength(0); }); it('should not delete attachments for non-matching message id', () => { const state = { - allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], + allConversations: [{ id: 1 }], + attachments: { + 1: [{ id: 1, message_id: 1 }], + }, }; const message = { conversation_id: 1, @@ -474,12 +477,13 @@ describe('#mutations', () => { }; mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toHaveLength(1); + expect(state.attachments[1]).toHaveLength(1); }); it('should not delete attachments if chat not found', () => { const state = { - allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], + allConversations: [{ id: 1 }], + attachments: { 1: [{ id: 1, message_id: 1 }] }, }; const message = { conversation_id: 2, @@ -488,7 +492,7 @@ describe('#mutations', () => { }; mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); - expect(state.allConversations[0].attachments).toHaveLength(1); + expect(state.attachments[1]).toHaveLength(1); }); });