## Description This PR fixes a data-corruption bug in the new conversation flow that surfaces in conversation search results as `Subject: undefined`. ### The Problem When creating a conversation through the "New conversation" modal without typing a subject (every non-email channel never shows the subject input, and email channels can be left blank), the conversation gets persisted with `additional_attributes.mail_subject = "undefined"` (the literal string). The bogus value shows up in conversation search results as `Subject: undefined`, which started rendering after #10843. ### Root Cause `createConversationPayload` in `app/javascript/dashboard/store/modules/contactConversations.js` appends the `mail_subject` field unconditionally: ```js payload.append('additional_attributes[mail_subject]', mailSubject); ``` `composeConversationHelper.js` only sets `payload.mailSubject` when `subject` is truthy, so when no subject is provided, `mailSubject` is destructured as `undefined` in `createConversationPayload`. `FormData.append` coerces `undefined` to the string `"undefined"`, and the backend persists it as-is into the JSONB column. ### The Fix Skip the append when `mailSubject` is falsy. The backend already treats a missing key the same as an empty subject (the email mailer falls back to a default subject when `mail_subject` is `nil`), so omitting the field is safe across channels. ### Key Changes - Guarded the `additional_attributes[mail_subject]` append in `createConversationPayload`. - Added a spec covering the case where `mailSubject` is omitted. > Note: existing rows persisted before this fix will continue to show `Subject: undefined` in search until cleaned up. A simple SQL cleanup for non-email inboxes: > > ```sql > UPDATE conversations > SET additional_attributes = additional_attributes - 'mail_subject' > FROM inboxes > WHERE conversations.inbox_id = inboxes.id > AND inboxes.channel_type <> 'Channel::Email' > AND conversations.additional_attributes->>'mail_subject' = 'undefined'; > ``` > > I'm leaving any data-cleanup migration out of this PR since it's a maintenance concern that may be handled differently per installation. ## Type of change - [X] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? 1. Open the "New conversation" modal on a non-email inbox (e.g., WhatsApp, SMS, API). 2. Create a conversation without filling any subject (field is not present on those, so regular flow). 3. Open the global search and search for the conversation. 4. **Before:** the result card shows `Subject: undefined`. **After:** the subject row is hidden. 5. Repeat on an email inbox leaving the subject blank — same result. 6. On an email inbox, type a subject and verify it still persists and renders correctly. 7. Run the new spec: `pnpm test contactConversations`. ## Checklist: - [X] My code follows the style guidelines of this project - [X] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [X] My changes generate no new warnings - [X] I have added tests that prove my fix is effective or that my feature works - [X] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules Co-authored-by: Sojan Jose <sojan@pepalo.com> Co-authored-by: Sony Mathew <sony@chatwoot.com>
326 lines
9.6 KiB
JavaScript
326 lines
9.6 KiB
JavaScript
import axios from 'axios';
|
|
import {
|
|
actions,
|
|
createMessagePayload,
|
|
createConversationPayload,
|
|
createWhatsAppConversationPayload,
|
|
} from '../../contactConversations';
|
|
import * as types from '../../../mutation-types';
|
|
import conversationList from './fixtures';
|
|
|
|
const commit = vi.fn();
|
|
global.axios = axios;
|
|
vi.mock('axios');
|
|
|
|
describe('#actions', () => {
|
|
describe('#get', () => {
|
|
it('sends correct actions if API is success', async () => {
|
|
axios.get.mockResolvedValue({ data: { payload: conversationList } });
|
|
await actions.get({ commit }, 1);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isFetching: true }],
|
|
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS,
|
|
{ id: 1, data: conversationList },
|
|
],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isFetching: false },
|
|
],
|
|
]);
|
|
});
|
|
it('sends correct actions if API is error', async () => {
|
|
axios.get.mockRejectedValue({ message: 'Incorrect header' });
|
|
await actions.get({ commit });
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isFetching: true }],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isFetching: false },
|
|
],
|
|
]);
|
|
});
|
|
});
|
|
|
|
describe('#create', () => {
|
|
it('sends correct actions if API is success', async () => {
|
|
axios.post.mockResolvedValue({ data: conversationList[0] });
|
|
await actions.create(
|
|
{ commit },
|
|
{
|
|
params: {
|
|
inboxId: 1,
|
|
message: { content: 'hi' },
|
|
contactId: 4,
|
|
sourceId: 5,
|
|
mailSubject: 'Mail Subject',
|
|
assigneeId: 6,
|
|
files: [],
|
|
},
|
|
isFromWhatsApp: false,
|
|
}
|
|
);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isCreating: true }],
|
|
[
|
|
types.default.ADD_CONTACT_CONVERSATION,
|
|
{ id: 4, data: conversationList[0] },
|
|
],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isCreating: false },
|
|
],
|
|
]);
|
|
});
|
|
it('sends correct actions with files if API is success', async () => {
|
|
axios.post.mockResolvedValue({ data: conversationList[0] });
|
|
await actions.create(
|
|
{ commit },
|
|
{
|
|
params: {
|
|
inboxId: 1,
|
|
message: { content: 'hi' },
|
|
contactId: 4,
|
|
sourceId: 5,
|
|
mailSubject: 'Mail Subject',
|
|
assigneeId: 6,
|
|
files: ['file1.pdf', 'file2.jpg'],
|
|
},
|
|
isFromWhatsApp: false,
|
|
}
|
|
);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isCreating: true }],
|
|
[
|
|
types.default.ADD_CONTACT_CONVERSATION,
|
|
{ id: 4, data: conversationList[0] },
|
|
],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isCreating: false },
|
|
],
|
|
]);
|
|
});
|
|
it('sends correct actions actions if API is success for whatsapp conversation', async () => {
|
|
axios.post.mockResolvedValue({ data: conversationList[0] });
|
|
await actions.create(
|
|
{ commit },
|
|
{
|
|
params: {
|
|
inboxId: 1,
|
|
message: {
|
|
content: 'hi',
|
|
template_params: {
|
|
name: 'hello_world',
|
|
category: 'MARKETING',
|
|
language: 'en_US',
|
|
processed_params: {},
|
|
},
|
|
},
|
|
contactId: 4,
|
|
sourceId: 5,
|
|
assigneeId: 6,
|
|
},
|
|
isFromWhatsApp: true,
|
|
}
|
|
);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isCreating: true }],
|
|
[
|
|
types.default.ADD_CONTACT_CONVERSATION,
|
|
{ id: 4, data: conversationList[0] },
|
|
],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isCreating: false },
|
|
],
|
|
]);
|
|
});
|
|
it('sends correct actions if API is error', async () => {
|
|
axios.post.mockRejectedValue({ message: 'Incorrect header' });
|
|
await expect(
|
|
actions.create(
|
|
{ commit },
|
|
{
|
|
params: {
|
|
inboxId: 1,
|
|
message: { content: 'hi' },
|
|
contactId: 4,
|
|
assigneeId: 6,
|
|
sourceId: 5,
|
|
mailSubject: 'Mail Subject',
|
|
},
|
|
isFromWhatsApp: false,
|
|
}
|
|
)
|
|
).rejects.toThrow(Error);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isCreating: true }],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isCreating: false },
|
|
],
|
|
]);
|
|
});
|
|
it('sends correct actions with files if API is error', async () => {
|
|
axios.post.mockRejectedValue({ message: 'Incorrect header' });
|
|
await expect(
|
|
actions.create(
|
|
{ commit },
|
|
{
|
|
params: {
|
|
inboxId: 1,
|
|
message: { content: 'hi' },
|
|
contactId: 4,
|
|
sourceId: 5,
|
|
mailSubject: 'Mail Subject',
|
|
assigneeId: 6,
|
|
files: ['file1.pdf', 'file2.jpg'],
|
|
},
|
|
isFromWhatsApp: false,
|
|
}
|
|
)
|
|
).rejects.toThrow(Error);
|
|
expect(commit.mock.calls).toEqual([
|
|
[types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG, { isCreating: true }],
|
|
[
|
|
types.default.SET_CONTACT_CONVERSATIONS_UI_FLAG,
|
|
{ isCreating: false },
|
|
],
|
|
]);
|
|
});
|
|
});
|
|
});
|
|
|
|
describe('createMessagePayload', () => {
|
|
it('creates message payload with cc and bcc emails', () => {
|
|
const payload = new FormData();
|
|
const message = {
|
|
content: 'Test message content',
|
|
cc_emails: 'cc@example.com',
|
|
bcc_emails: 'bcc@example.com',
|
|
};
|
|
|
|
createMessagePayload(payload, message);
|
|
|
|
expect(payload.get('message[content]')).toBe(message.content);
|
|
expect(payload.get('message[cc_emails]')).toBe(message.cc_emails);
|
|
expect(payload.get('message[bcc_emails]')).toBe(message.bcc_emails);
|
|
});
|
|
|
|
it('creates message payload without cc and bcc emails', () => {
|
|
const payload = new FormData();
|
|
const message = {
|
|
content: 'Test message content',
|
|
};
|
|
|
|
createMessagePayload(payload, message);
|
|
|
|
expect(payload.get('message[content]')).toBe(message.content);
|
|
expect(payload.get('message[cc_emails]')).toBeNull();
|
|
expect(payload.get('message[bcc_emails]')).toBeNull();
|
|
});
|
|
});
|
|
|
|
describe('createConversationPayload', () => {
|
|
it('creates conversation payload with message and attachments', () => {
|
|
const options = {
|
|
params: {
|
|
inboxId: '1',
|
|
message: {
|
|
content: 'Test message content',
|
|
},
|
|
sourceId: '12',
|
|
mailSubject: 'Test Subject',
|
|
assigneeId: '123',
|
|
},
|
|
contactId: '23',
|
|
files: ['file1.pdf', 'file2.jpg'],
|
|
};
|
|
|
|
const payload = createConversationPayload(options);
|
|
|
|
expect(payload.get('message[content]')).toBe(
|
|
options.params.message.content
|
|
);
|
|
expect(payload.get('inbox_id')).toBe(options.params.inboxId);
|
|
expect(payload.get('contact_id')).toBe(options.contactId);
|
|
expect(payload.get('source_id')).toBe(options.params.sourceId);
|
|
expect(payload.get('additional_attributes[mail_subject]')).toBe(
|
|
options.params.mailSubject
|
|
);
|
|
expect(payload.get('assignee_id')).toBe(options.params.assigneeId);
|
|
expect(payload.getAll('message[attachments][]')).toEqual(options.files);
|
|
});
|
|
|
|
it('creates conversation payload with message and without attachments', () => {
|
|
const options = {
|
|
params: {
|
|
inboxId: '1',
|
|
message: {
|
|
content: 'Test message content',
|
|
},
|
|
sourceId: '12',
|
|
mailSubject: 'Test Subject',
|
|
assigneeId: '123',
|
|
},
|
|
contactId: '23',
|
|
};
|
|
|
|
const payload = createConversationPayload(options);
|
|
|
|
expect(payload.get('message[content]')).toBe(
|
|
options.params.message.content
|
|
);
|
|
expect(payload.get('inbox_id')).toBe(options.params.inboxId);
|
|
expect(payload.get('contact_id')).toBe(options.contactId);
|
|
expect(payload.get('source_id')).toBe(options.params.sourceId);
|
|
expect(payload.get('additional_attributes[mail_subject]')).toBe(
|
|
options.params.mailSubject
|
|
);
|
|
expect(payload.get('assignee_id')).toBe(options.params.assigneeId);
|
|
expect(payload.getAll('message[attachments][]')).toEqual([]);
|
|
});
|
|
|
|
it('omits mail_subject when mailSubject is undefined', () => {
|
|
const options = {
|
|
params: {
|
|
inboxId: '1',
|
|
message: {
|
|
content: 'Test message content',
|
|
},
|
|
sourceId: '12',
|
|
assigneeId: '123',
|
|
},
|
|
contactId: '23',
|
|
};
|
|
|
|
const payload = createConversationPayload(options);
|
|
|
|
expect(payload.has('additional_attributes[mail_subject]')).toBe(false);
|
|
});
|
|
});
|
|
|
|
describe('createWhatsAppConversationPayload', () => {
|
|
it('creates conversation payload with message', () => {
|
|
const options = {
|
|
params: {
|
|
inboxId: '1',
|
|
message: {
|
|
content: 'Test message content',
|
|
},
|
|
sourceId: '12',
|
|
assigneeId: '123',
|
|
},
|
|
};
|
|
|
|
const payload = createWhatsAppConversationPayload(options);
|
|
|
|
expect(payload.message).toBe(options.params.message);
|
|
expect(payload.inbox_id).toBe(options.params.inboxId);
|
|
expect(payload.source_id).toBe(options.params.sourceId);
|
|
expect(payload.assignee_id).toBe(options.params.assigneeId);
|
|
});
|
|
});
|