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>
This commit is contained in:
co-authored by
Sojan Jose
Sony Mathew
parent
274e92e0e4
commit
e5c140158e
@@ -25,7 +25,9 @@ export const createConversationPayload = ({ params, contactId, files }) => {
|
||||
payload.append('inbox_id', inboxId);
|
||||
payload.append('contact_id', contactId);
|
||||
payload.append('source_id', sourceId);
|
||||
payload.append('additional_attributes[mail_subject]', mailSubject);
|
||||
if (mailSubject) {
|
||||
payload.append('additional_attributes[mail_subject]', mailSubject);
|
||||
}
|
||||
payload.append('assignee_id', assigneeId);
|
||||
|
||||
return payload;
|
||||
|
||||
@@ -282,6 +282,24 @@ describe('createConversationPayload', () => {
|
||||
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', () => {
|
||||
|
||||
Reference in New Issue
Block a user