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

fix: Prevent race condition in conversation dataFetched flag (#13492)

Browse Source 
Co-authored-by: Shivam Mishra <scm.mymail@gmail.com>
This commit is contained in:
Sivin Varghese
2026-02-10 15:23:14 +05:30
committed by GitHub
parent 6e397c7571
commit b252656984
5 changed files with 174 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -96,7 +96,7 @@ const actions = {
data: payload,
});
if (!payload.length) {
commit(types.SET_ALL_MESSAGES_LOADED);
commit(types.SET_ALL_MESSAGES_LOADED, data.conversationId);
}
} catch (error) {
// Handle error
@@ -191,7 +191,7 @@ const actions = {
async setActiveChat({ commit, dispatch }, { data, after }) {
commit(types.SET_CURRENT_CHAT_WINDOW, data);
commit(types.CLEAR_ALL_MESSAGES_LOADED);
commit(types.CLEAR_ALL_MESSAGES_LOADED, data.id);
if (data.dataFetched === undefined) {
try {
await dispatch('fetchPreviousMessages', {
@@ -199,7 +199,7 @@ const actions = {
before: data.messages[0].id,
conversationId: data.id,
});
data.dataFetched = true;
commit(types.SET_CHAT_DATA_FETCHED, data.id);
} catch (error) {
// Ignore error
}

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

@@ -63,14 +63,18 @@ export const mutations = {
_state.allConversations = [];
_state.selectedChatId = null;
},
[types.SET_ALL_MESSAGES_LOADED](_state) {
const [chat] = getSelectedChatConversation(_state);
chat.allMessagesLoaded = true;
[types.SET_ALL_MESSAGES_LOADED](_state, conversationId) {
const chat = getConversationById(_state)(conversationId);
if (chat) {
chat.allMessagesLoaded = true;
}
},
[types.CLEAR_ALL_MESSAGES_LOADED](_state) {
const [chat] = getSelectedChatConversation(_state);
chat.allMessagesLoaded = false;
[types.CLEAR_ALL_MESSAGES_LOADED](_state, conversationId) {
const chat = getConversationById(_state)(conversationId);
if (chat) {
chat.allMessagesLoaded = false;
}
},
[types.CLEAR_CURRENT_CHAT_WINDOW](_state) {
_state.selectedChatId = null;
@@ -91,6 +95,13 @@ export const mutations = {
chat.messages = data;
},
[types.SET_CHAT_DATA_FETCHED](_state, conversationId) {
const chat = getConversationById(_state)(conversationId);
if (chat) {
chat.dataFetched = true;
}
},
[types.SET_CURRENT_CHAT_WINDOW](_state, activeChat) {
if (activeChat) {
_state.selectedChatId = activeChat.id;

58
app/javascript/dashboard/store/modules/specs/conversations/actions.spec.js
View File

@@ -716,6 +716,64 @@ describe('#addMentions', () => {
});
});
describe('#setActiveChat', () => {
it('should commit SET_CHAT_DATA_FETCHED with conversation ID after fetch', async () => {
const localCommit = vi.fn();
const localDispatch = vi.fn().mockResolvedValue();
const data = { id: 42, messages: [{ id: 100 }] };
await actions.setActiveChat(
{ commit: localCommit, dispatch: localDispatch },
{ data, after: 99 }
);
expect(localCommit.mock.calls).toEqual([
[types.SET_CURRENT_CHAT_WINDOW, data],
[types.CLEAR_ALL_MESSAGES_LOADED, 42],
[types.SET_CHAT_DATA_FETCHED, 42],
]);
expect(localDispatch).toHaveBeenCalledWith('fetchPreviousMessages', {
after: 99,
before: 100,
conversationId: 42,
});
});
it('should not dispatch fetchPreviousMessages if dataFetched is already set', async () => {
const localCommit = vi.fn();
const localDispatch = vi.fn();
const data = { id: 42, messages: [{ id: 100 }], dataFetched: true };
await actions.setActiveChat(
{ commit: localCommit, dispatch: localDispatch },
{ data }
);
expect(localCommit.mock.calls).toEqual([
[types.SET_CURRENT_CHAT_WINDOW, data],
[types.CLEAR_ALL_MESSAGES_LOADED, 42],
]);
expect(localDispatch).not.toHaveBeenCalled();
});
it('should commit SET_CHAT_DATA_FETCHED by ID, not mutate the data object directly (race condition fix)', async () => {
const localCommit = vi.fn();
const localDispatch = vi.fn().mockResolvedValue();
const data = { id: 42, messages: [{ id: 100 }] };
await actions.setActiveChat(
{ commit: localCommit, dispatch: localDispatch },
{ data }
);
// The action must NOT set dataFetched on the data object directly
expect(data.dataFetched).toBeUndefined();
// Instead it commits a mutation that finds the conversation by ID in the store
expect(localCommit).toHaveBeenCalledWith(types.SET_CHAT_DATA_FETCHED, 42);
});
});
describe('#getInboxCaptainAssistantById', () => {
it('fetches inbox assistant by id', async () => {
axios.get.mockResolvedValue({

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

@@ -570,25 +570,84 @@ describe('#mutations', () => {
});
});
describe('#SET_ALL_MESSAGES_LOADED', () => {
it('should set allMessagesLoaded to true on selected chat', () => {
describe('#SET_CHAT_DATA_FETCHED', () => {
it('should set dataFetched to true on the conversation by ID', () => {
const state = {
allConversations: [{ id: 1, allMessagesLoaded: false }],
allConversations: [{ id: 1 }, { id: 2 }],
};
mutations[types.SET_CHAT_DATA_FETCHED](state, 1);
expect(state.allConversations[0].dataFetched).toBe(true);
expect(state.allConversations[1].dataFetched).toBeUndefined();
});
it('should do nothing if conversation is not found', () => {
const state = { allConversations: [{ id: 1 }] };
mutations[types.SET_CHAT_DATA_FETCHED](state, 999);
expect(state.allConversations[0].dataFetched).toBeUndefined();
});
it('should survive the race: SET_ALL_CONVERSATION replaces the object, then SET_CHAT_DATA_FETCHED still works', () => {
// 1. Initial state: conversation exists with dataFetched undefined
const state = {
allConversations: [{ id: 1, messages: [{ id: 'm1' }] }],
selectedChatId: 1,
};
mutations[types.SET_ALL_MESSAGES_LOADED](state);
const originalRef = state.allConversations[0];
// 2. Simulate SET_ALL_CONVERSATION replacing the object (WebSocket/polling)
// This copies dataFetched from the old object (still undefined)
mutations[types.SET_ALL_CONVERSATION](state, [
{ id: 1, name: 'refreshed', messages: [{ id: 'm2' }] },
]);
// The store now holds a NEW object, old reference is detached
const newRef = state.allConversations[0];
expect(newRef).not.toBe(originalRef);
expect(newRef.dataFetched).toBeUndefined();
// 3. SET_CHAT_DATA_FETCHED finds by ID — works on the current store object
mutations[types.SET_CHAT_DATA_FETCHED](state, 1);
expect(state.allConversations[0].dataFetched).toBe(true);
// Old detached reference is unaffected
expect(originalRef.dataFetched).toBeUndefined();
});
});
describe('#SET_ALL_MESSAGES_LOADED', () => {
it('should set allMessagesLoaded to true on the conversation by ID', () => {
const state = {
allConversations: [{ id: 1, allMessagesLoaded: false }, { id: 2 }],
};
mutations[types.SET_ALL_MESSAGES_LOADED](state, 1);
expect(state.allConversations[0].allMessagesLoaded).toBe(true);
expect(state.allConversations[1].allMessagesLoaded).toBeUndefined();
});
it('should do nothing if conversation is not found', () => {
const state = { allConversations: [{ id: 1 }] };
mutations[types.SET_ALL_MESSAGES_LOADED](state, 999);
expect(state.allConversations[0].allMessagesLoaded).toBeUndefined();
});
});
describe('#CLEAR_ALL_MESSAGES_LOADED', () => {
it('should set allMessagesLoaded to false on selected chat', () => {
it('should set allMessagesLoaded to false on the conversation by ID', () => {
const state = {
allConversations: [{ id: 1, allMessagesLoaded: true }],
selectedChatId: 1,
allConversations: [
{ id: 1, allMessagesLoaded: true },
{ id: 2, allMessagesLoaded: true },
],
};
mutations[types.CLEAR_ALL_MESSAGES_LOADED](state);
mutations[types.CLEAR_ALL_MESSAGES_LOADED](state, 1);
expect(state.allConversations[0].allMessagesLoaded).toBe(false);
expect(state.allConversations[1].allMessagesLoaded).toBe(true);
});
it('should do nothing if conversation is not found', () => {
const state = { allConversations: [{ id: 1, allMessagesLoaded: true }] };
mutations[types.CLEAR_ALL_MESSAGES_LOADED](state, 999);
expect(state.allConversations[0].allMessagesLoaded).toBe(true);
});
});
@@ -797,6 +856,34 @@ describe('#mutations', () => {
mutations[types.UPDATE_CONVERSATION](state, conversation);
expect(state.allConversations[0].status).toEqual('resolved');
});
it('should preserve dataFetched and allMessagesLoaded during update', () => {
const state = {
allConversations: [
{
id: 1,
status: 'open',
updated_at: 100,
messages: [{ id: 'msg1' }],
dataFetched: true,
allMessagesLoaded: true,
},
],
};
const conversation = {
id: 1,
status: 'resolved',
updated_at: 200,
messages: [{ id: 'msg2' }],
};
mutations[types.UPDATE_CONVERSATION](state, conversation);
expect(state.allConversations[0].status).toEqual('resolved');
expect(state.allConversations[0].dataFetched).toBe(true);
expect(state.allConversations[0].allMessagesLoaded).toBe(true);
expect(state.allConversations[0].messages).toEqual([{ id: 'msg1' }]);
});
});
describe('#UPDATE_CONVERSATION_CONTACT', () => {

1
app/javascript/dashboard/store/mutation-types.js
View File

@@ -64,6 +64,7 @@ export default {
SET_CONTEXT_MENU_CHAT_ID: 'SET_CONTEXT_MENU_CHAT_ID',
SET_CHAT_DATA_FETCHED: 'SET_CHAT_DATA_FETCHED',
SET_CHAT_LIST_FILTERS: 'SET_CHAT_LIST_FILTERS',
UPDATE_CHAT_LIST_FILTERS: 'UPDATE_CHAT_LIST_FILTERS',