[Discover] Removed chart data fetching from Discover and Unified Hist…

…ogram
elastic · Nov 2, 2022 · db92baa · db92baa
1 parent 9cccac2
commit db92baa
Show file tree

Hide file tree

Showing 21 changed files with 211 additions and 784 deletions.
diff --git a/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts b/src/plugins/discover/public/application/main/components/layout/use_discover_histogram.ts
@@ -12,7 +12,6 @@ import {
   getVisualizeInformation,
   triggerVisualizeActions,
 } from '@kbn/unified-field-list-plugin/public';
-import { buildChartData } from '@kbn/unified-histogram-plugin/public';
 import { useCallback, useEffect, useMemo, useState } from 'react';
 import { getUiActions } from '../../../../kibana_services';
 import { PLUGIN_ID } from '../../../../../common';
@@ -42,7 +41,7 @@ export const useDiscoverHistogram = ({
   isTimeBased: boolean;
   isPlainRecord: boolean;
 }) => {
-  const { storage, data } = useDiscoverServices();
+  const { storage } = useDiscoverServices();
 
   /**
    * Visualize
@@ -134,41 +133,15 @@ export const useDiscoverHistogram = ({
     [hitsFetchStatus, hitsTotal, isPlainRecord]
   );
 
-  const { fetchStatus: chartFetchStatus, response, error } = useDataState(savedSearchData$.charts$);
-
-  const { bucketInterval, chartData } = useMemo(
-    () =>
-      buildChartData({
-        data,
-        dataView,
-        timeInterval: state.interval,
-        response,
-      }),
-    [data, dataView, response, state.interval]
-  );
-
   const chart = useMemo(
     () =>
       isPlainRecord || !isTimeBased
         ? undefined
         : {
-            status: chartFetchStatus,
             hidden: state.hideChart,
             timeInterval: state.interval,
-            bucketInterval,
-            data: chartData,
-            error,
           },
-    [
-      bucketInterval,
-      chartData,
-      chartFetchStatus,
-      error,
-      isPlainRecord,
-      isTimeBased,
-      state.hideChart,
-      state.interval,
-    ]
+    [isPlainRecord, isTimeBased, state.hideChart, state.interval]
   );
 
   /**

diff --git a/src/plugins/discover/public/application/main/hooks/use_saved_search.ts b/src/plugins/discover/public/application/main/hooks/use_saved_search.ts
@@ -30,7 +30,6 @@ export interface SavedSearchData {
   main$: DataMain$;
   documents$: DataDocuments$;
   totalHits$: DataTotalHits$;
-  charts$: DataCharts$;
   availableFields$: AvailableFields$;
 }
 

diff --git a/src/plugins/discover/public/application/main/hooks/use_saved_search_messages.ts b/src/plugins/discover/public/application/main/hooks/use_saved_search_messages.ts
@@ -105,11 +105,6 @@ export function sendResetMsg(data: SavedSearchData, initialFetchStatus: FetchSta
     result: [],
     recordRawType,
   });
-  data.charts$.next({
-    fetchStatus: initialFetchStatus,
-    response: undefined,
-    recordRawType,
-  });
   data.totalHits$.next({
     fetchStatus: initialFetchStatus,
     result: undefined,

diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.test.ts b/src/plugins/discover/public/application/main/utils/fetch_all.test.ts
@@ -17,7 +17,6 @@ import { discoverServiceMock } from '../../../__mocks__/services';
 import { fetchAll } from './fetch_all';
 import {
   DataAvailableFieldsMsg,
-  DataChartsMessage,
   DataDocumentsMsg,
   DataMainMsg,
   DataTotalHitsMsg,
@@ -26,11 +25,9 @@ import {
 
 import { fetchDocuments } from './fetch_documents';
 import { fetchSql } from './fetch_sql';
-import { fetchChart } from './fetch_chart';
 import { fetchTotalHits } from './fetch_total_hits';
 import { buildDataTableRecord } from '../../../utils/build_data_record';
 import { dataViewMock } from '../../../__mocks__/data_view';
-import type { SearchResponse } from '@elastic/elasticsearch/lib/api/types';
 
 jest.mock('./fetch_documents', () => ({
   fetchDocuments: jest.fn().mockResolvedValue([]),
@@ -40,17 +37,12 @@ jest.mock('./fetch_sql', () => ({
   fetchSql: jest.fn().mockResolvedValue([]),
 }));
 
-jest.mock('./fetch_chart', () => ({
-  fetchChart: jest.fn(),
-}));
-
 jest.mock('./fetch_total_hits', () => ({
   fetchTotalHits: jest.fn(),
 }));
 
 const mockFetchDocuments = fetchDocuments as unknown as jest.MockedFunction<typeof fetchDocuments>;
 const mockFetchTotalHits = fetchTotalHits as unknown as jest.MockedFunction<typeof fetchTotalHits>;
-const mockFetchChart = fetchChart as unknown as jest.MockedFunction<typeof fetchChart>;
 const mockFetchSQL = fetchSql as unknown as jest.MockedFunction<typeof fetchSql>;
 
 function subjectCollector<T>(subject: Subject<T>): () => Promise<T[]> {
@@ -73,7 +65,6 @@ describe('test fetchAll', () => {
       main$: new BehaviorSubject<DataMainMsg>({ fetchStatus: FetchStatus.UNINITIALIZED }),
       documents$: new BehaviorSubject<DataDocumentsMsg>({ fetchStatus: FetchStatus.UNINITIALIZED }),
       totalHits$: new BehaviorSubject<DataTotalHitsMsg>({ fetchStatus: FetchStatus.UNINITIALIZED }),
-      charts$: new BehaviorSubject<DataChartsMessage>({ fetchStatus: FetchStatus.UNINITIALIZED }),
       availableFields$: new BehaviorSubject<DataAvailableFieldsMsg>({
         fetchStatus: FetchStatus.UNINITIALIZED,
       }),
@@ -98,9 +89,6 @@ describe('test fetchAll', () => {
     mockFetchDocuments.mockReset().mockResolvedValue([]);
     mockFetchSQL.mockReset().mockResolvedValue([]);
     mockFetchTotalHits.mockReset().mockResolvedValue(42);
-    mockFetchChart
-      .mockReset()
-      .mockResolvedValue({ totalHits: 42, response: {} as unknown as SearchResponse });
   });
 
   test('changes of fetchStatus when starting with FetchStatus.UNINITIALIZED', async () => {
@@ -157,25 +145,9 @@ describe('test fetchAll', () => {
     ]);
   });
 
-  test('emits loading and response on charts$ correctly', async () => {
-    const collect = subjectCollector(subjects.charts$);
-    searchSource.getField('index')!.isTimeBased = () => true;
-    await fetchAll(subjects, searchSource, false, deps);
-    expect(await collect()).toEqual([
-      { fetchStatus: FetchStatus.UNINITIALIZED },
-      { fetchStatus: FetchStatus.LOADING, recordRawType: 'document' },
-      {
-        fetchStatus: FetchStatus.COMPLETE,
-        recordRawType: 'document',
-        response: {},
-      },
-    ]);
-  });
-
   test('should use charts query to fetch total hit count when chart is visible', async () => {
     const collect = subjectCollector(subjects.totalHits$);
     searchSource.getField('index')!.isTimeBased = () => true;
-    mockFetchChart.mockResolvedValue({ totalHits: 32, response: {} as unknown as SearchResponse });
     await fetchAll(subjects, searchSource, false, deps);
     expect(await collect()).toEqual([
       { fetchStatus: FetchStatus.UNINITIALIZED },

diff --git a/src/plugins/discover/public/application/main/utils/fetch_all.ts b/src/plugins/discover/public/application/main/utils/fetch_all.ts
@@ -22,7 +22,6 @@ import {
 import { updateSearchSource } from './update_search_source';
 import { fetchDocuments } from './fetch_documents';
 import { fetchTotalHits } from './fetch_total_hits';
-import { fetchChart } from './fetch_chart';
 import { AppState } from '../services/discover_state';
 import { FetchStatus } from '../../types';
 import {
@@ -105,7 +104,6 @@ export function fetchAll(
     sendLoadingMsg(dataSubjects.main$, recordRawType);
     sendLoadingMsg(dataSubjects.documents$, recordRawType, query);
     sendLoadingMsg(dataSubjects.totalHits$, recordRawType);
-    sendLoadingMsg(dataSubjects.charts$, recordRawType);
 
     const isChartVisible =
       !hideChart && dataView.isTimeBased() && dataView.type !== DataViewType.ROLLUP;
@@ -115,8 +113,6 @@ export function fetchAll(
       useSql && query
         ? fetchSql(query, services.dataViews, data, services.expressions)
         : fetchDocuments(searchSource.createCopy(), fetchDeps);
-    const charts =
-      isChartVisible && !useSql ? fetchChart(searchSource.createCopy(), fetchDeps) : undefined;
     const totalHits =
       !isChartVisible && !useSql ? fetchTotalHits(searchSource.createCopy(), fetchDeps) : undefined;
     /**
@@ -162,18 +158,6 @@ export function fetchAll(
       // but their errors will be shown in-place (e.g. of the chart).
       .catch(sendErrorTo(dataSubjects.documents$, dataSubjects.main$));
 
-    charts
-      ?.then((chart) => {
-        dataSubjects.charts$.next({
-          fetchStatus: FetchStatus.COMPLETE,
-          response: chart.response,
-          recordRawType,
-        });
-
-        checkHitCount(chart.totalHits);
-      })
-      .catch(sendErrorTo(dataSubjects.charts$, dataSubjects.totalHits$));
-
     totalHits
       ?.then((hitCount) => {
         dataSubjects.totalHits$.next({
@@ -186,7 +170,7 @@ export function fetchAll(
       .catch(sendErrorTo(dataSubjects.totalHits$));
 
     // Return a promise that will resolve once all the requests have finished or failed
-    return Promise.allSettled([documents, charts, totalHits]).then(() => {
+    return Promise.allSettled([documents, totalHits]).then(() => {
       // Send a complete message to main$ once all queries are done and if main$
       // is not already in an ERROR state, e.g. because the document query has failed.
       // This will only complete main$, if it hasn't already been completed previously

diff --git a/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts b/src/plugins/discover/public/application/main/utils/fetch_chart.test.ts