diff --git a/web/ui/react-app/src/ExpressionInput.test.tsx b/web/ui/react-app/src/ExpressionInput.test.tsx index 07bb0b01ef..c7034cebce 100644 --- a/web/ui/react-app/src/ExpressionInput.test.tsx +++ b/web/ui/react-app/src/ExpressionInput.test.tsx @@ -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(); + 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(); + const btn = wrapper.find(Button).filterWhere(btn => btn.hasClass('execute-btn')); + btn.simulate('click'); + expect(spyExecuteQuery).toHaveBeenCalledTimes(1); }); }); diff --git a/web/ui/react-app/src/ExpressionInput.tsx b/web/ui/react-app/src/ExpressionInput.tsx index fb514884e0..0fd2dc5208 100644 --- a/web/ui/react-app/src/ExpressionInput.tsx +++ b/web/ui/react-app/src/ExpressionInput.tsx @@ -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 { @@ -26,7 +26,6 @@ class ExpressionInput extends Component { - 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) => { + 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: '', @@ -125,7 +125,8 @@ class ExpressionInput extends Component {downshift => ( @@ -175,7 +176,7 @@ class ExpressionInput extends Component - diff --git a/web/ui/react-app/src/Panel.test.tsx b/web/ui/react-app/src/Panel.test.tsx index e5d38841b6..fa133abe69 100644 --- a/web/ui/react-app/src/Panel.test.tsx +++ b/web/ui/react-app/src/Panel.test.tsx @@ -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(); + const panel = shallow(); 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(); + const panel = shallow(); 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(); + const graphPanel = mount(); 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(); + 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(); + 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); + }); + }); }); diff --git a/web/ui/react-app/src/Panel.tsx b/web/ui/react-app/src/Panel.tsx index e8ee99508c..66c6546287 100644 --- a/web/ui/react-app/src/Panel.tsx +++ b/web/ui/react-app/src/Panel.tsx @@ -68,36 +68,26 @@ class Panel extends Component { }; } - 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 { 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 { { { - this.setOptions({ type: 'table' }); - }} + onClick={() => this.handleChangeType(PanelType.Table)} > Table @@ -249,9 +243,7 @@ class Panel extends Component { { - this.setOptions({ type: 'graph' }); - }} + onClick={() => this.handleChangeType(PanelType.Graph)} > Graph diff --git a/web/ui/react-app/src/TimeInput.tsx b/web/ui/react-app/src/TimeInput.tsx index 5bee6e3495..43d0590bd1 100644 --- a/web/ui/react-app/src/TimeInput.tsx +++ b/web/ui/react-app/src/TimeInput.tsx @@ -27,7 +27,6 @@ interface TimeInputProps { time: number | null; // Timestamp in milliseconds. range: number; // Range in seconds. placeholder: string; - onChangeTime: (time: number | null) => void; } diff --git a/web/ui/react-app/src/PanelList.test.tsx b/web/ui/react-app/src/pages/PanelList.test.tsx similarity index 71% rename from web/ui/react-app/src/PanelList.test.tsx rename to web/ui/react-app/src/pages/PanelList.test.tsx index 1d7b230b30..64bde5061c 100755 --- a/web/ui/react-app/src/PanelList.test.tsx +++ b/web/ui/react-app/src/pages/PanelList.test.tsx @@ -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(); + const instance: any = panelList.instance(); + const updateURLSpy = jest.spyOn(instance, 'updateURL'); + instance.handleExecuteQuery('new'); + expect(updateURLSpy).toHaveBeenCalledTimes(1); + }); }); diff --git a/web/ui/react-app/src/pages/PanelList.tsx b/web/ui/react-app/src/pages/PanelList.tsx index c04e247bd4..25422451ee 100644 --- a/web/ui/react-app/src/pages/PanelList.tsx +++ b/web/ui/react-app/src/pages/PanelList.tsx @@ -96,7 +96,7 @@ class PanelList extends Component { + handleExecuteQuery = (query: string) => { const isSimpleMetric = this.state.metricNames.indexOf(query) !== -1; if (isSimpleMetric || !query.length) { return; @@ -110,6 +110,7 @@ class PanelList extends Component { 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 {panels.map(({ id, options }) => ( this.handleOptionsChanged(id, opts)}