Hide notebook feature when MDS enabled and remove security dashboard …

…API dependency calling (#201) (#202) * hide notebook feature when mds enabled Signed-off-by: tygao <[email protected]> * remove useraccess Signed-off-by: tygao <[email protected]> * add internal account api Signed-off-by: tygao <[email protected]> * test: update tests Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update mock function Signed-off-by: tygao <[email protected]> * update mock function Signed-off-by: tygao <[email protected]> * delete unused file Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> (cherry picked from commit e834cd5) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Jun 6, 2024 · bd1a617 · bd1a617
1 parent b583ee6
commit bd1a617
Show file tree

Hide file tree

Showing 18 changed files with 112 additions and 103 deletions.
diff --git a/common/constants/llm.ts b/common/constants/llm.ts
@@ -16,9 +16,12 @@ export const ASSISTANT_API = {
   ABORT_AGENT_EXECUTION: `${API_BASE}/abort`,
   REGENERATE: `${API_BASE}/regenerate`,
   TRACE: `${API_BASE}/trace`,
+  ACCOUNT: `${API_BASE}/account`,
 } as const;
 
 export const NOTEBOOK_API = {
   CREATE_NOTEBOOK: `${NOTEBOOK_PREFIX}/note`,
   SET_PARAGRAPH: `${NOTEBOOK_PREFIX}/set_paragraphs/`,
 };
+
+export const DEFAULT_USER_NAME = 'User';
diff --git a/opensearch_dashboards.json b/opensearch_dashboards.json
@@ -11,7 +11,6 @@
     "opensearchDashboardsUtils"
   ],
   "optionalPlugins": [
-    "securityDashboards",
     "dataSource",
     "dataSourceManagement"
   ],

