Skip to content

Commit

Permalink
[8.16] [Security Assistant] Fix KB output fields (elastic#196567) (el…
Browse files Browse the repository at this point in the history 
…astic#197118)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Security Assistant] Fix KB output fields
(elastic#196567)](elastic#196567)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Patryk
Kopyciński","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-21T18:32:06Z","message":"[Security
Assistant] Fix KB output fields (elastic#196567)\n\n## Summary\r\n\r\nFixes
Assistant Knowledge Base output fields field logic\r\nFixes Security
Assistant card not appearing on Serverless \r\nReverts Assistant Cog
wheel settings button when FF\r\n`assistantKnowledgeBaseByDefault` is
off\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/2460cf22-c02a-4513-98d1-5fbcd75d117b)","sha":"399aed9b19935651b979dc68ad88429a156dae2f","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v9.0.0","backport:prev-minor","Feature:Security
Assistant","Team:Security Generative
AI","v8.16.0","v8.17.0"],"title":"[Security Assistant] Reverts Assistant
Cog wheel settings button when assistantKnowledgeBaseByDefault FF is
off","number":196567,"url":"https://github.com/elastic/kibana/pull/196567","mergeCommit":{"message":"[Security
Assistant] Fix KB output fields (elastic#196567)\n\n## Summary\r\n\r\nFixes
Assistant Knowledge Base output fields field logic\r\nFixes Security
Assistant card not appearing on Serverless \r\nReverts Assistant Cog
wheel settings button when FF\r\n`assistantKnowledgeBaseByDefault` is
off\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/2460cf22-c02a-4513-98d1-5fbcd75d117b)","sha":"399aed9b19935651b979dc68ad88429a156dae2f"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196567","number":196567,"mergeCommit":{"message":"[Security
Assistant] Fix KB output fields (elastic#196567)\n\n## Summary\r\n\r\nFixes
Assistant Knowledge Base output fields field logic\r\nFixes Security
Assistant card not appearing on Serverless \r\nReverts Assistant Cog
wheel settings button when FF\r\n`assistantKnowledgeBaseByDefault` is
off\r\n\r\n\r\n![image](https://github.com/user-attachments/assets/2460cf22-c02a-4513-98d1-5fbcd75d117b)","sha":"399aed9b19935651b979dc68ad88429a156dae2f"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Patryk Kopyciński <[email protected]>
  • Loading branch information
@kibanamachine @patrykkopycinski
kibanamachine and patrykkopycinski authored Oct 21, 2024
1 parent 5248361 commit 18b517a
Show file tree
Hide file tree
Showing 8 changed files with 406 additions and 65 deletions.
107 changes: 107 additions & 0 deletions x-pack/packages/kbn-elastic-assistant/impl/assistant/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { Conversation } from '../assistant_context/types';
import * as all from './chat_send/use_chat_send';
import { useConversation } from './use_conversation';
import { AIConnector } from '../connectorland/connector_selector';
import { omit } from 'lodash';

jest.mock('../connectorland/use_load_connectors');
jest.mock('../connectorland/connector_setup');
Expand Down Expand Up @@ -138,6 +139,112 @@ describe('Assistant', () => {
>);
});

describe('persistent storage', () => {
it('should refetchCurrentUserConversations after settings save button click', async () => {
const chatSendSpy = jest.spyOn(all, 'useChatSend');
await renderAssistant();

fireEvent.click(screen.getByTestId('settings'));

jest.mocked(useFetchCurrentUserConversations).mockReturnValue({
data: {
...mockData,
welcome_id: {
...mockData.welcome_id,
apiConfig: { newProp: true },
},
},
isLoading: false,
refetch: jest.fn().mockResolvedValue({
isLoading: false,
data: {
...mockData,
welcome_id: {
...mockData.welcome_id,
apiConfig: { newProp: true },
},
},
}),
isFetched: true,
} as unknown as DefinedUseQueryResult<Record<string, Conversation>, unknown>);

await act(async () => {
fireEvent.click(screen.getByTestId('save-button'));
});

expect(chatSendSpy).toHaveBeenLastCalledWith(
expect.objectContaining({
currentConversation: {
apiConfig: { newProp: true },
category: 'assistant',
id: mockData.welcome_id.id,
messages: [],
title: 'Welcome',
replacements: {},
},
})
);
});

it('should refetchCurrentUserConversations after settings save button click, but do not update convos when refetch returns bad results', async () => {
jest.mocked(useFetchCurrentUserConversations).mockReturnValue({
data: mockData,
isLoading: false,
refetch: jest.fn().mockResolvedValue({
isLoading: false,
data: omit(mockData, 'welcome_id'),
}),
isFetched: true,
} as unknown as DefinedUseQueryResult<Record<string, Conversation>, unknown>);
const chatSendSpy = jest.spyOn(all, 'useChatSend');
await renderAssistant();

fireEvent.click(screen.getByTestId('settings'));
await act(async () => {
fireEvent.click(screen.getByTestId('save-button'));
});

expect(chatSendSpy).toHaveBeenLastCalledWith(
expect.objectContaining({
currentConversation: {
apiConfig: { connectorId: '123' },
replacements: {},
category: 'assistant',
id: mockData.welcome_id.id,
messages: [],
title: 'Welcome',
},
})
);
});

it('should delete conversation when delete button is clicked', async () => {
await renderAssistant();
const deleteButton = screen.getAllByTestId('delete-option')[0];
await act(async () => {
fireEvent.click(deleteButton);
});

await act(async () => {
fireEvent.click(screen.getByTestId('confirmModalConfirmButton'));
});

await waitFor(() => {
expect(mockDeleteConvo).toHaveBeenCalledWith(mockData.electric_sheep_id.id);
});
});
it('should refetchCurrentUserConversations after clear chat history button click', async () => {
await renderAssistant();
fireEvent.click(screen.getByTestId('chat-context-menu'));
fireEvent.click(screen.getByTestId('clear-chat'));
fireEvent.click(screen.getByTestId('confirmModalConfirmButton'));
await waitFor(() => {
expect(clearConversation).toHaveBeenCalled();
expect(refetchResults).toHaveBeenCalled();
});
});
});

describe('when selected conversation changes and some connectors are loaded', () => {
it('should persist the conversation id to local storage', async () => {
const getConversation = jest.fn().mockResolvedValue(mockData.electric_sheep_id);
Expand Down
13 changes: 12 additions & 1 deletion x-pack/packages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const mockContext = {
basePromptContexts: MOCK_QUICK_PROMPTS,
setSelectedSettingsTab,
http: {},
assistantFeatures: { assistantModelEvaluation: true },
assistantFeatures: { assistantModelEvaluation: true, assistantKnowledgeBaseByDefault: false },
selectedSettingsTab: 'CONVERSATIONS_TAB',
assistantAvailability: {
isAssistantEnabled: true,
Expand Down Expand Up @@ -136,6 +136,17 @@ describe('AssistantSettings', () => {
QUICK_PROMPTS_TAB,
SYSTEM_PROMPTS_TAB,
])('%s', (tab) => {
it('Opens the tab on button click', () => {
(useAssistantContext as jest.Mock).mockImplementation(() => ({
...mockContext,
selectedSettingsTab: tab === CONVERSATIONS_TAB ? ANONYMIZATION_TAB : CONVERSATIONS_TAB,
}));
const { getByTestId } = render(<AssistantSettings {...testProps} />, {
wrapper,
});
fireEvent.click(getByTestId(`${tab}-button`));
expect(setSelectedSettingsTab).toHaveBeenCalledWith(tab);
});
it('renders with the correct tab open', () => {
(useAssistantContext as jest.Mock).mockImplementation(() => ({
...mockContext,
Expand Down
124 changes: 123 additions & 1 deletion x-pack/packages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';
import {
EuiButton,
EuiButtonEmpty,
EuiIcon,
EuiModal,
EuiModalFooter,
EuiKeyPadMenu,
EuiKeyPadMenuItem,
EuiPage,
EuiPageBody,
EuiPageSidebar,
EuiSplitPanel,
} from '@elastic/eui';

Expand Down Expand Up @@ -76,7 +80,16 @@ export const AssistantSettings: React.FC<Props> = React.memo(
conversations,
conversationsLoaded,
}) => {
const { http, toasts, selectedSettingsTab, setSelectedSettingsTab } = useAssistantContext();
const {
assistantFeatures: {
assistantModelEvaluation: modelEvaluatorEnabled,
assistantKnowledgeBaseByDefault,
},
http,
toasts,
selectedSettingsTab,
setSelectedSettingsTab,
} = useAssistantContext();

useEffect(() => {
if (selectedSettingsTab == null) {
Expand Down Expand Up @@ -201,6 +214,115 @@ export const AssistantSettings: React.FC<Props> = React.memo(
return (
<StyledEuiModal data-test-subj={TEST_IDS.SETTINGS_MODAL} onClose={onClose}>
<EuiPage paddingSize="none">
{!assistantKnowledgeBaseByDefault && (
<EuiPageSidebar
paddingSize="xs"
css={css`
min-inline-size: unset !important;
max-width: 104px;
`}
>
<EuiKeyPadMenu>
<EuiKeyPadMenuItem
id={CONVERSATIONS_TAB}
label={i18n.CONVERSATIONS_MENU_ITEM}
isSelected={!selectedSettingsTab || selectedSettingsTab === CONVERSATIONS_TAB}
onClick={() => setSelectedSettingsTab(CONVERSATIONS_TAB)}
data-test-subj={`${CONVERSATIONS_TAB}-button`}
>
<>
<EuiIcon
type="editorComment"
size="xl"
css={css`
position: relative;
top: -10px;
`}
/>
<EuiIcon
type="editorComment"
size="l"
css={css`
position: relative;
transform: rotateY(180deg);
top: -7px;
`}
/>
</>
</EuiKeyPadMenuItem>
<EuiKeyPadMenuItem
id={QUICK_PROMPTS_TAB}
label={i18n.QUICK_PROMPTS_MENU_ITEM}
isSelected={selectedSettingsTab === QUICK_PROMPTS_TAB}
onClick={() => setSelectedSettingsTab(QUICK_PROMPTS_TAB)}
data-test-subj={`${QUICK_PROMPTS_TAB}-button`}
>
<>
<EuiIcon type="editorComment" size="xxl" />
<EuiIcon
type="bolt"
size="s"
color="warning"
css={css`
position: absolute;
top: 11px;
left: 14px;
`}
/>
</>
</EuiKeyPadMenuItem>
<EuiKeyPadMenuItem
id={SYSTEM_PROMPTS_TAB}
label={i18n.SYSTEM_PROMPTS_MENU_ITEM}
isSelected={selectedSettingsTab === SYSTEM_PROMPTS_TAB}
onClick={() => setSelectedSettingsTab(SYSTEM_PROMPTS_TAB)}
data-test-subj={`${SYSTEM_PROMPTS_TAB}-button`}
>
<EuiIcon type="editorComment" size="xxl" />
<EuiIcon
type="storage"
size="s"
color="success"
css={css`
position: absolute;
top: 11px;
left: 14px;
`}
/>
</EuiKeyPadMenuItem>
<EuiKeyPadMenuItem
id={ANONYMIZATION_TAB}
label={i18n.ANONYMIZATION_MENU_ITEM}
isSelected={selectedSettingsTab === ANONYMIZATION_TAB}
onClick={() => setSelectedSettingsTab(ANONYMIZATION_TAB)}
data-test-subj={`${ANONYMIZATION_TAB}-button`}
>
<EuiIcon type="eyeClosed" size="l" />
</EuiKeyPadMenuItem>
<EuiKeyPadMenuItem
id={KNOWLEDGE_BASE_TAB}
label={i18n.KNOWLEDGE_BASE_MENU_ITEM}
isSelected={selectedSettingsTab === KNOWLEDGE_BASE_TAB}
onClick={() => setSelectedSettingsTab(KNOWLEDGE_BASE_TAB)}
data-test-subj={`${KNOWLEDGE_BASE_TAB}-button`}
>
<EuiIcon type="notebookApp" size="l" />
</EuiKeyPadMenuItem>
{modelEvaluatorEnabled && (
<EuiKeyPadMenuItem
id={EVALUATION_TAB}
label={i18n.EVALUATION_MENU_ITEM}
isSelected={selectedSettingsTab === EVALUATION_TAB}
onClick={() => setSelectedSettingsTab(EVALUATION_TAB)}
data-test-subj={`${EVALUATION_TAB}-button`}
>
<EuiIcon type="crossClusterReplicationApp" size="l" />
</EuiKeyPadMenuItem>
)}
</EuiKeyPadMenu>
</EuiPageSidebar>
)}

<EuiPageBody paddingSize="none" panelled={true}>
<EuiSplitPanel.Outer grow={true}>
<EuiSplitPanel.Inner
Expand Down
9 changes: 9 additions & 0 deletions ...packages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings_button.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { OpenAiProviderType } from '@kbn/stack-connectors-plugin/common/openai/c

import { AssistantSettingsButton } from './assistant_settings_button';
import { welcomeConvo } from '../../mock/conversation';
import { CONVERSATIONS_TAB } from './const';

const setIsSettingsModalVisible = jest.fn();
const onConversationSelected = jest.fn();
Expand All @@ -31,6 +32,7 @@ const testProps = {
const setSelectedSettingsTab = jest.fn();
const mockUseAssistantContext = {
setSelectedSettingsTab,
assistantFeatures: {},
};
jest.mock('../../assistant_context', () => {
const original = jest.requireActual('../../assistant_context');
Expand All @@ -57,6 +59,13 @@ describe('AssistantSettingsButton', () => {
jest.clearAllMocks();
});

it('Clicking the settings gear opens the conversations tab', () => {
const { getByTestId } = render(<AssistantSettingsButton {...testProps} />);
fireEvent.click(getByTestId('settings'));
expect(setSelectedSettingsTab).toHaveBeenCalledWith(CONVERSATIONS_TAB);
expect(setIsSettingsModalVisible).toHaveBeenCalledWith(true);
});

it('Settings modal is visible and calls correct actions per click', () => {
const { getByTestId } = render(
<AssistantSettingsButton {...testProps} isSettingsModalVisible />
Expand Down
Loading

0 comments on commit 18b517a

Please sign in to comment.