update preview panel for external models (#252) (#253)

* feat: update preview panel for external models Signed-off-by: tygao <[email protected]> * feat: update preview panel for external models Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> (cherry picked from commit 9bbd25f) Co-authored-by: raintygao <[email protected]>
opensearch-project · Aug 30, 2023 · 728fcb1 · 728fcb1
1 parent 0710c49
commit 728fcb1
Show file tree

Hide file tree

Showing 6 changed files with 231 additions and 55 deletions.
diff --git a/public/components/preview_panel/__tests__/connector_details.test.tsx b/public/components/preview_panel/__tests__/connector_details.test.tsx
@@ -0,0 +1,36 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import userEvent from '@testing-library/user-event';
+import { render, screen } from '../../../../test/test_utils';
+import { ConnectorDetails } from '../connector_details';
+
+function setup({ name = 'name', id = 'id', description = 'description' }) {
+  const user = userEvent.setup({});
+  render(<ConnectorDetails name={name} id={id} description={description} />);
+  return { user };
+}
+
+describe('<ConnectorDetails />', () => {
+  it('should render connector details', () => {
+    setup({});
+    expect(screen.getByText('Connector name')).toBeInTheDocument();
+    expect(screen.getByText('Connector ID')).toBeInTheDocument();
+    expect(screen.getByText('Connector description')).toBeInTheDocument();
+  });
+
+  it('should render - when id is empty', () => {
+    setup({ id: '' });
+    expect(screen.getByText('-')).toBeInTheDocument();
+    expect(screen.queryByTestId('copyable-text-div')).not.toBeInTheDocument();
+  });
+
+  it('should render id and copy id button when id is not empty', () => {
+    setup({ id: 'connector-id' });
+    expect(screen.getByText('connector-id')).toBeInTheDocument();
+    expect(screen.queryByTestId('copyable-text-div')).toBeInTheDocument();
+  });
+});
diff --git a/public/components/preview_panel/__tests__/nodes_table.test.tsx b/public/components/preview_panel/__tests__/nodes_table.test.tsx
@@ -19,9 +19,9 @@ const NODES = [
   },
 ];
 
