[Lens] Fix crash when two layers xychart switches to pie (#75267)

* [Lens] fix crash on two layers and add a functional test with two layers switching to pie chart
elastic · Aug 18, 2020 · e04e05a · e04e05a
1 parent ce8a92e
commit e04e05a
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 31 deletions.
diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx
@@ -89,11 +89,12 @@ function LayerPanels(
 
  return (
  <EuiForm className="lnsConfigPanel">
- {layerIds.map((layerId) => (
+ {layerIds.map((layerId, index) => (
  <LayerPanel
  {...props}
  key={layerId}
  layerId={layerId}
+ dataTestSubj={`lns-layerPanel-${index}`}
  visualizationState={visualizationState}
  updateVisualization={setVisualizationState}
  updateDatasource={updateDatasource}

diff --git a/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/...k/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx
@@ -58,6 +58,7 @@ describe('LayerPanel', () => {
  onRemoveLayer: jest.fn(),
  dispatch: jest.fn(),
  core: coreMock.createStart(),
+ dataTestSubj: 'lns_layerPanel-0',
  };
  }
 

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx
@@ -36,6 +36,7 @@ const initialPopoverState = {
 export function LayerPanel(
  props: Exclude<ConfigPanelWrapperProps, 'state' | 'setState'> & {
  layerId: string;
+ dataTestSubj: string;
  isOnlyLayer: boolean;
  updateVisualization: StateSetter<unknown>;
  updateDatasource: (datasourceId: string, newState: unknown) => void;
@@ -50,7 +51,7 @@ export function LayerPanel(
  const dragDropContext = useContext(DragContext);
  const [popoverState, setPopoverState] = useState<DimensionPopoverState>(initialPopoverState);
 
- const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer } = props;
+ const { framePublicAPI, layerId, isOnlyLayer, onRemoveLayer, dataTestSubj } = props;
  const datasourcePublicAPI = framePublicAPI.datasourceLayers[layerId];
 
  useEffect(() => {
@@ -96,7 +97,7 @@ export function LayerPanel(
 
  return (
  <ChildDragDropProvider {...dragDropContext}>
- <EuiPanel className="lnsLayerPanel" paddingSize="s">
+ <EuiPanel data-test-subj={dataTestSubj} className="lnsLayerPanel" paddingSize="s">
  <EuiFlexGroup gutterSize="s" alignItems="flexStart" responsive={false}>
  <EuiFlexItem grow={false}>
  <LayerSettings

diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/suggestion_panel.tsx
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import _ from 'lodash';
+import _, { camelCase } from 'lodash';
 import React, { useState, useEffect, useMemo } from 'react';
 import { FormattedMessage } from '@kbn/i18n/react';
 import {
@@ -122,30 +122,32 @@ const SuggestionPreview = ({
 }) => {
  return (
  <EuiToolTip content={preview.title}>
- <EuiPanelFixed
- className={classNames('lnsSuggestionPanel__button', {
- // eslint-disable-next-line @typescript-eslint/naming-convention
- 'lnsSuggestionPanel__button-isSelected': selected,
- })}
- paddingSize="none"
- data-test-subj="lnsSuggestion"
- onClick={onSelect}
- >
- {preview.expression ? (
- <DebouncedPreviewRenderer
- ExpressionRendererComponent={ExpressionRendererComponent}
- expression={toExpression(preview.expression)}
- withLabel={Boolean(showTitleAsLabel)}
- />
- ) : (
- <span className="lnsSuggestionPanel__suggestionIcon">
- <EuiIcon size="xxl" type={preview.icon} />
- </span>
- )}
- {showTitleAsLabel && (
- <span className="lnsSuggestionPanel__buttonLabel">{preview.title}</span>
- )}
- </EuiPanelFixed>
+ <div data-test-subj={`lnsSuggestion-${camelCase(preview.title)}`}>
+ <EuiPanelFixed
+ className={classNames('lnsSuggestionPanel__button', {
+ // eslint-disable-next-line @typescript-eslint/naming-convention
+ 'lnsSuggestionPanel__button-isSelected': selected,
+ })}
+ paddingSize="none"
+ data-test-subj="lnsSuggestion"
+ onClick={onSelect}
+ >
+ {preview.expression ? (
+ <DebouncedPreviewRenderer
+ ExpressionRendererComponent={ExpressionRendererComponent}
+ expression={toExpression(preview.expression)}
+ withLabel={Boolean(showTitleAsLabel)}
+ />
+ ) : (
+ <span className="lnsSuggestionPanel__suggestionIcon">
+ <EuiIcon size="xxl" type={preview.icon} />
+ </span>
+ )}
+ {showTitleAsLabel && (
+ <span className="lnsSuggestionPanel__buttonLabel">{preview.title}</span>
+ )}
+ </EuiPanelFixed>
+ </div>
  </EuiToolTip>
  );
 };
@@ -204,8 +206,8 @@ export function SuggestionPanel({
  : undefined;
 
  return { suggestions: newSuggestions, currentStateExpression: newStateExpression };
+ // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [
- frame,
  currentDatasourceStates,
  currentVisualizationState,
  currentVisualizationId,

diff --git a/x-pack/test/functional/apps/lens/smokescreen.ts b/x-pack/test/functional/apps/lens/smokescreen.ts
@@ -11,6 +11,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
  const PageObjects = getPageObjects(['visualize', 'lens']);
  const find = getService('find');
  const listingTable = getService('listingTable');
+ const testSubjects = getService('testSubjects');
 
  describe('lens smokescreen tests', () => {
  it('should allow editing saved visualizations', async () => {
@@ -109,6 +110,54 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
  expect(await PageObjects.lens.getLayerCount()).to.eql(2);
  });
 
+ it('should switch from a multi-layer stacked bar to donut chart using suggestions', async () => {
+ await PageObjects.visualize.navigateToNewVisualization();
+ await PageObjects.visualize.clickVisType('lens');
+ await PageObjects.lens.goToTimeRange();
+
+ await PageObjects.lens.configureDimension({
+ dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension',
+ operation: 'terms',
+ field: 'geo.dest',
+ });
+
+ await PageObjects.lens.configureDimension({
+ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension',
+ operation: 'avg',
+ field: 'bytes',
+ });
+
+ await PageObjects.lens.createLayer();
+
+ await PageObjects.lens.configureDimension(
+ {
+ dimension: 'lnsXY_xDimensionPanel > lns-empty-dimension',
+ operation: 'terms',
+ field: 'geo.src',
+ },
+ 1
+ );
+
+ await PageObjects.lens.configureDimension(
+ {
+ dimension: 'lnsXY_yDimensionPanel > lns-empty-dimension',
+ operation: 'avg',
+ field: 'bytes',
+ },
+ 1
+ );
+ await PageObjects.lens.save('twolayerchart');
+ await testSubjects.click('lnsSuggestion-asDonut > lnsSuggestion');
+
+ expect(await PageObjects.lens.getLayerCount()).to.eql(1);
+ expect(await PageObjects.lens.getDimensionTriggerText('lnsPie_sliceByDimensionPanel')).to.eql(
+ 'Top values of geo.dest'
+ );
+ expect(await PageObjects.lens.getDimensionTriggerText('lnsPie_sizeByDimensionPanel')).to.eql(
+ 'Average of bytes'
+ );
+ });
+
  it('should allow transition from line chart to donut chart and to bar chart', async () => {
  await PageObjects.visualize.gotoVisualizationLandingPage();
  await listingTable.searchForItemWithName('lnsXYvis');

diff --git a/x-pack/test/functional/page_objects/lens_page.ts b/x-pack/test/functional/page_objects/lens_page.ts
@@ -89,10 +89,14 @@ export function LensPageProvider({ getService, getPageObjects }: FtrProviderCont
  * @param opts.dimension - the selector of the dimension being changed
  * @param opts.operation - the desired operation ID for the dimension
  * @param opts.field - the desired field for the dimension
+ * @param layerIndex - the index of the layer
  */
- async configureDimension(opts: { dimension: string; operation: string; field: string }) {
+ async configureDimension(
+ opts: { dimension: string; operation: string; field: string },
+ layerIndex = 0
+ ) {
  await retry.try(async () => {
- await testSubjects.click(opts.dimension);
+ await testSubjects.click(`lns-layerPanel-${layerIndex} > ${opts.dimension}`);
  await testSubjects.exists(`lns-indexPatternDimension-${opts.operation}`);
  });