add validation to desc field, show graph when data is present (#18009)

* add validation to desc field, show graph when data is present

* added unit test

---------

Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com>

openmetadata-ui/src/main/resources/ui/src/components/DataInsight/KPIChart.test.tsx

@@ -0,0 +1,90 @@
+/*
+ *  Copyright 2023 Collate.
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+import { render, screen } from '@testing-library/react';
+import React from 'react';
+import { MemoryRouter } from 'react-router-dom';
+import { act } from 'react-test-renderer';
+import { KPI_LIST } from '../../pages/KPIPage/KPIMock.mock';
+import KPIChart from './KPIChart';
+
+jest.mock('../../rest/KpiAPI', () => ({
+  getListKPIs: jest
+    .fn()
+    .mockImplementation(() => Promise.resolve({ data: KPI_LIST })),
+}));
+
+describe('Test KPIChart Component', () => {
+  const mockProps = {
+    chartFilter: {
+      startTs: 1234567890,
+      endTs: 1234567899,
+    },
+    kpiList: KPI_LIST,
+    isKpiLoading: false,
+    viewKPIPermission: true,
+    createKPIPermission: true,
+  };
+
+  it('Should render KPIChart component', async () => {
+    await act(async () => {
+      render(<KPIChart {...mockProps} />, {
+        wrapper: MemoryRouter,
+      });
+    });
+
+    const kpiCard = screen.getByTestId('kpi-card');
+
+    expect(kpiCard).toBeInTheDocument();
+  });
+
+  it('Should render EmptyGraphPlaceholder when no data is available', async () => {
+    await act(async () => {
+      render(<KPIChart {...mockProps} kpiList={[]} />, {
+        wrapper: MemoryRouter,
+      });
+    });
+
+    const emptyPlaceholder = screen.getByText(
+      'message.no-kpi-available-add-new-one'
+    );
+
+    expect(emptyPlaceholder).toBeInTheDocument();
+  });
+
+  it('Should render "Add KPI" button when no KPIs exist and user has create permission', async () => {
+    await act(async () => {
+      render(<KPIChart {...mockProps} kpiList={[]} />, {
+        wrapper: MemoryRouter,
+      });
+    });
+
+    const addButton = screen.getByText('label.add-entity');
+
+    expect(addButton).toBeInTheDocument();
+  });
+
+  it('Should not render "Add KPI" button when no create permission', async () => {
+    await act(async () => {
+      render(
+        <KPIChart {...mockProps} createKPIPermission={false} kpiList={[]} />,
+        {
+          wrapper: MemoryRouter,
+        }
+      );
+    });
+
+    const addButton = screen.queryByText('label.add-entity');
+
+    expect(addButton).not.toBeInTheDocument();
+  });
+});

openmetadata-ui/src/main/resources/ui/src/components/DataInsight/KPIChart.tsx

@@ -219,6 +219,12 @@ const KPIChart: FC<Props> = ({
     }
   }, [kpiList, chartFilter]);
 
+  const hasAtLeastOneData = useMemo(() => {
+    return kpiNames.some(
+      (key) => kpiResults[key] && kpiResults[key].length > 0
+    );
+  }, [kpiNames, kpiResults]);
+
   return (
     <Card
       className="data-insight-card"
@@ -235,7 +241,7 @@ const KPIChart: FC<Props> = ({
       }>
       {kpiList.length ? (
         <Row gutter={DI_STRUCTURE.rowContainerGutter}>
-          {!isEmpty(kpiResults) ? (
+          {hasAtLeastOneData ? (
             <>
               <Col span={DI_STRUCTURE.leftContainerSpan}>
                 <ResponsiveContainer

openmetadata-ui/src/main/resources/ui/src/pages/KPIPage/AddKPIPage.test.tsx

@@ -128,6 +128,26 @@ describe('Add KPI page', () => {
     expect(submitButton).toBeInTheDocument();
   });
 
+  it('should show validation error when description is empty', async () => {
+    render(<AddKPIPage />, { wrapper: MemoryRouter });
+
+    const submitButton = await screen.findByTestId('submit-btn');
+
+    await act(async () => {
+      fireEvent.click(submitButton);
+    });
+
+    const validationMessages = await screen.findAllByText(
+      'label.field-required'
+    );
+    // we have start date and end date field with the same label, hence we have 3 validation messages
+    // and description is the last field in the form
+    const lastValidationMessage =
+      validationMessages[validationMessages.length - 1];
+
+    expect(lastValidationMessage).toBeInTheDocument();
+  });
+
   it.skip('Should render the proper metric input based on metric type', async () => {
     render(<AddKPIPage />, { wrapper: MemoryRouter });
 

openmetadata-ui/src/main/resources/ui/src/pages/KPIPage/AddKPIPage.tsx

@@ -349,6 +349,14 @@ const AddKPIPage = () => {
               <Form.Item
                 label={t('label.description')}
                 name="description"
+                rules={[
+                  {
+                    required: true,
+                    message: t('label.field-required', {
+                      field: t('label.description-kpi'),
+                    }),
+                  },
+                ]}
                 trigger="onTextChange"
                 valuePropName="initialValue">
                 <RichTextEditor

openmetadata-ui/src/main/resources/ui/src/pages/KPIPage/KPIMock.mock.ts

@@ -11,6 +11,8 @@
  *  limitations under the License.
  */
 
+import { KpiTargetType } from '../../generated/dataInsight/kpi/kpi';
+
 export const KPI_CHARTS = [
   {
     id: 'd2f093d4-0ca8-42b8-8721-1c2a59951b59',
@@ -307,7 +309,7 @@ export const KPI_LIST = [
     displayName: 'Description KPI',
     fullyQualifiedName: 'description-kpi',
     description: '',
-    metricType: 'PERCENTAGE',
+    metricType: KpiTargetType.Percentage,
     dataInsightChart: {
       id: '7dc794d3-1881-408c-92fc-6182aa453bc8',
       type: 'dataInsightChart',
@@ -349,6 +351,7 @@ export const KPI_LIST = [
       previousVersion: 0.1,
     },
     deleted: false,
+    targetValue: 58,
   },
 ];