fix: guard agent sort against null names in assignment dropdown (#15125)
# Pull Request Template ## Description This PR fixes a crash where opening a conversation threw `TypeError: Cannot read properties of null (reading 'localeCompare')` and prevented the agent assignment dropdown from rendering. Since #14866, agent bots are included in the assignable agents list. `AgentBot#name` is not presence-validated, so system bots (account-less, global) can have a `null` name. Those nameless bots flowed into name-based operations that assumed a string, causing crashes and warnings across multiple surfaces: * **Assignment dropdown sort:** `getAgentsByAvailability` called `a.name.localeCompare(b.name)`, causing a `localeCompare` `TypeError`. * **Dropdown search:** `MultiselectDropdownItems` called `option.name.toLowerCase()`, causing a `toLowerCase` `TypeError`. * **Agent Bots settings:** `Avatar` received `name=null` for a `String` prop, triggering a Vue prop validation warning. ### What changed * Keep nameless agent bots in the assignment dropdown and render a `-` fallback label in `useAgentsList`. These are still valid, assignable-by-ID records: the assignable agents API includes accessible bots, and `Conversations::AssignmentService` assigns them by ID. Preserving them avoids hiding valid assignment targets. Bots are still included only when `includeAgentBots` is enabled. * Make the sort in `getAgentsByAvailability` null-safe by coercing missing names to an empty string (defense in depth). * Make the search filter in `MultiselectDropdownItems` null-safe (defense in depth). * Pass a null-safe `name` prop to `Avatar` in the Agent Bots settings list to eliminate the Vue prop validation warning. Fixes https://linear.app/chatwoot/issue/CW-7670/agent-assignment-dropdown-crashes-with-cannot-read-properties-of-null ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? 1. Have a system agent bot (`name: null`) that is assignable to an inbox. 2. Open any conversation in that inbox. * The agent assignment dropdown renders without console errors. * The nameless bot is listed with a `-` label and can be assigned. 3. Go to **Settings → Agent Bots**. * The page renders without the `Avatar` prop validation warning. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] 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
This commit is contained in:
@@ -1,9 +1,9 @@
|
||||
import { ref } from 'vue';
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import { useAgentsList } from '../useAgentsList';
|
||||
import { useMapGetter } from 'dashboard/composables/store';
|
||||
import { allAgentsData, formattedAgentsData } from './fixtures/agentFixtures';
|
||||
import * as agentHelper from 'dashboard/helper/agentHelper';
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { ref } from 'vue';
|
||||
import { useAgentsList } from '../useAgentsList';
|
||||
import { allAgentsData, formattedAgentsData } from './fixtures/agentFixtures';
|
||||
|
||||
// Mock vue-i18n
|
||||
vi.mock('vue-i18n', () => ({
|
||||
@@ -94,6 +94,32 @@ describe('useAgentsList', () => {
|
||||
expect(agentsList.value.length).toBe(formattedAgentsData.slice(1).length);
|
||||
});
|
||||
|
||||
it('keeps nameless agent bots and applies a fallback label', () => {
|
||||
const namelessBot = {
|
||||
id: 91,
|
||||
name: null,
|
||||
assignee_type: 'AgentBot',
|
||||
availability_status: 'offline',
|
||||
};
|
||||
mockUseMapGetter({
|
||||
'inboxAssignableAgents/getAssignableAgents': ref(() => [
|
||||
...allAgentsData,
|
||||
namelessBot,
|
||||
]),
|
||||
});
|
||||
|
||||
const { agentsList } = useAgentsList();
|
||||
// access the computed to trigger evaluation
|
||||
expect(agentsList.value).toBeDefined();
|
||||
|
||||
const passedAgents =
|
||||
agentHelper.getAgentsByUpdatedPresence.mock.calls[0][0];
|
||||
expect(passedAgents).toContainEqual({
|
||||
...namelessBot,
|
||||
name: '-',
|
||||
});
|
||||
});
|
||||
|
||||
it('handles empty assignable agents', () => {
|
||||
mockUseMapGetter({
|
||||
'inboxAssignableAgents/getAssignableAgents': ref(() => []),
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import { computed } from 'vue';
|
||||
import { useMapGetter } from 'dashboard/composables/store';
|
||||
import { useI18n } from 'vue-i18n';
|
||||
import {
|
||||
getAgentsByUpdatedPresence,
|
||||
getSortedAgentsByAvailability,
|
||||
} from 'dashboard/helper/agentHelper';
|
||||
import { computed } from 'vue';
|
||||
import { useI18n } from 'vue-i18n';
|
||||
|
||||
/**
|
||||
* A composable function that provides a list of agents for assignment.
|
||||
@@ -53,7 +53,11 @@ export function useAgentsList(
|
||||
* @type {import('vue').ComputedRef<Array>}
|
||||
*/
|
||||
const agentsList = computed(() => {
|
||||
const agents = assignableAgents.value || [];
|
||||
const agents = (assignableAgents.value || []).map(agent =>
|
||||
!agent.name && agent.assignee_type === 'AgentBot'
|
||||
? { ...agent, name: '-' }
|
||||
: agent
|
||||
);
|
||||
const agentsByUpdatedPresence = getAgentsByUpdatedPresence(
|
||||
agents,
|
||||
currentUser.value,
|
||||
|
||||
@@ -7,7 +7,7 @@
|
||||
export const getAgentsByAvailability = (agents, availability) => {
|
||||
return agents
|
||||
.filter(agent => agent.availability_status === availability)
|
||||
.sort((a, b) => a.name.localeCompare(b.name));
|
||||
.sort((a, b) => (a.name || '').localeCompare(b.name || ''));
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -26,6 +26,18 @@ describe('agentHelper', () => {
|
||||
offlineAgentsData
|
||||
);
|
||||
});
|
||||
|
||||
it('does not throw when an agent has a null name', () => {
|
||||
const agents = [
|
||||
{ id: 1, name: null, availability_status: 'offline' },
|
||||
{ id: 2, name: 'Zoe', availability_status: 'offline' },
|
||||
];
|
||||
|
||||
expect(() => getAgentsByAvailability(agents, 'offline')).not.toThrow();
|
||||
expect(
|
||||
getAgentsByAvailability(agents, 'offline').map(agent => agent.id)
|
||||
).toEqual([1, 2]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getSortedAgentsByAvailability', () => {
|
||||
|
||||
@@ -135,7 +135,7 @@ onMounted(() => {
|
||||
<BaseTableCell class="max-w-0">
|
||||
<div class="flex items-center gap-4 min-w-0">
|
||||
<Avatar
|
||||
:name="bot.name"
|
||||
:name="bot.name || ''"
|
||||
:src="bot.thumbnail"
|
||||
:size="40"
|
||||
class="flex-shrink-0"
|
||||
|
||||
@@ -68,6 +68,10 @@ const isAgentBot = computed(
|
||||
() => props.selectedItem?.assignee_type === 'AgentBot'
|
||||
);
|
||||
|
||||
const selectedItemName = computed(() =>
|
||||
!props.selectedItem?.name && isAgentBot.value ? '-' : props.selectedItem?.name
|
||||
);
|
||||
|
||||
const selectedThumbnail = computed(
|
||||
() => props.selectedItem?.thumbnail || props.selectedItem?.avatar_url
|
||||
);
|
||||
@@ -95,16 +99,16 @@ const selectedThumbnail = computed(
|
||||
<h4
|
||||
v-else
|
||||
class="items-center overflow-hidden text-sm leading-tight whitespace-nowrap text-ellipsis text-n-slate-12"
|
||||
:title="selectedItem.name"
|
||||
:title="selectedItemName"
|
||||
>
|
||||
{{ selectedItem.name }}
|
||||
{{ selectedItemName }}
|
||||
</h4>
|
||||
</div>
|
||||
<Avatar
|
||||
v-if="hasValue && hasThumbnail && (isAgentBot || !hasIcon)"
|
||||
:src="selectedThumbnail"
|
||||
:status="selectedItem.availability_status"
|
||||
:name="selectedItem.name"
|
||||
:name="selectedItemName"
|
||||
:icon-name="isAgentBot ? 'i-lucide-bot' : undefined"
|
||||
:size="24"
|
||||
hide-offline-status
|
||||
|
||||
@@ -53,7 +53,9 @@ export default {
|
||||
computed: {
|
||||
filteredOptions() {
|
||||
return this.options.filter(option => {
|
||||
return option.name.toLowerCase().includes(this.search.toLowerCase());
|
||||
return (option.name || '')
|
||||
.toLowerCase()
|
||||
.includes(this.search.toLowerCase());
|
||||
});
|
||||
},
|
||||
noResult() {
|
||||
|
||||
Reference in New Issue
Block a user