[Security Assistant] V2 Knowledge Base Settings feedback and fixes (e…

…lastic#194354) ## Summary This PR is a follow up to elastic#192665 and addresses a bunch of feedback and fixes including: - [X] Adds support for updating/editing entries - [X] Fixes initial loading experience of the KB Settings Setup/Table - [X] Fixes two bugs where `semantic_text` and `text` must be declared for `IndexEntries` to work - [X] Add new Settings Context Menu items for KB and Alerts - [X] Add support for `required` entries in initial prompt * See [this trace](https://smith.langchain.com/public/84a17a31-8ce8-4bd9-911e-38a854484dd8/r) for included knowledge. Note that the KnowledgeBaseRetrievalTool was not selected. * Note: All prompts were updated to include the `{knowledge_history}` placeholder, and _not behind the feature flag_, as this will just be the empty case until the feature flag is enabled. TODO (in this or follow-up PR): - [ ] Add suggestions to `index` and `fields` inputs - [ ] Adds URL deeplinking to securityAssistantManagement - [ ] Fix bug where updating entry does not re-create embeddings (see [comment](elastic#194354 (comment))) - [ ] Fix loading indicators when adding/editing entries - [ ] API integration tests for update API (@e40pud) ### Checklist Delete any items that are not applicable to this PR. - [X] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials * Docs being tracked in elastic/security-docs#5337 for when feature flag is enabled - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Patryk Kopycinski <contact@patrykkopycinski.com> (cherry picked from commit 7df3672)
kibanamachine · Oct 9, 2024 · 65d1a9d · 65d1a9d
1 parent 7cf64b7
commit 65d1a9d
Show file tree

Hide file tree

Showing 32 changed files with 686 additions and 143 deletions.
diff --git a/...kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/common_attributes.gen.ts b/...kbn-elastic-assistant-common/impl/schemas/knowledge_base/entries/common_attributes.gen.ts
@@ -106,7 +106,11 @@ export type BaseCreateProps = z.infer<typeof BaseCreateProps>;
 export const BaseCreateProps = BaseRequiredFields.merge(BaseDefaultableFields);
 
 export type BaseUpdateProps = z.infer<typeof BaseUpdateProps>;
-export const BaseUpdateProps = BaseCreateProps.partial();
+export const BaseUpdateProps = BaseCreateProps.partial().merge(
+  z.object({
+    id: NonEmptyString,
+  })
+);
 
 export type BaseResponseProps = z.infer<typeof BaseResponseProps>;
 export const BaseResponseProps = BaseRequiredFields.merge(BaseDefaultableFields.required());

diff --git a/...lastic-assistant-common/impl/schemas/knowledge_base/entries/common_attributes.schema.yaml b/...lastic-assistant-common/impl/schemas/knowledge_base/entries/common_attributes.schema.yaml
@@ -112,6 +112,12 @@ components:
       allOf:
         - $ref: "#/components/schemas/BaseCreateProps"
           x-modify: partial
+        - type: object
+          properties:
+            id:
+              $ref: "../../common_attributes.schema.yaml#/components/schemas/NonEmptyString"
+          required:
+            - id
 
     BaseResponseProps:
       x-inline: true

diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/assistant_header/index.tsx b/x-pack/packages/kbn-elastic-assistant/impl/assistant/assistant_header/index.tsx
@@ -5,16 +5,13 @@
  * 2.0.
  */
 
-import React, { useState, useMemo, useCallback } from 'react';
+import React, { useMemo, useCallback } from 'react';
 import { QueryObserverResult, RefetchOptions, RefetchQueryFilters } from '@tanstack/react-query';
 import {
   EuiFlexGroup,
   EuiFlexItem,
-  EuiPopover,
-  EuiContextMenu,
   EuiButtonIcon,
   EuiPanel,
-  EuiConfirmModal,
   EuiToolTip,
   EuiSkeletonTitle,
 } from '@elastic/eui';
@@ -29,6 +26,7 @@ import { FlyoutNavigation } from '../assistant_overlay/flyout_navigation';
 import { AssistantSettingsButton } from '../settings/assistant_settings_button';
 import * as i18n from './translations';
 import { AIConnector } from '../../connectorland/connector_selector';
+import { SettingsContextMenu } from '../settings/settings_context_menu/settings_context_menu';
 
 interface OwnProps {
   selectedConversation: Conversation | undefined;
@@ -94,21 +92,6 @@ export const AssistantHeader: React.FC<Props> = ({
     [selectedConversation?.apiConfig?.connectorId]
   );
 
-  const [isPopoverOpen, setPopover] = useState(false);
-
-  const onButtonClick = useCallback(() => {
-    setPopover(!isPopoverOpen);
-  }, [isPopoverOpen]);
-
-  const closePopover = useCallback(() => {
-    setPopover(false);
-  }, []);
-
-  const [isResetConversationModalVisible, setIsResetConversationModalVisible] = useState(false);
-
-  const closeDestroyModal = useCallback(() => setIsResetConversationModalVisible(false), []);
-  const showDestroyModal = useCallback(() => setIsResetConversationModalVisible(true), []);
-
   const onConversationChange = useCallback(
     (updatedConversation: Conversation) => {
       onConversationSelected({
@@ -119,32 +102,6 @@ export const AssistantHeader: React.FC<Props> = ({
     [onConversationSelected]
   );
 
-  const panels = useMemo(
-    () => [
-      {
-        id: 0,
-        items: [
-          {
-            name: i18n.RESET_CONVERSATION,
-            css: css`
-              color: ${euiThemeVars.euiColorDanger};
-            `,
-            onClick: showDestroyModal,
-            icon: 'refresh',
-            'data-test-subj': 'clear-chat',
-          },
-        ],
-      },
-    ],
-    [showDestroyModal]
-  );
-
-  const handleReset = useCallback(() => {
-    onChatCleared();
-    closeDestroyModal();
-    closePopover();
-  }, [onChatCleared, closeDestroyModal, closePopover]);
-
   return (
     <>
       <FlyoutNavigation
@@ -246,42 +203,12 @@ export const AssistantHeader: React.FC<Props> = ({
                 </EuiToolTip>
               </EuiFlexItem>
               <EuiFlexItem>
-                <EuiPopover
-                  button={
-                    <EuiButtonIcon
-                      aria-label="test"
-                      isDisabled={isDisabled}
-                      iconType="boxesVertical"
-                      onClick={onButtonClick}
-                      data-test-subj="chat-context-menu"
-                    />
-                  }
-                  isOpen={isPopoverOpen}
-                  closePopover={closePopover}
-                  panelPaddingSize="none"
-                  anchorPosition="downLeft"
-                >
-                  <EuiContextMenu initialPanelId={0} panels={panels} />
-                </EuiPopover>
+                <SettingsContextMenu isDisabled={isDisabled} onChatCleared={onChatCleared} />
               </EuiFlexItem>
             </EuiFlexGroup>
           </EuiFlexItem>
         </EuiFlexGroup>
       </EuiPanel>
-      {isResetConversationModalVisible && (
-        <EuiConfirmModal
-          title={i18n.RESET_CONVERSATION}
-          onCancel={closeDestroyModal}
-          onConfirm={handleReset}
-          cancelButtonText={i18n.CANCEL_BUTTON_TEXT}
-          confirmButtonText={i18n.RESET_BUTTON_TEXT}
-          buttonColor="danger"
-          defaultFocusedButton="confirm"
-          data-test-subj="reset-conversation-modal"
-        >
-          <p>{i18n.CLEAR_CHAT_CONFIRMATION}</p>
-        </EuiConfirmModal>
-      )}
     </>
   );
 };
diff --git a/x-pack/packages/kbn-elastic-assistant/impl/assistant/assistant_header/translations.ts b/x-pack/packages/kbn-elastic-assistant/impl/assistant/assistant_header/translations.ts
@@ -7,6 +7,34 @@
 
 import { i18n } from '@kbn/i18n';
 
+export const AI_ASSISTANT_SETTINGS = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.aiAssistantSettings',
+  {
+    defaultMessage: 'AI Assistant settings',
+  }
+);
+
+export const ANONYMIZATION = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.anonymization',
+  {
+    defaultMessage: 'Anonymization',
+  }
+);
+
+export const KNOWLEDGE_BASE = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.knowledgeBase',
+  {
+    defaultMessage: 'Knowledge Base',
+  }
+);
+
+export const ALERTS_TO_ANALYZE = i18n.translate(
+  'xpack.elasticAssistant.assistant.settings.alertsToAnalyze',
+  {
+    defaultMessage: 'Alerts to analyze',
+  }
+);
+
 export const RESET_CONVERSATION = i18n.translate(
   'xpack.elasticAssistant.assistant.settings.resetConversation',
   {

diff --git a/.../alerts/settings/alerts_settings.test.tsx → .../alerts_settings/alerts_settings.test.tsx b/.../alerts/settings/alerts_settings.test.tsx → .../alerts_settings/alerts_settings.test.tsx
@@ -9,8 +9,8 @@ import { render, screen, fireEvent } from '@testing-library/react';
 import React from 'react';
 
 import { AlertsSettings } from './alerts_settings';
-import { KnowledgeBaseConfig } from '../../assistant/types';
-import { DEFAULT_LATEST_ALERTS } from '../../assistant_context/constants';
+import { KnowledgeBaseConfig } from '../../types';
+import { DEFAULT_LATEST_ALERTS } from '../../../assistant_context/constants';
 
 describe('AlertsSettings', () => {
   beforeEach(() => {

diff --git a/.../impl/alerts/settings/alerts_settings.tsx → ...tings/alerts_settings/alerts_settings.tsx b/.../impl/alerts/settings/alerts_settings.tsx → ...tings/alerts_settings/alerts_settings.tsx
@@ -9,9 +9,9 @@ import { EuiFlexGroup, EuiFormRow, EuiFlexItem, EuiSpacer, EuiText } from '@elas
 import { css } from '@emotion/react';
 import React from 'react';
 
-import { KnowledgeBaseConfig } from '../../assistant/types';
-import { AlertsRange } from '../../knowledge_base/alerts_range';
-import * as i18n from '../../knowledge_base/translations';
+import { KnowledgeBaseConfig } from '../../types';
+import { AlertsRange } from '../../../knowledge_base/alerts_range';
+import * as i18n from '../../../knowledge_base/translations';
 
 export const MIN_LATEST_ALERTS = 10;
 export const MAX_LATEST_ALERTS = 100;

diff --git a/...s/settings/alerts_settings_management.tsx → ...s_settings/alerts_settings_management.tsx b/...s/settings/alerts_settings_management.tsx → ...s_settings/alerts_settings_management.tsx
@@ -7,19 +7,24 @@
 
 import { EuiPanel, EuiSpacer, EuiText, EuiTitle } from '@elastic/eui';
 import React from 'react';
-import { KnowledgeBaseConfig } from '../../assistant/types';
-import { AlertsRange } from '../../knowledge_base/alerts_range';
-import * as i18n from '../../knowledge_base/translations';
+import { KnowledgeBaseConfig } from '../../types';
+import { AlertsRange } from '../../../knowledge_base/alerts_range';
+import * as i18n from '../../../knowledge_base/translations';
 
 interface Props {
   knowledgeBase: KnowledgeBaseConfig;
   setUpdatedKnowledgeBaseSettings: React.Dispatch<React.SetStateAction<KnowledgeBaseConfig>>;
+  hasBorder?: boolean;
 }
 
+/**
+ * Replaces the AlertsSettings component used in the existing settings modal. Once the modal is
+ * fully removed we can delete that component in favor of this one.
+ */
 export const AlertsSettingsManagement: React.FC<Props> = React.memo(
-  ({ knowledgeBase, setUpdatedKnowledgeBaseSettings }) => {
+  ({ knowledgeBase, setUpdatedKnowledgeBaseSettings, hasBorder = true }) => {
     return (
-      <EuiPanel hasShadow={false} hasBorder paddingSize="l" title={i18n.ALERTS_LABEL}>
+      <EuiPanel hasShadow={false} hasBorder={hasBorder} paddingSize="l" title={i18n.ALERTS_LABEL}>
         <EuiTitle size="m">
           <h3>{i18n.ALERTS_LABEL}</h3>
         </EuiTitle>

diff --git a/...ages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings_management.test.tsx b/...ages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings_management.test.tsx
@@ -25,6 +25,7 @@ import {
   SYSTEM_PROMPTS_TAB,
 } from './const';
 import { mockSystemPrompts } from '../../mock/system_prompt';
+import { DataViewsContract } from '@kbn/data-views-plugin/public';
 
 const mockConversations = {
   [alertConvo.title]: alertConvo,
@@ -53,8 +54,13 @@ const mockContext = {
   },
 };
 
+const mockDataViews = {
+  getIndices: jest.fn(),
+} as unknown as DataViewsContract;
+
 const testProps = {
   selectedConversation: welcomeConvo,
+  dataViews: mockDataViews,
 };
 jest.mock('../../assistant_context');
 

diff --git a/.../packages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings_management.tsx b/.../packages/kbn-elastic-assistant/impl/assistant/settings/assistant_settings_management.tsx
@@ -9,6 +9,7 @@ import React, { useEffect, useMemo } from 'react';
 import { EuiAvatar, EuiPageTemplate, EuiTitle, useEuiShadow, useEuiTheme } from '@elastic/eui';
 
 import { css } from '@emotion/react';
+import { DataViewsContract } from '@kbn/data-views-plugin/public';
 import { Conversation } from '../../..';
 import * as i18n from './translations';
 import { useAssistantContext } from '../../assistant_context';
@@ -33,6 +34,7 @@ import { KnowledgeBaseSettingsManagement } from '../../knowledge_base/knowledge_
 import { EvaluationSettings } from '.';
 
 interface Props {
+  dataViews: DataViewsContract;
   selectedConversation: Conversation;
 }
 
@@ -41,7 +43,7 @@ interface Props {
  * anonymization, knowledge base, and evaluation via the `isModelEvaluationEnabled` feature flag.
  */
 export const AssistantSettingsManagement: React.FC<Props> = React.memo(
-  ({ selectedConversation: defaultSelectedConversation }) => {
+  ({ dataViews, selectedConversation: defaultSelectedConversation }) => {
     const {
       assistantFeatures: { assistantModelEvaluation: modelEvaluatorEnabled },
       http,
@@ -158,7 +160,9 @@ export const AssistantSettingsManagement: React.FC<Props> = React.memo(
           )}
           {selectedSettingsTab === QUICK_PROMPTS_TAB && <QuickPromptSettingsManagement />}
           {selectedSettingsTab === ANONYMIZATION_TAB && <AnonymizationSettingsManagement />}
-          {selectedSettingsTab === KNOWLEDGE_BASE_TAB && <KnowledgeBaseSettingsManagement />}
+          {selectedSettingsTab === KNOWLEDGE_BASE_TAB && (
+            <KnowledgeBaseSettingsManagement dataViews={dataViews} />
+          )}
           {selectedSettingsTab === EVALUATION_TAB && <EvaluationSettings />}
         </EuiPageTemplate.Section>
       </>