LeadMagnet/leadchat
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

fix: Update the logic to handle attachments in a conversation (#9784)

Browse Source 
When the chat is viewed, a function `fetchAllAttachments` is run to get all attachments for a particular conversation. This function, before updating the record creates the `attachments` property on the `chat` object in the store.
If in any case this function fails, the `attachments` property is not created, and when the code reaches the `dashboard/store/modules/conversations/index.js` the error occurs

This PR fixes it by ensuring that `SET_ALL_ATTACHMENTS` is always run. And it handles the default case as well


---

Sentry Issue:
[CHATWOOT-FRONTEND-APP-5Y](https://chatwoot-p3.sentry.io/issues/5459056982/)

```
TypeError: Cannot read properties of undefined (reading 'some')
  at forEach(./app/javascript/dashboard/store/modules/conversations/index.js:160:31)
  at Array.forEach(<anonymous>)
  at mutations(./app/javascript/dashboard/store/modules/conversations/index.js:159:27)
  at handler(./node_modules/vuex/dist/vuex.js:771:7)
  at forEach(./node_modules/vuex/dist/vuex.js:470:9)
  at Array.forEach(<anonymous>)
  at fn(./node_modules/vuex/dist/vuex.js:469:13)
  at Store.prototype._withCommit(./node_modules/vuex/dist/vuex.js:574:5)
  at Store.prototype.commit(./node_modules/vuex/dist/vuex.js:468:10)
  at this.commit(./node_modules/vuex/dist/vuex.js:420:21)
  at call(./app/javascript/dashboard/store/modules/conversations/actions.js:273:7)
  at tryCatch(./node_modules/videojs-record/dist/videojs.record.js:2868:27)
  at _invoke(./node_modules/videojs-record/dist/videojs.record.js:3088:32)
  at prototype[method](./node_modules/videojs-record/dist/videojs.record.js:2921:31)
  at as(/packs/js/application-cf716bca3c984faeb095.js:4:76)
  at as(/packs/js/application-cf716bca3c984faeb095.js:4:76)
  at nrWrapper(/app/accounts/81898/conversations/95:6:17817)
```

---------

Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com>
Co-authored-by: Sojan Jose <sojan@pepalo.com>
Co-authored-by: Pranav <pranav@chatwoot.com>
This commit is contained in:
Shivam Mishra
2024-08-07 06:43:41 +05:30
committed by GitHub
parent e330c2f6c0
commit e393bcf125
5 changed files with 84 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
app/javascript/dashboard/store/modules/conversations/actions.js
View File

@@ -12,6 +12,7 @@ import {
} from './helpers/actionHelpers'; } from './helpers/actionHelpers';
import messageReadActions from './actions/messageReadActions'; import messageReadActions from './actions/messageReadActions';
import messageTranslateActions from './actions/messageTranslateActions'; import messageTranslateActions from './actions/messageTranslateActions';
import * as Sentry from '@sentry/browser';
export const hasMessageFailedWithExternalError = pendingMessage => { export const hasMessageFailedWithExternalError = pendingMessage => {
// This helper is used to check if the message has failed with an external error. // This helper is used to check if the message has failed with an external error.
@@ -100,14 +101,24 @@ const actions = {
}, },
fetchAllAttachments: async ({ commit }, conversationId) => { fetchAllAttachments: async ({ commit }, conversationId) => {
let attachments = null;
try { try {
const { data } = await ConversationApi.getAllAttachments(conversationId); const { data } = await ConversationApi.getAllAttachments(conversationId);
attachments = data.payload;
} catch (error) {
// in case of error, log the error and continue
Sentry.setContext('Conversation', {
id: conversationId,
});
Sentry.captureException(error);
} finally {
// we run the commit even if the request fails
// this ensures that the `attachment` variable is always present on chat
commit(types.SET_ALL_ATTACHMENTS, { commit(types.SET_ALL_ATTACHMENTS, {
id: conversationId, id: conversationId,
data: data.payload, data: attachments,
}); });
} catch (error) {
// Handle error
} }
}, },

5
app/javascript/dashboard/store/modules/conversations/getters.js
View File

@@ -18,9 +18,8 @@ const getters = {
); );
return selectedChat || {}; return selectedChat || {};
}, },
getSelectedChatAttachments: (_state, _getters) => { getSelectedChatAttachments: ({ selectedChatId, attachments }) => {
const selectedChat = _getters.getSelectedChat; return attachments[selectedChatId] || [];
return selectedChat.attachments || [];
}, },
getChatListFilters: ({ conversationFilters }) => conversationFilters, getChatListFilters: ({ conversationFilters }) => conversationFilters,
getLastEmailInSelectedChat: (stage, _getters) => { getLastEmailInSelectedChat: (stage, _getters) => {

67
app/javascript/dashboard/store/modules/conversations/index.js
View File

@@ -10,6 +10,7 @@ import { emitter } from 'shared/helpers/mitt';
const state = { const state = {
allConversations: [], allConversations: [],
attachments: {},
listLoadingStatus: true, listLoadingStatus: true,
chatStatusFilter: wootConstants.STATUS_TYPE.OPEN, chatStatusFilter: wootConstants.STATUS_TYPE.OPEN,
chatSortFilter: wootConstants.SORT_BY_TYPE.LATEST, chatSortFilter: wootConstants.SORT_BY_TYPE.LATEST,
@@ -48,7 +49,6 @@ export const mutations = {
allMessagesLoaded: existingConversation.allMessagesLoaded, allMessagesLoaded: existingConversation.allMessagesLoaded,
messages: existingConversation.messages, messages: existingConversation.messages,
dataFetched: existingConversation.dataFetched, dataFetched: existingConversation.dataFetched,
attachments: existingConversation.attachments,
}; };
} }
}); });
@@ -78,10 +78,10 @@ export const mutations = {
} }
}, },
[types.SET_ALL_ATTACHMENTS](_state, { id, data }) { [types.SET_ALL_ATTACHMENTS](_state, { id, data }) {
const [chat] = _state.allConversations.filter(c => c.id === id); const attachments = _state.attachments[id] || [];
if (!chat) return;
Vue.set(chat, 'attachments', []); attachments.push(...data);
chat.attachments.push(...data); _state.attachments[id] = [...attachments];
}, },
[types.SET_MISSING_MESSAGES](_state, { id, data }) { [types.SET_MISSING_MESSAGES](_state, { id, data }) {
const [chat] = _state.allConversations.filter(c => c.id === id); const [chat] = _state.allConversations.filter(c => c.id === id);
@@ -144,42 +144,37 @@ export const mutations = {
Vue.set(chat, 'muted', false); Vue.set(chat, 'muted', false);
}, },
[types.ADD_CONVERSATION_ATTACHMENTS]({ allConversations }, message) { [types.ADD_CONVERSATION_ATTACHMENTS](_state, message) {
const { conversation_id: conversationId } = message; // early return if the message has not been sent, or has no attachments
const [chat] = getSelectedChatConversation({ if (message.status !== MESSAGE_STATUS.SENT || !message.attachments.length) {
allConversations, return;
selectedChatId: conversationId, }
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; // replace the attachments in the store
_state.attachments[id] = [...existingAttachments, ...attachmentsToAdd];
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);
}
});
}
}, },
[types.DELETE_CONVERSATION_ATTACHMENTS]({ allConversations }, message) { [types.DELETE_CONVERSATION_ATTACHMENTS](_state, message) {
const { conversation_id: conversationId } = message; if (message.status !== MESSAGE_STATUS.SENT) return;
const [chat] = getSelectedChatConversation({
allConversations, const { conversation_id: id } = message;
selectedChatId: conversationId, 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) { [types.ADD_MESSAGE]({ allConversations, selectedChatId }, message) {

28
app/javascript/dashboard/store/modules/specs/conversations/getters.spec.js
View File

@@ -245,30 +245,18 @@ describe('#getters', () => {
describe('#getSelectedChatAttachments', () => { describe('#getSelectedChatAttachments', () => {
it('Returns attachments in selected chat', () => { it('Returns attachments in selected chat', () => {
const state = {}; const attachments = {
const getSelectedChat = { 1: [
attachments: [ { id: 1, file_name: 'test1' },
{ { id: 2, file_name: 'test2' },
id: 1,
file_name: 'test1',
},
{
id: 2,
file_name: 'test2',
},
], ],
}; };
const selectedChatId = 1;
expect( expect(
getters.getSelectedChatAttachments(state, { getSelectedChat }) getters.getSelectedChatAttachments({ selectedChatId, attachments })
).toEqual([ ).toEqual([
{ { id: 1, file_name: 'test1' },
id: 1, { id: 2, file_name: 'test2' },
file_name: 'test1',
},
{
id: 2,
file_name: 'test2',
},
]); ]);
}); });
}); });

54
app/javascript/dashboard/store/modules/specs/conversations/mutations.spec.js
View File

@@ -313,7 +313,6 @@ describe('#mutations', () => {
{ {
id: 1, id: 1,
messages: [{ id: 1, content: 'test' }], messages: [{ id: 1, content: 'test' }],
attachments: [{ id: 1, name: 'test1.png' }],
dataFetched: true, dataFetched: true,
allMessagesLoaded: true, allMessagesLoaded: true,
}, },
@@ -325,7 +324,6 @@ describe('#mutations', () => {
id: 1, id: 1,
name: 'test', name: 'test',
messages: [{ id: 1, content: 'updated message' }], messages: [{ id: 1, content: 'updated message' }],
attachments: [{ id: 1, name: 'test.png' }],
dataFetched: true, dataFetched: true,
allMessagesLoaded: true, allMessagesLoaded: true,
}, },
@@ -335,7 +333,6 @@ describe('#mutations', () => {
id: 1, id: 1,
name: 'test', name: 'test',
messages: [{ id: 1, content: 'test' }], messages: [{ id: 1, content: 'test' }],
attachments: [{ id: 1, name: 'test1.png' }],
dataFetched: true, dataFetched: true,
allMessagesLoaded: true, allMessagesLoaded: true,
}, },
@@ -371,25 +368,28 @@ describe('#mutations', () => {
it('set all attachments', () => { it('set all attachments', () => {
const state = { const state = {
allConversations: [{ id: 1 }], allConversations: [{ id: 1 }],
attachments: {},
}; };
const data = [{ id: 1, name: 'test' }]; const data = [{ id: 1, name: 'test' }];
mutations[types.SET_ALL_ATTACHMENTS](state, { id: 1, data }); 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', () => { it('set attachments key even if the attachments are empty', () => {
const state = { const state = {
allConversations: [{ id: 1 }], allConversations: [{ id: 1 }],
attachments: {},
}; };
const data = []; const data = [];
mutations[types.SET_ALL_ATTACHMENTS](state, { id: 1, 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', () => { describe('#ADD_CONVERSATION_ATTACHMENTS', () => {
it('add conversation attachments', () => { it('add conversation attachments', () => {
const state = { const state = {
allConversations: [{ id: 1, attachments: [] }], allConversations: [{ id: 1 }],
attachments: {},
}; };
const message = { const message = {
conversation_id: 1, conversation_id: 1,
@@ -398,19 +398,13 @@ describe('#mutations', () => {
}; };
mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message);
expect(state.allConversations[0].attachments).toEqual( expect(state.attachments[1]).toEqual(message.attachments);
message.attachments
);
}); });
it('should not add duplicate attachments', () => { it('should not add duplicate attachments', () => {
const state = { const state = {
allConversations: [ allConversations: [{ id: 1 }],
{ attachments: { 1: [{ id: 1, name: 'existing' }] },
id: 1,
attachments: [{ id: 1, name: 'existing' }],
},
],
}; };
const message = { const message = {
conversation_id: 1, conversation_id: 1,
@@ -422,12 +416,12 @@ describe('#mutations', () => {
}; };
mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message);
expect(state.allConversations[0].attachments).toHaveLength(2); expect(state.attachments[1]).toHaveLength(2);
expect(state.allConversations[0].attachments).toContainEqual({ expect(state.attachments[1]).toContainEqual({
id: 1, id: 1,
name: 'existing', name: 'existing',
}); });
expect(state.allConversations[0].attachments).toContainEqual({ expect(state.attachments[1]).toContainEqual({
id: 2, id: 2,
name: 'new', name: 'new',
}); });
@@ -436,6 +430,9 @@ describe('#mutations', () => {
it('should not add attachments if chat not found', () => { it('should not add attachments if chat not found', () => {
const state = { const state = {
allConversations: [{ id: 1, attachments: [] }], allConversations: [{ id: 1, attachments: [] }],
attachments: {
1: [],
},
}; };
const message = { const message = {
conversation_id: 2, conversation_id: 2,
@@ -444,14 +441,17 @@ describe('#mutations', () => {
}; };
mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message); mutations[types.ADD_CONVERSATION_ATTACHMENTS](state, message);
expect(state.allConversations[0].attachments).toHaveLength(0); expect(state.attachments[1]).toHaveLength(0);
}); });
}); });
describe('#DELETE_CONVERSATION_ATTACHMENTS', () => { describe('#DELETE_CONVERSATION_ATTACHMENTS', () => {
it('delete conversation attachments', () => { it('delete conversation attachments', () => {
const state = { const state = {
allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], allConversations: [{ id: 1 }],
attachments: {
1: [{ id: 1, message_id: 1 }],
},
}; };
const message = { const message = {
conversation_id: 1, conversation_id: 1,
@@ -460,12 +460,15 @@ describe('#mutations', () => {
}; };
mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); 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', () => { it('should not delete attachments for non-matching message id', () => {
const state = { const state = {
allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], allConversations: [{ id: 1 }],
attachments: {
1: [{ id: 1, message_id: 1 }],
},
}; };
const message = { const message = {
conversation_id: 1, conversation_id: 1,
@@ -474,12 +477,13 @@ describe('#mutations', () => {
}; };
mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); 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', () => { it('should not delete attachments if chat not found', () => {
const state = { const state = {
allConversations: [{ id: 1, attachments: [{ id: 1, message_id: 1 }] }], allConversations: [{ id: 1 }],
attachments: { 1: [{ id: 1, message_id: 1 }] },
}; };
const message = { const message = {
conversation_id: 2, conversation_id: 2,
@@ -488,7 +492,7 @@ describe('#mutations', () => {
}; };
mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message); mutations[types.DELETE_CONVERSATION_ATTACHMENTS](state, message);
expect(state.allConversations[0].attachments).toHaveLength(1); expect(state.attachments[1]).toHaveLength(1);
}); });
}); });