React UI: Fix issue when changing query then time, the old query executed (#6437)

* React UI: Fix issue when changing query then time, the old query is executed

Signed-off-by: Dustin Hooten <dhooten@splunk.com>

* pr feedback

Signed-off-by: Dustin Hooten <dhooten@splunk.com>

* more pr feedback

Signed-off-by: Dustin Hooten <dhooten@splunk.com>
This commit is contained in:
Dustin Hooten 2019-12-12 15:22:12 -07:00 committed by Julius Volz
parent 67838643ee
commit fce2e131db
7 changed files with 128 additions and 68 deletions

View File

@ -23,6 +23,7 @@ describe('ExpressionInput', () => {
'Metric Names': metricNames,
},
executeQuery: (): void => {},
onExpressionChange: (): void => {},
loading: false,
};
@ -62,7 +63,7 @@ describe('ExpressionInput', () => {
expect(input.prop('type')).toEqual('textarea');
expect(input.prop('rows')).toEqual('1');
expect(input.prop('placeholder')).toEqual('Expression (press Shift+Enter for newlines)');
expect(expressionInput.state('value')).toEqual('node_cpu');
expect(input.prop('value')).toEqual('node_cpu');
});
describe('when autosuggest is closed', () => {
@ -94,6 +95,14 @@ describe('ExpressionInput', () => {
instance.handleInput();
expect(stateSpy).toHaveBeenCalled();
});
it('should call onExpressionChange', () => {
const spyOnExpressionChange = jest.fn();
const props = { ...expressionInputProps, onExpressionChange: spyOnExpressionChange };
const wrapper = mount(<ExpressionInput {...props} />);
const input = wrapper.find(Input);
input.simulate('input', { target: { value: 'prometheus_engine_' } });
expect(spyOnExpressionChange).toHaveBeenCalledTimes(1);
});
});
describe('onSelect', () => {
@ -101,7 +110,18 @@ describe('ExpressionInput', () => {
const instance: any = expressionInput.instance();
const stateSpy = jest.spyOn(instance, 'setState');
instance.setValue('foo');
expect(stateSpy).toHaveBeenCalledWith({ value: 'foo', height: 'auto' }, expect.anything());
expect(stateSpy).toHaveBeenCalledWith({ height: 'auto' }, expect.anything());
});
});
describe('onClick', () => {
it('executes the query', () => {
const spyExecuteQuery = jest.fn();
const props = { ...expressionInputProps, executeQuery: spyExecuteQuery };
const wrapper = mount(<ExpressionInput {...props} />);
const btn = wrapper.find(Button).filterWhere(btn => btn.hasClass('execute-btn'));
btn.simulate('click');
expect(spyExecuteQuery).toHaveBeenCalledTimes(1);
});
});

View File

@ -10,14 +10,14 @@ import { faSearch, faSpinner } from '@fortawesome/free-solid-svg-icons';
interface ExpressionInputProps {
value: string;
onExpressionChange: (expr: string) => void;
autocompleteSections: { [key: string]: string[] };
executeQuery: (expr: string) => void;
executeQuery: () => void;
loading: boolean;
}
interface ExpressionInputState {
height: number | string;
value: string;
}
class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputState> {
@ -26,7 +26,6 @@ class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputSta
constructor(props: ExpressionInputProps) {
super(props);
this.state = {
value: props.value,
height: 'auto',
};
}
@ -46,7 +45,9 @@ class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputSta
};
setValue = (value: string) => {
this.setState({ value, height: 'auto' }, this.setHeight);
const { onExpressionChange } = this.props;
onExpressionChange(value);
this.setState({ height: 'auto' }, this.setHeight);
};
componentDidUpdate(prevProps: ExpressionInputProps) {
@ -57,14 +58,13 @@ class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputSta
}
handleKeyPress = (event: React.KeyboardEvent<HTMLInputElement>) => {
const { executeQuery } = this.props;
if (event.key === 'Enter' && !event.shiftKey) {
this.executeQuery();
executeQuery();
event.preventDefault();
}
};
executeQuery = () => this.props.executeQuery(this.exprInputRef.current!.value);
getSearchMatches = (input: string, expressions: string[]) => {
return fuzzy.filter(input.replace(/ /g, ''), expressions, {
pre: '<strong>',
@ -125,7 +125,8 @@ class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputSta
};
render() {
const { value, height } = this.state;
const { executeQuery, value } = this.props;
const { height } = this.state;
return (
<Downshift onSelect={this.setValue}>
{downshift => (
@ -175,7 +176,7 @@ class ExpressionInput extends Component<ExpressionInputProps, ExpressionInputSta
value={value}
/>
<InputGroupAddon addonType="append">
<Button className="execute-btn" color="primary" onClick={this.executeQuery}>
<Button className="execute-btn" color="primary" onClick={executeQuery}>
Execute
</Button>
</InputGroupAddon>

View File

@ -8,27 +8,28 @@ import TimeInput from './TimeInput';
import DataTable from './DataTable';
import { GraphTabContent } from './graph/GraphTabContent';
const defaultProps = {
options: {
expr: 'prometheus_engine',
type: PanelType.Table,
range: 10,
endTime: 1572100217898,
resolution: 28,
stacked: false,
},
onOptionsChanged: (): void => {},
pastQueries: [],
metricNames: [
'prometheus_engine_queries',
'prometheus_engine_queries_concurrent_max',
'prometheus_engine_query_duration_seconds',
],
removePanel: (): void => {},
onExecuteQuery: (): void => {},
};
describe('Panel', () => {
const props = {
options: {
expr: 'prometheus_engine',
type: PanelType.Table,
range: 10,
endTime: 1572100217898,
resolution: 28,
stacked: false,
},
onOptionsChanged: (): void => {},
pastQueries: [],
metricNames: [
'prometheus_engine_queries',
'prometheus_engine_queries_concurrent_max',
'prometheus_engine_query_duration_seconds',
],
removePanel: (): void => {},
onExecuteQuery: (): void => {},
};
const panel = shallow(<Panel {...props} />);
const panel = shallow(<Panel {...defaultProps} />);
it('renders an ExpressionInput', () => {
const input = panel.find(ExpressionInput);
@ -48,7 +49,7 @@ describe('Panel', () => {
const onOptionsChanged = (opts: PanelOptions): void => {
results.push(opts);
};
const panel = shallow(<Panel {...props} onOptionsChanged={onOptionsChanged} />);
const panel = shallow(<Panel {...defaultProps} onOptionsChanged={onOptionsChanged} />);
const links = panel.find(NavLink);
[{ panelType: 'Table', active: true }, { panelType: 'Graph', active: false }].forEach(
(tc: { panelType: string; active: boolean }, i: number) => {
@ -66,8 +67,8 @@ describe('Panel', () => {
it('renders a TabPane with a TimeInput and a DataTable when in table mode', () => {
const tab = panel.find(TabPane).filterWhere(tab => tab.prop('tabId') === 'table');
const timeInput = tab.find(TimeInput);
expect(timeInput.prop('time')).toEqual(props.options.endTime);
expect(timeInput.prop('range')).toEqual(props.options.range);
expect(timeInput.prop('time')).toEqual(defaultProps.options.endTime);
expect(timeInput.prop('range')).toEqual(defaultProps.options.range);
expect(timeInput.prop('placeholder')).toEqual('Evaluation time');
expect(tab.find(DataTable)).toHaveLength(1);
});
@ -81,7 +82,7 @@ describe('Panel', () => {
resolution: 28,
stacked: false,
};
const graphPanel = mount(<Panel {...props} options={options} />);
const graphPanel = mount(<Panel {...defaultProps} options={options} />);
const controls = graphPanel.find(GraphControls);
graphPanel.setState({ data: { resultType: 'matrix', result: [] } });
const graph = graphPanel.find(GraphTabContent);
@ -91,4 +92,40 @@ describe('Panel', () => {
expect(controls.prop('stacked')).toEqual(options.stacked);
expect(graph.prop('stacked')).toEqual(options.stacked);
});
describe('when switching between modes', () => {
[{ from: PanelType.Table, to: PanelType.Graph }, { from: PanelType.Graph, to: PanelType.Table }].forEach(
({ from, to }: { from: PanelType; to: PanelType }) => {
it(`${from} -> ${to} nulls out data`, () => {
const props = {
...defaultProps,
options: { ...defaultProps.options, type: from },
};
const panel = shallow(<Panel {...props} />);
const instance: any = panel.instance();
panel.setState({ data: 'somedata' });
expect(panel.state('data')).toEqual('somedata');
instance.handleChangeType(to);
expect(panel.state('data')).toBeNull();
});
}
);
});
describe('when changing query then time', () => {
it('executes the new query', () => {
const initialExpr = 'time()';
const newExpr = 'time() - time()';
const panel = shallow(<Panel {...defaultProps} options={{ ...defaultProps.options, expr: initialExpr }} />);
const instance: any = panel.instance();
instance.executeQuery();
const executeQuerySpy = jest.spyOn(instance, 'executeQuery');
//change query without executing
panel.setProps({ options: { ...defaultProps.options, expr: newExpr } });
expect(executeQuerySpy).toHaveBeenCalledTimes(0);
//execute query implicitly with time change
panel.setProps({ options: { ...defaultProps.options, expr: newExpr, endTime: 1575744840 } });
expect(executeQuerySpy).toHaveBeenCalledTimes(1);
});
});
});

View File

@ -68,36 +68,26 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
};
}
componentDidUpdate(prevProps: PanelProps, prevState: PanelState) {
const prevOpts = prevProps.options;
const opts = this.props.options;
componentDidUpdate({ options: prevOpts }: PanelProps) {
const { endTime, range, resolution, type } = this.props.options;
if (
prevOpts.type !== opts.type ||
prevOpts.range !== opts.range ||
prevOpts.endTime !== opts.endTime ||
prevOpts.resolution !== opts.resolution ||
prevOpts.expr !== opts.expr
prevOpts.endTime !== endTime ||
prevOpts.range !== range ||
prevOpts.resolution !== resolution ||
prevOpts.type !== type
) {
if (prevOpts.type !== opts.type) {
// If the other options change, we still want to show the old data until the new
// query completes, but this is not a good idea when we actually change between
// table and graph view, since not all queries work well in both.
this.setState({ data: null });
}
this.executeQuery(opts.expr);
this.executeQuery();
}
}
componentDidMount() {
this.executeQuery(this.props.options.expr);
this.executeQuery();
}
executeQuery = (expr: string): void => {
executeQuery = (): void => {
const { expr } = this.props.options;
const queryStart = Date.now();
this.props.onExecuteQuery(expr);
if (this.props.options.expr !== expr) {
this.setOptions({ expr: expr });
}
if (expr === '') {
return;
}
@ -209,6 +199,11 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
this.setOptions({ resolution: resolution });
};
handleChangeType = (type: PanelType) => {
this.setState({ data: null });
this.setOptions({ type: type });
};
handleChangeStacking = (stacked: boolean) => {
this.setOptions({ stacked: stacked });
};
@ -221,6 +216,7 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
<Col>
<ExpressionInput
value={options.expr}
onExpressionChange={this.handleExpressionChange}
executeQuery={this.executeQuery}
loading={this.state.loading}
autocompleteSections={{
@ -239,9 +235,7 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
<NavItem>
<NavLink
className={options.type === 'table' ? 'active' : ''}
onClick={() => {
this.setOptions({ type: 'table' });
}}
onClick={() => this.handleChangeType(PanelType.Table)}
>
Table
</NavLink>
@ -249,9 +243,7 @@ class Panel extends Component<PanelProps & PathPrefixProps, PanelState> {
<NavItem>
<NavLink
className={options.type === 'graph' ? 'active' : ''}
onClick={() => {
this.setOptions({ type: 'graph' });
}}
onClick={() => this.handleChangeType(PanelType.Graph)}
>
Graph
</NavLink>

View File

@ -27,7 +27,6 @@ interface TimeInputProps {
time: number | null; // Timestamp in milliseconds.
range: number; // Range in seconds.
placeholder: string;
onChangeTime: (time: number | null) => void;
}

View File

@ -1,9 +1,11 @@
import * as React from 'react';
import { mount, shallow } from 'enzyme';
import PanelList from './pages/PanelList';
import Checkbox from './Checkbox';
import PanelList from './PanelList';
import Checkbox from '../Checkbox';
import { Alert, Button } from 'reactstrap';
import Panel from './Panel';
import Panel, { PanelDefaultOptions } from '../Panel';
import { decodePanelOptionsFromQueryString } from '../utils/urlParams';
import ExpressionInput from '../ExpressionInput';
describe('PanelList', () => {
it('renders a query history checkbox', () => {
@ -37,4 +39,12 @@ describe('PanelList', () => {
expect(btn.prop('color')).toEqual('primary');
expect(btn.children().text()).toEqual('Add Panel');
});
it('updates URL when a query is executed', () => {
const panelList = mount(<PanelList />);
const instance: any = panelList.instance();
const updateURLSpy = jest.spyOn(instance, 'updateURL');
instance.handleExecuteQuery('new');
expect(updateURLSpy).toHaveBeenCalledTimes(1);
});
});

View File

@ -96,7 +96,7 @@ class PanelList extends Component<RouteComponentProps & PathPrefixProps, PanelLi
});
};
handleQueryHistory = (query: string) => {
handleExecuteQuery = (query: string) => {
const isSimpleMetric = this.state.metricNames.indexOf(query) !== -1;
if (isSimpleMetric || !query.length) {
return;
@ -110,6 +110,7 @@ class PanelList extends Component<RouteComponentProps & PathPrefixProps, PanelLi
);
localStorage.setItem('history', JSON.stringify(extendedItems.slice(0, 50)));
this.updatePastQueries();
this.updateURL();
};
updateURL() {
@ -119,7 +120,7 @@ class PanelList extends Component<RouteComponentProps & PathPrefixProps, PanelLi
handleOptionsChanged = (id: string, options: PanelOptions) => {
const updatedPanels = this.state.panels.map(p => (id === p.id ? { ...p, options } : p));
this.setState({ panels: updatedPanels }, this.updateURL);
this.setState({ panels: updatedPanels });
};
addPanel = () => {
@ -181,7 +182,7 @@ class PanelList extends Component<RouteComponentProps & PathPrefixProps, PanelLi
</Row>
{panels.map(({ id, options }) => (
<Panel
onExecuteQuery={this.handleQueryHistory}
onExecuteQuery={this.handleExecuteQuery}
key={id}
options={options}
onOptionsChanged={opts => this.handleOptionsChanged(id, opts)}