Skip to content

Commit

Permalink
[8.x] Kb settings followup (elastic#195733) (elastic#196477)
Browse files Browse the repository at this point in the history 
# Backport

This will backport the following commits from `main` to `8.x`:
- [Kb settings followup
(elastic#195733)](elastic#195733)

<!--- 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-16T03:41:57Z","message":"Kb
settings followup
(elastic#195733)","sha":"983a3e5723f7c2ab6e33663e03355f431723b1b5","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["v9.0.0","release_note:feature","Feature:Security
Assistant","Team:Security Generative
AI","v8.16.0","backport:version"],"title":"Kb settings
followup","number":195733,"url":"https://github.com/elastic/kibana/pull/195733","mergeCommit":{"message":"Kb
settings followup
(elastic#195733)","sha":"983a3e5723f7c2ab6e33663e03355f431723b1b5"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195733","number":195733,"mergeCommit":{"message":"Kb
settings followup
(elastic#195733)","sha":"983a3e5723f7c2ab6e33663e03355f431723b1b5"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.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 16, 2024
1 parent 8514556 commit cbd40a8
Show file tree
Hide file tree
Showing 59 changed files with 1,458 additions and 794 deletions.
12 changes: 12 additions & 0 deletions src/plugins/data_views/public/data_views/data_views_api_client.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ describe('IndexPatternsApiClient', () => {
let indexPatternsApiClient: DataViewsApiClient;

beforeEach(() => {
jest.clearAllMocks();
fetchSpy = jest.spyOn(http, 'fetch').mockImplementation(() => Promise.resolve({}));
indexPatternsApiClient = new DataViewsApiClient(http as HttpSetup, () =>
Promise.resolve(undefined)
Expand Down Expand Up @@ -46,4 +47,15 @@ describe('IndexPatternsApiClient', () => {
version: '1', // version header
});
});

test('Correctly formats fieldTypes argument', async function () {
const fieldTypes = ['text', 'keyword'];
await indexPatternsApiClient.getFieldsForWildcard({
pattern: 'blah',
fieldTypes,
allowHidden: false,
});

expect(fetchSpy.mock.calls[0][1].query.field_types).toEqual(fieldTypes);
});
});
2 changes: 1 addition & 1 deletion src/plugins/data_views/public/data_views/data_views_api_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class DataViewsApiClient implements IDataViewsApiClient {
allow_no_index: allowNoIndex,
include_unmapped: includeUnmapped,
fields,
fieldTypes,
field_types: fieldTypes,
// default to undefined to keep value out of URL params and improve caching
allow_hidden: allowHidden || undefined,
include_empty_fields: includeEmptyFields,
Expand Down
12 changes: 2 additions & 10 deletions x-pack/packages/kbn-elastic-assistant/impl/assistant/assistant_overlay/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ import useEvent from 'react-use/lib/useEvent';
import { css } from '@emotion/react';
// eslint-disable-next-line @kbn/eslint/module_migration
import { createGlobalStyle } from 'styled-components';
import {
ShowAssistantOverlayProps,
useAssistantContext,
UserAvatar,
} from '../../assistant_context';
import { ShowAssistantOverlayProps, useAssistantContext } from '../../assistant_context';
import { Assistant, CONVERSATION_SIDE_PANEL_WIDTH } from '..';

const isMac = navigator.platform.toLowerCase().indexOf('mac') >= 0;
Expand All @@ -25,17 +21,14 @@ const isMac = navigator.platform.toLowerCase().indexOf('mac') >= 0;
* Modal container for Elastic AI Assistant conversations, receiving the page contents as context, plus whatever
* component currently has focus and any specific context it may provide through the SAssInterface.
*/
export interface Props {
currentUserAvatar?: UserAvatar;
}

export const UnifiedTimelineGlobalStyles = createGlobalStyle`
body:has(.timeline-portal-overlay-mask) .euiOverlayMask {
z-index: 1003 !important;
}
`;

export const AssistantOverlay = React.memo<Props>(({ currentUserAvatar }) => {
export const AssistantOverlay = React.memo(() => {
const [isModalVisible, setIsModalVisible] = useState(false);
// Why is this named Title and not Id?
const [conversationTitle, setConversationTitle] = useState<string | undefined>(undefined);
Expand Down Expand Up @@ -144,7 +137,6 @@ export const AssistantOverlay = React.memo<Props>(({ currentUserAvatar }) => {
onCloseFlyout={handleCloseModal}
chatHistoryVisible={chatHistoryVisible}
setChatHistoryVisible={toggleChatHistory}
currentUserAvatar={currentUserAvatar}
/>
</EuiFlyoutResizable>
<UnifiedTimelineGlobalStyles />
Expand Down
4 changes: 4 additions & 0 deletions ...assistant/impl/assistant/common/components/assistant_settings_management/flyout/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ interface Props {
onSaveCancelled: () => void;
onSaveConfirmed: () => void;
saveButtonDisabled?: boolean;
saveButtonLoading?: boolean;
}

const FlyoutComponent: React.FC<Props> = ({
Expand All @@ -38,9 +39,11 @@ const FlyoutComponent: React.FC<Props> = ({
onSaveCancelled,
onSaveConfirmed,
saveButtonDisabled = false,
saveButtonLoading = false,
}) => {
return flyoutVisible ? (
<EuiFlyout
data-test-subj={'flyout'}
ownFocus
onClose={onClose}
css={css`
Expand Down Expand Up @@ -74,6 +77,7 @@ const FlyoutComponent: React.FC<Props> = ({
onClick={onSaveConfirmed}
iconType="check"
disabled={saveButtonDisabled}
isLoading={saveButtonLoading}
fill
>
{i18n.FLYOUT_SAVE_BUTTON_TITLE}
Expand Down
1 change: 1 addition & 0 deletions ...t/impl/assistant/common/components/assistant_settings_management/inline_actions/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ export const useInlineActions = <T extends { isDefault?: boolean | undefined }>(
},
{
name: i18n.DELETE_BUTTON,
'data-test-subj': 'delete-button',
description: i18n.DELETE_BUTTON,
icon: 'trash',
type: 'icon',
Expand Down
106 changes: 0 additions & 106 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,7 +23,6 @@ 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 @@ -139,111 +138,6 @@ 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
8 changes: 2 additions & 6 deletions x-pack/packages/kbn-elastic-assistant/impl/assistant/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import { ChatSend } from './chat_send';
import { WELCOME_CONVERSATION_TITLE } from './use_conversation/translations';
import { getDefaultConnector } from './helpers';

import { useAssistantContext, UserAvatar } from '../assistant_context';
import { useAssistantContext } from '../assistant_context';
import { ContextPills } from './context_pills';
import { getNewSelectedPromptContext } from '../data_anonymization/get_new_selected_prompt_context';
import type { PromptContext, SelectedPromptContext } from './prompt_context/types';
Expand All @@ -61,7 +61,6 @@ const CommentContainer = styled('span')`
export interface Props {
chatHistoryVisible?: boolean;
conversationTitle?: string;
currentUserAvatar?: UserAvatar;
onCloseFlyout?: () => void;
promptContextId?: string;
setChatHistoryVisible?: Dispatch<SetStateAction<boolean>>;
Expand All @@ -75,7 +74,6 @@ export interface Props {
const AssistantComponent: React.FC<Props> = ({
chatHistoryVisible,
conversationTitle,
currentUserAvatar,
onCloseFlyout,
promptContextId = '',
setChatHistoryVisible,
Expand All @@ -90,12 +88,10 @@ const AssistantComponent: React.FC<Props> = ({
getLastConversationId,
http,
promptContexts,
setCurrentUserAvatar,
currentUserAvatar,
setLastConversationId,
} = useAssistantContext();

setCurrentUserAvatar(currentUserAvatar);

const [selectedPromptContexts, setSelectedPromptContexts] = useState<
Record<string, SelectedPromptContext>
>({});
Expand Down
4 changes: 2 additions & 2 deletions ...es/kbn-elastic-assistant/impl/assistant/settings/alerts_settings/alerts_settings.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ describe('AlertsSettings', () => {
);

const rangeSlider = screen.getByTestId('alertsRange');
fireEvent.change(rangeSlider, { target: { value: '10' } });
fireEvent.change(rangeSlider, { target: { value: '90' } });

expect(setUpdatedKnowledgeBaseSettings).toHaveBeenCalledWith({
latestAlerts: 10,
latestAlerts: 90,
});
});
});
63 changes: 63 additions & 0 deletions ...s/kbn-elastic-assistant/impl/assistant/settings/alerts_settings/alerts_settings_modal.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { useCallback } from 'react';
import {
EuiButton,
EuiButtonEmpty,
EuiModal,
EuiModalBody,
EuiModalFooter,
EuiModalHeader,
EuiModalHeaderTitle,
} from '@elastic/eui';
import { ALERTS_LABEL } from '../../../knowledge_base/translations';
import {
DEFAULT_CONVERSATIONS,
DEFAULT_PROMPTS,
useSettingsUpdater,
} from '../use_settings_updater/use_settings_updater';
import { AlertsSettings } from './alerts_settings';
import { CANCEL, SAVE } from '../translations';

interface AlertSettingsModalProps {
onClose: () => void;
}

export const AlertsSettingsModal = ({ onClose }: AlertSettingsModalProps) => {
const { knowledgeBase, setUpdatedKnowledgeBaseSettings, saveSettings } = useSettingsUpdater(
DEFAULT_CONVERSATIONS, // Alerts settings do not require conversations
DEFAULT_PROMPTS, // Alerts settings do not require prompts
false, // Alerts settings do not require conversations
false // Alerts settings do not require prompts
);

const handleSave = useCallback(() => {
saveSettings();
onClose();
}, [onClose, saveSettings]);

return (
<EuiModal onClose={onClose}>
<EuiModalHeader>
<EuiModalHeaderTitle>{ALERTS_LABEL}</EuiModalHeaderTitle>
</EuiModalHeader>
<EuiModalBody>
<AlertsSettings
knowledgeBase={knowledgeBase}
setUpdatedKnowledgeBaseSettings={setUpdatedKnowledgeBaseSettings}
/>
</EuiModalBody>
<EuiModalFooter>
<EuiButtonEmpty onClick={onClose}>{CANCEL}</EuiButtonEmpty>
<EuiButton type="submit" onClick={handleSave} fill>
{SAVE}
</EuiButton>
</EuiModalFooter>
</EuiModal>
);
};
23 changes: 6 additions & 17 deletions 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 @@ -64,12 +64,12 @@ jest.mock('../../assistant_context');

jest.mock('.', () => {
return {
AnonymizationSettings: () => <span data-test-subj="ANONYMIZATION_TAB-tab" />,
ConversationSettings: () => <span data-test-subj="CONVERSATIONS_TAB-tab" />,
EvaluationSettings: () => <span data-test-subj="EVALUATION_TAB-tab" />,
KnowledgeBaseSettings: () => <span data-test-subj="KNOWLEDGE_BASE_TAB-tab" />,
QuickPromptSettings: () => <span data-test-subj="QUICK_PROMPTS_TAB-tab" />,
SystemPromptSettings: () => <span data-test-subj="SYSTEM_PROMPTS_TAB-tab" />,
AnonymizationSettings: () => <span data-test-subj="anonymization-tab" />,
ConversationSettings: () => <span data-test-subj="conversations-tab" />,
EvaluationSettings: () => <span data-test-subj="evaluation-tab" />,
KnowledgeBaseSettings: () => <span data-test-subj="knowledge_base-tab" />,
QuickPromptSettings: () => <span data-test-subj="quick_prompts-tab" />,
SystemPromptSettings: () => <span data-test-subj="system_prompts-tab" />,
};
});

Expand Down Expand Up @@ -136,17 +136,6 @@ 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
Loading

0 comments on commit cbd40a8

Please sign in to comment.