IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Variables: Fixes Constant variable persistence confusion (#29407)

Browse Source 
* Variables: Fixes savequery for Constant and TextBox variables

* Refactor: reverts textbox changes

* Refactor: Fixes dashboard export and tests

* Refactor: hides or migrates Constant variables

* Tests: fixes snapshots
This commit is contained in:
Hugo Häggmark 2020-12-01 15:02:49 +01:00 committed by GitHub
parent c52fd933f6
commit fb622650f3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 233 additions and 40 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
public/app/features/dashboard/components/DashExportModal/DashboardExporter.test.ts
View File

@ -56,6 +56,7 @@ describe('given dashboard with repeated panels', () => {
type: 'constant',
current: { value: 'collectd', text: 'collectd' },
options: [],
query: 'collectd',
},
{
name: 'ds',

5
public/app/features/dashboard/components/DashExportModal/DashboardExporter.ts
View File

@ -182,16 +182,17 @@ export class DashboardExporter {
name: refName,
type: 'constant',
label: variable.label || variable.name,
value: variable.current.value,
value: variable.query,
description: '',
});
// update current and option
variable.query = '${' + refName + '}';
variable.options[0] = variable.current = {
variable.current = {
value: variable.query,
text: variable.query,
selected: false,
};
variable.options = [variable.current];
}
}

14
public/app/features/dashboard/containers/__snapshots__/DashboardPage.test.tsx.snap
View File

@ -80,7 +80,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1`
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -207,7 +207,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1`
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -302,7 +302,7 @@ exports[`DashboardPage Dashboard init completed Should render dashboard grid 1`
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -452,7 +452,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -579,7 +579,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -674,7 +674,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -774,7 +774,7 @@ exports[`DashboardPage When dashboard has editview url state should render setti
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],

8
public/app/features/dashboard/dashgrid/__snapshots__/DashboardGrid.test.tsx.snap
View File

@ -215,7 +215,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -437,7 +437,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -659,7 +659,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],
@ -881,7 +881,7 @@ exports[`DashboardGrid Can render dashboard grid Should render 1`] = `
],
"refresh": undefined,
"revision": undefined,
"schemaVersion": 26,
"schemaVersion": 27,
"snapshot": undefined,
"style": "dark",
"tags": Array [],

104
public/app/features/dashboard/state/DashboardMigrator.test.ts
View File

@ -4,6 +4,7 @@ import { PanelModel } from '../state/PanelModel';
import { GRID_CELL_HEIGHT, GRID_CELL_VMARGIN } from 'app/core/constants';
import { expect } from 'test/lib/common';
import { DataLinkBuiltInVars } from '@grafana/data';
import { VariableHide } from '../../variables/types';
jest.mock('app/core/services/context_srv', () => ({}));
@ -132,7 +133,7 @@ describe('DashboardModel', () => {
});
it('dashboard schema version should be set to latest', () => {
expect(model.schemaVersion).toBe(26);
expect(model.schemaVersion).toBe(27);
});
it('graph thresholds should be migrated', () => {
@ -798,6 +799,107 @@ describe('DashboardModel', () => {
expect(reactPanel.options.angular).toBeUndefined();
});
});
describe('when migrating constant variables so they are always hidden', () => {
let model: DashboardModel;
beforeEach(() => {
model = new DashboardModel({
templating: {
list: [
{
type: 'query',
hide: VariableHide.dontHide,
datasource: null,
allFormat: '',
},
{
type: 'query',
hide: VariableHide.hideLabel,
datasource: null,
allFormat: '',
},
{
type: 'query',
hide: VariableHide.hideVariable,
datasource: null,
allFormat: '',
},
{
type: 'constant',
hide: VariableHide.dontHide,
query: 'default value',
current: { selected: true, text: 'A', value: 'B' },
options: [{ selected: true, text: 'A', value: 'B' }],
datasource: null,
allFormat: '',
},
{
type: 'constant',
hide: VariableHide.hideLabel,
query: 'default value',
current: { selected: true, text: 'A', value: 'B' },
options: [{ selected: true, text: 'A', value: 'B' }],
datasource: null,
allFormat: '',
},
{
type: 'constant',
hide: VariableHide.hideVariable,
query: 'default value',
current: { selected: true, text: 'A', value: 'B' },
options: [{ selected: true, text: 'A', value: 'B' }],
datasource: null,
allFormat: '',
},
],
},
});
});
it('should have three variables after migration', () => {
expect(model.templating.list.length).toBe(6);
});
it('should not touch other variable types', () => {
expect(model.templating.list[0].hide).toEqual(VariableHide.dontHide);
expect(model.templating.list[1].hide).toEqual(VariableHide.hideLabel);
expect(model.templating.list[2].hide).toEqual(VariableHide.hideVariable);
});
it('should migrate visible constant variables to textbox variables', () => {
expect(model.templating.list[3]).toEqual({
type: 'textbox',
hide: VariableHide.dontHide,
query: 'default value',
current: { selected: true, text: 'default value', value: 'default value' },
options: [{ selected: true, text: 'default value', value: 'default value' }],
datasource: null,
allFormat: '',
});
expect(model.templating.list[4]).toEqual({
type: 'textbox',
hide: VariableHide.hideLabel,
query: 'default value',
current: { selected: true, text: 'default value', value: 'default value' },
options: [{ selected: true, text: 'default value', value: 'default value' }],
datasource: null,
allFormat: '',
});
});
it('should change current and options for hidden constant variables', () => {
expect(model.templating.list[5]).toEqual({
type: 'constant',
hide: VariableHide.hideVariable,
query: 'default value',
current: { selected: true, text: 'default value', value: 'default value' },
options: [{ selected: true, text: 'default value', value: 'default value' }],
datasource: null,
allFormat: '',
});
});
});
});
function createRow(options: any, panelDescriptions: any[]) {

23
public/app/features/dashboard/state/DashboardMigrator.ts
View File

@ -6,7 +6,7 @@ import kbn from 'app/core/utils/kbn';
// Types
import { PanelModel } from './PanelModel';
import { DashboardModel } from './DashboardModel';
import { DataLinkBuiltInVars, DataLink, urlUtil } from '@grafana/data';
import { DataLink, DataLinkBuiltInVars, urlUtil } from '@grafana/data';
// Constants
import {
DEFAULT_PANEL_SPAN,
@ -16,9 +16,9 @@ import {
GRID_COLUMN_COUNT,
MIN_PANEL_HEIGHT,
} from 'app/core/constants';
import { isMulti, isQuery } from 'app/features/variables/guard';
import { isConstant, isMulti, isQuery } from 'app/features/variables/guard';
import { alignCurrentWithMulti } from 'app/features/variables/shared/multiOptions';
import { VariableTag } from '../../variables/types';
import { VariableHide, VariableTag } from '../../variables/types';
export class DashboardMigrator {
dashboard: DashboardModel;
@ -31,7 +31,7 @@ export class DashboardMigrator {
let i, j, k, n;
const oldVersion = this.dashboard.schemaVersion;
const panelUpgrades = [];
this.dashboard.schemaVersion = 26;
this.dashboard.schemaVersion = 27;
if (oldVersion === this.dashboard.schemaVersion) {
return;
@ -575,6 +575,21 @@ export class DashboardMigrator {
});
}
if (oldVersion < 27) {
for (const variable of this.dashboard.templating.list) {
if (!isConstant(variable)) {
continue;
}
if (variable.hide === VariableHide.dontHide || variable.hide === VariableHide.hideLabel) {
variable.type = 'textbox';
}
variable.current = { selected: true, text: variable.query ?? '', value: variable.query ?? '' };
variable.options = [variable.current];
}
}
if (panelUpgrades.length === 0) {
return;
}

1
public/app/features/variables/adapters.ts
View File

@ -39,6 +39,7 @@ export interface VariableAdapter<Model extends VariableModel> {
picker: ComponentType<VariablePickerProps>;
editor: ComponentType<VariableEditorProps>;
reducer: Reducer<VariablesState>;
beforeAdding?: (model: any) => any;
}
export type VariableModels =

8
public/app/features/variables/constant/adapter.ts
View File

@ -31,11 +31,17 @@ export const createConstantVariableAdapter = (): VariableAdapter<ConstantVariabl
await dispatch(updateConstantVariableOptions(toVariableIdentifier(variable)));
},
getSaveModel: variable => {
const { index, id, state, global, ...rest } = cloneDeep(variable);
const { index, id, state, global, current, options, ...rest } = cloneDeep(variable);
return rest;
},
getValueForUrl: variable => {
return variable.current.value;
},
beforeAdding: model => {
const { current, options, query, ...rest } = cloneDeep(model);
const option = { selected: true, text: query, value: query };
return { ...rest, current: option, options: [option], query };
},
};
};

