[Discover] Persist hide chart option to local storage (elastic#114534)

* Persist hide chart to local storage * [Discover] Persist hide chart option to local storage * Fix state * Fix dependency check * Set chart state to undefined * Update unit test * Do not override saved search preferences * Fix missing import * Add a functional test * Add a functional test * Fix functional test Co-authored-by: Kibana Machine <[email protected]>
majagrubic · Oct 14, 2021 · e2226ef · e2226ef
1 parent 9460831
commit e2226ef
Show file tree

Hide file tree

Showing 7 changed files with 107 additions and 8 deletions.
diff --git a/src/plugins/discover/public/application/apps/main/components/chart/discover_chart.tsx b/src/plugins/discover/public/application/apps/main/components/chart/discover_chart.tsx
@@ -25,6 +25,7 @@ import { DiscoverServices } from '../../../../../build_services';
 import { useChartPanels } from './use_chart_panels';
 
 const DiscoverHistogramMemoized = memo(DiscoverHistogram);
+export const CHART_HIDDEN_KEY = 'discover:chartHidden';
 
 export function DiscoverChart({
   resetSavedSearch,
@@ -47,7 +48,8 @@ export function DiscoverChart({
 }) {
   const [showChartOptionsPopover, setShowChartOptionsPopover] = useState(false);
 
-  const { data } = services;
+  const { data, storage } = services;
+
   const chartRef = useRef<{ element: HTMLElement | null; moveFocus: boolean }>({
     element: null,
     moveFocus: false,
@@ -71,7 +73,8 @@ export function DiscoverChart({
     const newHideChart = !state.hideChart;
     stateContainer.setAppState({ hideChart: newHideChart });
     chartRef.current.moveFocus = !newHideChart;
-  }, [state, stateContainer]);
+    storage.set(CHART_HIDDEN_KEY, newHideChart);
+  }, [state.hideChart, stateContainer, storage]);
 
   const timefilterUpdateHandler = useCallback(
     (ranges: { from: number; to: number }) => {

diff --git a/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts b/src/plugins/discover/public/application/apps/main/services/use_discover_state.ts
@@ -34,7 +34,7 @@ export function useDiscoverState({
   savedSearch: SavedSearch;
   history: History;
 }) {
-  const { uiSettings: config, data, filterManager, indexPatterns } = services;
+  const { uiSettings: config, data, filterManager, indexPatterns, storage } = services;
   const useNewFieldsApi = useMemo(() => !config.get(SEARCH_FIELDS_FROM_SOURCE), [config]);
   const { timefilter } = data.query.timefilter;
 
@@ -53,13 +53,14 @@ export function useDiscoverState({
             config,
             data,
             savedSearch,
+            storage,
           }),
         storeInSessionStorage: config.get('state:storeInSessionStorage'),
         history,
         toasts: services.core.notifications.toasts,
         uiSettings: config,
       }),
-    [config, data, history, savedSearch, services.core.notifications.toasts]
+    [config, data, history, savedSearch, services.core.notifications.toasts, storage]
   );
 
   const { appStateContainer } = stateContainer;
@@ -160,11 +161,12 @@ export function useDiscoverState({
         config,
         data,
         savedSearch: newSavedSearch,
+        storage,
       });
       await stateContainer.replaceUrlAppState(newAppState);
       setState(newAppState);
     },
-    [indexPattern, services, config, data, stateContainer]
+    [services, indexPattern, config, data, storage, stateContainer]
   );
 
   /**
@@ -209,10 +211,11 @@ export function useDiscoverState({
       config,
       data,
       savedSearch,
+      storage,
     });
     stateContainer.replaceUrlAppState(newAppState);
     setState(newAppState);
-  }, [config, data, savedSearch, reset, stateContainer]);
+  }, [config, data, savedSearch, reset, stateContainer, storage]);
 
   /**
    * Trigger data fetching on indexPattern or savedSearch changes

diff --git a/src/plugins/discover/public/application/apps/main/utils/get_state_defaults.test.ts b/src/plugins/discover/public/application/apps/main/utils/get_state_defaults.test.ts
@@ -12,14 +12,18 @@ import { uiSettingsMock } from '../../../../__mocks__/ui_settings';
 import { indexPatternWithTimefieldMock } from '../../../../__mocks__/index_pattern_with_timefield';
 import { savedSearchMock } from '../../../../__mocks__/saved_search';
 import { indexPatternMock } from '../../../../__mocks__/index_pattern';
+import { discoverServiceMock } from '../../../../__mocks__/services';
 
 describe('getStateDefaults', () => {
+  const storage = discoverServiceMock.storage;
+
   test('index pattern with timefield', () => {
     savedSearchMock.searchSource = createSearchSourceMock({ index: indexPatternWithTimefieldMock });
     const actual = getStateDefaults({
       config: uiSettingsMock,
       data: dataPluginMock.createStartContract(),
       savedSearch: savedSearchMock,
+      storage,
     });
     expect(actual).toMatchInlineSnapshot(`
       Object {
@@ -49,6 +53,7 @@ describe('getStateDefaults', () => {
       config: uiSettingsMock,
       data: dataPluginMock.createStartContract(),
       savedSearch: savedSearchMock,
+      storage,
     });
     expect(actual).toMatchInlineSnapshot(`
       Object {

diff --git a/src/plugins/discover/public/application/apps/main/utils/get_state_defaults.ts b/src/plugins/discover/public/application/apps/main/utils/get_state_defaults.ts
@@ -18,6 +18,8 @@ import { DataPublicPluginStart } from '../../../../../../data/public';
 
 import { AppState } from '../services/discover_state';
 import { getDefaultSort, getSortArray } from '../components/doc_table';
+import { CHART_HIDDEN_KEY } from '../components/chart/discover_chart';
+import { Storage } from '../../../../../../kibana_utils/public';
 
 function getDefaultColumns(savedSearch: SavedSearch, config: IUiSettingsClient) {
   if (savedSearch.columns && savedSearch.columns.length > 0) {
@@ -33,17 +35,20 @@ export function getStateDefaults({
   config,
   data,
   savedSearch,
+  storage,
 }: {
   config: IUiSettingsClient;
   data: DataPublicPluginStart;
   savedSearch: SavedSearch;
+  storage: Storage;
 }) {
   const { searchSource } = savedSearch;
   const indexPattern = searchSource.getField('index');
 
   const query = searchSource.getField('query') || data.query.queryString.getDefaultQuery();
   const sort = getSortArray(savedSearch.sort ?? [], indexPattern!);
   const columns = getDefaultColumns(savedSearch, config);
+  const chartHidden = Boolean(storage.get(CHART_HIDDEN_KEY));
 
   const defaultState = {
     query,
@@ -54,13 +59,13 @@ export function getStateDefaults({
     index: indexPattern?.id,
     interval: 'auto',
     filters: cloneDeep(searchSource.getOwnField('filter')),
-    hideChart: undefined,
+    hideChart: chartHidden ? chartHidden : undefined,
     savedQuery: undefined,
   } as AppState;
   if (savedSearch.grid) {
     defaultState.grid = savedSearch.grid;
   }
-  if (savedSearch.hideChart) {
+  if (savedSearch.hideChart !== undefined) {
     defaultState.hideChart = savedSearch.hideChart;
   }
 

diff --git a/test/functional/apps/discover/_chart_hidden.ts b/test/functional/apps/discover/_chart_hidden.ts
@@ -0,0 +1,71 @@
+/*
+ * 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 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import expect from '@kbn/expect';
+import { FtrProviderContext } from '../../ftr_provider_context';
+
+export default function ({ getService, getPageObjects }: FtrProviderContext) {
+  const log = getService('log');
+  const esArchiver = getService('esArchiver');
+  const kibanaServer = getService('kibanaServer');
+  const PageObjects = getPageObjects(['common', 'discover', 'header', 'timePicker']);
+
+  const defaultSettings = {
+    defaultIndex: 'logstash-*',
+  };
+
+  describe('discover show/hide chart test', function () {
+    before(async function () {
+      log.debug('load kibana index with default index pattern');
+
+      await kibanaServer.importExport.load('test/functional/fixtures/kbn_archiver/discover.json');
+
+      // and load a set of makelogs data
+      await esArchiver.loadIfNeeded('test/functional/fixtures/es_archiver/logstash_functional');
+      await kibanaServer.uiSettings.replace(defaultSettings);
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.timePicker.setDefaultAbsoluteRange();
+    });
+
+    after(async () => {
+      await kibanaServer.uiSettings.unset('defaultIndex');
+    });
+
+    it('shows chart by default', async function () {
+      expect(await PageObjects.discover.isChartVisible()).to.be(true);
+    });
+
+    it('hiding the chart persists the setting', async function () {
+      await PageObjects.discover.toggleChartVisibility();
+      expect(await PageObjects.discover.isChartVisible()).to.be(false);
+
+      await PageObjects.common.navigateToApp('dashboard');
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.timePicker.setDefaultAbsoluteRange();
+      await PageObjects.header.waitUntilLoadingHasFinished();
+
+      expect(await PageObjects.discover.isChartVisible()).to.be(false);
+    });
+
+    it('persists hidden chart option on the saved search ', async function () {
+      const savedSearchTitle = 'chart hidden';
+      await PageObjects.discover.saveSearch(savedSearchTitle);
+
+      await PageObjects.discover.toggleChartVisibility();
+      expect(await PageObjects.discover.isChartVisible()).to.be(true);
+
+      await PageObjects.common.navigateToApp('discover');
+      await PageObjects.timePicker.setDefaultAbsoluteRange();
+      await PageObjects.header.waitUntilLoadingHasFinished();
+      expect(await PageObjects.discover.isChartVisible()).to.be(true);
+
+      await PageObjects.discover.loadSavedSearch(savedSearchTitle);
+      expect(await PageObjects.discover.isChartVisible()).to.be(false);
+    });
+  });
+}
diff --git a/test/functional/apps/discover/index.ts b/test/functional/apps/discover/index.ts
@@ -52,5 +52,6 @@ export default function ({ getService, loadTestFile }: FtrProviderContext) {
     loadTestFile(require.resolve('./_huge_fields'));
     loadTestFile(require.resolve('./_date_nested'));
     loadTestFile(require.resolve('./_search_on_page_load'));
+    loadTestFile(require.resolve('./_chart_hidden'));
   });
 }
diff --git a/test/functional/page_objects/discover_page.ts b/test/functional/page_objects/discover_page.ts
@@ -185,6 +185,17 @@ export class DiscoverPageObject extends FtrService {
     return await this.globalNav.getLastBreadcrumb();
   }
 
+  public async isChartVisible() {
+    return await this.testSubjects.exists('discoverChart');
+  }
+
+  public async toggleChartVisibility() {
+    await this.testSubjects.click('discoverChartOptionsToggle');
+    await this.testSubjects.exists('discoverChartToggle');
+    await this.testSubjects.click('discoverChartToggle');
+    await this.header.waitUntilLoadingHasFinished();
+  }
+
   public async getChartInterval() {
     await this.testSubjects.click('discoverChartOptionsToggle');
     await this.testSubjects.click('discoverTimeIntervalPanel');