From 769be876a5fcf733975361cf888457d296c868bc Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 3 May 2022 14:50:13 +0200 Subject: [PATCH] Loki, Prometheus: Remember preferred editor (#48580) * Loki: Remember default editor * Loki: Add tests * Prometheus: Set default editor type * Fix and refactor tests * Remove unused import --- .../components/LokiQueryEditorSelector.tsx | 9 +- .../loki/querybuilder/state.test.ts | 22 +++ .../components/PromQueryEditorByApp.test.tsx | 22 +-- .../components/PromQueryField.test.tsx | 157 ++++++++---------- .../PromQueryEditorSelector.test.tsx | 13 -- .../prometheus/querybuilder/state.test.ts | 6 +- .../prometheus/querybuilder/state.ts | 4 +- 7 files changed, 102 insertions(+), 131 deletions(-) create mode 100644 public/app/plugins/datasource/loki/querybuilder/state.test.ts diff --git a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryEditorSelector.tsx b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryEditorSelector.tsx index fe2c798aac7..ecc74b6efbd 100644 --- a/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryEditorSelector.tsx +++ b/public/app/plugins/datasource/loki/querybuilder/components/LokiQueryEditorSelector.tsx @@ -11,7 +11,7 @@ import { LokiQueryEditorProps } from '../../components/types'; import { LokiQuery } from '../../types'; import { lokiQueryModeller } from '../LokiQueryModeller'; import { buildVisualQueryFromString } from '../parsing'; -import { getQueryWithDefaults } from '../state'; +import { changeEditorMode, getQueryWithDefaults } from '../state'; import { LokiQueryBuilderContainer } from './LokiQueryBuilderContainer'; import { LokiQueryBuilderExplained } from './LokiQueryBuilderExplained'; @@ -26,9 +26,8 @@ export const LokiQueryEditorSelector = React.memo((props) const query = getQueryWithDefaults(props.query); const onEditorModeChange = useCallback( - (newMetricEditorMode: QueryEditorMode) => { - const change = { ...query, editorMode: newMetricEditorMode }; - if (newMetricEditorMode === QueryEditorMode.Builder) { + (newEditorMode: QueryEditorMode) => { + if (newEditorMode === QueryEditorMode.Builder) { const result = buildVisualQueryFromString(query.expr || ''); // If there are errors, give user a chance to decide if they want to go to builder as that can loose some data. if (result.errors.length) { @@ -36,7 +35,7 @@ export const LokiQueryEditorSelector = React.memo((props) return; } } - onChange(change); + changeEditorMode(query, newEditorMode, onChange); }, [onChange, query] ); diff --git a/public/app/plugins/datasource/loki/querybuilder/state.test.ts b/public/app/plugins/datasource/loki/querybuilder/state.test.ts new file mode 100644 index 00000000000..fddca4e2741 --- /dev/null +++ b/public/app/plugins/datasource/loki/querybuilder/state.test.ts @@ -0,0 +1,22 @@ +import { QueryEditorMode } from '../../prometheus/querybuilder/shared/types'; + +import { changeEditorMode, getQueryWithDefaults } from './state'; + +describe('getQueryWithDefaults(', () => { + it('should set defaults', () => { + expect(getQueryWithDefaults({ refId: 'A' } as any)).toEqual({ + editorMode: 'builder', + expr: '', + queryType: 'range', + refId: 'A', + }); + }); + + it('changing editor mode with blank query should change default', () => { + changeEditorMode({ refId: 'A', expr: '' }, QueryEditorMode.Code, (query) => { + expect(query.editorMode).toBe(QueryEditorMode.Code); + }); + + expect(getQueryWithDefaults({ refId: 'A' } as any).editorMode).toEqual(QueryEditorMode.Code); + }); +}); diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryEditorByApp.test.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryEditorByApp.test.tsx index 45f257cfabe..9d80cddb841 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryEditorByApp.test.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryEditorByApp.test.tsx @@ -1,5 +1,4 @@ import { render, RenderResult } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; import { noop } from 'lodash'; import React from 'react'; @@ -43,6 +42,7 @@ function setup(app: CoreApp): RenderResult & { onRunQuery: jest.Mock } { createQuery: jest.fn((q) => q), getInitHints: () => [], getPrometheusTime: jest.fn((date, roundup) => 123), + getQueryHints: jest.fn(() => []), languageProvider: { start: () => Promise.resolve([]), syntax: () => {}, @@ -96,24 +96,4 @@ describe('PromQueryEditorByApp', () => { expect(getByTestId('QueryEditorModeToggle')).toBeInTheDocument(); expect(queryByTestId(alertingTestIds.editor)).toBeNull(); }); - - it('should not run query onBlur in explore', async () => { - const { getByTestId, onRunQuery } = setup(CoreApp.Explore); - - const input = getByTestId('dummy-code-input'); - expect(input).toBeInTheDocument(); - await userEvent.type(input, 'metric'); - input.blur(); - expect(onRunQuery).not.toHaveBeenCalled(); - }); - - it('should run query onBlur in dashboard', async () => { - const { getByTestId, onRunQuery } = setup(CoreApp.Dashboard); - - const input = getByTestId('dummy-code-input'); - expect(input).toBeInTheDocument(); - await userEvent.type(input, 'metric'); - input.blur(); - expect(onRunQuery).toHaveBeenCalled(); - }); }); diff --git a/public/app/plugins/datasource/prometheus/components/PromQueryField.test.tsx b/public/app/plugins/datasource/prometheus/components/PromQueryField.test.tsx index 4a047c07ac8..0538d8c9455 100644 --- a/public/app/plugins/datasource/prometheus/components/PromQueryField.test.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromQueryField.test.tsx @@ -1,25 +1,46 @@ -import { render, screen } from '@testing-library/react'; +import { getByTestId, render, screen } from '@testing-library/react'; // @ts-ignore -import RCCascader from 'rc-cascader'; +import userEvent from '@testing-library/user-event'; import React from 'react'; -import { DataSourceInstanceSettings, PanelData, LoadingState, DataFrame } from '@grafana/data'; +import { PanelData, LoadingState, DataFrame, CoreApp } from '@grafana/data'; +import { PrometheusDatasource } from '../datasource'; import PromQlLanguageProvider from '../language_provider'; -import { PromOptions } from '../types'; import PromQueryField from './PromQueryField'; // the monaco-based editor uses lazy-loading and that does not work // well with this test, and we do not need the monaco-related // functionality in this test anyway, so we mock it out. -jest.mock('./monaco-query-field/MonacoQueryFieldWrapper', () => { - const fakeQueryField = () =>
prometheus query field
; +jest.mock('./monaco-query-field/MonacoQueryFieldLazy', () => { + const fakeQueryField = (props: any) => { + return ; + }; return { - MonacoQueryFieldWrapper: fakeQueryField, + MonacoQueryFieldLazy: fakeQueryField, }; }); +const defaultProps = { + datasource: { + languageProvider: { + start: () => Promise.resolve([]), + syntax: () => {}, + getLabelKeys: () => [], + metrics: [], + }, + getInitHints: () => [], + } as unknown as PrometheusDatasource, + query: { + expr: '', + refId: '', + }, + onRunQuery: () => {}, + onChange: () => {}, + history: [], +}; + describe('PromQueryField', () => { beforeAll(() => { // @ts-ignore @@ -27,97 +48,37 @@ describe('PromQueryField', () => { }); it('renders metrics chooser regularly if lookups are not disabled in the datasource settings', () => { - const datasource = { - languageProvider: { - start: () => Promise.resolve([]), - syntax: () => {}, - getLabelKeys: () => [], - metrics: [], - }, - getInitHints: () => [], - } as unknown as DataSourceInstanceSettings; - - const queryField = render( - {}} - onChange={() => {}} - history={[]} - /> - ); + const queryField = render(); expect(queryField.getAllByRole('button')).toHaveLength(1); }); it('renders a disabled metrics chooser if lookups are disabled in datasource settings', () => { - const datasource = { - languageProvider: { - start: () => Promise.resolve([]), - syntax: () => {}, - getLabelKeys: () => [], - metrics: [], - }, - getInitHints: () => [], - } as unknown as DataSourceInstanceSettings; - const queryField = render( - {}} - onChange={() => {}} - history={[]} - /> - ); + const props = defaultProps; + props.datasource.lookupsDisabled = true; + const queryField = render(); const bcButton = queryField.getByRole('button'); expect(bcButton).toBeDisabled(); }); it('renders an initial hint if no data and initial hint provided', () => { - const datasource = { - languageProvider: { - start: () => Promise.resolve([]), - syntax: () => {}, - getLabelKeys: () => [], - metrics: [], - }, - getInitHints: () => [{ label: 'Initial hint', type: 'INFO' }], - } as unknown as DataSourceInstanceSettings; - render( - {}} - onChange={() => {}} - history={[]} - /> - ); + const props = defaultProps; + props.datasource.lookupsDisabled = true; + props.datasource.getInitHints = () => [{ label: 'Initial hint', type: 'INFO' }]; + render(); + expect(screen.getByText('Initial hint')).toBeInTheDocument(); }); it('renders query hint if data, query hint and initial hint provided', () => { - const datasource = { - languageProvider: { - start: () => Promise.resolve([]), - syntax: () => {}, - getLabelKeys: () => [], - metrics: [], - }, - getInitHints: () => [{ label: 'Initial hint', type: 'INFO' }], - getQueryHints: () => [{ label: 'Query hint', type: 'INFO' }], - } as unknown as DataSourceInstanceSettings; + const props = defaultProps; + props.datasource.lookupsDisabled = true; + props.datasource.getInitHints = () => [{ label: 'Initial hint', type: 'INFO' }]; + props.datasource.getQueryHints = () => [{ label: 'Query hint', type: 'INFO' }]; render( {}} - onChange={() => {}} - history={[]} + {...props} data={ { series: [{ name: 'test name' }] as DataFrame[], @@ -126,6 +87,7 @@ describe('PromQueryField', () => { } /> ); + expect(screen.getByText('Query hint')).toBeInTheDocument(); expect(screen.queryByText('Initial hint')).not.toBeInTheDocument(); }); @@ -140,11 +102,12 @@ describe('PromQueryField', () => { const metrics = ['foo', 'bar']; const queryField = render( [], - }} + datasource={ + { + languageProvider: makeLanguageProvider({ metrics: [metrics] }), + getInitHints: () => [], + } as unknown as PrometheusDatasource + } {...defaultProps} /> ); @@ -164,6 +127,28 @@ describe('PromQueryField', () => { let labelBrowser = screen.getByRole('button'); expect(labelBrowser.textContent).toContain('Loading'); }); + + it('should not run query onBlur in explore', async () => { + const onRunQuery = jest.fn(); + const { container } = render(); + + const input = getByTestId(container, 'dummy-code-input'); + expect(input).toBeInTheDocument(); + await userEvent.type(input, 'metric'); + input.blur(); + expect(onRunQuery).not.toHaveBeenCalled(); + }); + + it('should run query onBlur in dashboard', async () => { + const onRunQuery = jest.fn(); + const { container } = render(); + + const input = getByTestId(container, 'dummy-code-input'); + expect(input).toBeInTheDocument(); + await userEvent.type(input, 'metric'); + input.blur(); + expect(onRunQuery).toHaveBeenCalled(); + }); }); function makeLanguageProvider(options: { metrics: string[][] }) { diff --git a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx index 73c5613b1db..3d3bd9821b9 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx +++ b/public/app/plugins/datasource/prometheus/querybuilder/components/PromQueryEditorSelector.test.tsx @@ -60,19 +60,6 @@ describe('PromQueryEditorSelector', () => { expectCodeEditor(); }); - it('shows code if new query', async () => { - render( - - ); - expectCodeEditor(); - }); - it('shows code editor when code mode is set', async () => { renderWithMode(QueryEditorMode.Code); expectCodeEditor(); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/state.test.ts b/public/app/plugins/datasource/prometheus/querybuilder/state.test.ts index cb05cae6fba..b0caa8c9f01 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/state.test.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/state.test.ts @@ -6,7 +6,7 @@ import { changeEditorMode, getQueryWithDefaults } from './state'; describe('getQueryWithDefaults(', () => { it('should set defaults', () => { expect(getQueryWithDefaults({ refId: 'A' } as any, CoreApp.Dashboard)).toEqual({ - editorMode: 'code', + editorMode: 'builder', expr: '', legendFormat: '__auto', range: true, @@ -16,7 +16,7 @@ describe('getQueryWithDefaults(', () => { it('should set both range and instant to true when in Explore', () => { expect(getQueryWithDefaults({ refId: 'A' } as any, CoreApp.Explore)).toEqual({ - editorMode: 'code', + editorMode: 'builder', expr: '', legendFormat: '__auto', range: true, @@ -25,7 +25,7 @@ describe('getQueryWithDefaults(', () => { }); }); - it('Changing editor mode with blank query should change default', () => { + it('changing editor mode with blank query should change default', () => { changeEditorMode({ refId: 'A', expr: '' }, QueryEditorMode.Code, (query) => { expect(query.editorMode).toBe(QueryEditorMode.Code); }); diff --git a/public/app/plugins/datasource/prometheus/querybuilder/state.ts b/public/app/plugins/datasource/prometheus/querybuilder/state.ts index 606153c393c..04a42555d40 100644 --- a/public/app/plugins/datasource/prometheus/querybuilder/state.ts +++ b/public/app/plugins/datasource/prometheus/querybuilder/state.ts @@ -16,7 +16,6 @@ export function changeEditorMode(query: PromQuery, editorMode: QueryEditorMode, onChange({ ...query, editorMode }); } -// @ts-ignore Will be used after builder is out of beta function getDefaultEditorMode(expr: string) { // If we already have an expression default to code view if (expr != null && expr !== '') { @@ -41,8 +40,7 @@ export function getQueryWithDefaults(query: PromQuery, app: CoreApp | undefined) let result = query; if (!query.editorMode) { - // Default to Code mode until we are out of beta with the builder, then use getDefaultEditorMode. - result = { ...query, editorMode: QueryEditorMode.Code }; + result = { ...query, editorMode: getDefaultEditorMode(query.expr) }; } if (query.expr == null) {