From e8eb73f271cea61b946a2d3e463ed32e18765c93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= Date: Tue, 27 Jul 2021 11:41:11 +0200 Subject: [PATCH] Graphite: Update text editor state on initial load (#37202) * Update current query when props change * Remove managed state in GraphiteTextEditor * Fix tests --- .../components/GraphiteTextEditor.tsx | 21 +++++++++------- .../plugins/datasource/graphite/query_ctrl.ts | 2 +- .../graphite/specs/query_ctrl.test.ts | 24 +++++++++---------- .../datasource/graphite/state/actions.ts | 2 ++ .../datasource/graphite/state/store.ts | 2 ++ 5 files changed, 30 insertions(+), 21 deletions(-) diff --git a/public/app/plugins/datasource/graphite/components/GraphiteTextEditor.tsx b/public/app/plugins/datasource/graphite/components/GraphiteTextEditor.tsx index 7ff0c096dfe..68a4d1bc591 100644 --- a/public/app/plugins/datasource/graphite/components/GraphiteTextEditor.tsx +++ b/public/app/plugins/datasource/graphite/components/GraphiteTextEditor.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useState } from 'react'; +import React, { useCallback } from 'react'; import { QueryField } from '@grafana/ui'; import { actions } from '../state/actions'; import { Dispatch } from 'redux'; @@ -9,19 +9,24 @@ type Props = { }; export function GraphiteTextEditor({ rawQuery, dispatch }: Props) { - const [currentQuery, updateCurrentQuery] = useState(rawQuery); + const updateQuery = useCallback( + (query: string) => { + dispatch(actions.updateQuery({ query })); + }, + [dispatch] + ); - const applyChanges = useCallback(() => { - dispatch(actions.updateQuery({ query: currentQuery })); - }, [dispatch, currentQuery]); + const runQuery = useCallback(() => { + dispatch(actions.runQuery()); + }, [dispatch]); return ( <> diff --git a/public/app/plugins/datasource/graphite/query_ctrl.ts b/public/app/plugins/datasource/graphite/query_ctrl.ts index a5f935b3dd7..4858c836dd3 100644 --- a/public/app/plugins/datasource/graphite/query_ctrl.ts +++ b/public/app/plugins/datasource/graphite/query_ctrl.ts @@ -142,7 +142,7 @@ export class GraphiteQueryCtrl extends QueryCtrl { } async targetTextChanged(event: ChangeEvent) { - await this.dispatch(actions.updateQuery({ query: event.target.value })); + // WIP: removed, handled by GraphiteTextEditor } updateModelTarget() { diff --git a/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts b/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts index f84aff4b6fd..a1643ec38c9 100644 --- a/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts +++ b/public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts @@ -4,6 +4,7 @@ import gfunc from '../gfunc'; import { GraphiteQueryCtrl } from '../query_ctrl'; import { TemplateSrvStub } from 'test/specs/helpers'; import { silenceConsoleOutput } from 'test/core/utils/silenceConsoleOutput'; +import { actions } from '../state/actions'; jest.mock('app/core/utils/promiseToDigest', () => ({ promiseToDigest: (scope: any) => { @@ -16,12 +17,13 @@ jest.mock('app/store/store', () => ({ })); const mockDispatch = dispatch as jest.Mock; -async function changeTarget(ctx: any, target: string, refId?: string): Promise { +/** + * Simulate switching to text editor, changing the query and switching back to visual editor + */ +async function changeTarget(ctx: any, target: string): Promise { await ctx.ctrl.toggleEditorMode(); - ctx.ctrl.state.target.target = target; - if (refId) { - ctx.ctrl.state.target.refId = refId; - } + await ctx.ctrl.dispatch(actions.updateQuery({ query: target })); + await ctx.ctrl.dispatch(actions.runQuery()); await ctx.ctrl.toggleEditorMode(); } @@ -283,9 +285,6 @@ describe('GraphiteQueryCtrl', () => { const newQuery = 'aliasByNode(scaleToSeconds(test.prod.*, 1), 2)'; ctx.ctrl.state.datasource.metricFindQuery = () => Promise.resolve([{ expandable: false }]); await changeTarget(ctx, newQuery); - await ctx.ctrl.targetTextChanged({ - target: { value: newQuery }, - } as any); }); it('should rebuild target after expression model', () => { @@ -323,7 +322,7 @@ describe('GraphiteQueryCtrl', () => { }, ]; - await ctx.ctrl.targetTextChanged({ target: { value: 'nested.query.count' } } as any); + await changeTarget(ctx, ctx.target.target); expect(ctx.ctrl.state.target.target).toBe('scaleToSeconds(#A, 60)'); @@ -334,15 +333,16 @@ describe('GraphiteQueryCtrl', () => { describe('when updating target used in other query', () => { beforeEach(async () => { ctx.ctrl.datasource.metricFindQuery = () => Promise.resolve([{ expandable: false }]); - await changeTarget(ctx, 'metrics.a.count', 'A'); + ctx.ctrl.target.refId = 'A'; + await changeTarget(ctx, 'metrics.foo.count'); ctx.ctrl.state.panelCtrl.panel.targets = [ctx.ctrl.target, { target: 'sumSeries(#A)', refId: 'B' }]; - await ctx.ctrl.targetTextChanged({ target: { value: 'metrics.a.count' } } as any); + await changeTarget(ctx, 'metrics.bar.count'); }); it('targetFull of other query should update', () => { - expect(ctx.ctrl.state.panelCtrl.panel.targets[1].targetFull).toBe('sumSeries(metrics.a.count)'); + expect(ctx.ctrl.state.panelCtrl.panel.targets[1].targetFull).toBe('sumSeries(metrics.bar.count)'); }); }); diff --git a/public/app/plugins/datasource/graphite/state/actions.ts b/public/app/plugins/datasource/graphite/state/actions.ts index 6caa597dfe2..9b129e05535 100644 --- a/public/app/plugins/datasource/graphite/state/actions.ts +++ b/public/app/plugins/datasource/graphite/state/actions.ts @@ -28,6 +28,7 @@ const updateFunctionParam = createAction<{ func: FuncInstance }>('update-functio // Text editor const updateQuery = createAction<{ query: string }>('update-query'); +const runQuery = createAction('run-current-query'); const toggleEditorMode = createAction('toggle-editor'); export const actions = { @@ -41,5 +42,6 @@ export const actions = { moveFunction, updateFunctionParam, updateQuery, + runQuery, toggleEditorMode, }; diff --git a/public/app/plugins/datasource/graphite/state/store.ts b/public/app/plugins/datasource/graphite/state/store.ts index 7a1e04d5598..82547b1ed7c 100644 --- a/public/app/plugins/datasource/graphite/state/store.ts +++ b/public/app/plugins/datasource/graphite/state/store.ts @@ -151,6 +151,8 @@ const reducer = async (action: Action, state: GraphiteQueryEditorState): Promise if (actions.updateQuery.match(action)) { state.target.target = action.payload.query; handleTargetChanged(state); + } + if (actions.runQuery.match(action)) { // handleTargetChanged() builds target from segments/tags/functions only, // it doesn't handle refresh when target is change explicitly state.panelCtrl.refresh();