-function setup({ nodes = NODES, loading = false }) {
+function setup({ nodes = NODES, loading = false, nodesStatus = 'Responding on 1 of 2 nodes' }) {
   const user = userEvent.setup({});
-  render(<NodesTable nodes={nodes} loading={loading} />);
+  render(<NodesTable nodes={nodes} loading={loading} nodesStatus={nodesStatus} />);
   return { user };
 }
 
@@ -31,6 +31,7 @@ describe('<NodesTable />', () => {
     expect(screen.getAllByRole('columnheader').length).toBe(2);
     expect(screen.getByText('id1')).toBeInTheDocument();
     expect(screen.getByText('id2')).toBeInTheDocument();
+    expect(screen.getByText('Responding on 1 of 2 nodes')).toBeInTheDocument();
   });
 
   it('should render status at first column with asc by default', () => {

diff --git a/public/components/preview_panel/__tests__/preview_panel.test.tsx b/public/components/preview_panel/__tests__/preview_panel.test.tsx
@@ -13,9 +13,6 @@ const MODEL = {
   id: 'id1',
   name: 'test',
   planningWorkerNodes: ['node-1', 'node-2', 'node-3'],
-  connector: {
-    name: 'Connector',
-  },
 };
 
 function setup({ model = MODEL, onClose = jest.fn() }) {
@@ -29,11 +26,30 @@ describe('<PreviewPanel />', () => {
     jest.clearAllMocks();
   });
 
-  it('should render id, name and source in panel', () => {
+  it('should render id, name in panel', () => {
     setup({});
     expect(screen.getByText('test')).toBeInTheDocument();
     expect(screen.getByText('id1')).toBeInTheDocument();
+  });
+
+  it('source should be local and should not render connector details when no connector params passed', async () => {
+    setup({});
+    expect(screen.getByText('Local')).toBeInTheDocument();
+    expect(screen.queryByText('Connector details')).not.toBeInTheDocument();
+  });
+
+  it('source should be external and should not render nodes details when connector params passed', async () => {
+    const modelWithConntector = {
+      ...MODEL,
+      connector: {
+        name: 'connector',
+      },
+    };
+    setup({
+      model: modelWithConntector,
+    });
     expect(screen.getByText('External')).toBeInTheDocument();
+    expect(screen.queryByText('Status by node')).not.toBeInTheDocument();
   });
 
   it('should call onClose when close panel', async () => {
@@ -44,7 +60,7 @@ describe('<PreviewPanel />', () => {
     expect(onClose).toHaveBeenCalled();
   });
 
-  it('should render loading when not responding and render partially state when responding', async () => {
+  it('should render loading when local model not responding and render partially state when responding', async () => {
     const request = jest.spyOn(APIProvider.getAPI('profile'), 'getModel');
     const mockResult = {
       id: 'model-1-id',
@@ -55,9 +71,10 @@ describe('<PreviewPanel />', () => {
     request.mockResolvedValue(mockResult);
     setup({});
     expect(screen.getByTestId('preview-panel-color-loading-text')).toBeInTheDocument();
-    await waitFor(() =>
-      expect(screen.getByText('Partially responding on 2 of 3 nodes')).toBeInTheDocument()
-    );
+    await waitFor(() => {
+      expect(screen.getByText('Partially responding')).toBeInTheDocument();
+      expect(screen.getByText('Responding on 2 of 3 nodes')).toBeInTheDocument();
+    });
   });
 
   it('should render not responding when no model profile API response', async () => {

diff --git a/public/components/preview_panel/connector_details.tsx b/public/components/preview_panel/connector_details.tsx
@@ -0,0 +1,62 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import React from 'react';
+import {
+  EuiDescriptionList,
+  EuiDescriptionListTitle,
+  EuiTitle,
+  EuiSpacer,
+  EuiDescriptionListDescription,
+  EuiFlexGroup,
+  EuiFlexItem,
+} from '@elastic/eui';
+import { CopyableText } from '../common';
+
+export const ConnectorDetails = (props: { name?: string; id?: string; description?: string }) => {
+  const { name, id, description } = props;
+  return (
+    <>
+      <EuiSpacer size="m" />
+      <EuiTitle size="s">
+        <h3>Connector details</h3>
+      </EuiTitle>
+      <EuiSpacer size="m" />
+      <EuiDescriptionList>
+        <EuiFlexGroup>
+          <EuiFlexItem>
+            <EuiDescriptionListTitle>
+              <EuiTitle size="xxs">
+                <h5>Connector name</h5>
+              </EuiTitle>
+            </EuiDescriptionListTitle>
+            <EuiDescriptionListDescription>{name}</EuiDescriptionListDescription>
+          </EuiFlexItem>
+          <EuiFlexItem>
+            <EuiDescriptionListTitle>
+              <EuiTitle size="xxs">
+                <h5>Connector ID</h5>
+              </EuiTitle>
+            </EuiDescriptionListTitle>
+            <EuiDescriptionListDescription>
+              {id ? (
+                <CopyableText text={id} iconLeft={false} tooltipText="Copy connector ID" />
+              ) : (
+                '-'
+              )}
+            </EuiDescriptionListDescription>
+          </EuiFlexItem>
+        </EuiFlexGroup>
+        <EuiSpacer size="m" />
+        <EuiDescriptionListTitle>
+          <EuiTitle size="xxs">
+            <h5>Connector description</h5>
+          </EuiTitle>
+        </EuiDescriptionListTitle>
+        <EuiDescriptionListDescription>{description}</EuiDescriptionListDescription>
+      </EuiDescriptionList>
+    </>
+  );
+};
diff --git a/public/components/preview_panel/index.tsx b/public/components/preview_panel/index.tsx
@@ -15,12 +15,15 @@ import {
   EuiDescriptionListDescription,
   EuiSpacer,
   EuiTextColor,
+  EuiFlexGroup,
+  EuiFlexItem,
 } from '@elastic/eui';
 import { APIProvider } from '../../apis/api_provider';
 import { useFetcher } from '../../hooks/use_fetcher';
 import { NodesTable } from './nodes_table';
 import { CopyableText } from '../common';
 import { ModelDeploymentProfile } from '../../apis/profile';
+import { ConnectorDetails } from './connector_details';
 
 export interface INode {
   id: string;
@@ -60,33 +63,45 @@ export const PreviewPanel = ({ onClose, model }: Props) => {
 
   const respondingStatus = useMemo(() => {
     if (loading) {
-      return (
-        <EuiTextColor color="subdued" data-test-subj="preview-panel-color-loading-text">
-          Loading...
-        </EuiTextColor>
-      );
+      return {
+        overall: (
+          <EuiTextColor color="subdued" data-test-subj="preview-panel-color-loading-text">
+            Loading...
+          </EuiTextColor>
+        ),
+        nodes: 'Loading...',
+      };
     }
     const deployedNodesNum = nodes.filter(({ deployed }) => deployed).length;
     const targetNodesNum = nodes.length;
     if (deployedNodesNum === 0) {
-      return (
-        <EuiHealth className="ml-modelStatusCell" color="danger">
-          Not responding on {targetNodesNum} of {targetNodesNum} nodes
-        </EuiHealth>
-      );
+      return {
+        overall: (
+          <EuiHealth className="ml-modelStatusCell" color="danger">
+            Not responding
+          </EuiHealth>
+        ),
+        nodes: `Not responding on ${targetNodesNum} of ${targetNodesNum} nodes`,
+      };
     }
     if (deployedNodesNum < targetNodesNum) {
-      return (
-        <EuiHealth className="ml-modelStatusCell" color="warning">
-          Partially responding on {deployedNodesNum} of {targetNodesNum} nodes
-        </EuiHealth>
-      );
+      return {
+        overall: (
+          <EuiHealth className="ml-modelStatusCell" color="warning">
+            Partially responding
+          </EuiHealth>
+        ),
+        nodes: `Responding on ${deployedNodesNum} of ${targetNodesNum} nodes`,
+      };
     }
-    return (
-      <EuiHealth className="ml-modelStatusCell" color="success">
-        Responding on {deployedNodesNum} of {targetNodesNum} nodes
-      </EuiHealth>
-    );
+    return {
+      overall: (
+        <EuiHealth className="ml-modelStatusCell" color="success">
+          Responding
+        </EuiHealth>
+      ),
+      nodes: `Responding on ${deployedNodesNum} of ${targetNodesNum} nodes`,
+    };
   }, [nodes, loading]);
 
   const onCloseFlyout = useCallback(() => {
@@ -95,26 +110,54 @@ export const PreviewPanel = ({ onClose, model }: Props) => {
 
   return (
     <EuiFlyout onClose={onCloseFlyout}>
-      <EuiFlyoutHeader>
-        <EuiTitle size="s">
-          <h3>{name}</h3>
+      <EuiFlyoutHeader hasBorder>
+        <EuiTitle size="m">
+          <h2>{name}</h2>
         </EuiTitle>
       </EuiFlyoutHeader>
       <EuiFlyoutBody>
         <EuiDescriptionList>
-          <EuiDescriptionListTitle>Model ID</EuiDescriptionListTitle>
+          <EuiFlexGroup>
+            <EuiFlexItem>
+              <EuiDescriptionListTitle>
+                <EuiTitle size="xxs">
+                  <h5>Status</h5>
+                </EuiTitle>
+              </EuiDescriptionListTitle>
+              <EuiDescriptionListDescription>
+                {respondingStatus.overall}
+              </EuiDescriptionListDescription>
+            </EuiFlexItem>
+            <EuiFlexItem>
+              <EuiDescriptionListTitle>
+                <EuiTitle size="xxs">
+                  <h5>Source</h5>
+                </EuiTitle>
+              </EuiDescriptionListTitle>
+              <EuiDescriptionListDescription>
+                {connector ? 'External' : 'Local'}
+              </EuiDescriptionListDescription>
+            </EuiFlexItem>
+          </EuiFlexGroup>
+          <EuiSpacer size="m" />
+          <EuiDescriptionListTitle>
+            <EuiTitle size="xxs">
+              <h5>Model ID</h5>
+            </EuiTitle>
+          </EuiDescriptionListTitle>
           <EuiDescriptionListDescription>
             <CopyableText text={id} iconLeft={false} tooltipText="Copy model ID" />
           </EuiDescriptionListDescription>
-          <EuiDescriptionListTitle>Source</EuiDescriptionListTitle>
-          <EuiDescriptionListDescription>
-            {connector ? 'External' : 'Local'}
-          </EuiDescriptionListDescription>
-          <EuiDescriptionListTitle>Model status by node</EuiDescriptionListTitle>
-          <EuiDescriptionListDescription>{respondingStatus}</EuiDescriptionListDescription>
         </EuiDescriptionList>
-        <EuiSpacer />
-        <NodesTable loading={loading} nodes={nodes} />
+        {connector ? (
+          <ConnectorDetails
+            id={connector.id}
+            name={connector.name}
+            description={connector.description}
+          />
+        ) : (
+          <NodesTable loading={loading} nodes={nodes} nodesStatus={respondingStatus.nodes} />
+        )}
       </EuiFlyoutBody>
     </EuiFlyout>
   );

diff --git a/public/components/preview_panel/nodes_table.tsx b/public/components/preview_panel/nodes_table.tsx
@@ -14,11 +14,16 @@ import {
   EuiEmptyPrompt,
   EuiCopy,
   EuiText,
+  EuiDescriptionList,
+  EuiDescriptionListTitle,
+  EuiTitle,
+  EuiSpacer,
+  EuiDescriptionListDescription,
 } from '@elastic/eui';
 import { INode } from './';
 
-export function NodesTable(props: { nodes: INode[]; loading: boolean }) {
-  const { nodes, loading } = props;
+export function NodesTable(props: { nodes: INode[]; loading: boolean; nodesStatus: string }) {
+  const { nodes, loading, nodesStatus } = props;
   const [sort, setSort] = useState<{ field: keyof INode; direction: Direction }>({
     field: 'deployed',
     direction: 'asc',
@@ -108,16 +113,28 @@ export function NodesTable(props: { nodes: INode[]; loading: boolean }) {
   );
 
   return (
-    <EuiBasicTable<INode>
-      columns={columns}
-      items={items}
-      sorting={{ sort }}
-      pagination={pagination}
-      onChange={handleTableChange}
-      loading={loading}
-      noItemsMessage={
-        loading ? <EuiEmptyPrompt body={<>Loading...</>} aria-label="loading nodes" /> : undefined
-      }
-    />
+    <>
+      <EuiSpacer size="l" />
+      <EuiDescriptionList>
+        <EuiDescriptionListTitle>
+          <EuiTitle size="s">
+            <h3>Status by node</h3>
+          </EuiTitle>
+        </EuiDescriptionListTitle>
+        <EuiDescriptionListDescription>{nodesStatus}</EuiDescriptionListDescription>
+      </EuiDescriptionList>
+      <EuiSpacer size="m" />
+      <EuiBasicTable<INode>
+        columns={columns}
+        items={items}
+        sorting={{ sort }}
+        pagination={pagination}
+        onChange={handleTableChange}
+        loading={loading}
+        noItemsMessage={
+          loading ? <EuiEmptyPrompt body={<>Loading...</>} aria-label="loading nodes" /> : undefined
+        }
+      />
+    </>
   );
 }