diff --git a/public/chat_header_button.test.tsx b/public/chat_header_button.test.tsx
@@ -92,11 +92,10 @@ describe('<HeaderChatButton />', () => {
     render(
       <HeaderChatButton
         application={applicationStart}
-        userHasAccess={true}
         messageRenderers={{}}
         actionExecutors={{}}
         assistantActions={{} as AssistantActions}
-        currentAccount={{ username: 'test_user', tenant: 'test_tenant' }}
+        currentAccount={{ username: 'test_user' }}
       />
     );
 
@@ -142,11 +141,10 @@ describe('<HeaderChatButton />', () => {
     render(
       <HeaderChatButton
         application={applicationServiceMock.createStartContract()}
-        userHasAccess={true}
         messageRenderers={{}}
         actionExecutors={{}}
         assistantActions={{} as AssistantActions}
-        currentAccount={{ username: 'test_user', tenant: 'test_tenant' }}
+        currentAccount={{ username: 'test_user' }}
       />
     );
     screen.getByLabelText('chat input').focus();
@@ -166,11 +164,10 @@ describe('<HeaderChatButton />', () => {
     render(
       <HeaderChatButton
         application={applicationServiceMock.createStartContract()}
-        userHasAccess={true}
         messageRenderers={{}}
         actionExecutors={{}}
         assistantActions={{} as AssistantActions}
-        currentAccount={{ username: 'test_user', tenant: 'test_tenant' }}
+        currentAccount={{ username: 'test_user' }}
       />
     );
     expect(screen.getByLabelText('chat input')).not.toHaveFocus();
@@ -183,29 +180,14 @@ describe('<HeaderChatButton />', () => {
     expect(screen.getByLabelText('chat input')).toHaveFocus();
   });
 
-  it('should disable chat input when no access', () => {
-    render(
-      <HeaderChatButton
-        application={applicationServiceMock.createStartContract()}
-        userHasAccess={false}
-        messageRenderers={{}}
-        actionExecutors={{}}
-        assistantActions={{} as AssistantActions}
-        currentAccount={{ username: 'test_user', tenant: 'test_tenant' }}
-      />
-    );
-    expect(screen.getByLabelText('chat input')).toBeDisabled();
-  });
-
   it('should not focus on chat input when no access and pressing global shortcut', () => {
     render(
       <HeaderChatButton
         application={applicationServiceMock.createStartContract()}
-        userHasAccess={false}
         messageRenderers={{}}
         actionExecutors={{}}
         assistantActions={{} as AssistantActions}
-        currentAccount={{ username: 'test_user', tenant: 'test_tenant' }}
+        currentAccount={{ username: 'test_user' }}
       />
     );
     expect(screen.getByLabelText('chat input')).not.toHaveFocus();

diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx
@@ -27,7 +27,6 @@ import { MountPointPortal } from '../../../src/plugins/opensearch_dashboards_rea
 
 interface HeaderChatButtonProps {
   application: ApplicationStart;
-  userHasAccess: boolean;
   messageRenderers: Record<string, MessageRenderer>;
   actionExecutors: Record<string, ActionExecutor>;
   assistantActions: AssistantActions;
@@ -74,7 +73,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
       flyoutFullScreen,
       setFlyoutVisible,
       setFlyoutComponent,
-      userHasAccess: props.userHasAccess,
       messageRenderers: props.messageRenderers,
       actionExecutors: props.actionExecutors,
       currentAccount: props.currentAccount,
@@ -92,7 +90,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
       flyoutFullScreen,
       selectedTabId,
       preSelectedTabId,
-      props.userHasAccess,
       props.messageRenderers,
       props.actionExecutors,
       props.currentAccount,
@@ -157,9 +154,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
   }, []);
 
   useEffect(() => {
-    if (!props.userHasAccess) {
-      return;
-    }
     const onGlobalMouseUp = (e: KeyboardEvent) => {
       if (e.ctrlKey && e.key === '/') {
         inputRef.current?.focus();
@@ -170,7 +164,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
     return () => {
       document.removeEventListener('keydown', onGlobalMouseUp);
     };
-  }, [props.userHasAccess]);
+  }, []);
 
   useEffect(() => {
     const handleSuggestion = (event: { suggestion: string }) => {
@@ -235,7 +229,6 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
               )}
             </span>
           }
-          disabled={!props.userHasAccess}
         />
         <ChatContext.Provider value={chatContextValue}>
           <ChatStateProvider>

diff --git a/public/components/__tests__/chat_window_header_title.test.tsx b/public/components/__tests__/chat_window_header_title.test.tsx
@@ -22,7 +22,11 @@ import { DataSourceServiceMock } from '../../services/data_source_service.mock';
 const setup = ({
   messages = [],
   ...rest
-}: { messages?: IMessage[]; conversationId?: string | undefined } = {}) => {
+}: {
+  messages?: IMessage[];
+  conversationId?: string | undefined;
+  dataSource?: DataSourceServiceMock;
+} = {}) => {
   const useCoreMock = {
     services: {
       ...coreMock.createStart(),
@@ -38,7 +42,7 @@ const setup = ({
         }),
         reload: jest.fn(),
       },
-      dataSource: new DataSourceServiceMock(),
+      dataSource: rest.dataSource ?? new DataSourceServiceMock(false),
     },
   };
   useCoreMock.services.http.put.mockImplementation(() => Promise.resolve());
@@ -170,6 +174,19 @@ describe('<ChatWindowHeaderTitle />', () => {
     });
   });
 
+  it('should hide save to notebook button when MDS enabled', async () => {
+    const { renderResult } = setup({
+      messages: [{ type: 'input', contentType: 'text', content: 'foo' }],
+      dataSource: new DataSourceServiceMock(true),
+    });
+
+    fireEvent.click(renderResult.getByLabelText('toggle chat context menu'));
+
+    expect(
+      renderResult.queryByRole('button', { name: 'Save to notebook' })
+    ).not.toBeInTheDocument();
+  });
+
   it('should disable "Save to notebook" button when message does not include input', async () => {
     const { renderResult } = setup({
       messages: [{ type: 'output', content: 'bar', contentType: 'markdown' }],

diff --git a/public/components/chat_window_header_title.tsx b/public/components/chat_window_header_title.tsx
@@ -12,7 +12,7 @@ import {
   EuiButtonIcon,
   EuiToolTip,
 } from '@elastic/eui';
-import React, { useCallback, useState } from 'react';
+import React, { useCallback, useMemo, useState } from 'react';
 import { useChatContext } from '../contexts';
 import { useChatActions, useChatState, useSaveChat } from '../hooks';
 import { NotebookNameModal } from './notebook/notebook_name_modal';
@@ -57,6 +57,11 @@ export const ChatWindowHeaderTitle = React.memo(() => {
     [chatContext, core.services.conversations]
   );
 
+  const displayNotebookFeature = useMemo(() => {
+    // Notebook dashboard API doesn't support MDS for now, so we hide saving to notebook feature when MDS enabled.
+    return !core.services.dataSource.isMDSEnabled();
+  }, [core.services.dataSource.isMDSEnabled]);
+
   const handleSaveNotebookModalClose = () => {
     setSaveNotebookModalOpen(false);
   };
@@ -87,17 +92,19 @@ export const ChatWindowHeaderTitle = React.memo(() => {
     >
       New conversation
     </EuiContextMenuItem>,
-    <EuiContextMenuItem
-      key="save-as-notebook"
-      onClick={() => {
-        closePopover();
-        setSaveNotebookModalOpen(true);
-      }}
-      // User only can save conversation when he send a message at least.
-      disabled={chatState.messages.every((item) => item.type !== 'input')}
-    >
-      Save to notebook
-    </EuiContextMenuItem>,
+    displayNotebookFeature && (
+      <EuiContextMenuItem
+        key="save-as-notebook"
+        onClick={() => {
+          closePopover();
+          setSaveNotebookModalOpen(true);
+        }}
+        // User only can save conversation when he send a message at least.
+        disabled={chatState.messages.every((item) => item.type !== 'input')}
+      >
+        Save to notebook
+      </EuiContextMenuItem>
+    ),
   ];
 
   return (

diff --git a/public/contexts/__tests__/chat_context.test.tsx b/public/contexts/__tests__/chat_context.test.tsx
@@ -17,10 +17,9 @@ describe('useChatContext', () => {
       flyoutFullScreen: true,
       setFlyoutVisible: jest.fn(),
       setFlyoutComponent: jest.fn(),
-      userHasAccess: true,
       messageRenderers: {},
       actionExecutors: {},
-      currentAccount: { username: 'foo', tenant: '' },
+      currentAccount: { username: 'foo' },
       setTitle: jest.fn(),
       setInteractionId: jest.fn(),
     };

diff --git a/public/contexts/chat_context.tsx b/public/contexts/chat_context.tsx
@@ -18,7 +18,6 @@ export interface IChatContext {
   flyoutFullScreen: boolean;
   setFlyoutVisible: React.Dispatch<React.SetStateAction<boolean>>;
   setFlyoutComponent: React.Dispatch<React.SetStateAction<React.ReactNode | null>>;
-  userHasAccess: boolean;
   messageRenderers: Record<string, MessageRenderer>;
   actionExecutors: Record<string, ActionExecutor>;
   currentAccount?: UserAccount;

diff --git a/public/hooks/use_chat_actions.test.tsx b/public/hooks/use_chat_actions.test.tsx
@@ -76,11 +76,9 @@ describe('useChatActions hook', () => {
     setInteractionId: setInteractionIdMock,
     flyoutVisible: false,
     flyoutFullScreen: false,
-    userHasAccess: false,
     messageRenderers: {},
     currentAccount: {
       username: '',
-      tenant: '',
     },
   };
 

diff --git a/public/plugin.tsx b/public/plugin.tsx
@@ -32,6 +32,7 @@ import {
 } from './services';
 import { ConfigSchema } from '../common/types/config';
 import { DataSourceService } from './services/data_source_service';
+import { ASSISTANT_API, DEFAULT_USER_NAME } from '../common/constants/llm';
 
 export const [getCoreStart, setCoreStart] = createGetterSetter<CoreStart>('CoreStart');
 
@@ -46,7 +47,7 @@ export const IncontextInsightComponent: React.FC<{ props: any }> = (props) => (
 );
 
 interface UserAccountResponse {
-  data: { roles: string[]; user_name: string; user_requested_tenant?: string };
+  user_name: string;
 }
 
 export class AssistantPlugin
@@ -76,27 +77,15 @@ export class AssistantPlugin
     const actionExecutors: Record<string, ActionExecutor> = {};
     const assistantActions: AssistantActions = {} as AssistantActions;
     /**
-     * Returns {@link UserAccountResponse}. Provides default roles and user
-     * name if security plugin call fails.
+     * Returns {@link UserAccountResponse}. Provides user name.
      */
-    const getAccount: () => Promise<UserAccountResponse> = (() => {
-      let account: UserAccountResponse;
-      return async () => {
-        if (setupDeps.securityDashboards === undefined)
-          return { data: { roles: ['all_access'], user_name: 'dashboards_user' } };
-        if (account === undefined) {
-          account = await core.http
-            .get<UserAccountResponse>('/api/v1/configuration/account')
-            .catch((e) => {
-              console.error(`Failed to request user account information: ${String(e.body || e)}`);
-              return { data: { roles: [], user_name: '' } };
-            });
-        }
-        return account;
-      };
-    })();
-    const checkAccess = (account: Awaited<ReturnType<typeof getAccount>>) =>
-      account.data.roles.some((role) => ['all_access', 'assistant_user'].includes(role));
+    const getAccount: () => Promise<UserAccountResponse> = async () => {
+      const account = await core.http.get<UserAccountResponse>(ASSISTANT_API.ACCOUNT).catch((e) => {
+        console.error(`Failed to request user account information: ${String(e.body || e)}`);
+        return { user_name: DEFAULT_USER_NAME };
+      });
+      return account;
+    };
 
     const dataSourceSetupResult = this.dataSourceService.setup({
       uiSettings: core.uiSettings,
@@ -116,8 +105,7 @@ export class AssistantPlugin
           dataSource: this.dataSourceService,
         });
         const account = await getAccount();
-        const username = account.data.user_name;
-        const tenant = account.data.user_requested_tenant ?? '';
+        const username = account.user_name;
         this.incontextInsightRegistry?.setIsEnabled(this.config.incontextInsight.enabled);
 
         coreStart.chrome.navControls.registerRight({
@@ -126,11 +114,10 @@ export class AssistantPlugin
             <CoreContext.Provider>
               <HeaderChatButton
                 application={coreStart.application}
-                userHasAccess={checkAccess(account)}
                 messageRenderers={messageRenderers}
                 actionExecutors={actionExecutors}
                 assistantActions={assistantActions}
-                currentAccount={{ username, tenant }}
+                currentAccount={{ username }}
               />
             </CoreContext.Provider>
           ),
@@ -152,7 +139,6 @@ export class AssistantPlugin
         actionExecutors[actionType] = execute;
       },
       chatEnabled: () => this.config.chat.enabled,
-      userHasAccess: async () => await getAccount().then(checkAccess),
       assistantActions,
       registerIncontextInsight: this.incontextInsightRegistry.register.bind(
         this.incontextInsightRegistry

diff --git a/public/services/data_source_service.mock.ts b/public/services/data_source_service.mock.ts
@@ -4,12 +4,23 @@
  */
 
 export class DataSourceServiceMock {
-  constructor() {}
+  private _isMDSEnabled = true;
+  constructor(isMDSEnabled?: boolean) {
+    this._isMDSEnabled = isMDSEnabled ?? true;
+  }
 
   getDataSourceQuery() {
+    const result = this._isMDSEnabled
+      ? {
+          dataSourceId: '',
+        }
+      : {};
     return new Promise((resolve) => {
-      resolve({ dataSourceId: '' });
+      resolve(result);
     });
-    // return { dataSourceId: '' };
+  }
+
+  isMDSEnabled() {
+    return this._isMDSEnabled;
   }
 }
diff --git a/public/tabs/chat/chat_page.tsx b/public/tabs/chat/chat_page.tsx
@@ -60,7 +60,7 @@ export const ChatPage: React.FC<ChatPageProps> = (props) => {
         <EuiSpacer size="xs" />
         <ChatInputControls
           loading={chatState.llmResponding}
-          disabled={messagesLoading || chatState.llmResponding || !chatContext.userHasAccess}
+          disabled={messagesLoading || chatState.llmResponding}
         />
         <EuiSpacer size="s" />
       </EuiFlyoutFooter>

diff --git a/public/tabs/chat/chat_page_content.test.tsx b/public/tabs/chat/chat_page_content.test.tsx
@@ -45,7 +45,6 @@ describe('<ChatPageContent />', () => {
       },
       currentAccount: {
         username: 'test_user',
-        tenant: 'private',
       },
     });
-Original file line number
+Diff line change
@@ Expand Up / @@ -45,7 +45,6 @@ describe('<ChatPageContent />', () => { @@
           },
           currentAccount: {
             username: 'test_user',
-            tenant: 'private',
           },
         });
@@ Expand Down @@