6
public/app/features/variables/editor/VariableEditorEditor.tsx
View File

@ -152,7 +152,11 @@ export class VariableEditorEditorUnConnected extends PureComponent<Props> {
placeholder="optional display name"
ariaLabel={selectors.pages.Dashboard.Settings.Variables.Edit.General.generalLabelInput}
/>
<VariableHideSelect onChange={this.onHideChange} hide={this.props.variable.hide} />
<VariableHideSelect
onChange={this.onHideChange}
hide={this.props.variable.hide}
type={this.props.variable.type}
/>
</InlineFieldRow>
<VariableTextField

9
public/app/features/variables/editor/VariableHideSelect.tsx
View File

@ -1,5 +1,5 @@
import React, { PropsWithChildren, useMemo } from 'react';
import { SelectableValue } from '@grafana/data';
import { SelectableValue, VariableType } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors';
import { VariableSelectField } from '../editor/VariableSelectField';
@ -8,6 +8,7 @@ import { VariableHide } from '../types';
interface Props {
onChange: (option: SelectableValue<VariableHide>) => void;
hide: VariableHide;
type: VariableType;
}
const HIDE_OPTIONS = [
@ -16,9 +17,13 @@ const HIDE_OPTIONS = [
{ label: 'Variable', value: VariableHide.hideVariable },
];
export function VariableHideSelect({ onChange, hide }: PropsWithChildren<Props>) {
export function VariableHideSelect({ onChange, hide, type }: PropsWithChildren<Props>) {
const value = useMemo(() => HIDE_OPTIONS.find(o => o.value === hide) ?? HIDE_OPTIONS[0], [hide]);
if (type === 'constant') {
return null;
}
return (
<VariableSelectField
name="Hide"

22
public/app/features/variables/state/actions.test.ts
View File

@ -58,7 +58,7 @@ import {
import { initialState } from '../pickers/OptionsPicker/reducer';
import { cleanVariables } from './variablesReducer';
import { expect } from '../../../../test/lib/common';
import { VariableRefresh } from '../types';
import { ConstantVariableModel, VariableRefresh } from '../types';
import { updateVariableOptions } from '../query/reducer';
import { setVariableQueryRunner, VariableQueryRunner } from '../query/VariableQueryRunner';
@ -399,7 +399,15 @@ describe('shared actions', () => {
data: {
global: false,
index: 1,
model: { ...constant, name: 'constant1', id: 'constant1', global: false, index: 1 },
model: {
...constant,
name: 'constant1',
id: 'constant1',
global: false,
index: 1,
current: { selected: true, text: '', value: '' },
options: [{ selected: true, text: '', value: '' }],
} as ConstantVariableModel,
},
}),
changeVariableNameSucceeded({ type: 'constant', id: 'constant1', data: { newName: 'constant1' } }),
@ -434,7 +442,15 @@ describe('shared actions', () => {
data: {
global: false,
index: 1,
model: { ...constant, name: 'constant1', id: 'constant1', global: false, index: 1 },
model: {
...constant,
name: 'constant1',
id: 'constant1',
global: false,
index: 1,
current: { selected: true, text: '', value: '' },
options: [{ selected: true, text: '', value: '' }],
} as ConstantVariableModel,
},
}),
changeVariableNameSucceeded({ type: 'constant', id: 'constant1', data: { newName: 'constant1' } }),

65
public/app/features/variables/state/sharedReducer.test.ts
View File

@ -1,6 +1,5 @@
import cloneDeep from 'lodash/cloneDeep';
import { default as lodashDefaults } from 'lodash/defaults';
import { LoadingState } from '@grafana/data';
import { LoadingState, VariableType } from '@grafana/data';
import { reducerTester } from '../../../../test/core/redux/reducerTester';
import {
@ -16,7 +15,7 @@ import {
variableStateFetching,
variableStateNotStarted,
} from './sharedReducer';
import { QueryVariableModel, VariableHide } from '../types';
import { ConstantVariableModel, QueryVariableModel, VariableHide, VariableOption } from '../types';
import { ALL_VARIABLE_TEXT, ALL_VARIABLE_VALUE, toVariablePayload } from './types';
import { variableAdapters } from '../adapters';
import { createQueryVariableAdapter } from '../query/adapter';
@ -24,30 +23,70 @@ import { initialQueryVariableModelState } from '../query/reducer';
import { getVariableState, getVariableTestContext } from './helpers';
import { initialVariablesState, VariablesState } from './variablesReducer';
import { changeVariableNameSucceeded } from '../editor/reducer';
import { createConstantVariableAdapter } from '../constant/adapter';
import { initialConstantVariableModelState } from '../constant/reducer';
variableAdapters.setInit(() => [createQueryVariableAdapter()]);
variableAdapters.setInit(() => [createQueryVariableAdapter(), createConstantVariableAdapter()]);
describe('sharedReducer', () => {
describe('when addVariable is dispatched', () => {
it('then state should be correct', () => {
const model = ({
const model: any = {
name: 'name from model',
type: 'type from model',
current: undefined,
} as unknown) as QueryVariableModel;
};
const payload = toVariablePayload({ id: '0', type: 'query' }, { global: true, index: 0, model });
const expected: QueryVariableModel = {
...initialQueryVariableModelState,
id: 'name from model',
global: true,
index: 0,
name: 'name from model',
type: ('type from model' as unknown) as VariableType,
current: ({} as unknown) as VariableOption,
};
const payload = toVariablePayload({ id: 'name from model', type: 'query' }, { global: true, index: 0, model });
reducerTester<VariablesState>()
.givenReducer(sharedReducer, { ...initialVariablesState })
.whenActionIsDispatched(addVariable(payload))
.thenStateShouldEqual({
[0]: {
...lodashDefaults({}, model, initialQueryVariableModelState),
id: '0',
global: true,
index: 0,
},
['name from model']: expected,
});
});
});
describe('when addVariable is dispatched for a constant model', () => {
it('then state should be correct', () => {
const model: any = {
name: 'constant',
type: 'constant',
query: 'a constant',
current: { selected: true, text: 'A', value: 'A' },
options: [{ selected: true, text: 'A', value: 'A' }],
};
const expected: ConstantVariableModel = {
...initialConstantVariableModelState,
id: 'constant',
global: true,
index: 0,
name: 'constant',
type: 'constant',
query: 'a constant',
current: { selected: true, text: 'a constant', value: 'a constant' },
options: [{ selected: true, text: 'a constant', value: 'a constant' }],
};
const payload = toVariablePayload({ id: 'constant', type: 'constant' }, { global: true, index: 0, model });
reducerTester<VariablesState>()
.givenReducer(sharedReducer, { ...initialVariablesState })
.whenActionIsDispatched(addVariable(payload))
.thenStateShouldEqual({
['constant']: expected,
});
});
});

7
public/app/features/variables/state/sharedReducer.ts
View File

@ -16,8 +16,11 @@ const sharedReducerSlice = createSlice({
reducers: {
addVariable: (state: VariablesState, action: PayloadAction<VariablePayload<AddVariable>>) => {
const id = action.payload.id ?? action.payload.data.model.name; // for testing purposes we can call this with an id
const initialState = cloneDeep(variableAdapters.get(action.payload.type).initialState);
const model = cloneDeep(action.payload.data.model);
const adapter = variableAdapters.get(action.payload.type);
const initialState = cloneDeep(adapter.initialState);
const model = adapter.beforeAdding
? adapter.beforeAdding(action.payload.data.model)
: cloneDeep(action.payload.data.model);
const variable = {
...lodashDefaults({}, model, initialState),