PR Feedback.

- Adjusted table layout. - Added skeleton loader. - Refactored route, route params and associated types.
elastic · Jan 22, 2024 · b68e259 · b68e259
1 parent 5810d43
commit b68e259
Show file tree

Hide file tree

Showing 24 changed files with 315 additions and 144 deletions.
diff --git a/x-pack/plugins/dataset_quality/common/data_streams_stats/data_stream_details.ts b/x-pack/plugins/dataset_quality/common/data_streams_stats/data_stream_details.ts
@@ -5,16 +5,6 @@
  * 2.0.
  */
 
-import { DataStreamDetailsType } from './types';
-
-export class DataStreamDetails {
+export interface DataStreamDetails {
   createdOn?: number;
-
-  private constructor(obj: DataStreamDetails) {
-    this.createdOn = obj.createdOn;
-  }
-
-  public static create(dataStreamDetails: DataStreamDetailsType) {
-    return new DataStreamDetails(dataStreamDetails);
-  }
 }
diff --git a/x-pack/plugins/dataset_quality/common/data_streams_stats/data_stream_stat.ts b/x-pack/plugins/dataset_quality/common/data_streams_stats/data_stream_stat.ts
@@ -5,12 +5,14 @@
  * 2.0.
  */
 
+import { DataStreamType } from '../types';
+import { indexNameToDataStreamParts } from '../utils';
 import { Integration } from './integration';
 import { DataStreamStatType } from './types';
 
 export class DataStreamStat {
   rawName: string;
-  type: string;
+  type: DataStreamType;
   name: DataStreamStatType['name'];
   namespace: string;
   title: string;
@@ -34,9 +36,7 @@ export class DataStreamStat {
   }
 
   public static create(dataStreamStat: DataStreamStatType) {
-    const [type, ...dataStreamParts] = dataStreamStat.name.split('-');
-    const namespace = dataStreamParts[dataStreamParts.length - 1];
-    const dataset = dataStreamParts.slice(0, dataStreamParts.length - 1).join('-');
+    const { type, dataset, namespace } = indexNameToDataStreamParts(dataStreamStat.name);
 
     const dataStreamStatProps = {
       rawName: dataStreamStat.name,

diff --git a/x-pack/plugins/dataset_quality/common/data_streams_stats/types.ts b/x-pack/plugins/dataset_quality/common/data_streams_stats/types.ts
@@ -5,9 +5,8 @@
  * 2.0.
  */
 
-import { APIClientRequestParamsOf, APIReturnType } from '../rest/create_call_dataset_quality_api';
+import { APIClientRequestParamsOf, APIReturnType } from '../rest';
 import { DataStreamStat } from './data_stream_stat';
-import { DataStreamDetails } from './data_stream_details';
 
 export type GetDataStreamsStatsParams =
   APIClientRequestParamsOf<`GET /internal/dataset_quality/data_streams/stats`>['params'];
@@ -29,8 +28,9 @@ export type DataStreamDegradedDocsStatServiceResponse = DegradedDocsStatType[];
 export type DegradedDocsStatType = GetDataStreamsDegradedDocsStatsResponse['degradedDocs'][0];
 
 export type GetDataStreamDetailsParams =
-  APIClientRequestParamsOf<`GET /internal/dataset_quality/data_streams/details`>['params']['query'];
+  APIClientRequestParamsOf<`GET /internal/dataset_quality/data_streams/{dataStream}/details`>['params']['path'];
 export type GetDataStreamDetailsResponse =
-  APIReturnType<`GET /internal/dataset_quality/data_streams/details`>;
-export type DataStreamDetailsServiceResponse = DataStreamDetails;
-export type DataStreamDetailsType = Partial<DataStreamDetailsServiceResponse>;
+  APIReturnType<`GET /internal/dataset_quality/data_streams/{dataStream}/details`>;
+
+export type { DataStreamStat } from './data_stream_stat';
+export type { DataStreamDetails } from './data_stream_details';
diff --git a/x-pack/plugins/dataset_quality/common/types/dataset_types.ts b/x-pack/plugins/dataset_quality/common/types/dataset_types.ts
@@ -0,0 +1,19 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+// https://github.com/gcanti/io-ts/blob/master/index.md#union-of-string-literals
+import * as t from 'io-ts';
+
+export const dataStreamTypesRt = t.keyof({
+  logs: null,
+  metrics: null,
+  traces: null,
+  synthetics: null,
+  profiling: null,
+});
+
+export type DataStreamType = t.TypeOf<typeof dataStreamTypesRt>;
diff --git a/x-pack/plugins/dataset_quality/common/types/index.ts b/x-pack/plugins/dataset_quality/common/types/index.ts
@@ -0,0 +1,8 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export * from './dataset_types';
diff --git a/x-pack/plugins/dataset_quality/common/utils/dataset_name.test.ts b/x-pack/plugins/dataset_quality/common/utils/dataset_name.test.ts
@@ -0,0 +1,55 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import {
+  dataStreamPartsToIndexName,
+  streamPartsToIndexPattern,
+  indexNameToDataStreamParts,
+} from './dataset_name';
+
+describe('dataset_name', () => {
+  describe('streamPartsToIndexPattern', () => {
+    it('returns the correct index pattern', () => {
+      expect(
+        streamPartsToIndexPattern({
+          typePattern: 'logs',
+          datasetPattern: '*nginx.access*',
+        })
+      ).toEqual('logs-*nginx.access*');
+    });
+  });
+
+  describe('dataStreamPartsToIndexName', () => {
+    it('returns the correct index name', () => {
+      expect(
+        dataStreamPartsToIndexName({
+          type: 'logs',
+          dataset: 'nginx.access',
+          namespace: 'default',
+        })
+      ).toEqual('logs-nginx.access-default');
+    });
+  });
+
+  describe('indexNameToDataStreamParts', () => {
+    it('returns the correct data stream name', () => {
+      expect(indexNameToDataStreamParts('logs-nginx.access-default')).toEqual({
+        type: 'logs',
+        dataset: 'nginx.access',
+        namespace: 'default',
+      });
+    });
+
+    it('handles the case where the dataset name contains a hyphen', () => {
+      expect(indexNameToDataStreamParts('logs-heartbeat-8-default')).toEqual({
+        type: 'logs',
+        dataset: 'heartbeat-8',
+        namespace: 'default',
+      });
+    });
+  });
+});
diff --git a/x-pack/plugins/dataset_quality/common/utils/dataset_name.ts b/x-pack/plugins/dataset_quality/common/utils/dataset_name.ts
@@ -0,0 +1,40 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { DataStreamType } from '../types';
+
+export interface DataStreamNameParts {
+  type: DataStreamType;
+  dataset: string;
+  namespace: string;
+}
+
+export const streamPartsToIndexPattern = ({
+  typePattern,
+  datasetPattern,
+}: {
+  datasetPattern: string;
+  typePattern: string;
+}) => {
+  return `${typePattern}-${datasetPattern}`;
+};
+
+export const dataStreamPartsToIndexName = ({ type, dataset, namespace }: DataStreamNameParts) => {
+  return `${type}-${dataset}-${namespace}`;
+};
+
+export const indexNameToDataStreamParts = (dataStreamName: string) => {
+  const [type, ...dataStreamParts] = dataStreamName.split('-');
+  const namespace = dataStreamParts[dataStreamParts.length - 1];
+  const dataset = dataStreamParts.slice(0, dataStreamParts.length - 1).join('-');
+
+  return {
+    type: type as DataStreamType,
+    dataset,
+    namespace,
+  };
+};
diff --git a/x-pack/plugins/dataset_quality/common/utils/index.ts b/x-pack/plugins/dataset_quality/common/utils/index.ts
@@ -0,0 +1,8 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export * from './dataset_name';
diff --git a/x-pack/plugins/dataset_quality/public/components/dataset_quality/columns.tsx b/x-pack/plugins/dataset_quality/public/components/dataset_quality/columns.tsx
@@ -23,6 +23,7 @@ import { ES_FIELD_TYPES, KBN_FIELD_TYPES } from '@kbn/field-types';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n-react';
 import React, { Dispatch, SetStateAction } from 'react';
+import { css } from '@emotion/react';
 import {
   DEGRADED_QUALITY_MINIMUM_PERCENTAGE,
   POOR_QUALITY_MINIMUM_PERCENTAGE,
@@ -133,6 +134,11 @@ export const getDatasetQualityTableColumns = ({
         );
       },
       width: '40px',
+      css: css`
+        &.euiTableCellContent {
+          padding: 0;
+        }
+      `,
     },
     {
       name: nameColumnName,

diff --git a/x-pack/plugins/dataset_quality/public/components/dataset_quality/table.tsx b/x-pack/plugins/dataset_quality/public/components/dataset_quality/table.tsx
@@ -39,6 +39,7 @@ export const Table = () => {
       <EuiSpacer size="s" />
       <EuiHorizontalRule margin="none" style={{ height: 2 }} />
       <EuiBasicTable
+        tableLayout="auto"
         sorting={sort}
         onChange={onTableChange}
         pagination={pagination}

diff --git a/x-pack/plugins/dataset_quality/public/components/flyout/dataset_summary.tsx b/x-pack/plugins/dataset_quality/public/components/flyout/dataset_summary.tsx
@@ -13,13 +13,12 @@ import {
   flyoutDatasetDetailsText,
   flyoutDatasetLastActivityText,
 } from '../../../common/translations';
-import { DataStreamStat } from '../../../common/data_streams_stats/data_stream_stat';
-import { DataStreamDetails } from '../../../common/data_streams_stats/data_stream_details';
-import { FieldsList } from './fields_list';
+import { DataStreamStat, DataStreamDetails } from '../../../common/data_streams_stats';
+import { FieldsList, FieldsListLoading } from './fields_list';
 
 interface DatasetSummaryProps {
   fieldFormats: FieldFormatsStart;
-  dataStreamDetails: DataStreamDetails;
+  dataStreamDetails?: DataStreamDetails;
   dataStreamStat: DataStreamStat;
 }
 
@@ -32,9 +31,9 @@ export function DatasetSummary({
     ES_FIELD_TYPES.DATE,
   ]);
   const formattedLastActivity = dataFormatter.convert(dataStreamStat.lastActivity);
-  const formattedCreatedOn = dataStreamDetails.createdOn
+  const formattedCreatedOn = dataStreamDetails?.createdOn
     ? dataFormatter.convert(dataStreamDetails.createdOn)
-    : '';
+    : '-';
 
   return (
     <FieldsList
@@ -52,3 +51,7 @@ export function DatasetSummary({
     />
   );
 }
+
+export function DatasetSummaryLoading() {
+  return <FieldsListLoading />;
+}
diff --git a/x-pack/plugins/dataset_quality/public/components/flyout/fields_list.tsx b/x-pack/plugins/dataset_quality/public/components/flyout/fields_list.tsx
@@ -13,6 +13,8 @@ import {
   EuiSpacer,
   EuiTitle,
   EuiHorizontalRule,
+  EuiSkeletonTitle,
+  EuiSkeletonText,
 } from '@elastic/eui';
 
 export function FieldsList({
@@ -47,3 +49,34 @@ export function FieldsList({
     </EuiPanel>
   );
 }
+
+export function FieldsListLoading() {
+  return (
+    <EuiPanel hasBorder grow={false}>
+      <EuiFlexGroup direction="column" gutterSize="none">
+        <EuiFlexGroup>
+          <EuiFlexItem grow={1}>
+            <EuiSkeletonTitle size="s" />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+        <EuiHorizontalRule margin="s" />
+        <EuiFlexGroup>
+          <EuiFlexItem grow={1}>
+            <EuiSkeletonText size="m" lines={1} />
+          </EuiFlexItem>
+          <EuiFlexItem grow={2}>
+            <EuiSkeletonText lines={1} />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+        <EuiFlexGroup>
+          <EuiFlexItem grow={1}>
+            <EuiSkeletonText size="m" lines={1} />
+          </EuiFlexItem>
+          <EuiFlexItem grow={2}>
+            <EuiSkeletonText lines={1} />
+          </EuiFlexItem>
+        </EuiFlexGroup>
+      </EuiFlexGroup>
+    </EuiPanel>
+  );
+}
diff --git a/x-pack/plugins/dataset_quality/public/components/flyout/flyout.tsx b/x-pack/plugins/dataset_quality/public/components/flyout/flyout.tsx
@@ -12,14 +12,14 @@ import {
   EuiFlyout,
   EuiFlyoutBody,
   EuiFlyoutFooter,
-  EuiLoadingLogo,
   EuiSpacer,
 } from '@elastic/eui';
-import React from 'react';
+import React, { Fragment } from 'react';
+import { DEFAULT_DATASET_TYPE } from '../../../common/constants';
 import { DataStreamStat } from '../../../common/data_streams_stats/data_stream_stat';
 import { flyoutCancelText } from '../../../common/translations';
 import { useDatasetQualityFlyout } from '../../hooks';
-import { DatasetSummary } from './dataset_summary';
+import { DatasetSummary, DatasetSummaryLoading } from './dataset_summary';
 import { Header } from './header';
 import { IntegrationSummary } from './integration_summary';
 
@@ -29,33 +29,40 @@ interface FlyoutProps {
 }
 
 export function Flyout({ dataset, closeFlyout }: FlyoutProps) {
-  const { dataStreamStat, dataStreamDetails, loading, fieldFormats } = useDatasetQualityFlyout({
-    datasetQuery: `${dataset.name}-${dataset.namespace}`,
+  const {
+    dataStreamStat,
+    dataStreamDetails,
+    dataStreamStatLoading,
+    dataStreamDetailsLoading,
+    fieldFormats,
+  } = useDatasetQualityFlyout({
+    type: DEFAULT_DATASET_TYPE,
+    dataset: dataset.name,
+    namespace: dataset.namespace,
   });
 
   return (
     <EuiFlyout onClose={closeFlyout} ownFocus={false} data-component-name={'datasetQualityFlyout'}>
       <>
         <Header dataStreamStat={dataset} />
-        {loading ? (
-          <EuiFlyoutBody>
-            <EuiFlexGroup justifyContent="center">
-              <EuiLoadingLogo logo="logoObservability" size="l" />
-            </EuiFlexGroup>
-          </EuiFlyoutBody>
-        ) : (
-          <EuiFlyoutBody>
-            <DatasetSummary
-              dataStreamStat={dataStreamStat}
-              dataStreamDetails={dataStreamDetails}
-              fieldFormats={fieldFormats}
-            />
-            <EuiSpacer />
-            {dataStreamStat.integration && (
-              <IntegrationSummary integration={dataStreamStat.integration} />
-            )}
-          </EuiFlyoutBody>
-        )}
+        <EuiFlyoutBody>
+          {dataStreamStatLoading || dataStreamDetailsLoading ? (
+            <DatasetSummaryLoading />
+          ) : dataStreamStat ? (
+            <Fragment>
+              <DatasetSummary
+                dataStreamStat={dataStreamStat}
+                dataStreamDetails={dataStreamDetails}
+                fieldFormats={fieldFormats}
+              />
+              <EuiSpacer />
+              {dataStreamStat.integration && (
+                <IntegrationSummary integration={dataStreamStat.integration} />
+              )}
+            </Fragment>
+          ) : null}
+        </EuiFlyoutBody>
+
         <EuiFlyoutFooter>
           <EuiFlexGroup justifyContent="spaceBetween">
             <EuiFlexItem grow={false}>