Skip to content

Commit bfffd55

Browse files
committed
AIChat: fix optionChanged to cover object values
1 parent 78477dc commit bfffd55

File tree

10 files changed

+299
-37
lines changed

10 files changed

+299
-37
lines changed

packages/devextreme/js/__internal/grids/grid_core/ai_assistant/__tests__/ai_assistant_view.test.ts

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,11 @@ describe('AIAssistantView', () => {
290290
});
291291

292292
it('should call hide when aiAssistant.enabled changes to false', () => {
293-
const { aiAssistantView } = createAIAssistantView();
293+
const { aiAssistantView, setEnabled } = createAIAssistantView();
294294
const hideSpy = jest.spyOn(aiAssistantView, 'hide');
295295

296+
setEnabled(false);
297+
296298
aiAssistantView.optionChanged({
297299
name: 'aiAssistant' as const,
298300
fullName: 'aiAssistant.enabled' as const,
@@ -304,7 +306,7 @@ describe('AIAssistantView', () => {
304306
expect(hideSpy).toHaveBeenCalledTimes(1);
305307
});
306308

307-
it('should call updateOptions on aiChatInstance for non-enabled sub-options', () => {
309+
it('should call updateOptions on aiChatInstance for title change', () => {
308310
const { aiAssistantView } = createAIAssistantView();
309311

310312
const aiChatInstance = (AIChat as jest.Mock)
@@ -319,6 +321,55 @@ describe('AIAssistantView', () => {
319321
});
320322

321323
expect(aiChatInstance.updateOptions).toHaveBeenCalledTimes(1);
324+
expect(aiChatInstance.updateOptions).toHaveBeenCalledWith(
325+
expect.any(Object),
326+
true,
327+
false,
328+
);
329+
});
330+
331+
it('should call updateOptions on aiChatInstance for chat options change', () => {
332+
const { aiAssistantView } = createAIAssistantView();
333+
334+
const aiChatInstance = (AIChat as jest.Mock)
335+
.mock.results[0].value as { updateOptions: jest.Mock };
336+
337+
aiAssistantView.optionChanged({
338+
name: 'aiAssistant' as const,
339+
fullName: 'aiAssistant.chat' as const,
340+
value: { speechToTextEnabled: false },
341+
previousValue: {},
342+
handled: false,
343+
});
344+
345+
expect(aiChatInstance.updateOptions).toHaveBeenCalledTimes(1);
346+
expect(aiChatInstance.updateOptions).toHaveBeenCalledWith(
347+
expect.any(Object),
348+
false,
349+
true,
350+
);
351+
});
352+
353+
it('should call updateOptions with both flags when object value contains title and chat', () => {
354+
const { aiAssistantView } = createAIAssistantView();
355+
356+
const aiChatInstance = (AIChat as jest.Mock)
357+
.mock.results[0].value as { updateOptions: jest.Mock };
358+
359+
aiAssistantView.optionChanged({
360+
name: 'aiAssistant' as const,
361+
fullName: 'aiAssistant' as const,
362+
value: { title: 'New title', chat: { speechToTextEnabled: false } },
363+
previousValue: { title: 'Old title' },
364+
handled: false,
365+
});
366+
367+
expect(aiChatInstance.updateOptions).toHaveBeenCalledTimes(1);
368+
expect(aiChatInstance.updateOptions).toHaveBeenCalledWith(
369+
expect.any(Object),
370+
true,
371+
true,
372+
);
322373
});
323374

324375
it('should not throw when aiChatInstance is not created for non-enabled sub-options', () => {

packages/devextreme/js/__internal/grids/grid_core/ai_assistant/__tests__/ai_assistant_view_controller.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ describe('AIAssistantViewController', () => {
126126
expect(mockView.visibilityChanged.remove).toHaveBeenCalledWith(expect.any(Function));
127127
expect(removeOrder).toBeLessThan(addOrder);
128128
});
129+
130+
it('should use the same handler reference for remove and add', () => {
131+
const { mockView } = createAIAssistantViewController();
132+
133+
const removedHandler = mockView.visibilityChanged.remove.mock.calls[0][0];
134+
const addedHandler = mockView.visibilityChanged.add.mock.calls[0][0];
135+
136+
expect(removedHandler).toBe(addedHandler);
137+
});
129138
});
130139

131140
describe('optionChanged', () => {
@@ -210,5 +219,59 @@ describe('AIAssistantViewController', () => {
210219

211220
expect(mockHeaderPanel.applyToolbarItem).toHaveBeenCalledTimes(1);
212221
});
222+
223+
it('should set handled to true when object value contains enabled', () => {
224+
const options: Record<string, unknown> = { 'aiAssistant.enabled': false };
225+
const { controller, mockHeaderPanel } = createAIAssistantViewController(options);
226+
227+
options['aiAssistant.enabled'] = true;
228+
229+
const args = {
230+
name: 'aiAssistant' as const,
231+
fullName: 'aiAssistant' as const,
232+
value: { enabled: true },
233+
previousValue: { enabled: false },
234+
handled: false,
235+
};
236+
237+
controller.optionChanged(args);
238+
239+
expect(args.handled).toBe(true);
240+
expect(mockHeaderPanel.applyToolbarItem).toHaveBeenCalledTimes(1);
241+
});
242+
243+
it('should set handled to true when object value contains title', () => {
244+
const options: Record<string, unknown> = { 'aiAssistant.enabled': true };
245+
const { controller, mockHeaderPanel } = createAIAssistantViewController(options);
246+
247+
const args = {
248+
name: 'aiAssistant' as const,
249+
fullName: 'aiAssistant' as const,
250+
value: { title: 'New title', chat: { speechToTextEnabled: false } },
251+
previousValue: { title: 'Old title' },
252+
handled: false,
253+
};
254+
255+
controller.optionChanged(args);
256+
257+
expect(args.handled).toBe(true);
258+
expect(mockHeaderPanel.applyToolbarItem).toHaveBeenCalledTimes(1);
259+
});
260+
261+
it('should not set handled when object value contains only chat/popup options', () => {
262+
const { controller } = createAIAssistantViewController();
263+
264+
const args = {
265+
name: 'aiAssistant' as const,
266+
fullName: 'aiAssistant' as const,
267+
value: { chat: { speechToTextEnabled: false, showMessageTimestamp: true } },
268+
previousValue: {},
269+
handled: false,
270+
};
271+
272+
controller.optionChanged(args);
273+
274+
expect(args.handled).toBe(false);
275+
});
213276
});
214277
});
Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
import {
2+
describe, expect, it,
3+
} from '@jest/globals';
4+
5+
import {
6+
isChatOptions,
7+
isEnabledOption,
8+
isPopupOptions,
9+
isTitleOption,
10+
} from '../utils';
11+
12+
describe('isEnabledOption', () => {
13+
it('should return true for enabled option names', () => {
14+
expect(isEnabledOption('aiAssistant.enabled', true)).toBe(true);
15+
expect(isEnabledOption('aiAssistant', {
16+
enabled: false,
17+
title: 'AI Assistant',
18+
})).toBe(true);
19+
});
20+
21+
it('should return false for non-enabled option names', () => {
22+
expect(isEnabledOption('aiAssistant.title', 'Title')).toBe(false);
23+
expect(isEnabledOption('aiAssistant.popup', {})).toBe(false);
24+
expect(isEnabledOption('aiAssistant', { title: 'Title' })).toBe(false);
25+
expect(isEnabledOption('aiAssistant', 'string')).toBe(false);
26+
});
27+
});
28+
29+
describe('isTitleOption', () => {
30+
it('should return true for title option names', () => {
31+
expect(isTitleOption('aiAssistant.title', 'New Title')).toBe(true);
32+
expect(isTitleOption('aiAssistant', {
33+
title: 'New Title',
34+
chat: { speechToTextEnabled: false },
35+
})).toBe(true);
36+
});
37+
38+
it('should return false for non-title option names', () => {
39+
expect(isTitleOption('aiAssistant.enabled', true)).toBe(false);
40+
expect(isTitleOption('aiAssistant.chat', {})).toBe(false);
41+
expect(isTitleOption('aiAssistant', { enabled: true })).toBe(false);
42+
expect(isTitleOption('aiAssistant', 'string')).toBe(false);
43+
});
44+
});
45+
46+
describe('isPopupOptions', () => {
47+
it('should return true for popup option names', () => {
48+
expect(isPopupOptions('aiAssistant.popup', {})).toBe(true);
49+
expect(isPopupOptions('aiAssistant.popup.width', 400)).toBe(true);
50+
expect(isPopupOptions('aiAssistant', { popup: { width: 400 } })).toBe(true);
51+
expect(isPopupOptions('aiAssistant', {
52+
popup: { width: 400 },
53+
title: 'AI Assistant',
54+
})).toBe(true);
55+
});
56+
57+
it('should return false for non-popup option names', () => {
58+
expect(isPopupOptions('aiAssistant.chat', {})).toBe(false);
59+
expect(isPopupOptions('aiAssistant.enabled', true)).toBe(false);
60+
expect(isPopupOptions('aiAssistant', { chat: {} })).toBe(false);
61+
expect(isPopupOptions('aiAssistant', {
62+
chat: { showAvatar: false },
63+
title: 'AI Assistant',
64+
})).toBe(false);
65+
});
66+
});
67+
68+
describe('isChatOptions', () => {
69+
it('should return true for chat option names', () => {
70+
expect(isChatOptions('aiAssistant.chat', {})).toBe(true);
71+
expect(isChatOptions('aiAssistant.chat.showAvatar', false)).toBe(true);
72+
expect(isChatOptions('aiAssistant', { chat: { showAvatar: false } })).toBe(true);
73+
expect(isChatOptions('aiAssistant', {
74+
chat: { speechToTextEnabled: false },
75+
title: 'AI Assistant',
76+
})).toBe(true);
77+
});
78+
79+
it('should return false for non-chat option names', () => {
80+
expect(isChatOptions('aiAssistant.popup', {})).toBe(false);
81+
expect(isChatOptions('aiAssistant.enabled', true)).toBe(false);
82+
expect(isChatOptions('aiAssistant', { popup: {} })).toBe(false);
83+
expect(isChatOptions('aiAssistant', {
84+
popup: { width: 400 },
85+
title: 'AI Assistant',
86+
})).toBe(false);
87+
});
88+
});

packages/devextreme/js/__internal/grids/grid_core/ai_assistant/ai_assistant_view.ts

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ import { isString } from '@js/core/utils/type';
66
import type { Message, Properties as ChatProperties } from '@js/ui/chat';
77
import type { Properties as PopupProperties } from '@js/ui/popup';
88
import { AI_ASSISTANT_POPUP_OFFSET } from '@ts/grids/grid_core/ai_assistant/const';
9+
import {
10+
isChatOptions,
11+
isEnabledOption,
12+
isPopupOptions,
13+
isTitleOption,
14+
} from '@ts/grids/grid_core/ai_assistant/utils';
915
import type { ColumnHeadersView } from '@ts/grids/grid_core/column_headers/m_column_headers';
1016
import type { OptionChanged } from '@ts/grids/grid_core/m_types';
1117
import type { RowsView } from '@ts/grids/grid_core/views/m_rows_view';
@@ -110,19 +116,29 @@ export class AIAssistantView extends View {
110116

111117
public optionChanged(args: OptionChanged): void {
112118
if (args.name === 'aiAssistant') {
113-
const [, secondLevel] = args.fullName.split('.');
114-
switch (secondLevel) {
115-
case 'enabled':
116-
if (args.value) {
117-
this?._invalidate();
118-
} else {
119-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
120-
this?.hide();
121-
}
122-
break;
123-
default:
124-
this.aiChatInstance?.updateOptions(this.getAIChatConfig());
119+
const enabledChanged = isEnabledOption(args.fullName, args.value);
120+
121+
if (enabledChanged) {
122+
if (this.isVisible()) {
123+
this._invalidate();
124+
} else {
125+
// eslint-disable-next-line @typescript-eslint/no-floating-promises
126+
this.hide();
127+
}
125128
}
129+
130+
const popupOptionsChanged = isTitleOption(args.fullName, args.value)
131+
|| isPopupOptions(args.fullName, args.value);
132+
const chatOptionsChanged = isChatOptions(args.fullName, args.value);
133+
134+
if (popupOptionsChanged || chatOptionsChanged) {
135+
this.aiChatInstance?.updateOptions(
136+
this.getAIChatConfig(),
137+
popupOptionsChanged,
138+
chatOptionsChanged,
139+
);
140+
}
141+
126142
args.handled = true;
127143
} else {
128144
super.optionChanged(args);

packages/devextreme/js/__internal/grids/grid_core/ai_assistant/ai_assistant_view_controller.ts

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,17 +11,19 @@ import type { ToolbarItem } from '@ts/grids/new/grid_core/toolbar/types';
1111

1212
import { ViewController } from '../m_modules';
1313
import type { AIAssistantView } from './ai_assistant_view';
14+
import { isEnabledOption, isTitleOption } from './utils';
1415

1516
export class AIAssistantViewController extends ViewController {
1617
private aiAssistantView?: AIAssistantView;
1718

1819
private headerPanel?: HeaderPanel;
1920

20-
private getAiAssistantButton(): dxElementWrapper | undefined {
21+
private readonly visibilityChangedHandler = (visible: boolean): void => {
2122
const className = this.addWidgetPrefix(CLASSES.aiAssistantButton);
23+
const aiAssistantButton: dxElementWrapper | undefined = this.headerPanel?.element()?.find(`.${className}`);
2224

23-
return this.headerPanel?.element()?.find(`.${className}`) as dxElementWrapper | undefined;
24-
}
25+
aiAssistantButton?.toggleClass(ACTIVE_STATE_CLASS, visible);
26+
};
2527

2628
public init(): void {
2729
this.aiAssistantView = this.getView('aiAssistantView');
@@ -35,17 +37,16 @@ export class AIAssistantViewController extends ViewController {
3537
this.headerPanel?.registerToolbarItem(AI_ASSISTANT_BUTTON_NAME, aiAssistantToolbarItem);
3638
}
3739

38-
const visibilityChangedHandler = (visible: boolean): void => {
39-
this.getAiAssistantButton()?.toggleClass(ACTIVE_STATE_CLASS, visible);
40-
};
41-
42-
this.aiAssistantView?.visibilityChanged?.remove(visibilityChangedHandler);
43-
this.aiAssistantView.visibilityChanged?.add(visibilityChangedHandler);
40+
this.aiAssistantView?.visibilityChanged?.remove(this.visibilityChangedHandler);
41+
this.aiAssistantView?.visibilityChanged?.add(this.visibilityChangedHandler);
4442
}
4543

4644
public optionChanged(args: OptionChanged): void {
4745
if (args.name === 'aiAssistant') {
48-
if (args.fullName === 'aiAssistant.enabled' || args.fullName === 'aiAssistant.title') {
46+
const enabledChanged = isEnabledOption(args.fullName, args.value);
47+
const titleChanged = isTitleOption(args.fullName, args.value);
48+
49+
if (enabledChanged || titleChanged) {
4950
this.syncAiAssistantItem();
5051
args.handled = true;
5152
}
@@ -79,7 +80,7 @@ export class AIAssistantViewController extends ViewController {
7980

8081
const aiAssistantToolbarItemClass = this.headerPanel?.getToolbarButtonClass(
8182
this.addWidgetPrefix(CLASSES.aiAssistantButton),
82-
);
83+
) ?? '';
8384
const aiAssistantToolbarItemStateClass = isActive ? ACTIVE_STATE_CLASS : '';
8485

8586
return {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { isObject } from '@js/core/utils/type';
2+
3+
export const isEnabledOption = (optionName: string, value: unknown): boolean => optionName.startsWith('aiAssistant.enabled')
4+
|| (optionName === 'aiAssistant' && isObject(value) && 'enabled' in value);
5+
6+
export const isTitleOption = (optionName: string, value: unknown): boolean => optionName.startsWith('aiAssistant.title')
7+
|| (optionName === 'aiAssistant' && isObject(value) && 'title' in value);
8+
9+
export const isPopupOptions = (optionName: string, value: unknown): boolean => optionName.startsWith('aiAssistant.popup')
10+
|| (optionName === 'aiAssistant' && isObject(value) && 'popup' in value);
11+
12+
export const isChatOptions = (optionName: string, value: unknown): boolean => optionName.startsWith('aiAssistant.chat')
13+
|| (optionName === 'aiAssistant' && isObject(value) && 'chat' in value);

0 commit comments

Comments
 (0)