Skip to content

Commit

Permalink
[Maps] fix app crash where layer details panel calls getId on undefin…
Browse files Browse the repository at this point in the history
…ed (#31816) (#31844)

* [Maps] fix app crash where details panel calls getId on undefined

* add jest test ensuring no errors when selectedLayer is undefined
  • Loading branch information
nreese authored Feb 23, 2019
1 parent f561ff1 commit 0de8002
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 9 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/actions/store_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ export function removeSelectedLayer() {
const state = getState();
const layerId = getSelectedLayerId(state);
dispatch(removeLayer(layerId));
dispatch(setSelectedLayer(null));
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`LayerPanel is rendered 1`] = `
<EuiFlexGroup
alignItems="stretch"
component="div"
direction="column"
gutterSize="none"
justifyContent="flexStart"
responsive={true}
wrap={false}
>
<EuiFlyoutHeader
className="mapLayerPanel__header"
hasBorder={true}
>
<EuiFlexGroup
alignItems="center"
component="div"
direction="row"
gutterSize="s"
justifyContent="flexStart"
responsive={false}
wrap={false}
>
<EuiFlexItem
component="div"
grow={false}
>
<EuiButtonIcon
aria-label="Fit to bounds"
color="primary"
iconSize="m"
iconType="vector"
onClick={[Function]}
type="button"
>
Fit
</EuiButtonIcon>
</EuiFlexItem>
<EuiFlexItem
component="div"
grow={true}
>
<EuiTitle
size="s"
textTransform="none"
>
<h2>
layer 1
</h2>
</EuiTitle>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer
size="xs"
/>
<div
className="mapLayerPanel__sourceDetails"
>
<EuiAccordion
buttonContent="Source details"
id="accordion1"
initialIsOpen={false}
paddingSize="none"
>
<EuiText
color="subdued"
grow={true}
size="s"
>
<EuiSpacer
size="xs"
/>
<p
className="mapLayerPanel__sourceDetail"
key="source prop1"
>
<strong>
source prop1
</strong>
<span>
you get one chance to set me
</span>
</p>
</EuiText>
</EuiAccordion>
</div>
</EuiFlyoutHeader>
<EuiFlyoutBody
className="mapLayerPanel__body"
>
<SettingsPanel />
<EuiSpacer
size="s"
/>
<EuiPanel
grow={true}
hasShadow={false}
paddingSize="m"
>
<JoinEditor />
</EuiPanel>
<EuiSpacer
size="s"
/>
<StyleTabs
layer={
Object {
"getDisplayName": [Function],
"getId": [Function],
"getImmutableSourceProperties": [Function],
"getLayerTypeIconName": [Function],
"isJoinable": [Function],
}
}
/>
</EuiFlyoutBody>
<EuiFlyoutFooter
className="mapLayerPanel__footer"
>
<FlyoutFooter
hasStateChanged={[Function]}
/>
</EuiFlyoutFooter>
</EuiFlexGroup>
`;

exports[`LayerPanel should render empty panel when selectedLayer is null 1`] = `""`;
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,14 @@ import { updateFlyout, FLYOUT_STATE } from '../../../store/ui';
import {
setSelectedLayer,
removeSelectedLayer,
rollbackToTrackedLayerStateForSelectedLayer,
removeTrackedLayerStateForSelectedLayer
} from '../../../actions/store_actions';

const mapDispatchToProps = (dispatch) => {
return {
cancelLayerPanel: async () => {
await dispatch(updateFlyout(FLYOUT_STATE.NONE));
await dispatch(rollbackToTrackedLayerStateForSelectedLayer());
await dispatch(setSelectedLayer(null));
cancelLayerPanel: () => {
dispatch(updateFlyout(FLYOUT_STATE.NONE));
dispatch(setSelectedLayer(null));
},
saveLayerEdits: () => {
dispatch(updateFlyout(FLYOUT_STATE.NONE));
Expand All @@ -29,7 +27,6 @@ const mapDispatchToProps = (dispatch) => {
removeLayer: () => {
dispatch(updateFlyout(FLYOUT_STATE.NONE));
dispatch(removeSelectedLayer());
dispatch(setSelectedLayer(null));
}
};
};
Expand Down
14 changes: 11 additions & 3 deletions x-pack/plugins/maps/public/components/layer_panel/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ import {
EuiLink,
} from '@elastic/eui';

export class LayerPanel extends React.Component {
export class LayerPanel extends React.Component {

static getDerivedStateFromProps(nextProps, prevState) {
const nextId = nextProps.selectedLayer.getId();
const nextId = nextProps.selectedLayer ? nextProps.selectedLayer.getId() : null;
if (nextId !== prevState.prevId) {
return {
displayName: '',
Expand Down Expand Up @@ -59,6 +59,10 @@ export class LayerPanel extends React.Component {
}

loadDisplayName = async () => {
if (!this.props.selectedLayer) {
return;
}

const displayName = await this.props.selectedLayer.getDisplayName();
if (!this._isMounted || displayName === this.state.displayName) {
return;
Expand All @@ -68,7 +72,7 @@ export class LayerPanel extends React.Component {
}

loadImmutableSourceProperties = async () => {
if (this.state.hasLoadedSourcePropsForLayer) {
if (this.state.hasLoadedSourcePropsForLayer || !this.props.selectedLayer) {
return;
}

Expand Down Expand Up @@ -116,6 +120,10 @@ export class LayerPanel extends React.Component {
render() {
const { selectedLayer } = this.props;

if (!selectedLayer) {
return null;
}

return (
<EuiFlexGroup
direction="column"
Expand Down
87 changes: 87 additions & 0 deletions x-pack/plugins/maps/public/components/layer_panel/view.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

jest.mock('./style_tabs', () => ({
StyleTabs: () => {
return (<div>mockStyleTabs</div>);
}
}));

jest.mock('./join_editor', () => ({
JoinEditor: () => {
return (<div>mockJoinEditor</div>);
}
}));

jest.mock('./flyout_footer', () => ({
FlyoutFooter: () => {
return (<div>mockFlyoutFooter</div>);
}
}));

jest.mock('./settings_panel', () => ({
SettingsPanel: () => {
return (<div>mockSettingsPanel</div>);
}
}));

import React from 'react';
import { shallow } from 'enzyme';

import { LayerPanel } from './view';

const mockLayer = {
getId: () => { return '1'; },
getDisplayName: () => { return 'layer 1'; },
getImmutableSourceProperties: () => {
return [
{ label: 'source prop1', value: 'you get one chance to set me' }
];
},
isJoinable: () => { return true; },
getLayerTypeIconName: () => { return 'vector'; }
};

const defaultProps = {
selectedLayer: mockLayer,
hasStateChanged: () => {},
fitToBounds: () => {},
};

describe('LayerPanel', () => {
test('is rendered', async () => {
const component = shallow(
<LayerPanel
{...defaultProps}
/>
);

// Ensure all promises resolve
await new Promise(resolve => process.nextTick(resolve));
// Ensure the state changes are reflected
component.update();

expect(component)
.toMatchSnapshot();
});

test('should render empty panel when selectedLayer is null', async () => {
const component = shallow(
<LayerPanel
{...defaultProps}
selectedLayer={undefined}
/>
);

// Ensure all promises resolve
await new Promise(resolve => process.nextTick(resolve));
// Ensure the state changes are reflected
component.update();

expect(component)
.toMatchSnapshot();
});
});

0 comments on commit 0de8002

Please sign in to comment.