[Chore] Refactor columns passing, fix bugs

* pass columns explicitly as props * fix branding in core mocks * fix `toBeUndefined()` usage in tests * remove leftover comment * fix test subject * condense types Signed-off-by: Josh Romero <rmerqg@amazon.com>
opensearch-project · Oct 28, 2022 · 02118f2 · 02118f2
1 parent 6f79a05
commit 02118f2
Show file tree

Hide file tree

Showing 11 changed files with 42 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -58,9 +58,9 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Adding @zhongnansu as maintainer. ([#2590](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2590))
 
 ### 🪛 Refactoring
-* [MD] Refactor data source error handling ([#2661](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2661))
 
-* Refactor and improve Discover field summaries ([#2391](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2391))
+- [MD] Refactor data source error handling ([#2661](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2661))
+- Refactor and improve Discover field summaries ([#2391](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2391))
 
 ### 🔩 Tests
 

diff --git a/src/core/public/mocks.ts b/src/core/public/mocks.ts
@@ -83,6 +83,7 @@ function createCoreSetupMock({
     uiSettings: uiSettingsServiceMock.createSetupContract(),
     injectedMetadata: {
       getInjectedVar: injectedMetadataServiceMock.createSetupContract().getInjectedVar,
+      getBranding: injectedMetadataServiceMock.createSetupContract().getBranding,
     },
   };
 

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field.test.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field.test.tsx
@@ -100,8 +100,9 @@ function getComponent({
 
   const props = {
     indexPattern,
+    columns: [],
     field: finalField,
-    getDetails: jest.fn(() => ({ buckets: [], error: '', exists: 1, total: 1, columns: [] })),
+    getDetails: jest.fn(() => ({ buckets: [], error: '', exists: 1, total: 1 })),
     onAddFilter: jest.fn(),
     onAddField: jest.fn(),
     onRemoveField: jest.fn(),

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field.tsx
@@ -40,6 +40,10 @@ import { getFieldTypeName } from './lib/get_field_type_name';
 import './discover_field.scss';
 
 export interface DiscoverFieldProps {
+  /**
+   * the selected columns displayed in the doc table in discover
+   */
+  columns: string[];
   /**
    * The displayed field
    */
@@ -76,6 +80,7 @@ export interface DiscoverFieldProps {
 }
 
 export function DiscoverField({
+  columns,
   field,
   indexPattern,
   onAddField,
@@ -228,9 +233,10 @@ export function DiscoverField({
       </EuiPopoverTitle>
       {infoIsOpen && (
         <DiscoverFieldDetails
-          indexPattern={indexPattern}
-          field={field}
+          columns={columns}
           details={getDetails(field)}
+          field={field}
+          indexPattern={indexPattern}
           onAddFilter={onAddFilter}
         />
       )}

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field_details.test.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field_details.test.tsx
@@ -59,8 +59,9 @@ const indexPattern = getStubIndexPattern(
 
 describe('discover sidebar field details', function () {
   const defaultProps = {
+    columns: [],
+    details: { buckets: [], error: '', exists: 1, total: 1 },
     indexPattern,
-    details: { buckets: [], error: '', exists: 1, total: 1, columns: [] },
     onAddFilter: jest.fn(),
   };
 
@@ -176,7 +177,7 @@ describe('discover sidebar field details', function () {
       details: { ...defaultProps.details, error: errText },
     });
     expect(findTestSubject(comp, 'fieldVisualizeContainer').length).toBe(1);
-    expect(findTestSubject(comp, 'fieldVisualizeBucket').length).toBe(0);
+    expect(findTestSubject(comp, 'fieldVisualizeBucketContainer').length).toBe(0);
     expect(findTestSubject(comp, 'fieldVisualizeError').length).toBe(1);
     expect(findTestSubject(comp, 'fieldVisualizeError').text()).toBe(errText);
 

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_field_details.tsx b/src/plugins/discover/public/application/components/sidebar/discover_field_details.tsx
@@ -43,16 +43,18 @@ import { IndexPatternField, IndexPattern } from '../../../../../data/public';
 import './discover_field_details.scss';
 
 interface DiscoverFieldDetailsProps {
+  columns: string[];
+  details: FieldDetails;
   field: IndexPatternField;
   indexPattern: IndexPattern;
-  details: FieldDetails;
   onAddFilter: (field: IndexPatternField | string, value: string, type: '+' | '-') => void;
 }
 
 export function DiscoverFieldDetails({
+  columns,
+  details,
   field,
   indexPattern,
-  details,
   onAddFilter,
 }: DiscoverFieldDetailsProps) {
   const warnings = getWarnings(field);
@@ -61,25 +63,23 @@ export function DiscoverFieldDetails({
 
   useEffect(() => {
     const checkIfVisualizable = async () => {
-      const visualizable = await isFieldVisualizable(field, indexPattern.id, details.columns).catch(
+      const visualizable = await isFieldVisualizable(field, indexPattern.id, columns).catch(
         () => false
       );
 
       setShowVisualizeLink(visualizable);
       if (visualizable) {
-        const href = await getVisualizeHref(field, indexPattern.id, details.columns).catch(
-          () => ''
-        );
+        const href = await getVisualizeHref(field, indexPattern.id, columns).catch(() => '');
         setVisualizeLink(href || '');
       }
     };
     checkIfVisualizable();
-  }, [field, indexPattern.id, details.columns]);
+  }, [field, indexPattern.id, columns]);
 
   const handleVisualizeLinkClick = (event: React.MouseEvent<HTMLAnchorElement, MouseEvent>) => {
     // regular link click. let the uiActions code handle the navigation and show popup if needed
     event.preventDefault();
-    triggerVisualizeActions(field, indexPattern.id, details.columns);
+    triggerVisualizeActions(field, indexPattern.id, columns);
   };
 
   return (

diff --git a/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx b/src/plugins/discover/public/application/components/sidebar/discover_sidebar.tsx
@@ -117,8 +117,8 @@ export function DiscoverSidebar({
   );
 
   const getDetailsByField = useCallback(
-    (ipField: IndexPatternField) => getDetails(ipField, hits, columns, selectedIndexPattern),
-    [hits, columns, selectedIndexPattern]
+    (ipField: IndexPatternField) => getDetails(ipField, hits, selectedIndexPattern),
+    [hits, selectedIndexPattern]
   );
 
   const popularLimit = services.uiSettings.get(FIELDS_LIMIT_SETTING);
@@ -199,6 +199,7 @@ export function DiscoverSidebar({
                       className="dscSidebar__item"
                     >
                       <DiscoverField
+                        columns={columns}
                         field={field}
                         indexPattern={selectedIndexPattern}
                         onAddField={onAddField}
@@ -274,6 +275,7 @@ export function DiscoverSidebar({
                       className="dscSidebar__item"
                     >
                       <DiscoverField
+                        columns={columns}
                         field={field}
                         indexPattern={selectedIndexPattern}
                         onAddField={onAddField}
@@ -304,6 +306,7 @@ export function DiscoverSidebar({
                   className="dscSidebar__item"
                 >
                   <DiscoverField
+                    columns={columns}
                     field={field}
                     indexPattern={selectedIndexPattern}
                     onAddField={onAddField}

diff --git a/src/plugins/discover/public/application/components/sidebar/lib/field_calculator.test.ts b/src/plugins/discover/public/application/components/sidebar/lib/field_calculator.test.ts
@@ -105,7 +105,7 @@ describe('field_calculator', function () {
     });
 
     it('should count array terms independently', function () {
-      expect(groups['foo,bar']).toBe(undefined);
+      expect(groups['foo,bar']).toBeUndefined();
       expect(groups.foo.count).toBe(5);
       expect(groups.bar.count).toBe(3);
       expect(groups.baz.count).toBe(1);
@@ -196,7 +196,7 @@ describe('field_calculator', function () {
       expect(extensions.buckets).toBeInstanceOf(Array);
       const buckets = extensions.buckets as Bucket[];
       expect(buckets.length).toBe(5);
-      expect(extensions.error).toBe(undefined);
+      expect(extensions.error).toBeUndefined();
     });
 
     it('counts only distinct values if less than default', function () {
@@ -206,7 +206,7 @@ describe('field_calculator', function () {
       expect(extensions.buckets).toBeInstanceOf(Array);
       const buckets = extensions.buckets as Bucket[];
       expect(buckets.length).toBe(4);
-      expect(extensions.error).toBe(undefined);
+      expect(extensions.error).toBeUndefined();
     });
 
     it('counts only distinct values if less than specified count', function () {
@@ -216,7 +216,7 @@ describe('field_calculator', function () {
       expect(extensions.buckets).toBeInstanceOf(Array);
       const buckets = extensions.buckets as Bucket[];
       expect(buckets.length).toBe(4);
-      expect(extensions.error).toBe(undefined);
+      expect(extensions.error).toBeUndefined();
     });
 
     it('counts the top 3 values', function () {
@@ -226,23 +226,23 @@ describe('field_calculator', function () {
       const buckets = extensions.buckets as Bucket[];
       expect(buckets.length).toBe(3);
       expect(_.map(buckets, 'value')).toEqual(['html', 'gif', 'php']);
-      expect(extensions.error).toBe(undefined);
+      expect(extensions.error).toBeUndefined();
     });
 
     it('fails to analyze geo and attachment types', function () {
       params.field = indexPattern.fields.getByName('point') as IndexPatternField;
-      expect(getFieldValueCounts(params).error).not.toBe(undefined);
+      expect(getFieldValueCounts(params).error).not.toBeUndefined();
 
       params.field = indexPattern.fields.getByName('area') as IndexPatternField;
-      expect(getFieldValueCounts(params).error).not.toBe(undefined);
+      expect(getFieldValueCounts(params).error).not.toBeUndefined();
 
       params.field = indexPattern.fields.getByName('request_body') as IndexPatternField;
-      expect(getFieldValueCounts(params).error).not.toBe(undefined);
+      expect(getFieldValueCounts(params).error).not.toBeUndefined();
     });
 
     it('fails to analyze fields that are in the mapping, but not the hits', function () {
       params.field = indexPattern.fields.getByName('ip') as IndexPatternField;
-      expect(getFieldValueCounts(params).error).not.toBe(undefined);
+      expect(getFieldValueCounts(params).error).not.toBeUndefined();
     });
 
     it('counts the total hits', function () {

diff --git a/src/plugins/discover/public/application/components/sidebar/lib/field_calculator.ts b/src/plugins/discover/public/application/components/sidebar/lib/field_calculator.ts
@@ -28,7 +28,6 @@
  * under the License.
  */
 
-// import _ from 'lodash';
 import { i18n } from '@osd/i18n';
 import { IndexPattern, IndexPatternField } from 'src/plugins/data/public';
 import { FieldValueCounts } from '../types';

diff --git a/src/plugins/discover/public/application/components/sidebar/lib/get_details.ts b/src/plugins/discover/public/application/components/sidebar/lib/get_details.ts
@@ -36,15 +36,13 @@ import { IndexPattern, IndexPatternField } from '../../../../../../data/public';
 export function getDetails(
   field: IndexPatternField,
   hits: Array<Record<string, unknown>>,
-  columns: string[],
   indexPattern?: IndexPattern
 ) {
   const defaultDetails = {
     error: '',
     exists: 0,
     total: 0,
     buckets: [],
-    columns,
   };
   if (!indexPattern) {
     return {

diff --git a/src/plugins/discover/public/application/components/sidebar/types.ts b/src/plugins/discover/public/application/components/sidebar/types.ts
@@ -33,19 +33,15 @@ export interface IndexPatternRef {
   title: string;
 }
 
-export interface FieldValueCounts {
-  error?: string;
-  exists?: number;
-  total?: number;
-  buckets?: Bucket[];
-  missing?: number;
-}
 export interface FieldDetails {
   error: string;
   exists: number;
   total: number;
   buckets: Bucket[];
-  columns: string[];
+}
+
+export interface FieldValueCounts extends Partial<FieldDetails> {
+  missing?: number;
 }
 
 export interface Bucket {