Files
e5c140158e fix(new-conversation): stop writing literal "undefined" as mail_subject (#14383)
## 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>
2026-06-14 08:57:38 +05:30

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);
});
});