Skip to content

Commit

Permalink
Security Assistant 8.14 BC1 fixes (elastic#181410)
Browse files Browse the repository at this point in the history
## Summary
 
BC1 Security Assistant fixes
- View in assistant in Attack discovery now properly propagates the
title and the context of the conversation
- Fixed spacing around the Icon on the Welcome screen
- Fixed clearing the text input after the message was sent
- Fixed showing proper dates for the comments in `isStreaming` mode
- Fixed scrolling to the bottom in `isFlyoutMode`
- Added `Add connector` button when the Connector selector was empty
- Extracted `View in assistant` logic to the separate hook

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
patrykkopycinski and kibanamachine authored Apr 26, 2024
1 parent 2654a61 commit 4ae9f39
Show file tree
Hide file tree
Showing 30 changed files with 260 additions and 193 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ export const ConversationUpdateProps = z.object({

export type ConversationCreateProps = z.infer<typeof ConversationCreateProps>;
export const ConversationCreateProps = z.object({
/**
* The conversation id.
*/
id: z.string().optional(),
/**
* The conversation title.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ components:
required:
- title
properties:
id:
type: string
description: The conversation id.
title:
type: string
description: The conversation title.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ const Container = styled.div`
display: flex;
justify-content: center;
align-items: center;
margin-top: ${euiThemeVars.euiSizeXXL};
margin-bottom: ${euiThemeVars.euiSizeL};
:before,
:after {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,11 @@ import { TestProviders } from '../../mock/test_providers/test_providers';

jest.mock('./use_chat_send');

const handleButtonSendMessage = jest.fn();
const handleOnChatCleared = jest.fn();
const handlePromptChange = jest.fn();
const handleSendMessage = jest.fn();
const handleRegenerateResponse = jest.fn();
const testProps: Props = {
handleButtonSendMessage,
handleOnChatCleared,
handlePromptChange,
handleSendMessage,
Expand Down Expand Up @@ -51,7 +49,7 @@ describe('ChatSend', () => {
expect(getByTestId('prompt-textarea')).toHaveTextContent(promptText);
fireEvent.click(getByTestId('submit-chat'));
await waitFor(() => {
expect(handleButtonSendMessage).toHaveBeenCalledWith(promptText);
expect(handleSendMessage).toHaveBeenCalledWith(promptText);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ export interface Props extends Omit<UseChatSend, 'abortStream'> {
* Allows the user to clear the chat and switch between different system prompts.
*/
export const ChatSend: React.FC<Props> = ({
handleButtonSendMessage,
handleOnChatCleared,
handlePromptChange,
handleSendMessage,
Expand All @@ -46,11 +45,16 @@ export const ChatSend: React.FC<Props> = ({
const promptValue = useMemo(() => (isDisabled ? '' : userPrompt ?? ''), [isDisabled, userPrompt]);

const onSendMessage = useCallback(() => {
handleButtonSendMessage(promptTextAreaRef.current?.value?.trim() ?? '');
}, [handleButtonSendMessage, promptTextAreaRef]);
handleSendMessage(promptTextAreaRef.current?.value?.trim() ?? '');
handlePromptChange('');
}, [handleSendMessage, promptTextAreaRef, handlePromptChange]);

useAutosizeTextArea(promptTextAreaRef?.current, promptValue);

useEffect(() => {
handlePromptChange(promptValue);
}, [handlePromptChange, promptValue]);

return (
<EuiFlexGroup
gutterSize="none"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,12 @@ describe('use chat send', () => {
expect(setPromptTextPreview).toHaveBeenCalledWith('new prompt');
expect(setUserPrompt).toHaveBeenCalledWith('new prompt');
});
it('handleButtonSendMessage sends message with context prompt when a valid prompt text is provided', async () => {
it('handleSendMessage sends message with context prompt when a valid prompt text is provided', async () => {
const promptText = 'prompt text';
const { result } = renderHook(() => useChatSend(testProps), {
wrapper: TestProviders,
});
result.current.handleButtonSendMessage(promptText);
expect(setUserPrompt).toHaveBeenCalledWith('');
result.current.handleSendMessage(promptText);

await waitFor(() => {
expect(sendMessage).toHaveBeenCalled();
Expand All @@ -108,7 +107,7 @@ describe('use chat send', () => {
);
});
});
it('handleButtonSendMessage sends message with only provided prompt text and context already exists in convo history', async () => {
it('handleSendMessage sends message with only provided prompt text and context already exists in convo history', async () => {
const promptText = 'prompt text';
const { result } = renderHook(
() =>
Expand All @@ -118,8 +117,7 @@ describe('use chat send', () => {
}
);

result.current.handleButtonSendMessage(promptText);
expect(setUserPrompt).toHaveBeenCalledWith('');
result.current.handleSendMessage(promptText);

await waitFor(() => {
expect(sendMessage).toHaveBeenCalled();
Expand Down Expand Up @@ -150,8 +148,7 @@ describe('use chat send', () => {
const { result } = renderHook(() => useChatSend(testProps), {
wrapper: TestProviders,
});
result.current.handleButtonSendMessage(promptText);
expect(setUserPrompt).toHaveBeenCalledWith('');
result.current.handleSendMessage(promptText);

await waitFor(() => {
expect(reportAssistantMessageSent).toHaveBeenNthCalledWith(1, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export interface UseChatSendProps {

export interface UseChatSend {
abortStream: () => void;
handleButtonSendMessage: (m: string) => void;
handleOnChatCleared: () => void;
handlePromptChange: (prompt: string) => void;
handleSendMessage: (promptText: string) => void;
Expand Down Expand Up @@ -209,14 +208,6 @@ export const useChatSend = ({
});
}, [currentConversation, http, removeLastMessage, sendMessage, setCurrentConversation, toasts]);

const handleButtonSendMessage = useCallback(
(message: string) => {
handleSendMessage(message);
setUserPrompt('');
},
[handleSendMessage, setUserPrompt]
);

const handleOnChatCleared = useCallback(async () => {
const defaultSystemPromptId = getDefaultSystemPrompt({
allSystemPrompts,
Expand Down Expand Up @@ -246,7 +237,6 @@ export const useChatSend = ({

return {
abortStream,
handleButtonSendMessage,
handleOnChatCleared,
handlePromptChange,
handleSendMessage,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ export const ConversationSettings: React.FC<ConversationSettingsProps> = React.m
isDisabled={isDisabled}
onConnectorSelectionChange={handleOnConnectorSelectionChange}
selectedConnectorId={selectedConnector?.id}
isFlyoutMode={isFlyoutMode}
/>
</EuiFormRow>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const getMessageFromRawResponse = (
rawResponse: FetchConnectorExecuteResponse
): ClientMessage => {
const { response, isStream, isError } = rawResponse;
const dateTimeString = new Date().toLocaleString(); // TODO: Pull from response
const dateTimeString = new Date().toISOString(); // TODO: Pull from response
if (rawResponse) {
return {
role: 'assistant',
Expand Down
18 changes: 11 additions & 7 deletions x-pack/packages/kbn-elastic-assistant/impl/assistant/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ const AssistantComponent: React.FC<Props> = ({
} = useFetchAnonymizationFields();

// Connector details
const { data: connectors, isFetched: areConnectorsFetched } = useLoadConnectors({
const { data: connectors, isFetchedAfterMount: areConnectorsFetched } = useLoadConnectors({
http,
});
const defaultConnector = useMemo(() => getDefaultConnector(connectors), [connectors]);
Expand All @@ -208,6 +208,10 @@ const AssistantComponent: React.FC<Props> = ({
if (conversationId) {
const updatedConversation = await getConversation(conversationId);

if (updatedConversation) {
setCurrentConversation(updatedConversation);
}

return updatedConversation;
}
},
Expand Down Expand Up @@ -358,6 +362,12 @@ const AssistantComponent: React.FC<Props> = ({
}
// when scrollHeight changes, parent is scrolled to bottom
parent.scrollTop = parent.scrollHeight;

if (isFlyoutMode) {
(
commentsContainerRef.current?.childNodes[0].childNodes[0] as HTMLElement
).lastElementChild?.scrollIntoView();
}
});

const getWrapper = (children: React.ReactNode, isCommentContainer: boolean) =>
Expand Down Expand Up @@ -390,9 +400,6 @@ const AssistantComponent: React.FC<Props> = ({
setEditingSystemPromptId(
getDefaultSystemPrompt({ allSystemPrompts, conversation: refetchedConversation })?.id
);
if (refetchedConversation) {
setCurrentConversation(refetchedConversation);
}
setCurrentConversationId(cId);
}
},
Expand Down Expand Up @@ -521,7 +528,6 @@ const AssistantComponent: React.FC<Props> = ({

const {
abortStream,
handleButtonSendMessage,
handleOnChatCleared,
handlePromptChange,
handleSendMessage,
Expand Down Expand Up @@ -1002,7 +1008,6 @@ const AssistantComponent: React.FC<Props> = ({
isDisabled={isSendingDisabled}
shouldRefocusPrompt={shouldRefocusPrompt}
userPrompt={userPrompt}
handleButtonSendMessage={handleChatSend}
handleOnChatCleared={handleOnChatCleared}
handlePromptChange={handlePromptChange}
handleSendMessage={handleChatSend}
Expand Down Expand Up @@ -1122,7 +1127,6 @@ const AssistantComponent: React.FC<Props> = ({
isDisabled={isSendingDisabled}
shouldRefocusPrompt={shouldRefocusPrompt}
userPrompt={userPrompt}
handleButtonSendMessage={handleButtonSendMessage}
handleOnChatCleared={handleOnChatCleared}
handlePromptChange={handlePromptChange}
handleSendMessage={handleChatSend}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function getCombinedMessage({
// trim ensures any extra \n and other whitespace is removed
content: content.trim(),
role: 'user', // we are combining the system and user messages into one message
timestamp: new Date().toLocaleString(),
timestamp: new Date().toISOString(),
replacements,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import { EuiTextArea } from '@elastic/eui';
import React, { useCallback, useEffect, forwardRef } from 'react';
import React, { useCallback, forwardRef } from 'react';
import { css } from '@emotion/react';

import * as i18n from './translations';
Expand Down Expand Up @@ -42,10 +42,6 @@ export const PromptTextArea = forwardRef<HTMLTextAreaElement, Props>(
[value, onPromptSubmit, handlePromptChange]
);

useEffect(() => {
handlePromptChange(value);
}, [handlePromptChange, value]);

return (
<EuiTextArea
css={css`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { act, renderHook } from '@testing-library/react-hooks';

import { useAssistantOverlay } from '.';
import { waitFor } from '@testing-library/react';

const mockUseAssistantContext = {
registerPromptContext: jest.fn(),
Expand All @@ -22,6 +23,24 @@ jest.mock('../../assistant_context', () => {
useAssistantContext: () => mockUseAssistantContext,
};
});
jest.mock('../use_conversation', () => {
return {
useConversation: jest.fn(() => ({
currentConversation: { id: 'conversation-id' },
})),
};
});
jest.mock('../helpers');
jest.mock('../../connectorland/helpers');
jest.mock('../../connectorland/use_load_connectors', () => {
return {
useLoadConnectors: jest.fn(() => ({
data: [],
error: null,
isSuccess: true,
})),
};
});

describe('useAssistantOverlay', () => {
beforeEach(() => {
Expand All @@ -48,13 +67,15 @@ describe('useAssistantOverlay', () => {
)
);

expect(mockUseAssistantContext.registerPromptContext).toHaveBeenCalledWith({
category,
description,
getPromptContext,
id,
suggestedUserPrompt,
tooltip,
await waitFor(() => {
expect(mockUseAssistantContext.registerPromptContext).toHaveBeenCalledWith({
category,
description,
getPromptContext,
id,
suggestedUserPrompt,
tooltip,
});
});
});

Expand Down
Loading

0 comments on commit 4ae9f39

Please sign in to comment.